[TS 5.4.0-beta] instaceof narrowing impacted by prototype property

[acutmore]

🔎 Search Terms

instanceof, narrowing, prototype

🕗 Version & Regression Information

• This changed between versions 5.3.3 and 5.4.0-beta.
• This changed on 5.4.0-dev.20231130 (commits)

⏯ Playground Link

https://www.typescriptlang.org/play?target=99&ts=5.4.0-dev.20231130#code/CYUwxgNghgTiAEkoGdnwOoEtgHMQBd4BvAKHnNGXxgHsBPACgEoAueANxuwG4SBfEiVBI4iGgDsqGbHnwBhCVRgBXMPhow2pcuXEgA7szZZcBXgKHhoosIsIBZEOOUnZc6Ki1kdew62LeOjqYyI7OADIh+GwARjQ0ECBQ4rxBfPAAZNKm+Kk6AA606vh0+SBsJWU0AGbZbnYqahrc8AD0rfAAPAC03fBwALY07JjiOPD4ABYh8FCc2GggMLQw-LwklQhhLjIE8AC88ACSkvjJYCAAKqUgnZs18NuuBO4oyAB868LWCLanj04djkKjcHk9dvIPGgstofAYjADnM98GtBO10RjMVjsTjcXj0YJEoQ-vgnNFEUDZPAAD7wZTiUDVUYgYC8b6wX52DjGCHrTC1BjseCjKjnEBgwHIpgBILkElkg4cFrtLq9eC+eBLFbC8TwACsADoACwGgAM3RiBCg-EEIAAHvkNIQiAIgA

💻 Code

(reduced down from large repo as best I could)

declare class Widget {
   destroy(): void;
}

declare const WidgetConstructor: {
    new(): Widget;
}

declare const MenuWidgetClass: {
    new(): {
        isMenuList: boolean;
    } & Widget;
    prototype: typeof WidgetConstructor; // <-- removing this avoids error
};

type MenuWidget = InstanceType<typeof MenuWidgetClass>;

declare const MenuWidget: typeof MenuWidgetClass & {
    new(): MenuWidget
};

/////////////////////////////////////////////////////////

let content: MenuWidget | undefined;
declare const v: Widget;

if (v instanceof MenuWidget) {
      content = v; // <-- new error in 5.4.0-beta
}

New behavior

TypeScript 5.4.0-beta appears to be consulting the .prototype property of the type in a way that it wasn't before resulting in the instanceof narrowing to narrow to the base class.

Previous behavior

The code in 5.3.3 was expecting the instanceof to narrow to the return type of the function being consulted.

Additional information about the issue

Note: this is more of a change report than a bug report, as I don't think this was announced in the 5.4.0-beta breaking changes. I suspect the change is an improvement and is catching a potential issue that was previously unsafe.

[Andarist]

This changed on 5.4.0-dev.20231130 (commits)

FYI - each nightly version has package.json#gitHead. That information can give you a much more accurate list of the commits added between versions. A better commits list can be found here. Based on the listed commits the likely culprit is this PR - I'll investigate this.

[acutmore]

A better commits list can be found here.

Thanks. That is what I used (the githead from package.json). Our two lists of commits are the same, just in reverse order?

EDIT: Remembering that after looking up the commit I did switch the url to the date based param, but that can be less accurate. I agree the commit-range url is much better! thanks again!

[RyanCavanaugh]

If you're curious to bisect something, every-ts is the best tool for the job

[acutmore]

Oh that's awesome. I've been manually bisecting by updating the playground url 😅

[jakebailey]

Off topic-ish, but the issue template links to it; is there something we can do to make it more obvious?

[acutmore]

Maybe if I hadn't raised a few tickets in the past I would have been less hasty to fill in the form without really reading the extra info bits.

That said, maybe also putting a link to it in a HTML comment in the text box would make it more visible. As harder to not read the inputs.

[RyanCavanaugh]

Bisect confirmed #54753

[Andarist]

I think this works kind of as expected right now.

The break is related to this change:

type P = (typeof MenuWidget)["prototype"]; // TS 5.3: any, TS 5.4: new () => Widget
const p = MenuWidget.prototype; // TS 5.3: any, TS 5.4: new () => Widget

It boils down to the fact that a concrete prototype property trumps the .prototype that comes from the "object function property augment". We can verify that by going to the definition of this property - in TS 5.3 it brings us to two locations. One of them is inside interface Function. This change to prototype has been spotted in the process of reviewing #54753 (albeit in a different context) and it has been accepted as working as intended now. I think it makes sense to keep the behavior as otherwise some patterns become impossible because there is no way to opt-out of your types being impacted by this augment.

That said, it has a very surprising effect on this given example. I think though that it just uncovers a weird implicit implementation detail. It is surprising that the official InstanceType doesn't quite follow the logic embedded in getInstanceType (that is used by instanceof).

The "true" InstanceType (one that would mimic what getInstanceType does internally) would look something like this:

type IfAny<T, Y, N> = 0 extends 1 & T ? Y : N;

type ConstructorType = abstract new (...args: never) => any;

type Constructed<T extends ConstructorType> = T extends abstract new (
  ...args: never
) => infer R
  ? R
  : any;

type TryConstruct<T> = T extends ConstructorType ? Constructed<T> : {};

type InstanceType2<T> = "prototype" extends keyof T
  ? IfAny<T["prototype"], TryConstruct<T>, T["prototype"]>
  : TryConstruct<T>;

[acutmore]

Thank you so much for that analysis @Andarist! I can confirm back in the original project that the ["prototype"] lookup has gone from Function.prototype's any to the actual concrete prototype property of the type. And agree that this is overall an improvement. Chalking it up as a "positive breaking change".

This change is only causing one error in a widget that is still using our 'legacy' class system that pre-dates ES2015 classes (thus all the manual class-like type generation), and we can work around this in our code until the widget gets migrated.

Thanks again!

[typescript-bot]

This issue has been marked as "Not a Defect" and has seen no recent activity. It has been automatically closed for house-keeping purposes.