Generate undefined instead of null in typescript

[amol-c]

• [x] feature

I think there has been an intense argument going around with null vs undefined in the JS community. But, in the typescript world, pretty much microsoft has recommended to avoid null and use undefined.

Can we have an option argument to generate undefined instead of null in typescript? Coming from swift world, it will be pretty useful to just deal with undefined than adding null to the mix.

Any thoughts?

[danilobuerger]

GraphQL uses null and undefined, so the typings would be wrong by not including null. Also see https://github.com/graphql/graphql-js/issues/1298#issuecomment-377365916

[irace]

I’m a strong +1 on this request.

While I do understand that the generated typings would be “wrong” in a sense, I don’t think that would actually be of any practical concern to a client-side developer dealing with generated TypeScript models.

From the comment linked to above:

null means that the field requested is not available (either due to being non-existent or from an error) while undefined (or absent from response) means that the field was not requested.

This is true, but since we’re generating our TypeScript models’ interfaces from our actual queries, the interfaces would never include a field that was not requested. As such, the distinction between “non-existent” and “not-requested” is not valuable.

[danilobuerger]

Following your logic

This is true, but since we’re generating our TypeScript models’ interfaces from our actual queries, the interfaces would never include a field that was not requested.

The typings should then include null and not undefined, not the other way around.

[danilobuerger]

Whats the usecase for wanting to use undefined instead of null? What problem does it solve?

[irace]

The distinction between undefined and null – while valuable in JavaScript – does not have the same value in TypeScript. As such, the TypeScript community has largely made a standard practice of just using undefined instead of null (here are Microsoft’s coding guidelines, which @amol-c linked to above).

[irace]

As such, it’s frustrating to try to interoperate generated models with nullable types into a codebase that uses undefined everywhere. Imagine a simple React component whose props look as follows:

interface P {
  name?: string;
}

I would not be able to pass apolloGeneratedModel.name into this component without first converting null to undefined – an unnecessary step in a codebase that has already decided that the distinction between null and undefined is not adding value, and that undefined is to be used in all cases as such.

[danilobuerger]

As such, the TypeScript community has largely made a standard practice of just using undefined instead of null

Source? Just because the TypeScript team doesn't use null doesn't mean its standard practice.

Lets assume we change it to undefined:

// graphql schema
type SomeType {
  bar: String
}
type Query {
  foo: SomeType
}

// graphql query
query someQuery {
  foo {
    bar
  }
}

// typescript bindings
interface someQuery_foo {
  __typename: "SomeType";
  bar?: string;
}
interface someQuery {
  foo?: someQuery_foo;
}

// return from server
{
  "data": {
    "foo": null
  }
}

// code
if (data.foo !== undefined) {
  // TypeScript now assumes this is of type someQuery_foo
  data.foo.bar
  // Uncaught TypeError: Cannot read property 'bar' of null
}

Nice! Now we have what we don't want. People shooting themselves in the foot because the typings are wrong.

[irace]

Right, I should have been more clear.

Simply changing the code generation would not be sufficient – the Apollo client would also need to be configured in such a way that it actually does the null → undefined conversion as well.

[amol-c]

As a temporary fix for our typescript codebase, we are using tpt-optchain (its a fork from rimeto/ts-optchain. Looks like the original library may have added the support) which pretty much treats null and undefined as the same.

This also makes optional unwrapping a bit easier.

[danilobuerger]

@irace How would you then handle partial results (which are not implemented yet in apollo-client but described here: https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/designs/errors.md)? If there is no more null you can't differentiate between a nullable type being present but null and it not being present because of an error therefor undefined?

[irace]

@danilobuerger Good question – I admittedly have not read this proposal, let me read it and get back to you.

Thanks for hearing me out.

[kevinthecity]

If there is no more null you can't differentiate between a nullable type being present but null and it not being present because of an error therefor undefined?

@danilobuerger It seems like if that proposal were implemented, that logic could / would be abstracted away from the consumer of the API inside of the Apollo client - as consumers we just want optional models, we don't really care about how the underlying implementation delivers it to us.

Similar to how a java / kotlin / swift client with generated models wouldn't be able to distinguish between null / undefined since it's not a feature of the language, ideally typescript shouldn't either.

[irace]

Yeah, I think the Swift point is a good one. Swift does not have separate notions of null vs. undefined, yet Apollo still supports generating Swift models.

If you’re building a TypeScript application and have decided to use undefined everywhere and null nowhere, then perhaps it’s more useful to think of that codebase as more analogous to a Swift codebase than to JavaScript one.

[irace]

@danilobuerger I read through the errors proposal but, like @kevinthecity, I’m having a hard time concretely envisioning how this would work on platforms that don’t have separate undefined vs. null implementations. Is there any documentation or sample code?

[kevinthecity]

Adding in here - worth noting that I think this is only necessary to convert null -> undefined for query params, and not for mutation params.

Because of how JS works, you can't pass undefined as a parameter to null something out, since it actually won't be included in the dictionary, thus we need to preserve null in the type definition for variables.

[kambing86]

TL;DR: if you are doing generic programming using typescript, then deal with null value, because most likely you will have to check where is the null and use/discard based on your application eg. patch using knex or send payload to API, not just change the generated type and pretend the null is not there

Long comment below:

As such, the TypeScript community has largely made a standard practice of just using undefined instead of null

I don't agree and please let us know the source, and if you read carefully https://github.com/Microsoft/TypeScript/wiki/Coding-guidelines#null-and-undefined

AGAIN: This is NOT a prescriptive guideline for the TypeScript community

below is the reason from one contributer from TS https://github.com/Microsoft/TypeScript/issues/9653#issuecomment-232181143

The TS compiler internally opted to use only undefined. Part of that is we do not have dependencies on other libraries, so we can be as prescriptive as we want. Some APIs, e.g. DOM, will return T | null consistently, so there is no way around undefined. So i would say it is up to you and your dependencies.

TS team opted to do that because they are dealing with a very specific scope of programming

maybe you should check their source how many nullsss / tslint:disable:no-null-keyword are found in their code while they confused people that null should not be used, easy link here https://github.com/Microsoft/TypeScript/blob/master/src/compiler/sourcemap.ts

they still have null type in some of the functions, especially parameter because when getting data, you want to check if null is there

you can try not putting null into data or returning null (still controversial), but ignoring null is a big mistake

so when you are using no-null-keyword from tslint, it won't flag out the type definition null in the code below

let sourcesContent: (string | null)[] | undefined;

use only undefined !== ignoring null

[kambing86]

for server code, unless there is a feature for apollo server to strip out all the null / replace with undefined, the generated type should still stick to null

for client code, you still have to deal with null as what @danilobuerger said, because there is no undefined in JSON

@irace How would you then handle partial results (which are not implemented yet in apollo-client but described here: https://github.com/apollographql/apollo-client/blob/master/packages/apollo-client/designs/errors.md)? If there is no more null you can't differentiate between a nullable type being present but null and it not being present because of an error therefor undefined?

[ArmorDarks]

Just try typeof item === 'object' when you want to type guard property of the object to be an object. You'd be glad that GraphQL explicitly stated that null expected here, not just optional property, otherwise TS won't even complain about code which will likely fail in runtime. (hint — null is an object too).

So, generating types as optinals (which are equal to T | undefined) when they can be null is extremely misleading unless TS will provide an option to treat optionals as T | undefined | null.

[HosseinAgha]

@ArmorDarks We never simply use typeof item === 'object'(without further checks) ever in our frontend codebase because JS considers null, an object.
IMO adding a flag to Apollo (with a warning for edge cases in docs) is not a bad idea as it will eliminate a lot of unnecessary boilerplate from our components TypeScript Property Types.

[justinmchase]

Related: https://github.com/apollographql/apollo-client/issues/5977

[Sytten]

I was also facing a similar issue, so here my thought on it. null should still be supported because it is possible for a client to send an explicit null value and this is a different case from not sending the field. An ORM would interpret an explicit null as "erase the field from the database" while an undefined is interpreted as "don't touch this field" (in the case of an Update of a CRUD app).