Skip to content

Shorter tuple inference gets picked over longer inference when both come from rests #57981

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

Open
Andarist opened this issue Mar 28, 2024 · 1 comment
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Milestone

Comments

@Andarist
Copy link
Contributor

πŸ”Ž Search Terms

inference contravariant covariance rest shorter longer tuples

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

https://www.typescriptlang.org/play?ts=5.5.0-dev.20240327#code/CYUwxgNghgTiAEAzArgOzAFwJYHtVNQB4BBeEADwxFWAGd44pg8IBPeNAa1RwHdUA2gF0AfAAoAsACh48WAHMAXPDEA6dQtrLiASngBeEfABuOLMAA002etWbt0ndoDc0sHloZ4WVIhAw4YAMCMTEoZVRkAFsAI389Q3gAbwBfC3gARnSAJh1neAB6AvgAUXIAB3AqIOy5GHlo6gxadJjkL3kcLwBmVTFsgFYBgBYdaSLZWQA9AH54d1RPb19-QOUBcPhI2P8hNw8vHz8AkGBa-RCwiOi4mFaZ652YBKNU9Iy8wuKcTnHiydm8wOy2OgWy622tz2UgWSyOq1O3WCiFQoU2kP890etxeyTSmTk9FhGE+Ex+f0m8EBxJBCOA3XWGWhQA

πŸ’» Code

declare function fn<A extends readonly unknown[]>(
  arg: (...args: A) => void,
  ...args: A
): A;
const inferred = fn((a: number) => {}, 1, 2); // Expected 2 arguments, but got 3.(2554)
//    ^? const inferred: [a: number]
const inferred2 = fn((a: number, b?: number) => {}, 1); // ok
//    ^? const inferred2: [number]
const inferred3 = fn((a: number, b?: number) => {}, 1 as const); // ok
//    ^? const inferred3: [1]

πŸ™ Actual behavior

inferred is [a: number] and the function call fails

πŸ™‚ Expected behavior

inferred should be [number, number] and the function call should succeed.

Additional information about the issue

Based on inferred2 and inferred3 we can see that the covariant inference is preferred in a situation like this. The problem is the .length mismatch between those 2 candidates but since the contravariant candidate comes from a rest position it should be ignored when comparing those 2 candidates and the covariant inference should be preferred

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases labels Apr 12, 2024
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Apr 12, 2024
@Andarist
Copy link
Contributor Author

Andarist commented Aug 15, 2024

An extra test case in favor of this that I found when investigating #55192 (TS playground):

export function createSelector<TState, TProps extends number[], TDerivedState>(
  selector: (state: TState, ...props: TProps) => TDerivedState,
  getDependents: (state: TState, ...props: TProps) => unknown,
): (state: TState, ...props: TProps) => TDerivedState {
  return (state, ...args) => {
    getDependents(state, ...args);
    return selector(state, ...args);
  };
}

type State = Record<number, string>;

export const getById = createSelector(
  (...[state, id]: [State, id?: number]) => id && state[id],
  (state) => state,
);

const data = getById({ 1: "test" }, 1);

The problem here is that we end up with [id?: number] and [] contravariant candidates and based on those the shorter one ([]) gets selected through getCommonSubtype call in getContravariantInference

This test case involves 2 contravariant inferences unlike the first one that had a mix of contra and covariant inferences from rests

@Andarist Andarist changed the title Shorter tuple contravariant inference gets picked over longer covariant inference when both come from rests Shorter tuple inference gets picked over longer inference when both come from rests Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted You can do this Possible Improvement The current behavior isn't wrong, but it's possible to see that it might be better in some cases
Projects
None yet
Development

No branches or pull requests

2 participants