`unknown` args generation is overly broad for `required` JSON arguments

[jackfischer]

Code

// GENERATED by @edgedb/generate v0.5.3

import type {Executor} from "edgedb";

export type ExampleInsertArgs = {
  readonly "json": unknown;
};

export type ExampleInsertReturns = {
  "id": string;
};

export function ExampleInsert(client: Executor, args: ExampleInsertArgs): Promise<ExampleInsertReturns> {
  return client.queryRequiredSingle(`\
insert Example {
    optionalJson := <json>$json
}`, args);

}

(Note that $json is <json> and not <optional json>)

Schema

module default {
    type Example {
        optionalJson: json;
    }
}

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.

export type ExampleInsertArgs = {
  readonly json: NonNullable<unknown>;
};

Versions (please complete the following information):

• EdgeDB version: 5.2
• EdgeDB CLI version: 5.0.0
• edgedb-js version: 1.5.4
• @edgedb/generate version: 0.5.3
• TypeScript version: 5.4.5

[jackfischer]

@scotttrinh I'm putting that TS knowledge to work

[scotttrinh]

Yeah, this is interesting because null is a valid JSON value, but null has a special meaning w.r.t. parameters. I'll have a think about this and see how we want to deal with it.

[jackfischer]

Ah, I see what you mean. So null is not not assignable to required JSON.

[scotttrinh]

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.

undefined should definitely be excluded here, and unknown isn't really the right type for JSON since JSON is a pretty narrow subset of types in reality.

[jackfischer]

I wonder if there is a correctness issue right now; the only way to write null for a JSON field is to have it be optional in the query, and when you do that it writes it as empty set and not as null?

[scotttrinh]

when you do that it writes it as empty set and not as null

Yeah, I believe this is a fundamental issue exactly like that. Making json inputs strictly string-y "fixes" that since it disambiguates between "null" (the JSON value of null) and "\"null\"" (the JSON value of the string null) but that's not an ergonomic trade-off I'm willing to make 😅

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!

[jackfischer]

Really appreciate the openness with all the thinking