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

Rule idea: detect when unknown props are passed to a component #2071

Closed
cypherfunc opened this issue Dec 3, 2018 · 5 comments
Closed

Rule idea: detect when unknown props are passed to a component #2071

cypherfunc opened this issue Dec 3, 2018 · 5 comments

Comments

@cypherfunc
Copy link

My team really wants syntax highlighting in our editor (e.g. Webstorm) when a component is called with a prop that isn't defined in that component's propTypes.

e.g.

class MyComponent extends Component {
  ...
}

MyComponent.PropTypes = {
  foo: PropType.string,
};

...

<MyComponent
    bar={...} // this line would be marked as a warning/error
    foo={"some string"}
/>

Obviously this wouldn't catch props passed dynamically (e.g. <MyComponent {...someObject} />), but we think that's an anti-pattern anyway. (See #1094).

The widely-accepted solution seems to be https://npmjs.com/prop-types-exact, but adding the dependency and usage to each of our components would be a bit of a pain, and only gives run-time warnings.

related links:
https://stackoverflow.com/questions/48705910/does-react-warn-of-additional-props-that-a-component-wasnt-expecting

facebook/prop-types#11

@ljharb
Copy link
Member

ljharb commented Dec 3, 2018

This isn’t really possible to do with static analysis - but you can (and should) use https://npmjs.com/prop-types-exact and configure your tests to fail on propType warnings.

@cypherfunc
Copy link
Author

Webstorm/eslint already identifies when a component is called, and reads information from its propTypes (like whether a prop is required). It seems like all the necessary information is already used for other inspections/rules.

Although I'm starting to see how this would be limited; it could only be checked when a component is directly called (i.e. without being wrapped in HOCs), and props are not spread from an object.

class MyComponent extends Component {
    static propTypes = {foo: PropType.string};
    ...
}

...

<MyComponent
    bar={...} // this line would be marked as a warning/error
    foo={"some string"}
/>

...

const someObject = {bar: "something", foo: "another thing"};

<MyComponent
    {...someObject} // would not be marked, but could be disallowed with another rule.
/>

...

const MyWrappedComponent = withSomeHOC(MyComponent);

<MyWrappedComponent
    bar={...} // this line should be marked, but wouldnt be.
    foo={"some string"}
/>

The third example would be difficult/impossible to statically check, but is also avoidable by using render-props/child functions instead of HOCs.

<withSomeRenderProp
    render={(foo, bar) => (
        <MyComponent
            bar={bar} // this line would be correctly marked as a warning/error
            foo={foo}
        />
    )}
/>

@ljharb
Copy link
Member

ljharb commented Dec 4, 2018

Prop names can be statically checked within the component itself, because we can reasonably force the author to write them statically - but not all consumers will even necessarily be linted (think a published package, not a top-level app) or necessarily following the same linter config or code style (think different teams in a larger monorepo). Separately, adding cross-file linting is a huge complexity and performance burden, so even if it were a reliable approach, it’d not necessarily be a good idea.

@cypherfunc
Copy link
Author

Fair enough. Thanks for talking me through it; I was having a really hard time finding any info on why this rule didn't exist.

@ljharb
Copy link
Member

ljharb commented Dec 4, 2018

For what it's worth, I really wish this was possible - but since it's not, I'd prefer a rule like #1547/#1455

@ljharb ljharb closed this as completed Dec 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants