Skip to content

React props' generic not correctly inferred #41879

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
strblr opened this issue Dec 8, 2020 · 6 comments
Closed

React props' generic not correctly inferred #41879

strblr opened this issue Dec 8, 2020 · 6 comments
Assignees
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros

Comments

@strblr
Copy link

strblr commented Dec 8, 2020

TypeScript Version: 4.0.3

Search Terms: generic, inference, react, prop, extends

Code

import * as React from "react";

// Variable type

type Pagination = {
  limit: number;
  page: number;
};

// Demo Component

type Props<TVariables extends Pagination> = {
  variables: TVariables;
  setVariables(variables: TVariables): void;
};

function Demo<TVariables extends Pagination>(props: Props<TVariables>) {
  return null;
}

// App

export default function App() {
  const [variables, setVariables] = React.useState({
    test: "demo",
    limit: 10,
    page: 1
  });

  return <Demo variables={variables} setVariables={setVariables} />;
}

Expected behavior: I expected TVariables to be inferred from variables and so setVariables={setVariables} to work just fine.

Actual behavior: setVariables={setVariables} gives the following error :

(method) setVariables(variables: Pagination): void
Type 'Dispatch<SetStateAction<{ test: string; limit: number; page: number; }>>' is not assignable to type '(variables: Pagination) => void'.
  Types of parameters 'value' and 'variables' are incompatible.
    Type 'Pagination' is not assignable to type 'SetStateAction<{ test: string; limit: number; page: number; }>'.
      Property 'test' is missing in type 'Pagination' but required in type '{ test: string; limit: number; page: number; }'.ts(2322)
App.tsx(25, 5): 'test' is declared here.
App.tsx(14, 3): The expected type comes from property 'setVariables' which is declared here on type 'IntrinsicAttributes & Props<Pagination>'

Playground Link: CodeSandbox

Solutions that weirdly work:

I found three solutions that make the issue disappear. So the problem is not fixing the error at all cost, but rather to understand why it's there in the first place.

1- Explicitly setting TVariables using typeof works :

<Demo<typeof variables> variables={variables} setVariables={setVariables} />;

2- Explicitly narrowing what is passed to setVariables makes the code work :

<Demo variables={variables} setVariables={variables => setVariables(variables)} />;

3- The weirdest of all : changing the syntax for the Props definition by using an alternative syntactical form which I thought equivalent - works :

type Props<TVariables extends Pagination> = {
  variables: TVariables;
  setVariables: (variables: TVariables) => void;
  // instead of : 
  // setVariables(variables: TVariables): void;
};
@AviVahl
Copy link

AviVahl commented Dec 8, 2020

@RyanCavanaugh RyanCavanaugh added the Needs Investigation This issue needs a team member to investigate its status. label Dec 10, 2020
@RyanCavanaugh
Copy link
Member

@weswigham this doesn't look right to me -- the variables inference candidate should have highest priority and fix TVariables to the three-property anonymous type?

@weswigham
Copy link
Member

Here's a react-less repro for the bot:

// Variable type

type Pagination = {
  limit: number;
  page: number;
};

// Demo Component

type Props<TVariables extends Pagination> = {
  variables: TVariables;
  setVariables(variables: TVariables): void;
};

function Demo<TVariables extends Pagination>(props: Props<TVariables>) {
  return null;
}

// App

type exprType = {
    test: string;
    limit: 10;
    page: 1;
};

export default function App() {
  const [variables, setVariables]: [exprType, (arg: exprType | ((p: exprType) => exprType)) => void] = null as any;

  return Demo({
      variables,
      setVariables
  });
}

@typescript-bot typescript-bot added the Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros label Jan 5, 2021
@weswigham
Copy link
Member

this doesn't look right to me -- the variables inference candidate should have highest priority and fix TVariables to the three-property anonymous type?

Nope! Both the variables and setVariables occurrences of TVariables have the same inference priority (none of our priority reducing rules come into play whatsoever). The two inferences, exprType and exprType | ((p: exprType) => exprType) are combined into just the later, which does not pass the check against the constraint Pagination, and so Pagination becomes the inference. In strict mode, when you use an => function for setVariables, we instead make exprType | ((p: exprType) => exprType) a contravariant inference, while the exprType inference is a still a normal covariant inference (again at the same priority) - when we have a covariant inference, we discard all contravariant inferences - thus, we choose exprType, which passes the Pagination constraint check, and becomes the inference. That's why using an arrow function in strict mode fixes the issue - we do much more selective inference there.

@weswigham
Copy link
Member

So.... you could say this is unfortunate but working as intended? We've already "fixed" this - but only for arrow functions, and only in strict mode. I assume we limited inference changes to only those situations for compatibility reasons, for example #27357.

@weswigham weswigham added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Needs Investigation This issue needs a team member to investigate its status. labels Jan 5, 2021
@typescript-bot
Copy link
Collaborator

👋 Hi, I'm the Repro bot. I can help narrow down and track compiler bugs across releases! This comment reflects the current state of the repro in this issue running against the nightly TypeScript. If something changes, I will post a new comment.


Comment by @weswigham

❌ Failed: -

  • Type '(arg: exprType | ((p: exprType) => exprType)) => void' is not assignable to type '(variables: Pagination) => void'. Types of parameters 'arg' and 'variables' are incompatible. Type 'Pagination' is not assignable to type 'exprType | ((p: exprType) => exprType)'. Property 'test' is missing in type 'Pagination' but required in type 'exprType'.

Historical Information

Comment by @weswigham

Version Reproduction Outputs
3.7.5, 3.8.2, 3.9.2, 4.0.2, 4.1.2, Nightly

❌ Failed: -

  • Type '(arg: exprType | ((p: exprType) => exprType)) => void' is not assignable to type '(variables: Pagination) => void'. Types of parameters 'arg' and 'variables' are incompatible. Type 'Pagination' is not assignable to type 'exprType | ((p: exprType) => exprType)'. Property 'test' is missing in type 'Pagination' but required in type 'exprType'.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Has Repro This issue has compiler-backed repros: https://aka.ms/ts-repros
Projects
None yet
Development

No branches or pull requests

5 participants