edgedb
Allow assigning subtypes in queries
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.
I can't really wrap my head around the type generation code, but this does seem to fix my use case!
There seems to be something wrong with the default exports though. I'm getting this, but the one at the bottom looks very off.
I'm guessing it's from this part of my schema? Looks like it was mis-parsed though.
I don't really understand how TypeSet works, but it seems wrong that it contains Cardinality.AtMostOne | Cardinality.One | Cardinality.Empty when this field only has a cardinality of exactly Cardinality.One?
@Arrow7000 Thanks for digging in here!
There seems to be something wrong with the default exports though. I'm getting this, but the one at the bottom looks very off.
Is this a regression in this branch compared to master? We recently released a change to allow more unions (https://github.com/edgedb/edgedb-js/pull/279) which I know you got a chance to see as well, but none of this code should've broken any of that union parsing.
I don't really understand how TypeSet works, but it seems wrong that it contains Cardinality.AtMostOne | Cardinality.One | Cardinality.Empty when this field only has a cardinality of exactly Cardinality.One?
I'll take a look at this, but it's likely due to the subtype inclusion being too permissive. Can you share what $StrategyGamer and the other related types look like in your generated code?
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.
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?
@Arrow7000
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!