Negated instanceof check doesn't seem to narrow the types correctly in an edge case

[jstasiak]

🔎 Search Terms

instanceof narrowing
instanceof negation
narrowing negation
negative type narrowing
big.js

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about type narrowing/instanceof checks

⏯ Playground Link

https://codesandbox.io/s/typescript-playground-export-forked-78znfm?file=%2Findex.ts

💻 Code

import { Big } from "big.js";

function f(value: Big | string) {
  if (value instanceof Big) {
    // Expected type after narrowing: Big
    // Got: Big
    // Matches expectations
    console.log(value);
  }
  if (!(value instanceof Big)) {
    // Expected type after narrowing: string
    // Got: Big | string
    // Doesn't match expectations
    console.log(value);
  }
}

Dependencies (package.json):

{
  "dependencies": {
    "@types/big.js": "6.2.0",
    "big.js": "6.2.1",
    "typescript": "5.4.0-dev.20231205"
  },
  "description": "TypeScript playground exported Sandbox",
  "name": "TypeScript Playground Export",
  "version": "0.0.0"
}

🙁 Actual behavior

Inside the if (!(value instanceof Big)) block the type of value is not narrowed to Big, it remains Big | string

🙂 Expected behavior

Inside the if (!(value instanceof Big)) block I expect the type of value to be narrowed to not be Big, so: string.

The if (value instanceof Big) condition does narrow Big | string to Big so I'd expect the opposite condition to do the opposite.

Additional information about the issue

The issue is present with @types/big.js 6.2+.
It's not present with @types/big.js 6.1.0.

I've been unable to artificially reproduce this issue using a single source file without referring to the big.js type definitions.

As far as I can tell it's introduced in this DefinitelyTyped change: DefinitelyTyped/DefinitelyTyped#66163 (which is a response to a bug report here, #50058 (comment))

cc @andrewbranch this may interest you

I didn't know if I should report this to TS or the DT project, I went with TS.

[fatcerberus]

instanceof intentionally doesn’t narrow in the false case because TS is structurally typed; being not instanceof Foo doesn’t mean it isn’t still a Foo according to the type system.

[jstasiak]

I suspect it's a bit more complicated than that.

That's because with this locally defined class TS narrows the types the way I'd expect it to while the conditions are the same:

class Whatever {}

function f(value: Whatever | string) {
    if (value instanceof Whatever) {
        // value is Whatever
        console.log(value)
    }
    if (!(value instanceof Whatever)) {
        // vale is string
        console.log(value)
    }
}

https://www.typescriptlang.org/play?ts=5.4.0-dev.20231205#code/MYGwhgzhAEDqAWYAuBTAbigTtA3gXwCgCAzAVwDtgkBLAe3OmIAo0wRSUAuORVDbAD7QISTNXIBzAJS4C0edGrFoLNh0XkRYSilrKEydFhk45C88HoRaIFADoQtCavYopZ+YXNKVAQhfq4lo6ejyG-FImHubQlpo29o7OrK7u5oR4QA

[fatcerberus]

Huh, that's interesting; IIRC the compiler devs have explicitly said in the past that instanceof doesn't narrow in the negative case. Have to wait for a maintainer to chime in, I guess.

[jstasiak]

Yup, agreed. Thank you for the background information.

[MartinJohns]

Worth pointing out that instanceof in general is not a very safe operator. This code compiles fine, but runs into a runtime error:

class Whatever { val: string = 'abc' }

function f(value: Whatever | string) {
    if (value instanceof Whatever) {
        console.log(value.val);
    }

    if (!(value instanceof Whatever)) {
        console.log(value.charAt(0));
    }
}

f({ val: 'foo' })

[jstasiak]

Ugh, I managed to forget about this aspect of instanceof behavior.

[RyanCavanaugh]

instanceof used to not negatively narrow, but we changed it after much complaining.

A minimal inlined definition of Big would be very useful here

[fatcerberus]

Ugh, I managed to forget about this aspect of instanceof behavior.

It’s not really a problem with instanceof per se, the operator is doing what it’s supposed to; it’s that the type narrowing TS does for it is kind of a lie (albeit a convenient one), given the structural typing. If your classes have any private fields then it’s safer because then you can’t get something of the class type without using new.

[jstasiak]

Yeah, what I meant to say: I forget that classes/class instances participate in the same structural typing mechanism as other types, it always trips me as I expect them not to.

@RyanCavanaugh if by "inline" you mean "in the same file" then I made several attempts and failed. This snippet, for example, which is as close to the original big.js type definitions as possible and rather minimal doesn't exhibit the problem:

namespace ExternalModule {
    namespace Big {
        export interface BigConstructor {
            new(value: string): Big;
            (value: string): Big;
            (): BigConstructor;
            readonly Big: BigConstructor;
        }

        export interface Big {
            abs(): Big;
        }
    }

    export declare const Big: Big.BigConstructor;
    export type Big = Big.Big;
}


function f(value: ExternalModule.Big | string) {
    if (value instanceof ExternalModule.Big) {
        // value is Big
        console.log(value);
    }
    if (!(value instanceof ExternalModule.Big)) {
        // value is string, works correctly :|
        console.log(value);
    }
}

I tried to get as close to big.js' index.d.ts but while their version has

declare const Big: Big.BigConstructor;
type Big = Big.Big;

// The export is the same as type/value combo symbol 'Big'.
export = Big;
export as namespace Big;

I had to settle for

    export declare const Big: Big.BigConstructor;
    export type Big = Big.Big;

because I don't think the export = and export as namespace statements are permitted inline.

I suspect these error statements are causing the problem.

Any tips for reproducing this inline? I'm out of ideas.

[jstasiak]

I ran out of ideas and am unable to reproduce this inline, using just a single file. I'm afraid I give up.

[jstasiak]

Hey @RyanCavanaugh, may I ask if we should interpret "closed this as completed" as "working as intended/won't fix"?

[RyanCavanaugh]

Closing because we need a repro that doesn't depend on a large dependency in order to investigate and one was not provided

[jstasiak]

Got it, appreciate the clarification.