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

Class property type inference failure when destructuring return value of generic function #51203

Closed
paul-go opened this issue Oct 17, 2022 · 9 comments
Labels
Not a Defect This behavior is one of several equally-correct options

Comments

@paul-go
Copy link

paul-go commented Oct 17, 2022

Bug Report

🔎 Search Terms

Class Property
Destructuring
Type Inference
Generic

🕗 Version & Regression Information

  • This is a crash: No
  • The issue occurs on the latest nightly (v4.9.0-dev.20221017), as well as on version 4.7.4. I haven't tested on other versions, but my suspicion is that this has been a hidden bug for a while.

⏯ Playground Link

Playground link with relevant code

💻 Code

class Foo {
    constructor() {
        [this.x, this.y] = init();
    }

    x; // Any
    y; // Any
}

function init<T>() {
    return [0, ""] as [number, string];
}

🙁 Actual behavior

The x and y properties are typed as any.

🙂 Expected behavior

The x and y properties should be typed as number and string respectively.

Further investigation

If the return value of the function is captured in a local variable, and then destructured from there, everything works as expected:

class Foo {
    constructor() {
        const bar = init();
        [this.x, this.y] = bar;
    }

    x; // number
    y; // string
}

function init<T>() {
    return [0, ""] as [number, string];
}

Or, if the generic argument is omitted, everything works as expected:

class Foo {
    constructor() {
        [this.x, this.y] = init();
    }

    x; // number
    y; // string
}

function init() {
    return [0, ""] as [number, string];
}

Making use the generic type argument T does not appear to resolve the issue:

class Foo {
    constructor() {
        [this.x, this.y] = init(false);
    }

    x; // Any, should be number
    y; // Any, should be boolean
}

function init<T>(value: T) {
    return [0, value] as [number, T];
}
@fatcerberus
Copy link

fatcerberus commented Oct 17, 2022

For context:

'x' implicitly has type 'any' because it does not have a type annotation and is referenced directly or indirectly in its own initializer.

Looks like a design limitation. Essentially x and y become part of the context for the generic call: TS wants to know T, but first it needs to know what the type of x is (because contextual typing plays into generic inference), but for that it needs to instantiate T, etc. That the return value is actually non-generic doesn't matter because it doesn't have a usable first-order function signature until it's instantiated init. So the end result is you hit a circularity and the compiler gives up.

tl;dr: the type of a property is both the input and output of a generic instantiation, the compiler can't resolve the cycle and so falls back to any

@fatcerberus
Copy link

As an additional note: If you're wondering why the compiler thinks T might depend on the type of x and y, it's due to the same contextual typing mechanism that makes this work:

let x: Promise<number> = new Promise(resolve => resolve("foo"));  // error on resolve call

@paul-go
Copy link
Author

paul-go commented Oct 17, 2022

Well, I'm not familiar enough with the granularities of the type checker logic in order to refute what you're saying, but the error is still generated in the case when a default generic type argument is defined for T:

class Foo {
    constructor() {
        [this.x, this.y] = init();
    }

    x; // Still fails
    y; // Still fails
}

function init<T = string>() {
    return [0, ""];
}

If I'm understanding what you're saying correctly, then I would imagine that such a work-around should at least dodge the issue. But the error persists. Also worth noting that if the generic argument is provided at the call site rather than in the signature, everything works as expected:

class Foo {
    constructor() {
        [this.x, this.y] = init<string>();
    }

    x; // number
    y; // string
}

function init<T>() {
    return [0, ""];
}

@fatcerberus
Copy link

The default is only used when TS can’t successfully infer a type for the generic. It’s still going to try to infer it through the usual means, and thus hit the aforementioned circularity (which will indeed end with T getting its default type, to be clear). At that point though the property in question now has an “implicit any” as its type and the compiler can’t walk it back because that would require more than one round of inference. Hence the design limitation.

That providing the type argument at the call site dodges the error is fully expected - it bypasses generic inference (there’s no reason to prefer an inference over a type provided explicitly).

@fatcerberus
Copy link

fatcerberus commented Oct 17, 2022

Basically, any case where type inference X depends on type inference Y and vice versa will likely be detected as circular, even if there’s an alternate way to infer X that avoids the circularity, because inference is only done in a single pass. TS doesn’t do full unification (see #30134).

@fatcerberus
Copy link

fatcerberus commented Oct 17, 2022

Short version: TS tries to infer both T and this.prop “at the same time”. Both fail because the inferences depend on each other. T, being a type parameter, has a working failure case (fall back to default/constraint), prop does not and gets flagged as implicit-any. There is no mechanism to try inference again and you get stuck with implicit-anys. 😢

Fix would be to do type inference in multiple rounds (allowing failed inferences to be retried using information from the previous round), but responses to similar issues in the past have suggested it’s a performance issue.

@RyanCavanaugh RyanCavanaugh added the Not a Defect This behavior is one of several equally-correct options label Oct 17, 2022
@RyanCavanaugh
Copy link
Member

This is a true circularity, as outlined above.

@fatcerberus
Copy link

fatcerberus commented Oct 17, 2022

Well, technically, it’s a finite spiral that’s just being viewed from above—T has a working fallback mechanism that would resolve the circularity if not for the restriction that types must all be inferable in a single pass.

@github-actions
Copy link

github-actions bot commented Jun 8, 2023

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Not a Defect This behavior is one of several equally-correct options
Projects
None yet
Development

No branches or pull requests

3 participants