Skip to content

Rule Proposal: no-unused-prop-types #28

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
MitchLillie opened this issue Sep 20, 2018 · 5 comments
Closed

Rule Proposal: no-unused-prop-types #28

MitchLillie opened this issue Sep 20, 2018 · 5 comments

Comments

@MitchLillie
Copy link

There is a lot of discussion in eslint-plugin-react about the no-unused-prop-types rule. It seems to cause plenty of users issues, but especially Redux users, as it does not make an attempt to check mapStateToProps, mapDispatchToProps, or mergeProps for props that are used. This rule should completely replace react/no-unused-prop-types.

The following would be an example of correct code for this rule:

import React, { Component } from 'react';
export const mapStateToProps = (state, ownProps) => ({
  myData: getMyData(state, ownProps.myProp),
});

export class MyComponent extends Component {
  render = () => (
    <MyOtherComponent
      myData={this.props.myData}
    />
  )
}

MyComponent.propTypes = {
  myProp: PropTypes.string.isRequired
};

export default connect(mapStateToProps)(MyComponent);
@ljharb
Copy link
Collaborator

ljharb commented Sep 20, 2018

You may be able to use eslint-rule-composer for this, and avoid having to re-implement the bulk of the rule.

@DianaSuvorova
Copy link
Owner

Interesting, I haven't heard of eslint-rule-composer- seems like a very handy until. let me see if having no-unused-prop-types rule on redux side can solve problems people have with the react's version of it.

@DianaSuvorova
Copy link
Owner

looks like it should pretty straightforward to correctly decide on props usage for the example above (with the help of eslint-rule-composer).
Although, in my experience it is very common practice to have component and redux container in separate files. @ljharb do you know if it is possible/good idea to write eslint rule potentially spanning multiple files?

@ljharb
Copy link
Collaborator

ljharb commented Sep 30, 2018

The only plugin that i know looks at multiple files is the import plugin - and it only does that to build up a dependency map.

I don’t actually think we have any code at Airbnb where the redux container and the component it wraps are separate; either way, i think that’s not something you need to handle in the first iteration.

@DianaSuvorova
Copy link
Owner

@ljharb , @MitchLillie I am planning to merge #31, lmk if you have any comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants