Description
Acknowledgement
- I acknowledge that issues using this template may be closed without further explanation at the maintainer's discretion.
Comment
This GitHub issue contains feedback on the TS 5.6-beta release from the team that is responsible for keeping Google's internal software working with the latest version of TypeScript.
Executive summary
- We do not expect to have significant difficulty in upgrading Google to TS 5.6.
- Some changes to our TypeScript code are required to make it compile with TS 5.6.
- We observed regressions in relation to generic type inference (5.6 regression: Incorrect param type inference for type with all optional props #59656)
- Detail sections below explain the changes to our code we expect to make to unblock the upgrade.
Impact summary
| Change description | Announced | Libraries affected |
|---|---|---|
| Disallowed Nullish and Truthy Checks | Yes | 0.070% |
| lib.d.ts Changes | Yes | 0.004% |
| Correct override Checks on Computed Properties | Yes | 0.004% |
| Co- vs. contra-variant inference improvements | No | 0.004% |
The Announced column indicates whether we were able to connect the observed change with a section in the TS5.6-beta announcement.
The following sections give more detailed explanations of the changes listed above.
Announced Changes
This section reviews all announced changes from the TS5.6-beta announcement, whether we support these changes at Google, how we will resolve pre-existing issues for these changes (if applicable), and other thoughts.
Disallowed Nullish and Truthy Checks
We support this change. The check uncovers real unintentional runtime behavior.
As part of this migration, we will add // @ts-ignore suppressions to silence pre-existing errors with a note advising code authors to revisit the code and fix these genuine problems.
Includes the following errors: TS2869, TS2870, TS2871, TS2872, TS2873
Iterator Helper Methods
We support this change.
As these are new, none of our codebase in TS utilizes these so nothing was impacted.
However, the wide number of new unique iterators added is somewhat disruptive for our TS to JS interoperability infrastructure but we have a fairly simple workaround for now (e.g. ArrayIterator, MapIterator, SetIterator, StringIterator etc).
Strict Builtin Iterator Checks (and --strictBuiltinIteratorReturn)
We support this change. We typically interact with Iterator types via methods like [...it], Array.from(it), and for (const val of it) {} so this change is less interesting to us.
We did notice some new compiler errors in our copy of the vscode code base but I can see they have been resolved already so we'll update: microsoft/vscode#222009
Support for Arbitrary Module Identifiers
Presently, we do not plan on utilizing this feature in Google. We support TypeScript adding support for this though.
The changes to the TSC API broke a handful of code locations utilizing the TypeScript API that expected an identifier but now have to support string literals. That is, for the code that assumes propertyName in ImportSpecifier and ExportSpecifier is an Identifier will need to be updated to account for ModuleExportName which also accepts a StringLiteral. For now, we will resolve these instances by either throwing an exception for string literal values or emitting a diagnostic (where possible).
The --noUncheckedSideEffectImports Option
We support this change.
Note: We already enforced this in our build infrastructure. The error simply shows up earlier now in the stack.
The --noCheck Option
We support this change.
Note: This seems useful for incremental development scenarios where we want our edit-refresh cycle to be as short as possible.
Allow --build with Intermediate Errors
We support this change.
Note: We do not utilize the --build flag.
Region-Prioritized Diagnostics in Editors
We support this change.
Note: Editor-specific changes don't impact us.
Search Ancestor Configuration Files for Project Ownership
We support this change.
Note: Editor-specific changes don't impact us though.
lib.d.ts Changes
We support this change.
The changes here were pretty sweeping and sometimes at odds with other typings we already had in the codebase.
As part of this migration, we will add // @ts-ignore suppressions to silence pre-existing errors with a note advising code authors to update the code.
It should be noted that there is a lot of noise in the lib.dom.d.ts updates including many dropped MDN comments as well as new ones added. The general order and structure of some types changed as well. This makes is somewhat difficult to identify the meaningful changes.
In addition, it would be helpful for the lib.dom.d.ts file to be directly checked into github so that updates are viewable in the repo itself to detect unwanted diffs. At the moment, this file appears to be generated as part of the TSC release process and so its changes are harder to detect.
.tsbuildinfo is Always Written
We support this change.
Note: We do not utilize the --build flag.
Respecting File Extensions and package.json from within node_modules
We support this change.
Note: We presently don't use .mts nor .cts file extensions though it's probable that we will support these as we start relying on Open Source projects that use them.
Correct override Checks on Computed Properties
We support this change.
This change identified a number of code patterns in our codebase that needed override.
We will resolve these by adding the missing overrides.
Unannounced Changes
Improved logic to chooses co- vs. contra-variant inference
We support this change (provided that observed regressions are resolved).
#57909 introduced improvements to the compiler that fixes two issues.
We observed several new types of inference issues in our codebase that appear to have stemmed from this.
#59656 captures one issue where I was able to make a reproduction.
Another class of issues surrounds some of our test infrastructure that uses generics to render templates. We've observed typing errors when passing in object literals containing parameters for the templates. These include optional parameters disappearing or becoming required in the parameter object bag type.
We're hoping that #59709 resolves both of these classes of issues.
Once that is submitted, we will rebuild the failing targets and report back if the issue persists (hopefully with a simplified reproduction for the second issue).