Google feedback on TS 4.7-beta

[frigus02]

This GitHub issue contains feedback on the TS 4.7-beta release from the team that is responsible for keeping Google's internal software working with the latest version of TypeScript.

Executive summary

• Some changes to our TypeScript code are required to make it compile with TS 4.7.
• Detail sections below explain the changes to our code we expect to make to unblock the upgrade.

Impact summary

Change description	Announced	Libraries affected
Type Parameter No Longer Assignable to {} in strictNullChecks	yes	0.24%
lib/d.ts changes	yes	0.022%
TS2774 & TS2801 now fire for secondary conditions in if statements	no	0.017%
TS2731 now fires for keyof T	no	0.008%
Unclassified	-	0.014%

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

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

Changes which were announced

Type Parameter No Longer Assignable to {} in strictNullChecks

We see type errors similar to the ones mentioned in the release announcement.

We're looking into adding extends object to generic type parameters where possible and casts to any to silence the error everywhere else.

lib/d.ts changes

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

We observed the following breakdown within this category:

• fetch now also accepts URL in addition to RequestInfo for the first parameter

• AbortSignal has additional members reason and throwIfAborted

• ImageData has additional member colorSpace

• HTMLVideoElement has additional members requestVideoFrameCallback and cancelVideoFrameCallback

• Object.assign now rejects null/undefined for the first parameter

• Object.freeze has an additional overload, which causes some type errors like:

  interface Data { prop: Array<'a'|'b'> }
  export const DATA: Data = Object.freeze({prop: ['a']});
  //           ^^^^ Type 'Readonly<{ prop: string[]; }>' is not assignable to type 'Data'.

We expect to add casts to any to silence the error and apply proper fixes over time.

Changes which were not announced

TS2774 & TS2801 now fire for secondary conditions in if statements

This wasn't specifically mentioned in the release announcement, but looks like a decent improvement. Example:

declare let flag: boolean;
declare function foo(): void;
declare function bar(): Promise<void>;
if (flag && foo) foo();
//          ^^^ This condition will always return true since this function is always defined. Did you mean to call it instead?(2774)
if (flag && bar()) {}
//          ^^^^^ This condition will always return true since this 'Promise<void>' is always defined.(2801)

We expect to add casts to any to silence the error and apply proper fixes over time.

TS2731 now fires for keyof T

This wasn't specifically mentioned in the release announcement, but looks like a decent improvement. Example:

function foo<T>(key: keyof T) {
  console.log(`Key: "${key}"`);
  //                   ^^^ Implicit conversion of a 'symbol' to a 'string' will fail at runtime. Consider wrapping this expression in 'String(...)'.(2731)
}

We expect to add casts to any to silence the error and apply proper fixes over time.

[DanielRosenwasser]

I think that the

Implicit conversion of a 'symbol' to a 'string' will fail at runtime. Consider wrapping this epression in a 'String(...)'.

error message was introduced by #44578, and it seems to be a breaking change that @amcasey and I saw a bit on internal codebases.

I think that you can instead write string & keyof T in some of those examples.

---

fetch now also accepts URL in addition to RequestInfo for the first parameter

@amcasey and I ran into this on internal codebases as well... Kind of painful.

---

Type Parameter No Longer Assignable to {} in strictNullChecks

This is one of the biggest ones I think we have seen manual intervention with. How are you feeling about these updates right now?

[fatcerberus]

Is there something I should be watching out for w.r.t. the aforementioned fetch change? Making it more permissive shouldn’t be a breaking change…

[DanielRosenwasser]

I think the specific thing I found was an instance of trying to stub/mock out a custom fetch, where the code ended up with issues because it hadn't handled the new case.

[frigus02]

I think that you can instead write string & keyof T in some of those examples.

Thanks for the hint. We'll check if this is something we can add automatically to the affected code.

---

Type Parameter No Longer Assignable to {} in strictNullChecks

This is one of the biggest ones I think we have seen manual intervention with. How are you feeling about these updates right now?

Null check improvements seen generally nice to have to me. We haven't dug deeper into the errors, yet. It's hard to say for me at the moment if this helped catch real errors. I'll try to remember to report back when I know more. Some notes, though:

• I appreciate the "This type parameter probably needs an extends object constraint." diagnostic. I hope that makes it possible to automatically add extends object to the relevant locations. Although I suspect adding extends object might cause a ripple effect up the call chain in certain places. I hope it's not too bad, though. 🙂

• It would be great to have this diagnostic in more situations. Here's one example I found in our code base, that's fixed by adding extends object, but that doesn't have the special diagnostic:

  function extract<T>(obj: {}, other: Partial<T>) {
    (obj as T);
  // ^^^^^^^^
  // Conversion of type '{}' to type 'T' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
  //     'T' could be instantiated with an arbitrary type which could be unrelated to '{}'.(2352)
  }

---

Is there something I should be watching out for w.r.t. the aforementioned fetch change? Making it more permissive shouldn’t be a breaking change…

Similar to what Daniel said, we see this error in stubs/mocks. E.g.:

window.fetch = async (r: RequestInfo) => {
^^^^^^^^^^^^ TS2322: Type '(r: RequestInfo) => Promise<Response>' is not assignable to type '{ (input: RequestInfo | URL, init?: RequestInit | undefined): Promise<Response>. Types of parameters 'r' and 'input' are incompatible. Type 'RequestInfo | URL' is not assignable to type 'RequestInfo'.
  // do some tests and return mocked response...
};

I think those failures are expected.

[fatcerberus]

I think those failures are expected.

Okay, yeah, that's going to happen any time a built-in function becomes more permissive because function parameters are contravariant. You generally want the error because it alerts you to the fact that your monkey-patched stub doesn't handle the new case(s).

That being said, I thought fetch() always accepted an URL? I've never passed anything but a string to it...

[frigus02]

That being said, I thought fetch() always accepted an URL? I've never passed anything but a string to it...

You're right that string was always possible. It's now also possible to pass e.g. new URL(...). fetch changed from accepting RequestInfo to RequestInfo | URL. Where type RequestInfo = Request | string;.

[fatcerberus]

Ah, see, I thought URL itself was an alias for string and got really confused. Not a huge fan of type aliased unions containing primitive types for this reason. Thanks!

[DanielRosenwasser]

It would be great to have this diagnostic in more situations.

So for the example that you gave

function extract<T>(obj: {}, other: Partial<T>) {
  (obj as T);
// ^^^^^^^^
// Conversion of type '{}' to type 'T' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
//     'T' could be instantiated with an arbitrary type which could be unrelated to '{}'.(2352)
}

@weswigham's PR at #48861 should allow casting between {} and T.

[frigus02]

Thanks, @DanielRosenwasser. I didn't realize this is unintentional.

I've looked at a handful of the other errors that are probably caused by "Type Parameter No Longer Assignable to {} in strictNullChecks" and don't have the new diagnostic. I think they are all variations of the example I gave and can be resolved by adding extends unknown to the type parameter. This makes me think they will all be fixed by #48861.

[frigus02]

We imported 4.7-rc. As expected, we no longer see the breakages caused by "Type Parameter No Longer Assignable to {} in strictNullChecks", since this has been backed out in #48923.

We don't see any other major changes compared to the beta.

Though we did find that the Object.freeze typing addition causes a build failure because we don't include BigInt typings, yet. See #49101.