Skip to content

Generic JSX props don't work properly when props contain generic functions #20891

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
alshain opened this issue Dec 25, 2017 · 5 comments
Closed
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@alshain
Copy link

alshain commented Dec 25, 2017

In 2.5.3, I was using JSX components with generic props containing functions successfully by wrapping stateful ones with stateless components, as also described in #6395 and in this comment

However, in 2.6.x and typescript@next, it breaks:

  Type '{ a: T; b: T[]; c: (t: T) => number; }' is not assignable to type 'Readonly<Props<{}>>'.
    Types of property 'c' are incompatible.
      Type '(t: T) => number' is not assignable to type '(t: {}) => number'.
        Types of parameters 't' and 't' are incompatible.
          Type '{}' is not assignable to type 'T'.
src/bug.tsx(32,18): error TS2322: Type '{ a: number; b: number[]; c: (x: number) => number; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<Comp2<{}>> & Readonly<{ children?: ReactNode; }> &...'.
  Type '{ a: number; b: number[]; c: (x: number) => number; }' is not assignable to type 'Readonly<Props<{}>>'.
    Types of property 'c' are incompatible.
      Type '(x: number) => number' is not assignable to type '(t: {}) => number'.
        Types of parameters 'x' and 't' are incompatible.
          Type '{}' is not assignable to type 'number'.

TypeScript Version: 2.7.0-dev.20171224, 2.6.2

Code

import * as React from 'react';

interface Props<T> {
  a: T
  b: T[]
  c: (t: T) => number // This addition breaks it
}


class Comp2<T> extends React.Component<Props<T>> {
  render(){
    return null
  }
}


function Comp1<T>(props: Props<T>) {
    return <Comp2 {...props} /> // Error
}
  

const props = {
  a: 1,
  b: [2],
  c: (x: number) => x
}

const a = <Comp1 {...props} />

const b = <Comp2 {...props} /> // Same error

Expected behavior:

I expect it to work. It worked on 2.5.3

Actual behavior:

It breaks as soon as I add generic functions to the props

Config used:

{
    "compilerOptions": {
      "outDir": "build/dist",
      "module": "commonjs",
      "target": "es5",
      "lib": ["es6", "dom"],
      "sourceMap": true,
      "allowJs": true,
      "jsx": "react",
      "moduleResolution": "node",
      "rootDir": "src",
      "forceConsistentCasingInFileNames": true,
      "noImplicitReturns": true,
      "noImplicitThis": true,
      "noImplicitAny": true,
      "strictNullChecks": true,
      "suppressImplicitAnyIndexErrors": true,
      "experimentalDecorators": true,
      "emitDecoratorMetadata": true,
      "strict": true
      //"noUnusedLocals": true
    },
    "exclude": [
      "node_modules",
      "build",
      "scripts",
      "acceptance-tests",
      "webpack",
      "jest",
      "src/setupTests.ts"
    ],
    "types": [
      "typePatches"
    ]
  } 

The example works as expected on 2.5.3.

@weswigham
Copy link
Member

weswigham commented Dec 29, 2017

For posterity, this changed is due to your config containing strict: true, and 2.6 and onward introducing the strictFunctionTypes flag. If you set strictFunctionTypes to false you should no longer have the new errors.

As explanation for why what you've got changes:

interface Props<T> {
  a: T
  b: T[]
}

is fine. You'll note there are no function types in your props, so we don't do any strict variance checking on anything. Once you add

  c: (t: T) => number // This addition breaks it

we start doing strict function type checking for the added property.

@alshain
Copy link
Author

alshain commented Dec 30, 2017

@weswigham Thank you for the reply.

From what I understand, variance doesn't come into play in my example, so this is not "working as intended", right?

I just want to make sure I understand correctly whether you're explaining why the example doesn't and shouldn't work or why the bug? appears in 2.6.

@weswigham
Copy link
Member

weswigham commented Jan 2, 2018

Oh, it's not working as intended, for sure. The issue is actually that since we don't do any type inference when instantiating JSX attributes, we end up comparing the generic with its constraint (as when we instantiate the target we fill the generics with their constraints), and strict variance checking causes us to invert the directionality of that check at a callback input position (because variance is inverted there, structurally), causing us to check if {} is assignable to T or number. (Whereas without strict function types we'd check if T or number is assignable to {} or the above case, since our default case without that flag is to treat function arguments bivariantly)

I'm just saying that you can disable strictFunctionTypes to suppress the error, if you need to.

@oriSomething
Copy link

@weswigham I understand where strictFunctionTypes behavior of JSX of components with generics comes from and the reasons. Personally, I think it doesn't makes it any more useful with JSX.
I find myself do <T = any> every component declaration (class C<T = any> extends …). Which in the ends makes me adds everywhere declarations like:

<ClassComponent
  value={value as T}
  // If I forget `: T` onChange it will became `any` which can cause runtime errors in some cases
  onChange={(newValue: T) => {
    value = newValue;
  }}
/>;

So in the end I find myself with looser typing

@weswigham
Copy link
Member

weswigham commented Jan 24, 2018

@oriSomething #21383 should fix the typechecker behavior there (assuming the generics on ClassComponent are of an inferrable structure), however #21382 may prevent it from affecting the type you see when you mouseover in an editor or request completions (though it should still be checked correctly by your build tool).

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants