Google feedback on TS 4.5-beta

[rahul-kamat]

This GitHub issue contains feedback on the TS 4.5-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
  4.5.
• Some changes to our TypeScript code are required to make it compile with TS
  4.5.
  • About a third of the new errors reported by TS 4.5-beta are clearly
    related to the announced changes.
  • The remaining new TS errors are few. Some seem to be legitimate source
    code errors but others will need further inspection.
  • Detail sections below explain the changes to our code we expect to make
    to unblock the upgrade.

Impact summary

Change description	Announced	Libraries affected
lib/d.ts Changes	yes	0.006%
Added isTypeOnly attribute to ImportSpecifier and ExportSpecifier in typescript.d.ts	no	0.003%
Promise.all Changes	yes	0.003%
Template String Types Changes	yes	0.003%
Awaited Type Changes	yes	0.001%

The Announced column indicates whether we were able to connect the observed
change with a section in the
TS4.5-beta announcement.

The following sections give more detailed explanations of the changes listed
above.

Changes which were announced

lib/d.ts Changes

We support the typing improvements. Generally it's clear what's changing.

Our fix will be casting away the errors in existing code.

We observed the following breakdown within this category:

• HTMLElement related breakages : 65%
• Others : 35%

Promise.all Changes

Providing more than 1 parameter to Promise.all breaks.

Examples:

• Promise.all<boolean> builds successfully.
• Promise.all<boolean, boolean> fails - TS2558: Expected 1 type arguments,
  but got 2.
• Promise.all<void, boolean, void> fails - TS2558: Expected 1 type
  arguments, but got 3.
• Promise.all<string|null, string|null> fails - TS2558: Expected 1 type
  arguments, but got 2.

Exactly how we resolve this error will be determined later.

Template String Types Changes

Template string literals are sometimes recognized as a type when explicitly
being cast to a different type.

Example:

• return `r${variable1}` as Type1; fails - TS2352: Conversion of type
  'r${variable1}' to type 'Type1' may be a mistake because neither type
  sufficiently overlaps with the other.

We will likely resolve this error by inserting a cast to any for the error
along with a TODO comment containing the error message and a request to the
team owning the code to investigate and make a more correct fix when they can.

Awaited Type Changes

Type inference with Awaited fails in some cases.

Some of the breakages we have seen look like:

• TS2352: Conversion of type 'Awaited<T>' to type 'NonNullable<T>' may be a
  mistake because neither type sufficiently overlaps with the other.
• TS2322: Type 'Awaited<Row<T, K>>[K]' is not assignable to type 'T[K]'.

We will likely resolve this error by inserting a cast to any for the error
along with a TODO comment containing the error message and a request to the
team owning the code to investigate and make a more correct fix when they can.

Changes which were not announced

Added isTypeOnly attribute to ImportSpecifier and ExportSpecifier in typescript.d.ts

ImportSpecifier and ExportSpecifier in typescript.d.ts
have an added isTypeOnly attribute that is not optional. Any code that
includes calls createImportSpecifier, updateImportSpecifier,
createExportSpecifier, updateExportSpecifier breaks because of this added
isTypeOnly parameter. This breaks Angular and our tsickle tool.

Compare(ImportSpecifier):

• TS4.4 - ImportSpecifier interface
• TS4.5 - ImportSpecifier interface

Compare(ExportSpecifier):

• TS4.4 - ExportSpecifier interface
• TS4.5 - ExportSpecifier interface

Our fix would be adding the isTypeOnly parameter to all
ts.factory.(create/update)(Import/Export)Specifier calls.

[DanielRosenwasser]

Providing more than 1 parameter to Promise.all breaks.

So the hope here is that you can omit all the type arguments to Promise.all from now on. I'd be hope that doing so would cause no new issues.

[andrewbranch]

The direct equivalent though is to wrap the multiple type arguments in a tuple. Promise.all<T, U>(...) → Promise.all<[T, U]>(...).

[IllusionMH]

@andrewbranch looks like it might be not always that straightforward #46651

[rahul-kamat]

Providing more than 1 parameter to Promise.all breaks.

So the hope here is that you can omit all the type arguments to Promise.all from now on. I'd be hope that doing so would cause no new issues.

It seems like type inference with Promise.all when all the type arguments are omitted still causes failures.
However, as mentioned by @andrewbranch, wrapping the type arguments in a tuple resolves our breakages.

Thanks for clarifying how Promise.all works!

[rahul-kamat]

I've tested TS 4.5-rc and the results were identical to TS 4.5-beta with the addition of the following:

• About 67% of new errors reported by TS 4.5-rc are clearly related to lib.dom.d.ts changes.

Impact summary

Change description	Announced	Libraries affected
lib.dom.d.ts Changes	no	0.001%

The Announced column indicates whether we were able to connect the observed
change with a section in the TS4.5-rc announcement or in TypeScript 4.5 libdom changes.

lib.dom.d.ts Changes

src/lib/dom.generated.d.ts has been updated in Update TypeScript DOM Libs
between the release of TS 4.5-beta and TS 4.5-rc.
This code generates lib/lib.dom.d.ts.

The following two changes that update the Event type to
MediaRecorderErrorEvent have caused breakages with error
TS2717: Subsequent property declarations must have the same type. Property 'error' must be of type 'MediaRecorderErrorEvent', but here has type 'Event'.

Compare(MediaRecorderEventMap):

• TS 4.5-beta - "error": Event; in MediaRecorderEventMap interface
• TS 4.5-rc - "error": MediaRecorderErrorEvent; in MediaRecorderEventMap interface

Compare(MediaRecorder):

• TS 4.5-beta - onerror: ((this: MediaRecorder, ev: Event) => any) | null;
  in MediaRecorder interface
• TS 4.5-rc - onerror: ((this: MediaRecorder, ev: MediaRecorderErrorEvent) => any) | null;
  in MediaRecorder interface

Exactly how we resolve this error will be determined later. We will likely
resolve this error by either:

1. Changing lib/lib.dom.d.ts internally to temporarily change the
   MediaRecorderErrorEvent types to Event.
2. Inserting a cast to any for the error along with a TODO comment
   containing the error message and a request to the team owning the code to
   change any to MediaRecorderErrorEvent when they can.

[tronguye]

Hey all, commenting on this thread as it's the only existing one that makes mention of MediaRecorderErrorEvent.

On our team, we upgraded from TS 3.8 to 4.4 and noticed that the MediaRecorder.onerror declaration had changed to type the error as an Event although the spec calls for a MediaRecorderErrorEvent. Can anyone chime in on why this change happened? Is it related to rahul's beta testing results?

[tronguye]

Looking again, I see that Chromium attaches an error to the generic event instead of using MediaRecorderErrorEvent