instanceof should not narrow in false branch; or structural assignment to class types should not be allowed

[AnyhowStep]

TypeScript Version: 3.5.1

Search Terms:

instanceof, narrow, false branch

Code

class A {
  x! : number
}
class B {
  y! : number
}

//Neither A nor B, nominally
//Is A, structurally
const aOrB : A|B = { x : 32 };

if (aOrB instanceof A) {
  const a : A = aOrB; //OK
} else {
  const b : B = aOrB; //Expected to fail; is OK
}

Expected behavior:

A|B should not narrow to B in the false branch.

It is possible for aOrB to structurally match A but not nominally match A.
instanceof only checks for a nominal match; not structural match.

Actual behavior:

A|B is narrowed to B in the false branch and will cause run-time bugs.

Playground Link: Playground

Related Issues:

#32774

I'm sure many people have known of this behaviour for a long time, and I never use classes or instanceof much. So, I never bothered to create an issue for it. However, the above PR finally motivated me to bring this up.

I tried to look for "instanceof narrow false branch" on the issues page but couldn't find anything related.

I'm sure this will get closed as "Working as Intended" or "Duplicate" but I feel like I needed to get this out of my system, anyway =x

[kitsonk]

Essentially duplicate of #202

TypeScript is a structural type system. That is because JavaScript is mostly a structural system, but only becomes nominal with instanceof. The change of behaviour you are suggesting would be a breaking change, and it was an intentional decision to have a situation that wasn't 100% correct, but where usability outweighed.

It has been a long roadmap item to experiment with nominal typing in TypeScript of which #33038 is one potential solution. There are a couple others that the TypeScript team are actually working on.

[AnyhowStep]

It's different from #202 and #33038 because they're trying to make stuff like string and other non-class types become nominally typed.

Applying structural typing to interfaces and non-classes works fine, in general, but starts leaking (like the example above) when applied to classes and instanceof or constructor checks.

So, either one of the following should have failed (if we didn't care about usability/backwards compat),

• const aOrB : A|B = { x : 32 };
  • Assignment currently allowed because of structural typing
• const b : B = aOrB;
  • Narrowing currently allowed because of usability

But both are allowed at the moment and this introduces the unsoundness.

---

How often do people try to assign an object literal to a class type?

[RyanCavanaugh]

This is an intentional trade-off. Analysis of real code showed that people either use instanceof, or use structural-class matching, but not both.

[austincummings]

@RyanCavanaugh This was however taken into account in PR #32774, granted that's not merged yet so there are no inconsistencies yet. But I did ask here about that and it was determined that there shouldn't be narrowing in the false case. Should that PR be changed to be consistent with instanceof or stay how it is?

[AnyhowStep]

There's another reason to not want to narrow in the false branch.

//package-foo@1.1.0
export class A {}
export class B {}

//package-foo@1.2.0
export class A {}
export class B {}

//f.ts
//Let's say we're importing from 1.1.0 indirectly, somehow
import {A, B} from "indirectly-uses-package-foo-1.1.0";

function f (aOrB : A|B) {
  if (aOrB instanceof A) {
    //Correctly narrow to A
    console.log("I received A");
  } else {
    //Incorrectly narrow to B
    console.log("I received B");
  }
}

//index.ts
//Let's say we're importing from 1.2.0 indirectly, somehow
import {A, B} from "indirectly-uses-package-foo-1.2.0";
import {f} from "./f";

f(new A());

Given the above, the output will be,

> I received B

However, we passed in A. We just failed the instanceof check because they're from different packages.

---

The call f(new A()), however, might pass because 1.2.0's class A is structurally assignable to 1.1.0's class A.

But, internally, f() is not doing a structural check, it's doing a nominal check.

---

Adding a private property already makes classes nominal (correctly so), why not go all the way?

[typescript-bot]

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.