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
undefined
everywhere. Imagine a simple React component whose props look as follows:I would not be able to pass
apolloGeneratedModel.name
into this component without first convertingnull
toundefined
– an unnecessary step in a codebase that has already decided that the distinction betweennull
andundefined
is not adding value, and thatundefined
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:
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
→undefined
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 explicitnull
value and this is a different case from not sending the field. An ORM would interpret an explicitnull
as “erase the field from the database” while anundefined
is 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
null
and notundefined
, not the other way around.Source? Just because the TypeScript team doesn’t use
null
doesn’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
undefined
andnull
– 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 usingundefined
instead 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
null
value, because most likely you will have to check where is thenull
and use/discard based on your application eg. patch using knex or send payload to API, not just change the generated type and pretend thenull
is 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
null
sss /tslint:disable:no-null-keyword
are found in their code while they confused people thatnull
should not be used, easy link here https://github.com/Microsoft/TypeScript/blob/master/src/compiler/sourcemap.tsthey still have
null
type in some of the functions, especially parameter because when getting data, you want to check ifnull
is thereyou can try not putting
null
into data or returningnull
(still controversial), but ignoringnull
is a big mistakeso when you are using
no-null-keyword
from tslint, it won’t flag out the type definitionnull
in the code belowuse only
undefined
!== ignoringnull
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 thatnull
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 benull
is 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
/undefined
since it’s not a feature of the language, ideally typescript shouldn’t either.GraphQL uses
null
andundefined
, so the typings would be wrong by not includingnull
. Also see https://github.com/graphql/graphql-js/issues/1298#issuecomment-377365916