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

no-unused-prop-types does not recognize use of prop inside Array.map() #819

Closed
ghost opened this issue Sep 11, 2016 · 9 comments
Closed
Assignees
Labels

Comments

@ghost
Copy link

ghost commented Sep 11, 2016

In the following example, no-unused-prop-types will flag a non-use of arrayOfObjWithFoo.*.foo:

    static PropTypes = {
      arrayOfObjWithFoo: PropTypes.arrayOf(
        PropTypes.shape({ foo: PropTypes.string.isRequired })
      ).isRequired
    };

    render() {
      const fooItems = this.props.arrayOfObjWithFoo.map(objWithFoo => {
        <Item foo={objWithFoo.foo} />
      });
    }
@EvHaus
Copy link
Collaborator

EvHaus commented Sep 30, 2016

Duplicate of #871.

The no-unused-prop-types rule does not support shape props at the moment as such detection is very difficult. If you use shape props, I recommend setting the skipShapeProps option to true on the rule.

@ljharb
Copy link
Member

ljharb commented Sep 30, 2016

Again, this is still a bug. If it doesn't support shape props, it should not warn on them at all.

@EvHaus
Copy link
Collaborator

EvHaus commented Oct 1, 2016

If you set the skipShapeProps option to true, it should stop warning on all shape props. Is that not working?

@ljharb
Copy link
Member

ljharb commented Oct 1, 2016

I'm saying that if that option is required for the rule to be usable when using shapes, then the rule should never check shapes.

@EvHaus EvHaus self-assigned this Oct 1, 2016
@EvHaus
Copy link
Collaborator

EvHaus commented Oct 1, 2016

@ljharb That's a valid point. I'll look into whether or not it makes sense to set skipShapeProps to true as a default.

@merlinstardust
Copy link

I agree that skipShapeProps should default to true if it can't properly detect usage of props in shape.

The current behavior contradicts the forbid-prop-types rule since that encourages the use of shape instead of object.

@ljharb
Copy link
Member

ljharb commented Dec 31, 2016

@EvNaverniouk any progress here?

@EvHaus
Copy link
Collaborator

EvHaus commented Dec 31, 2016

@ljharb Just took a few minutes now to look into this but wasn't able to get far. This issue is talking about 2 things:

  1. The code snippet @smsenesac originally posted is supposedly throwing an error when it shouldn't.
    I cannot reproduce the failure. I added a test for this here: EvHaus@15e2fd8 but the test isn't failing for me. So I wonder if maybe this has already been fixed in another commit.
  2. Setting skipShapeProps to true by default.
    This is already done. See latest master code here: https://github.com/yannickcr/eslint-plugin-react/blob/master/lib/rules/no-unused-prop-types.js#L51

Don't think there are any further action items on this issue. Can we close?

@ljharb
Copy link
Member

ljharb commented Dec 31, 2016

Seems reasonable.

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

No branches or pull requests

4 participants