Provide upgrade tooling for --strictOptionalProperties

[DanielRosenwasser]

Background

For TypeScript 4.4, we anticipate that users using --strictOptionalProperties will have fairly mechanical transformations on individual properties. For example

interface Options {
  maxRetries?: number;
  headers?: KnownHeaders & Record<string, string>;
  callback?: (path: string) => void;
}

probably should just get rewritten to

interface Options {
  maxRetries?: number | undefined;
  headers?: (KnownHeaders & Record<string, string>) | undefined;
  callback?: ((path: string) => void) | undefined;
}

We'd like to provide some basic tooling to run through a project's declarations to ensure that properties are "upgraded" appropriately.

Basic constraints

• We should not introduce syntax errors in the generated code.
• We should ensure that the generated code's formatting is reasonable in 90% of cases, and does not drastically change the formatting of the original file.
• We should have an easy way to point the tool at a tsconfig.json, or a set of understood files to make this work correctly.
• We should not run through node_modules, lib.d.ts, or other files that are outside the control of a user.

Open questions

• What should the tool use? ts-morph? something else?
• How should we distribute this tool? As something standalone, or as a plugin for a broader refactoring tool?

[orta]

@sandersn made a tslint rule with a --fix for DT in microsoft/dtslint#335, could we recommend or upstream this (and/or) do the same for eslint?

[a-tarasyuk]

@orta I think many projects use typescript-eslint, so I suppose no-redundant-undefined should be there too (I can work on this if needed). What do you think?

[sandersn]

Like I did in microsoft/dtslint#335, I would call the rule redundant-undefined, since it should be configurable, and behave differently for parameters and properties.

[OliverJAsh]

I noticed a bunch of DefinitelyTyped PRs to add undefined to optional properties. I'm curious what this change will mean for React, specifically props like className: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/46c3dc08d8efb51686530f24cea216ab8c17d8be/types/react/index.d.ts#L1823

className={undefined} should not be allowed as this will produce HTML such as className="undefined". /cc @sandersn

[orta]

The key with these automated PRs is that they are 'backwards' compatible in the sense that 4.4 behavior with exactOptionalPropertyTypes enabled is gives you the same type system behavior as 4.3 without it. As it's 'legal' in TS to pass undefined to className in 4.3 strict.

However, post the PRs it now easy to switch the semantics to be more accurate on a case by case basis, in comparison to before where support for exactOptionalPropertyTypes would be very unreliable with modules on DT (because few modules would opt-in to the more accurate syntax)

So for React, that className change can have the undefined removed from the string | undefined type after the automated PR happens

[sandersn]

Quick notes from the tslint rule redundant-undefined I upgraded in microsoft/dtslint#335, if you want to use that as a basis for an eslint rule:

• It does not introduce syntax errors and puts parentheses around arrows only. It also skips any and unions with any since that fails another DT lint rule.
• Just adding | undefined after a union is 90% reasonable. Of course, the 80% case is a single type and 90% just adds simple unions. But I think even more complex types of the last 10% look OK most of the time.
• It uses tslint to read tsconfig.
• It skips node_modules and lib.d.ts, but it's clumsy and incomplete.

[a-tarasyuk]

eslint implementation of microsoft/dtslint#335

[andrewbranch]

As discussed in #45032, using a TS codefix across a whole codebase is a poor migration strategy for --exactOptionalPropertyTypes. We feel like @a-tarasyuk’s eslint rule should be good for a bulk migration, while @sandersn’s codefix in #45032 can be useful for one-off fixes in your editor. @iisaduan’s codefix runner should be releasable pretty soon and produces great results for plenty of other codefixes, but we don’t recommend it for --exactOptionalPropertyTypes.

[NickGerleman]

As discussed in #45032, using a TS codefix across a whole codebase is a poor migration strategy for --exactOptionalPropertyTypes. We feel like @a-tarasyuk’s eslint rule should be good for a bulk migration, while @sandersn’s codefix in #45032 can be useful for one-off fixes in your editor. @iisaduan’s codefix runner should be releasable pretty soon and produces great results for plenty of other codefixes, but we don’t recommend it for --exactOptionalPropertyTypes.

I've been curious about the comments on it not being a good migration strategy. The strategy was applied to large projects of DefinitelyTyped in a single go, including React Native in DefinitelyTyped/DefinitelyTyped@694c663 before we brought the typings into our own package.

It has since been applied inconsistently, and making matters more confusing, our actual source is Flow, which does allow passing undefined. So, to not be more strict than the source, we need to add this. But it is no guarantee it actually works.

Since all of the properties are already modded to allow explicit undefined, I did add the ESLint rule for now.