apollo-tooling
apollo-tooling copied to clipboard
apollographql
Emit TypeScript string unions instead of enums by default
This fixes #1044
- [x] Update CHANGELOG.md* with your change (include reference to issue & this PR)
- [x] Make sure all of the significant new logic is covered by tests
- [x] Rebase your changes on master so that they can be merged easily
- [x] Make sure all tests and linter rules pass
*Make sure changelog entries note which project(s) has been affected. See older entries for examples on what this looks like.
@grantila: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/
I think this is great. For me it makes sense that this is the default behaviour but pub potentially better to make string unions opt in instead to not introduce a breaking change
+1 for this. It can be an option for now and be made the default on next breaking change?
I would love to have string unions instead of enums. Hope this gets merged soon 🤞
@RyanCavanaugh hi As you said from a TS perspective it was an improvement, I'd be curious why you think so. Is there an official resource from TS team that would explain your thinking, or could you share more insights please?
would love to have this merged, the main reason is I have different Enums with the same values and would love o avoid "casting" them
Any movement on this? it would be huge for our use case, since we have server + client side sharing of code, we are getting collisions like:
enum ServerEnum {
VALUE = "VALUE",
}
enum GqlGenerated {
VALUE = "VALUE"
}
query.valueField as GqlGenerated === ServerEnum.VALUE // type error since enums dont overlap
It's been a while so let me take a minute to explain why the current behavior is really suboptimal from a developer experience perspective.
Let's say you write two different queries that both pull GitHub's repostory -> pullRequests -> node -> state field. You'll get two different enum types for "MERGED", "OPEN", "CLOSED". Because there's no merging of these, you can't write code like this
if (prFromQuery1.state === prFromQuery2.state) { ...
because an inherent behavior of string enums is that they're designed to prevent these sorts of comparisons from occurring.
This isn't desirable at all; it should be legal to compare two values from two different queries if they have some overlap. TypeScript will still detect mistakes here - if you have two queries that produce unions that are fully disjoint, you'll get an error.
String literal unions are just a much more accurate representation of what's going on here -- the underlying string values in GraphQL schemas are intended to be nonopaque, but string enums assume a certain level of opaqueness that defeats common patterns you would want to validly write in GraphQL code.
Would anyone mind if I take this over and get it merged in?
The restriction to enums has made things difficult for my team when dealing with unit test and type checking our mocked data. We have naturally moved towards just not typing mocks at all, which has become a pain.
Some teams don't use typescript for type checking. Typescript can be a useful forced code documentation tool and track the flow of data throughout the application. My team has this mindset. For us, we just need to know that the field is equal to any given string. The enum provides no value and instead has hindered us as a whole.
Having the option to use type aliases instead of enums works better for our development workflow.
We are in the same boat and want to use string unions as well instead of enums.
However, why change the default behavior? This might break certain workflows of people who rely on enums as default behavior.
Couldn't this just be an optional flag, similar to enumsAsTypes in https://graphql-code-generator.com/docs/plugins/typescript ?
It would be great to have an option to generate either enums of union types. I personally prefer union types in the majority of the cases and would like to see this happening.
What is stopping from moving this forward?
In the mean time, I started rethinking type conversions completely and built (relatively newly released) typeconv which converts between TypeScript, GraphQL, JSON Schema and Open API (in any order). Soon it'll also be able to also convert to/from suretype (released two days ago). These are young projects, but I'll try to be active and solve problems as they are reported.
typeconv produces string unions, not by default but always. This is because enums in GraphQL and enums in TypeScript are two semantically different things, and aren't convertible two-way. Sure you technically can convert GraphQL enums to TypeScript enums, but not always the other way around, because; what would enum E { Foo = 1, Bar = 3 } become in GraphQL?
I would like to see this merged as well.
To proceed with this someone with write access on this PR propably needs to resolve the merge conflicts and then check if that one CircleCI test still fails.
As someone pretty new to TypeScript I didn't know about this, but you can convert an string enum to a string union like so:
type WeekdayType = `${Weekday}`;
Note that this only works in TS version at least TS4.1 Found here: https://stackoverflow.com/a/52396706/2696867
Posted just in case that helps out anyone else here.
All that said, I would love to see this merged or added as a CLI option to choose enums vs unions. What's blocking this?
I describe a situation where this PR would be really helpful in https://github.com/microsoft/TypeScript/issues/39127#issuecomment-1003219341 . What's the status of this PR?
@RyanCavanaugh @gabrielferreiraa Both of you approved this PR and no one commented anything against it, please resolve the conflicts and merge.
@sharshuv-quotient The people who approved the PR are not maintainers of the repo and do not have permission to merge.
For the people still following this PR: I commented last year that I would like to see this getting merged, but I do no longer think that this will happen. The readme states that this whole project is going to be deprecated.
[2022-01-21] Note - Upcoming Deprecation Plans: We (Apollo) are working towards fully deprecating this repository and its related projects. Most of the functionality in this repository has been replaced by newer projects and the rest will be soon. We'll share detailed migration documentation when everything here is ready to be officially deprecated, but just a heads up in case you're planning on adopting anything here for a new project (which you still can of course if the tooling here works for you - support for this tooling will be minimal however).
Instead we migrated to GraphQL Code Generator in our project some months ago. If it helps, someone else described some GraphQL Code Generation options that should produce output similar to apollo client:codegen here.