Internal Typescript Conversion Roadmap

[mike-north]

We've allowed implicit any in .ts files for a while. We should have some sort of plan for eventually requiring them to be explicit. This will entail typing function args, among other things.

cc: @runspired

[runspired]

A Roadmap! 🚀

Background

For some time we've allowed implicit-any since our migration is incremental (and slow due to low contributor availability for doing the work). This also works well for converting new files to typescript or making bugfixes as it means these things were not forcing functions for larger type refactors beyond the scope of the work being done. Recently, we hit enough typescript adoption that it became time to firm up our usage and determine the course to get us all the way. ~60% of library code at this point is written in typescript (however, most of our tests are still written in javascript). One of the largest hurdles to converting our library code, refactoring Model to a native class, has also been cleared. With this in mind we've begun refining our tsconfig, and once #7537 lands noImplicitAny will be the only deactivated strict rule. Obviously we want to change this.

Relatedly, when we configured this project we kept allowJs disabled even though we were (and still mostly are when including tests) a JS project. This was because enabling it caused type-checking of JS files, including using JSDoc comments to infer type. Unfortunately, this conflicts with the yuidocs we use for the api documentation, and led to incorrect type inferences and type errors throughout that would have been unfixable. These inferred types would leak into typescript's type checking of TS files, and were worse than an implicit any. For these reasons we decided against allowJs, feeling there wasn't much of an option.

However, when allowJs is disabled, typescript code that calls JS code that calls typescript code loses access to the type information it would have otherwise had. This is also less-than-ideal. Fortunately, the typescript team appears to have realized this wasn't ideal for many teams and provided an escape hatch, albeit one we did not learn about until recently. When allowJs is enabled, we can disable JS typechecking as JSDoc annotation usage by disabling checkJs. Armed with this context, we've determine our migration can be best completed in the following steps:

Caveats

For the purposes of typescript migration, we are considering three goal posts:

• all library code using typescript in strict mode
• all tests of library code using typescript in strict mode
• all JS files for infra/blueprints etc (e.g. not library code) converted to ts.

This roadmap applies specifically to library and test code, but not to infra code at this time. Note that while the steps below are concrete actions to be taken, most all of this work can be done in parallel.

Additionally, this roadmap is NOT a roadmap for EmberData to publish its own types for consumers.

Step 1: tsconfig -> allowJs: true

We will re-enable allowJs to maximize the correctness of the types we've already produced, this results in ~50-100 issues when running yarn problems today. Fixing these ensures that in as many places as possible we are not relying on ts treating something as an any when we could have had robust type information.

Step 2: tsconfig -> noImplicitAny: true + opt-in looser-mode

Unfortunately, typescript does not have a built in way to incrementally migrate to stricter settings. If it did, our ideal setup would be to enable noImplicitAny while simultaneously disabling it for all files individually, then working to remove it from each file one at a time.

Typescript does however allow nested tsconfig.json files that extend from the root file and specify what files they cover (does not need to be the same set), which will allow us to achieve the same solution in effect as these nested configs can then specify specific files they cover. These files do not actually need to be nested,

We will utilize this by moving our existing config to root-tsconfig.json and adding a new config at tsconfig.json that extends it. Our root confit will be updated to activate noImplicitAny while our extended config will disable noImplicitAny and list all current files in its files list. Newly created files will be required to begin life in strict mode (which will happen automatically since they won't be listed in this list), while JS files will be allowed to remain looser when initially converted to TS by being added to this list. This list will function as our "burndown" to complete the migration. When no files remain in this list, and no js files remain in tests/addon/app directories, then we've achieved the goal and can remove our extended config and utilize only the base config once again.

There are also a few community solutions for incrementally migrating to strict mode which we could consider for value-add in addition to this approach.

One is to setup Betterer which allows you to create a test which passes (and self-updates) if you've reduced the number of ts errors from running in a stricter config and fails if you've increased the number of ts errors when run in a stricter config. This is basically a regression test to ensure only forward progress towards strict-mode is made.

Another is a VS Code Plugin that allows per-file and per-directory opt-in to strict mode. This isn't quite as ideal as opt-out of strict, and it also doesn't cause build errors, it will only error within the vscode editor.

Step 3: configure @typescript-eslint to run tsc based checks

We will configure a similar eslint setup to the tsconfig setup such that current files will operate with the current checks. New files, and files converted to be strict will be required to pass the additional checks provided by plugin:@typescript-eslint/recommended-requiring-type-checking.

The extended eslint config for TS files that are not yet strict will deactivate the following rules until such time as all files it covers are converted to strict or all files it covers have resolved violations of these rules. While currently we also have violations for @typescript-eslint/no-explicit-any, this rule will receive additional special handling, see step 6 for more.

{
  rules: {
        '@typescript-eslint/require-await': 'off',
        '@typescript-eslint/no-unsafe-member-access': 'off',
        '@typescript-eslint/no-unsafe-call': 'off',
        '@typescript-eslint/no-unsafe-return': 'off',
        '@typescript-eslint/restrict-template-expressions': 'off',
        '@typescript-eslint/no-unsafe-assignment': 'off',
        '@typescript-eslint/restrict-plus-operands': 'off',
        '@typescript-eslint/unbound-method': 'off',
        '@typescript-eslint/no-floating-promises': 'off',
        '@typescript-eslint/no-unnecessary-type-assertion': 'off',
        '@typescript-eslint/no-misused-promises': 'off',
  }
}

Step 4: convert remaining js files to ts files

After we've configured our config for the noImplicitAny migration, we should begin moving all remaining JS files to TS files. These conversions will be allowed to type as minimally as is required for tsc to compile and the converted file may be added to the burndown list in the extended config. This is the only situation in which a ts file will be allowed to be added to this config.

Step 5: resolve noImplicitAny errors

Once we've converted all files to TS, no imports should result in an any type (and any that still do we need to address with either upstreaming more types to @types/ or in our types/ overrides). We begin migrating TS files in small batches as possible to strict mode, improving types as necessary. Use of explicit any will be prohibited in files converted to be strict (we already have reached the point of enough typings that most explicit any usage has been removed, though ~100 remain, and new anys are something we actively work to avoid adding in code-review).

Step 6: remove all use of explicit any and lint-against it

We will configure and enable no-explicit-any. Existing usages will be ignored inline and no new usages will be accepted unless an irrefutable argument for why it is necessary exists. The locations of these inline ignores will function as a burn down list and a file cannot be marked strict until it contains no usages of explicit any. Note this is intentionally different from step-3 because we wish to enable this rule as quickly as possible.

[runspired]

I've completed the configuration changes required for steps 2 & 3 in #7537

[runspired]

We should probably also analyze re-activating the lib check to ensure things in our types overrides are typed well enough, currently the config is off:

"skipLibCheck": true,