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: All fields reported as unused with PropTypes.arrayOf(PropTypes.exact) #2850

Closed
asbjornh opened this issue Nov 4, 2020 · 5 comments · Fixed by #2883
Closed

Comments

@asbjornh
Copy link

asbjornh commented Nov 4, 2020

It seems like when a list of objects is typed using PropTypes.arrayOf(PropTypes.exact({ ... })) all properties of the inner object are reported as not in use by no-unused-prop-types regardless of them being used or not. This is not the case with PropTypes.arrayOf(PropTypes.shape({ ... })).

I would expect PropTypes.arrayOf(PropTypes.exact) and PropTypes.arrayOf(PropTypes.shape) to behave the same when checking for unused properties
I would expect the two below cases to have no errors as they are very similar. However, case A has no errors, while case B reports that a.*.b is not in use.

How to reproduce

Case A:

No errors reported

const Comp = ({ a }) => <div>{a.map(({ b }) => <div>{b}</div>)}</div>;

Comp.propTypes = {
  a: PropTypes.arrayOf(PropTypes.shape({ b: PropTypes.string }))
};

Case B:

Error: 'a.*.b' PropType is defined but prop is never used

const Comp = ({ a }) => <div>{a.map(({ b }) => <div>{b}</div>)}</div>;

Comp.propTypes = {
  a: PropTypes.arrayOf(PropTypes.exact({ b: PropTypes.string }))
};

.eslintrc.json

{
  "parser": "babel-eslint",
  "plugins": ["react"],
  "rules": {
    "react/no-unused-prop-types": 1
  }
}

package.json

{
  "dependencies": {
    "babel-eslint": "^10.1.0",
    "eslint": "^7.12.1",
    "eslint-plugin-react": "^7.21.5"
  }
}
@ljharb
Copy link
Member

ljharb commented Nov 4, 2020

I definitely would not expect them to behave the same - in a shape, all properties are optional unless .isRequired, and the properties in a shape don't need to be used for "the shape matches this interface" to be useful.

@asbjornh
Copy link
Author

asbjornh commented Nov 4, 2020

Sorry, I think I've explained myself badly.

It's unexpected that case B above produces an error for no-unused-prop-types when the property b clearly is in use.

Am I misunderstanding something?

@ljharb
Copy link
Member

ljharb commented Nov 5, 2020

ahhh, yes, I agree - case B should not be producing any warnings.

@asbjornh
Copy link
Author

asbjornh commented Nov 5, 2020

I'm trying to take a look at this, but I'm seeing something weird.

const Comp = ({ a, c, e, g }) => <div>
  {a.map((props) => <div>{props.b}</div>)}
  {c.map(({ d }) => <div>{d}</div>)}
  {e.map((props) => <div>{props.f}</div>)}
  {g.map(({ h }) => <div>{h}</div>)}
</div>;
Comp.propTypes = {
  a: PropTypes.arrayOf(PropTypes.shape({ b: PropTypes.string })),
  c: PropTypes.arrayOf(PropTypes.shape({ d: PropTypes.string })),
  e: PropTypes.arrayOf(PropTypes.exact({ f: PropTypes.string })),
  g: PropTypes.arrayOf(PropTypes.exact({ h: PropTypes.string })),
};

Using the versions and config from my first post, the above code results in one error when running eslint:

11:42  warning  'g.*.h' PropType is defined but prop is never used  react/no-unused-prop-types

However, If I add the same code to tests/lib/no-unused-prop-types.js I get these two errors:

      AssertionError [ERR_ASSERTION]: Should have no errors but had 2: [
  {
    ruleId: 'no-unused-prop-types',
    severity: 1,
    message: "'c.*.d' PropType is defined but prop is never used",
    line: 9,
    column: 42,
    nodeType: 'Identifier',
    endLine: 9,
    endColumn: 43
  },
  {
    ruleId: 'no-unused-prop-types',
    severity: 1,
    message: "'g.*.h' PropType is defined but prop is never used",
    line: 11,
    column: 42,
    nodeType: 'Identifier',
    endLine: 11,
    endColumn: 43
  }
]

The currently published version (7.21.5) seems to support destructuring of properties in .map for PropTypes.shape, but not for PropTypes.exact. In master there doesn't seem to be support for destructured properties in .map for either shape or exact. Has support for destructuring objects in a .map been dropped on purpose since the latest release or something like that?

EDIT:
It doesn't seem like this is an issue for class components at all. The below code doesn't produce errors with eslint or when added to test/lib/no-unused-prop-types.js:

class Comp {
  render() {
    return (<div>
      {this.props.a.map((props) => <div>{props.b}</div>)}
      {this.props.c.map(({ d }) => <div>{d}</div>)}
      {this.props.e.map((props) => <div>{props.f}</div>)}
      {this.props.g.map(({ h }) => <div>{h}</div>)}
    </div>);
  }
}
Comp.propTypes = {
  a: PropTypes.arrayOf(PropTypes.shape({ b: PropTypes.string })),
  c: PropTypes.arrayOf(PropTypes.shape({ d: PropTypes.string })),
  e: PropTypes.arrayOf(PropTypes.exact({ f: PropTypes.string })),
  g: PropTypes.arrayOf(PropTypes.exact({ h: PropTypes.string })),
};

@ljharb
Copy link
Member

ljharb commented Nov 5, 2020

It is a regression if destructured props no longer work, and it seems reasonable to ensure they do work for both exact and shape.

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

Successfully merging a pull request may close this issue.

2 participants