5.6 regression: Inference related regression: Wider covariant inference gets picked over more narrow contravariant one

[trevorade]

🔎 Search Terms

Inference

🕗 Version & Regression Information

• This changed between versions 5.5.4 and 5.6.0-beta (still happening in 5.6.1-rc)

⏯ Playground Link

https://www.typescriptlang.org/play/?ts=5.6.1-rc#code/JYOwLgpgTgZghgYwgAgGIHt3IN4ChnJgCeADhAFzIDkMmVA3PsuiWMOiHADYAKULAfkoAjTFwhwQjAL65cxMsgDCHAM4BXALbQAPEwD0+5KGbqwyQ8gCqIBOk3bwoAOanzYABbBVyLqBQARDDAAB4QqgGEHijeGhAAdEwAKrgAfMgAvMgAFHBQzpRJAJSZ6QBu6MAAJoy4VRAIXHkoMOq2bBzIqvYQqG0IOikEECGQIFU+GFiWKg4QTiCu6GZR3r7+yEGh4ZGeMapxianZTHYgGtpQlCrnWrpJqQA0TPXw6lxgSYXPRYW19Y1msgzqpzLR0DcLtBrmo7lAdFNUvRkHIQeYoOF3uYst1tH1bNlwZC4Y8cAoKNRwVRpEVGJYCMgAHoCXBAA

💻 Code

interface Foo {
  type: 'foo';
  optionalProp?: boolean;
}

type Consumer<
  // in out // Uncommenting out this line "fixes" the issue.
  T
> = (arg: T) => void;

declare function someFunc<T
  extends Foo // Commenting out this line "fixes" the issue.
>(
  consumer: Consumer<T>,
  defaultT: T,
): T;

declare const fooConsumer: Consumer<Foo>; 

const result = someFunc(fooConsumer, {type: 'foo'});
//    ^?

🙁 Actual behavior

result is of type { type: "foo"; }

🙂 Expected behavior

result is of type Foo as it was in previous releases

Additional information about the issue

See the comments in the example code for two tweaks to make for the code to work as expected. I get adding in out changing things. I'm less clear on why removing the extends should make any difference.

(Google note: relevant source location http://shortn/_eCS9djFiAR)

[trevorade]

To be clear, this is a real builder error observed in Google that has been boiled down to a minimal reproduction with changed names.

[trevorade]

Confirmed that #59709 does not resolve this issue.

[RyanCavanaugh]

This seems correct? Let's say you wrote this:

interface Foo {
  type: 'foo';
  optionalProp?: boolean;
}

type Consumer<T> = (arg: T) => void;

declare function someFunc<T extends Foo>(consumer: Consumer<T>, defaultT: T): T;

declare const fooConsumer: Consumer<Foo>;

const result = someFunc(fooConsumer, { type: 'foo', extra: "bar" });
result.extra; // should be OK!

[trevorade]

So I have at least three more classes of similar regressions to this we're seeing in Google that I need to investigate and then create simplified repros.

My Consumer name was bad. Here's the real code with names changed to make it a little less clear on what you might infer to be the correct behavior:

function functionName<C extends SomeUnionOfTypes>(
  arg1: Map<SomeType, AnotherType<YetAnotherType<C>>>,
  arg2: C,
  arg3: SomeType,
  arg4: unknown,
): C

Looking at this, it's less clear if we should prefer the literal type of C as extracted from arg1 or arg2.

The behavior has changed though which is causing new regressions.

[trevorade]

I was about to log a bug for this last largest class but, thinking about it more, I believe it boils down to this bug as well.

Here's the example:

declare class TestEnvironment<Args> {
    constructor(template?: (args: Args) => Element, defaultTemplateData?: Args);
    render(data?: Args): void;
}

declare function myElRender(data: MyRenderParams): Element;

type MyRenderParams = {
    state: State,
    optBool?: boolean,
};

enum State {
    OPEN,
    CLOSE,
}


const env = new TestEnvironment(myElRender, {state: State.OPEN});

function doTheRender({
  state = State.OPEN,
  optBool = false,
}: {state?: State, optBool?: boolean} = {}) {
  env.render({state, optBool});
}

Or, as a TS Playground:
https://www.typescriptlang.org/play/?ts=5.6.1-rc#code/CYUwxgNghgTiAEkoGdnwComQFwKIDsA3ASxgHt8BbEfbAHgEEYBzZAPngG8AoePxCjhgBXMNjIwAFNhCUADtBkB+AFzxJsVmqasAlPAC8HXBFk1sAGnigAZlGERsmeYpAARKNiir4O5LoBuXn44fFApYE9vbRZ-NUIyYmAggF9ublAkOHgbYXwxYgp4SgBPEwAlGnDJSK81AFkSyrCQGAAFWChKOPgTM1og7mwSuQRG5vCOmC60Ay5gvhxPEDUAZS8ZCwX4MjlsACEyMggfACMj0yh8LZTBmmFKeHXl+f54AHk23AA5LbeAYQAMu9VrgbuluGBBNh4DRCIZ4PgQAB3DBYPBEUgUai0SSlCpVVpWThLGRrDYgAB0nx+KUC6Vy+WwhXw1jI6AAFiAJq1JDxFhSEc8ZNSvr9grsDhcEXYIMgQDc1CSKT5hQqdntDsczhcQFcUgjOHTXrCiJTQtVlcsrJKtRA6aluEA

TestEnvironment is inferring the type of Args as {state: State} instead of MyRenderParams as it previously did.

---

From the sample runs I performed, I detected 26 different tests failing this same way.

This particular pattern impacts thousands of our tests in the Google codebase so is more important.

If you feel this is significantly different, I can log a separate issue for this regression.

[RyanCavanaugh]

TestEnvironment as written just needs some help if the intended contract is that the template argument is the sole determinant of Args. This inference in both 5.5 and 5.6 doesn't match what you want to happen in cases where you pass optBool to defaultTemplateData

declare class TestEnvironment<Args> {
    constructor(template?: (args: Args) => Element, defaultTemplateData?: Args);
    render(data?: Args): void;
}

declare function myElRender(data: MyRenderParams): Element;

type MyRenderParams = {
    state: State,
    optBool?: boolean,
};

enum State {
    OPEN,
    CLOSE,
}

function f1() {
  const env = new TestEnvironment(myElRender, {state: State.OPEN});
  // OK
  env.render({ state: State.OPEN });
}

function f2() {
  const env = new TestEnvironment(myElRender, {state: State.OPEN, optBool: true});
  // optBool is required now??
  env.render({ state: State.OPEN });
}

This is a very good situation for using NoInfer, which fixes it in both cases and clarifies which argument the inference is supposed to happen from

    , defaultTemplateData?: NoInfer<Args>

[trevorade]

I understand that I can rewrite code to make it compile with the new inference changes. I'm not sure that we should have to though.

My concern is that large swaths of pre-existing TypeScript is just going to stop compiling correctly around the world in this version. If that's okay, then okay.

For this particular case on our end, it's a little tricky because the TestEnvironment class is written in JavaScript, not TypeScript. I need to test how much I can influence the generated type declarations in this case.

[Andarist]

Note that the title of this issue is a little bit misguided. Both of those types are inferred here internally. The fact that one of them was "named" was always irrelevant. It's all about the algorithm that chooses the covariant vs contravariant inference candidates to be selected as the final inferred type.

[trevorade]

Note that the title of this issue is a little bit misguided. Both of those types are inferred here internally. The fact that one of them was "named" was always irrelevant. It's all about the algorithm that chooses the covariant vs contravariant inference candidates to be selected as the final inferred type.

I'm happy to change the title. Can you suggest a more accurate one?

[Andarist]

Wider covariant inference gets picked over more narrow contravariant one

[RyanCavanaugh]

My concern is that large swaths of pre-existing TypeScript is just going to stop compiling correctly around the world in this version. If that's okay, then okay.

The available data we have is that this isn't going to be a disruptive change. The expected number of inference changes between versions isn't zero. The behavior change here is basically a "bug fix" in other contexts and it's expected to see similar-looking cases have similar-looking changes.

[trevorade]

The available data we have is that this isn't going to be a disruptive change. The expected number of inference changes between versions isn't zero. The behavior change here is basically a "bug fix" in other contexts and it's expected to see similar-looking cases have similar-looking changes.

OK. That's good to hear. I'm familiar with the tools to resolve these issues on our end (NoInfer + multiple template params where one extends the other) so I can make updates in our code.

Thanks for being patient with me as I log these regressions. Still getting a feel for this process :)

[typescript-bot]

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