Road to Typescript

[IvanGoncharov]

This is an issue is to discuss our plan to convert this library to TypeScript.
Note: We will still continue to provide full Flow typings in the same package.

This effort is in line with the ongoing conversion of GraphQL IDE packages (GraphiQL, GraphQL LSP, etc.): https://github.com/graphql/graphiql/tree/convert-to-ts

Note this decision is not related to the technical aspect of Flow vs TS.
GraphQL.js has complete Flow coverage and it is excellent for ongoing maintenance. But our goal is to increase the number of active contributors and by switching codebase to TypeScript we hope to make the codebase feel more familiar and easy to contribute to.

Big thanks to @JacksonKearl from Apollo team for making PoC for such conversion and exploring different conversion strategies.

Before we start discussing the action plan here is a few important points:

• 14.x.x is stable release so we shouldn't do any potentially breaking things in this release.
• We need to do 15.0.0 anyway since it becomes hard to maintain support for Node6 since ESLint and other projects stop supporting it. Plus it allows us simply remove all deprecated stuff that was scheduled for removal in this release.
• Even after we release 15.0.0 we will still need to support 14.x.x as a branch and backport all critical or security-related bugfixes until 16.0.0.

Giving these constraints here is a plan I want to propose:

• 1. Freeze all non-critical merges for the duration of this conversion
• 2. I copied all *.d.ts from @types/graphql into tstypes folder and working on removing it from DefinetlyTyped. I included this typings in 14.5.0 release as the baseline for our future work.
• 3. Synchronize *.d.ts files with Flow types (some stuff is missing and a lot of types are based on 14.0.0 release) and identify all changes that were done in @types/graphql but weren't reflected in Flow. We will release these changes as 14.5.1 release.
• 4. After we are sure that *.d.ts files are complete and don't break existing TS libraries we can branch of 14.x.x and switch master to 15.0.0-alpha.1. Remove all stuff marked as deprecated and was scheduled for removal in v15.
• 5. We will start manually converting files one by one to TS and extract Flow typings into separate files. I propose first to convert all test files (should be pretty fast since they don't use a lot of typings) and latter switch to converting the rest of the project. We can schedule frequent 15.0.0-alpha.x releases just to get the early feedback.
• 6. After we converted all files we can switch to 15.0.0-beta.x and ask the different project to test both our TS and Flow typings. After we fix all the issues we would release 15.0.0 and finish this conversion effort.

Also, I think it makes sense use this opportunity and fully convert our codebase to ES6 and idiomatic TypeScript. It means we can finally get rid of pre-ES6 classes constructs and also some weird constructs caused by Flow technical limitations
and other similar artifacts.

CC: @kassens @Neitsch @jbaxleyiii @JacksonKearl @martijnwalraven @mjmahone @acao @19majkel94 @tgriesser @nodkz @benjie

[IvanGoncharov]

We need to decide which minimal TypeScript version we should target?
For 14.x.x we stuck with 2.6 since we inherit it from DefinetlyTyped and changing it would be a breaking change.
However, for 15.0.0 we need can update it.
I read through TypeScript changelog and we absolutely need unknown since Flow's mixed is very common in our source code.
That's mean it should 3.0 or above.

@acao @benjie What TS version are you targeting for graphql-ide?
Does anyone oppose setting it to be above 3.0?

With Flow, we were always targeting the latest release so do we need to support 3.1, 3.2, 3.3, 3.4, 3.5 or we can target upcoming 3.6 from the beginning?

[IvanGoncharov]

Just release 14.5.2 with update TS typings 🎉
Big thanks to @JacksonKearl who single-handedly syncronize 102 *.d.ts in less than two days 👏

@kassens @Neitsch @jbaxleyiii @martijnwalraven @mjmahone @acao @19majkel94 @tgriesser @nodkz @benjie Can you please update to 14.5.2 or at least try it locally and provide feedback?
Ideally, this would be the last release in 14.x.x line before we move it into the branch and switch master to 15.0.0-alpha.1. So it would be great to resolve all major issues before we do that.

[eschan]

i just tried to upgrade to 14.5.2 and getting the following build error.

node_modules/graphql/utilities/coerceInputValue.d.ts:2:30 - error TS2307: Cannot find module 'tstypes/error'.

2 import { GraphQLError } from 'tstypes/error';
                               ~~~~~~~~~~~~~~~

node_modules/graphql/validation/validate.d.ts:6:19 - error TS2307: Cannot find module 'tstypes/tsutils/Maybe'.

6 import Maybe from 'tstypes/tsutils/Maybe';
                    ~~~~~~~~~~~~~~~~~~~~~~~

[JacksonKearl]

@eschan I’ll take a look at that, thanks!

[JacksonKearl]

@eschan This should be fixed with #2120.

[IvanGoncharov]

@eschan @JacksonKearl released 14.5.3 📦 with this fix.

[eschan]

works! 💯

[Neitsch]

Regarding step 5: What are the thoughts on converting everything by hand, as opposed to running a script that converts everything (https://github.com/bcherny/flow-to-typescript), and then cleaning up any broken tests afterwards?

[JacksonKearl]

@Neitsch I actually have branch up on the Apollo fork that does exactly that. https://github.com/apollographql/graphql-js/tree/ts-merge-all-at-once-wip

Issues experienced:

1. The converter errors out with unhelpful messages on most of the larger files (and even some small ones). Apparently some Flow constructs we were using aren't handled for whatever reason. This means you end up converting the "difficult" files by hand anyways.
2. The outputted types are still Flow-y. But this I mean they don't take advantage of any of TS's features, and we lose power Flow has that TS doesn't. So the result code is strictly worse-typed. For instance, we would lose things like sealed objects, and do not gain things like private class members (unless we were to make some crazy flow transform that converted underscored vars to private, which could be cool).
3. The Flow code is missing lots of types where Flow is able to infer them but TS cannot (local function parameters, for instance), which results in lots of errors under any strict flags, which we'd like to use. (~1500 errors in the codebase when strict: true is set)

[MichalLytek]

Can you please update to 14.5.2 or at least try it locally and provide feedback?

All works in TypeGraphQL, no TS errors 😉

[Neitsch]

@JacksonKearl thank you for the quick response. Makes sense! 👍

@IvanGoncharov will do the bump this week and report back

[acao]

@JacksonKearl I highly recommend @benjie's fork for flow-to-typescript. Some of these issues are addressed, amongst others. We are experimenting with using it for the IDE ecosystem, however most of the migration effort so far has been manual.
@IvanGoncharov this all sounds great and we will be following a somewhat analagous route on the IDE side. The typescript version I'm using for the monaco mode development has been latest, 3.5.3 or so.
@Neitsch and I are digging in on the LSP, we've already been underway with converting GraphiQL to typescript, and hope to release soon with our own type definitions, if not most of the packages converted. We will similarly continue to maintain the flow types, 96K downloads a week just for our flow types package!?