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

React proptypes mess with checkJs type checking #30007

Closed
kiyutink opened this issue Feb 8, 2019 · 9 comments
Closed

React proptypes mess with checkJs type checking #30007

kiyutink opened this issue Feb 8, 2019 · 9 comments
Labels
External Relates to another program, environment, or user action which we cannot control.

Comments

@kiyutink
Copy link

kiyutink commented Feb 8, 2019

  • VSCode Version: 1.31.0
  • OS Version: Windows 10

Steps to Reproduce:

  1. Create a class component
  2. Pass this component to another component into a prop with type "PropTypes.func"
  3. The prop-types package doesn't identify this as an error, but TypeScript service picks up these typings and reports this as an error

image
image
image

Does this issue occur when all extensions are disabled?: Yes

@mjbvz mjbvz self-assigned this Feb 9, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Feb 9, 2019

Please provide a small code example as text (not just screenshots)

@kiyutink
Copy link
Author

kiyutink commented Feb 9, 2019

https://codesandbox.io/s/lxor0y5jn7
here's a minimal code example to reproduce

@mjbvz mjbvz transferred this issue from microsoft/vscode Feb 20, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Feb 20, 2019

Tested with typescript@3.4.0-dev.20190220

Self contained example:

// @ts-check
import React from "react";
import PropTypes from "prop-types";

class ClassComponent extends React.Component {
  render() { return null; }
}

const ComponentWithPropTypes = () => { return null; };

ComponentWithPropTypes.propTypes = { component: PropTypes.func };

const z = (
  <ComponentWithPropTypes component={ClassComponent} />
);

The error is on ClassComponent in component={ClassComponent}:

Type 'typeof ClassComponent' is not assignable to type '(...args: any[]) => any'.
  Type 'typeof ClassComponent' provides no match for the signature '(...args: any[]): any'.ts(2322)
index.jsx(11, 38): The expected type comes from property 'component' which is declared here on type 'IntrinsicAttributes & {} & Pick<InferProps<{ component: Requireable<(...args: any[]) => any>; }>, "component">'

@mjbvz mjbvz removed their assignment Feb 20, 2019
@AnyhowStep
Copy link
Contributor

Should one even consider classes "regular" functions?

typeof will tell you it's of type function but I feel like, in general, allowing the assignment is just asking for trouble.

If I have class Foo, what does it even mean to call Foo()?

But I guess there is no propTypes.class and no real way to know if a function is meant to be a class constructor during run time.

It could be changed to allow constructors but that would just break types for people who expect to be able to call someFunc() and not risk a class being passed

@jcready
Copy link

jcready commented Feb 21, 2019

Should one even consider classes "regular" functions?

Before ES6 all classes were just regular functions.

@weswigham
Copy link
Member

weswigham commented Feb 21, 2019

I'd file this over at Definitely Typed, where the type definitions lie. PropTypes.func specifically requires a function type, which TS recognizes distinctly from construct-able types. If you wanted func to validate against constructors, too, it'd need to be defined as Requireable<(...args: any[]) => any | new (...args: any[]) => any> instead of Requireable<(...args: any[]) => any>.

@weswigham
Copy link
Member

Though @AnyhowStep is definitely right - it's unsafe to simply call something that's only been validated as "functiony" since, as he states, a class would throw when called. @kiyutink it might be better to use instanceOf if you know the base of the component, or possibly oneOf(func, instanceOf(Component)).

@weswigham
Copy link
Member

weswigham commented Feb 21, 2019

I think prop-types defines an elementType validator which is actually exactly what you want (a Validator<React.Component>), but is currently missing from the declaration file. Should probably go add it, tbh. Someone put up a PR for it, so it should be published soonish.

@RyanCavanaugh RyanCavanaugh added the External Relates to another program, environment, or user action which we cannot control. label Feb 27, 2019
@typescript-bot
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Relates to another program, environment, or user action which we cannot control.
Projects
None yet
Development

No branches or pull requests

7 participants