TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

`asserts this is T` does not narrow type of `this`

Open OmarTawfik opened this issue 1 year ago • 4 comments

🔎 Search Terms

  • asserts this is T
  • narrow
  • assertions
  • I also checked #59707, but it is unrelated (generic types).

🕗 Version & Regression Information

  • Playground: v5.7.0-dev.20241004
  • At least since v5.6.2

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/MYGwhgzhAECC0G8BQ1oHsB2ICeARApgGYCWG+AJgJIawAUAlIiqtMJhGiPgHQhoDmtAESwh9ANzMAvkmaQI+AE4AXOvQBc0eUuUxlAC2IwjcRNBkzZoedABCTVNpVrNT3dAMmT8BOaSXlbAAHfGgAYWgAXlMAHztJJEIAVwxgZWJMLSgdOmBNMI0shRUYYGhvJktk1PTM5XwIZQBGWjzwxmRUYG43NUku7kwcAhIyKhoGcWgAemnoAFFFRTRFTQAFZZCVbGgAciG8IlIKalhd6HI0BugMNGVofAAPI3u64NDdsN3uaA20LcCewOI2O4zOFyuMFu9yeL3QGA87z2tm+-lk1TSGQR9UaACZWvkOnJss5WhJmN1gUcxqdJjM5gB1FYAawgAEI0dAgA

💻 Code

class A {
  onlyDefinedInA() {
    console.log("A");
  }

  assertA(): asserts this is A { }
}


class B {
  assertA(): asserts this is A { }
}

type C = A | B;

function assertA(c: C): asserts c is A {
}

function test1(c: C) {
  c.assertA();
  c.onlyDefinedInA(); // Error: Property 'onlyDefinedInA' does not exist on type 'C'. Property 'onlyDefinedInA' does not exist on type 'B'.
}

function test2(c: C) {
  assertA(c);
  c.onlyDefinedInA(); // Works!
}

Workbench Repro

🙁 Actual behavior

Type of c is not narrowed after the method call, producing an error.

🙂 Expected behavior

I expect the member function to to perform type narrowing.

Additional information about the issue

No response

OmarTawfik avatar Oct 04 '24 14:10 OmarTawfik

I also checked https://github.com/microsoft/TypeScript/issues/59707, but it is unrelated (generic types).

It's still related - the generic types used there are red herring, the issue described there is not specific to them. See my comment in that issue, you have the same situation here - when the compiler sees a union it tries to create a composite signature from all of the constituents of that type. That process deliberately skips assertions.

Andarist avatar Oct 04 '24 15:10 Andarist

@Andarist I see. Thanks for confirming.

should it work or should it error in a visible way like @jcalz is suggesting?

Is there a technical reason that prevents the compiler from making it work? or is it just not implemented yet? If it was the former, then I think it should be an explicit error, to save future users time debugging/root causing this like I did. If it was the latter, then I suggest keeping the issue open, and maybe adding a few pointers on how to go about fixing it.

OmarTawfik avatar Oct 05 '24 13:10 OmarTawfik

Not quite sure what to do with this.

It's obviously safe to maintain the predicateness when the asserted type in every signature is identical.

But only identical identical (i.e. mutual subtyping is not sufficient) is safe to do that, and we run into problems when we expose "identity" as a concept because you run into problems, e.g. if instead you had written

type X1 = { y: string; x: string };
type X2 = { x: string; y: string };

class A {
  onlyDefinedInA() {
    console.log("A");
  }

  assertA(): asserts this is X1 { }
}


class B {
  assertA(): asserts this is X2 { }
}

type C = A | B;

then this "wouldn't work" and we get bug reports that "X1 and X2 are identical" for some definition of "identical" and it just looks like a different kind of bug.

Or, worse, people start writing new cursed variants of IsEqual<T1, T2> and later when we fix this in some as-yet-undiscovered better way, we break that utility type.

So anyway I don't disagree it's suboptimal, but I am really not convinced that any known cure is better than the disease in this situation. boolean is still a sound inference here.

RyanCavanaugh avatar Oct 16 '24 16:10 RyanCavanaugh

@RyanCavanaugh thanks for the detailed follow up!

In that case, at least until there is a better/future-proof way to support this, I wonder if it can be an explicit error in the meantime, to save future users time debugging/root causing this like I did. Unfortunately the current type information surfaced in the IDE indicates it should succeed (playground link above):

Image

OmarTawfik avatar Oct 17 '24 02:10 OmarTawfik

This is related to #28159

you can't change the value of a variable partway through a function. For the same reason that

let foo: number = 5;
foo = '' as string; // error: you already committed to foo being a number

you also get that

let self: this = this;
self = this as X1; // not going to work for the same reason

I agree it's unfortunate since this would be a useful functionality for builders so you don't have to construct a builder in one giant chain

SebastienGllmt avatar Dec 07 '24 05:12 SebastienGllmt

A similar issue:

playground

class Foo {
    private a?: string;
    private b: number = 42;


    ensureA(): asserts this is Foo & {a: string} {
        if (!this.a) {
            this.a = ""
        }
    }

    test() {
        this.ensureA();

        // `this` is `never` unexpectedly
        this.a.includes('foo');
        this.b;
    }
}

JounQin avatar May 14 '25 10:05 JounQin

@JounQin This one is a different issue. The original issue here is strongly related to unions. You are mixing a private and public a members in that intersection and that reduces the containing type to never. This works as expected.

Andarist avatar May 14 '25 10:05 Andarist

You are mixing a private and public a members in that intersection and that reduces the containing type to never. This works as expected.

@Andarist Seems reasonable, thanks. Updated playground with public member.


But is that possible to make my issue work as expected without mark a as public?

JounQin avatar May 14 '25 11:05 JounQin

But is that possible to make my issue work as expected without mark a as public?

I don't think so. You can't have an overlapping public member with a private one of the same name.

Andarist avatar May 14 '25 11:05 Andarist

I don't think so. You can't have an overlapping public member with a private one of the same name.

That's really an unfortunate limitation then. :(

JounQin avatar May 14 '25 11:05 JounQin