-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow assigning subtypes in queries #606
base: master
Are you sure you want to change the base?
Conversation
@Arrow7000 Thanks for digging in here!
Is this a regression in this branch compared to
I'll take a look at this, but it's likely due to the subtype inclusion being too permissive. Can you share what |
Going to set this as a draft for now while I add a bunch of tests in a separate set of PRs that branch from this. But, if anyone is already reviewing this definitely feel free to continue and report back here. |
The code before assumed that subtypes were structurally assignable to their supertype, but via overloads the subtypes might have incompatible shapes. Now we specifically list the subtypes of an abstract type as assignable to the supertype.
15c61aa
to
f427427
Compare
Conflicts: packages/generate/test/globals.test.ts packages/generate/test/group.test.ts packages/generate/test/insert.test.ts packages/generate/test/literals.test.ts packages/generate/test/params.test.ts packages/generate/test/queries.test.ts
Hey @scotttrinh, just checking in on this PR. I don't know if this was ready to merge or not, was it waiting on anything else? |
Yeah, it's mostly waiting on me to have the time to work out the implications "downstream" from implementing what is essentially nominal typing within the TypeScript client. No ETA on this, but it's not ready to merge yet. We're hard at work on finish up some big stuff for EdgeDB 4.0 coming soon, but after that I'm going to circle back to a bunch of WIP stuff on the JS client side. I appreciate your patience! |
Closes #592
This adds a new property to the type that contains all of the subtypes of a given type, so that when testing assignment, instead of comparing objects structurally we compare just that the current type is a supertype of the assigning type by comparing the listed subtypes of the assigning object.
In other words, if a type chain is A -> B -> C -> D, then type A will reference all 4 types, and attempts to assign C (which contains C and D) will succeed.
Note that this will break if anyone was relying on structural typing before since if two unrelated types were being used before but shared the same structure, the old assignment type was typechecking. I think it's probably the case that it would've been a runtime error, so I'm not concerned that there are valid use cases, but it's worth having a think about it before running with this.