Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Typescript to 2.0 #419

Merged
merged 3 commits into from
Jul 19, 2016
Merged

Update Typescript to 2.0 #419

merged 3 commits into from
Jul 19, 2016

Conversation

mquandalle
Copy link
Contributor

@mquandalle mquandalle commented Jul 18, 2016

The soon-to-be-released v2.0 of TypeScript will bring a lot of new cool features. Cherry-picking the releases notes, I believe the apollo-client will especially benefit from the discriminated union types (microsoft/TypeScript#9163), the smarter flow analysis, and the explicit null and undefined checks.

This PR updates the Typescript compiler to use the v2.0-beta. There is one logic error detected by the new flow analysis:

106             result: action.result,
                               ~~~~~~

src/data/store.ts(106,28): error TS2339: Property 'result' does not exist on type 'QueryResultAction | QueryErrorAction | QueryInitAction | QueryResultClientAction | QueryStopActio...'.

I haven’t opt-in the strictNullChecks option as I’m not that comfortable with the code base, but it sure would be great if someone else could do it.

@stubailo
Copy link
Contributor

Thanks for doing this! I'll check it out sometime soon.

@stubailo
Copy link
Contributor

Will this in any way affect people who are using Apollo Client with Typescript 1.x?

@mquandalle
Copy link
Contributor Author

No, not until the types definition file is modified. With this PR, it’s just the compilation-step analysis that is enhanced, but the .js and .d.ts builds are the same.

@stubailo
Copy link
Contributor

There is one logic error detected by the new flow analysis:

That's odd, since there is a result field on the QueryResultAction type..

@mquandalle
Copy link
Contributor Author

Hum, after some investigations, I think this is an instance of microsoft/TypeScript#7719.

Updating graphql-tag was also required because its incorect types definition
was raising errors with TS 2.0, see apollographql/graphql-tag#1.

I haven’t opt-in for the `strictNullChecks` option yet, for that’s certainly
something to do to get the full benefit of this new TS release.
@stubailo
Copy link
Contributor

Looks good to go for me, gonna merge once the tests pass.

@stubailo stubailo merged commit de10f00 into master Jul 19, 2016
@stubailo stubailo removed the ready label Jul 19, 2016
@mquandalle mquandalle deleted the typescript-2.0 branch July 21, 2016 09:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants