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)
As such, it’s frustrating to try to interoperate generated models with nullable types into a codebase that uses
undefinedeverywhere. Imagine a simple React component whose props look as follows:I would not be able to pass
apolloGeneratedModel.nameinto this component without first convertingnulltoundefined– an unnecessary step in a codebase that has already decided that the distinction betweennullandundefinedis not adding value, and thatundefinedis 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:
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
null→undefinedconversion as well.I was also facing a similar issue, so here my thought on it.
nullshould still be supported because it is possible for a client to send an explicitnullvalue and this is a different case from not sending the field. An ORM would interpret an explicitnullas “erase the field from the database” while anundefinedis interpreted as “don’t touch this field” (in the case of an Update of a CRUD app).Following your logic
The typings should then include
nulland notundefined, not the other way around.Source? Just because the TypeScript team doesn’t use
nulldoesn’t mean its standard practice.Lets assume we change it to
undefined:Nice! Now we have what we don’t want. People shooting themselves in the foot because the typings are wrong.
The distinction between
undefinedandnull– 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 usingundefinedinstead ofnull(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
nullvalue, because most likely you will have to check where is thenulland use/discard based on your application eg. patch using knex or send payload to API, not just change the generated type and pretend thenullis not thereLong comment below:
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
below is the reason from one contributer from TS https://github.com/Microsoft/TypeScript/issues/9653#issuecomment-232181143
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-keywordare found in their code while they confused people thatnullshould not be used, easy link here https://github.com/Microsoft/TypeScript/blob/master/src/compiler/sourcemap.tsthey still have
nulltype in some of the functions, especially parameter because when getting data, you want to check ifnullis thereyou can try not putting
nullinto data or returningnull(still controversial), but ignoringnullis a big mistakeso when you are using
no-null-keywordfrom tslint, it won’t flag out the type definitionnullin the code belowuse only
undefined!== ignoringnullJust 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 thatnullexpected here, not just optional property, otherwise TS won’t even complain about code which will likely fail in runtime. (hint —nullis an object too).So, generating types as optinals (which are equal to
T | undefined) when they can benullis extremely misleading unless TS will provide an option to treat optionals asT | undefined | null.@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/undefinedsince it’s not a feature of the language, ideally typescript shouldn’t either.GraphQL uses
nullandundefined, so the typings would be wrong by not includingnull. Also see https://github.com/graphql/graphql-js/issues/1298#issuecomment-377365916