-
Notifications
You must be signed in to change notification settings - Fork 65
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
unknown
args generation is overly broad for required arguments
#981
Comments
unknown
args generation is overly broadunknown
args generation is overly broad for required arguments
@scotttrinh I'm putting that TS knowledge to work |
Yeah, this is interesting because |
Ah, I see what you mean. So null is not not assignable to required JSON. |
Yeah, but I'm not sure about the semantics here. I'll want to align this with the other clients (e.g. python) so maybe there is some existing prior art there.
|
I wonder if there is a correctness issue right now; the only way to write |
Yeah, I believe this is a fundamental issue exactly like that. Making json inputs strictly string-y "fixes" that since it disambiguates between Haven't had the chance to really dig into this problem yet, though, so maybe there are still some solutions we haven't stumbled upon. It's on my radar to focus on this though! |
Really appreciate the openness with all the thinking |
Code
(Note that
$json
is<json>
and not<optional json>
)Schema
Error or desired behavior
Writing
ExampleInsert(client, { json: null })
passes type checks but fails at runtime inside the client with:MissingArgumentError: argument json is required, but received null
.We did this wrong and hit this on two separate occasions production which was quite painful.
If we generated this instead, it solves the problem and properly tells the user statically that this is not allowed.
Versions (please complete the following information):
edgedb-js
version: 1.5.4@edgedb/generate
version: 0.5.3The text was updated successfully, but these errors were encountered: