apollo-tooling: Generate undefined instead of null in typescript

  • 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?

About this issue

  • Original URL
  • State: open
  • Created 6 years ago
  • Reactions: 72
  • Comments: 21 (5 by maintainers)

Most upvoted comments

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.

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.

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 nullundefined conversion as well.

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).

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.

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.

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).

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

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.

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.

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