Skip to content

Expected error in generic class types used in recursive reference #37627

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

Closed
dimitryvolkov opened this issue Mar 26, 2020 · 3 comments
Closed
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@dimitryvolkov
Copy link

TypeScript Version: 3.8.0-dev.20200118

Search Terms:
variance contravariance covariance
Code

class Success<T, E> {
    constructor(readonly value: T) { };
    public isSuccess(): this is Failure<T, E> {
        return true;
    }
}

class Failure<T, E>  {
    constructor(readonly error: E) { };
    public isSuccess(): this is Success<T, E> {
        return false;
    }
}

type Result<T, E> = Success<T, E> | Failure<T, E>;

function Test(n: Number): Result<number, Error> {
    return new Success("test");
}

Expected behavior:
Expected error:

Type 'Success<string, Error>' is not assignable to type 'Result<number, Error>'.
  Type 'Success<string, Error>' is not assignable to type 'Success<number, Error>'.
    Type 'string' is not assignable to type 'number'

Actual behavior:
No error

The code is part of an attempt to create a result object pattern in TS.
Through some bisection I have managed to narrow this down to 1a10e71

It appears this was considered an error up to that commit.
I have tried my luck in understanding the that PR but at the moment sadly I am unable to fully understand it
.
I was able to make the code correctly (assuming it is correct behavior) display the error by referencing both T and E in Success and Error respectively a-la:

class Success<T, E> {
    private _: E;
    constructor(readonly value: T) { };
    public isSuccess(): this is Failure<T, E> {
        return true;
    }
}

class Failure<T, E>  {
    private _: T;
    constructor(readonly error: E) { };
    public isSuccess(): this is Success<T, E> {
        return false;
    }
}

Trying to read the PR I assumed this was some how related to recursiveness of the types but again fully understanding it alludes me.

Another thing that alludes me is that if add the following code (which I understand is an error on its own):

declare class Success<T,E> {
    constructor(readonly value: T);
}

The expected behavior seemed to occur, I attempted this after being baffled as to why using an existing behavior (https://github.com/gDelgado14/neverthrow) yielded the expected behavior and my attempts at copying the library's code verbatim into my code did not.

If this indeed intended behavior I would love nothing more than to understand why is it so and how the aforementioned commit lead to it as I failed to deduce it on my own

Playground Link: Playground Link

Related Issues: #37400

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Mar 27, 2020
@RyanCavanaugh
Copy link
Member

The specific return types of type predicates are not considered as part of variance measurements, so Success has no variance measure of E and Error has no variance measure of T - they're basically fully evaporated from the types.

This specific use case doesn't seem to make any sense, since a Success is never a Failure and vice versa.

@dimitryvolkov
Copy link
Author

Thank you for the prompt response!
In this specific use case I redacted some code in order to get a minimal working repro, originally both of them had both methods isSuccess / isFailure each returning relevant values, allowing a Result value which I can call isSuccess / isFailure on and get the respective value, if it makes the use case make more sense

class Success<T, E> {
    constructor(readonly value: T) { };
    public isSuccess(): this is Failure<T, E> {
        return true;
    }
   public isFailure(): this is Failure<T, E> {
        return false;
    }
}

class Failure<T, E>  {
    constructor(readonly error: E) { };
    public isSuccess(): this is Success<T, E> {
        return false;
    }
   public isFailure(): this is Failure<T, E> {
        return true;
    }
}

I am still baffled as to why this errors when I bring "declare class" into the scope or when this code is imported from an external module, as I mentioned the code above appears almost verbatim in an external library (https://github.com/gDelgado14/neverthrow), if I import it and write the Test function the code throws an error, if I copy the code from the library into the same scope as the Test function the code does not.

@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants