Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

this type is not assignable to NonNullable<this> #22934

Closed
athasach opened this issue Mar 27, 2018 · 13 comments
Closed

this type is not assignable to NonNullable<this> #22934

athasach opened this issue Mar 27, 2018 · 13 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@athasach
Copy link

TypeScript Version: 2.8.1

Search Terms: NonNullable, this

Code

class X {
    fn(): void {
        // Type 'this' is not assignable to type 'NonNullable<this>'.
        //  Type 'X' is not assignable to type 'NonNullable<this>'.
        const x: NonNullable<this> = this;
        //    ~
    }
}

Expected behavior:

A reference of type this should be assignable to a NonNullable<this> within a method.

Actual behavior:

this is not assignable to NonNullable<this>.

Playground Link:

http://www.typescriptlang.org/play/#src=class%20X%20%7B%0D%0A%20%20%20%20fn()%3A%20void%20%7B%0D%0A%20%20%20%20%20%20%20%20const%20x%3A%20NonNullable%3Cthis%3E%20%3D%20this%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A

Related Issues:

@aschmois
Copy link

That's because a generic is a class definition not an instance. this is an instance. Your code should look like:

class X {
    fn(): void {
        const x: NonNullable<X> = this;
    }
}

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Mar 27, 2018
@RyanCavanaugh
Copy link
Member

@sandersn seems like a bug but I might be missing something extra-subtle

@mhegazy mhegazy assigned ahejlsberg and unassigned sandersn Mar 27, 2018
@mhegazy mhegazy added this to the TypeScript 2.9 milestone Mar 27, 2018
@aschmois
Copy link

Why should dynamic objects be allowed as generic definitions?

@athasach
Copy link
Author

athasach commented Mar 28, 2018

I may have whittled down the actual issue I was dealing with too much. Here is an example of the bug I was trying to be succinct with:

export class Entry<T extends Table> {
    private _table: T | null = null;
    createSubsetForDirectory(): void {
        const entry = new Entry<T>();
        // error TS2345: Argument of type 'Entry<T>' is not assignable to parameter of type 'Entry<NonNullable<T>>'.
        //  Type 'T' is not assignable to type 'NonNullable<T>'.
        //    Type 'Table' is not assignable to type 'NonNullable<T>'.
        this._table!.fn(entry);
        //              ~~~~~
    }
}


export abstract class Table {
    fn(directoryEntry: Entry<this>): this | null {
        return null;
    }
}

strictNullChecks must be enabled for the error to appear.

Playground Link:

http://www.typescriptlang.org/play/#src=export%20class%20Entry%3CT%20extends%20Table%3E%20%7B%0D%0A%20%20%20%20private%20_table%3A%20T%20%7C%20null%20%3D%20null%3B%0D%0A%20%20%20%20createSubsetForDirectory()%3A%20void%20%7B%0D%0A%20%20%20%20%20%20%20%20const%20entry%20%3D%20new%20Entry%3CT%3E()%3B%0D%0A%20%20%20%20%20%20%20%20this._table!.fn(entry)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0A%0D%0Aexport%20abstract%20class%20Table%20%7B%0D%0A%20%20%20%20fn(directoryEntry%3A%20Entry%3Cthis%3E)%3A%20this%20%7C%20null%20%7B%0D%0A%20%20%20%20%20%20%20%20return%20null%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A

@athasach
Copy link
Author

@aschmois In this case, I'm using this as a polymorphic this type. See http://www.typescriptlang.org/docs/handbook/advanced-types.html#polymorphic-this-types

@RyanCavanaugh
Copy link
Member

@athasach that's actually a much more serious repro as it's a regression from 2.7. That code worked as far back as 2.0

@ahejlsberg
Copy link
Member

Pinging @weswigham. I suspect this is related to #22096.

@mhegazy mhegazy assigned weswigham and unassigned ahejlsberg Mar 28, 2018
@mhegazy mhegazy modified the milestones: TypeScript 2.9, TypeScript 2.8.2 Mar 28, 2018
@ahejlsberg
Copy link
Member

I will say that the current behavior doesn't seem totally wrong. In general, a type parameter T isn't assignable to NonNullable<T>. Rather, it is the other way around. And, in the specific example, the this instance might be null or undefined (although it is pretty unlikely).

@weswigham
Copy link
Member

weswigham commented Mar 28, 2018

Pinging @weswigham. I suspect this is related to #22096.

This is just what falls out from conditional type assignability rules, I suppose. The example in the OP didn't use the ! operator at all, so it really doesn't have anything to do with changing ! to also use NonNullable (other than providing another place where people can see the behavior):

class X {
    fn(): void {
        // Type 'this' is not assignable to type 'NonNullable<this>'.
        //  Type 'X' is not assignable to type 'NonNullable<this>'.
        const x: NonNullable<this> = this;
        //    ~
    }
}

It seems like the issue is mostly that NonNullable<T> where T's domain does not include null or undefined (because, eg, T extends Table, or in the case of the OP, this extends X) needs to collapse to just T, but right now it stays generic. @ahejlsberg I think this is something you changed a week or two before the release actually - making conditional types stay generic more often (here)?

@ahejlsberg
Copy link
Member

Yes, I did make a change where distributive conditional types are always deferred if the checked type is generic, but I think it stayed generic before as well.

It may be that that ! operator should do a bit more work, i.e. remove null and undefined that are known to be present (as we used to do) and only return a NonNullable<T> if the resulting T could actually be nullable (which wouldn't be the case in the example above after we remove explicit null and undefined). I'm not sure it is feasible to have the NonNullable<T> type collapse as you suggest.

@weswigham
Copy link
Member

only return a NonNullable if the resulting T could actually be nullable

I'm pretty sure it'd be correct to (re)build that logic into conditional types in general at that point, since nothing about that logic would be specific to either null or undefined. One of the motivating examples for conditional types is non-null types; it'd be off that ! could do the right thing but a conditional type couldn't.

And again, the original issue doesn't involve ! - the postfix operator is just another way to witness the same problem!

@ahejlsberg
Copy link
Member

@weswigham In principle I agree, but devising the exact logic by which this would happen isn't trivial. We currently defer resolution for distributive conditional types when the check type is generic. Even in cases where we know that the check will always go a certain way (e.g. because of a constraint), we still need to defer because the generic type may be instantiated to a union type that we want to distribute over. Only when we can conclude that distributing over a union wouldn't affect the outcome can we actually consider resolving early. I think it would pretty much amount to recognizing Exclude- and Extract-like types. It may be worth doing, but building it into the getNonNullableType function would definitely be simpler.

@jakebailey
Copy link
Member

jakebailey commented Jul 11, 2023

This one was fixed by the infamous #49119, as that changed the definition of NonNullable<T> to T & {}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
Development

No branches or pull requests

8 participants