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

False positive on react/no-unused-prop-types #1764

Closed
guioconnor opened this issue Apr 11, 2018 · 6 comments · Fixed by #2301
Closed

False positive on react/no-unused-prop-types #1764

guioconnor opened this issue Apr 11, 2018 · 6 comments · Fixed by #2301

Comments

@guioconnor
Copy link

I came across some code looking like this where myProp was reportedly not being used, even though it was.

render() {
    const { props } = this;
    const { myProp } = props;
    someFunction(myProp)
}

MyComponent.propTypes = {
  myProp: PropTypes.number,
};

It seems the double desctructure confused the rule because a change to the code below fixed it.

render() {
    const { myProp } = this.props;
    someFunction(myProp)
}

I think it may be similar to #933, except this one is not such an anti-pattern. It looks very strange on my reduced version, but in a context with more variables can be more natural.

@ljharb
Copy link
Member

ljharb commented Apr 11, 2018

I think destructuring props off of this is pretty rare, but i agree it should work.

@guioconnor
Copy link
Author

guioconnor commented Apr 11, 2018

In my case the code was better after the fix, but I agree it should work.

@philip-peterson
Copy link

Hello! Just showing my support for this bug to be fixed :-) 👍

@matthargett
Copy link

Running into this all over our codebase. Here's a reduction of one of the cases, first the output, then the code:

C:\w\rnps-home-ui\packages\home-ui\bug.js
  5:13  error  'maxItems' PropType is defined but prop is never used                react/no-unused-prop-types
  6:27  error  'itemOpacityOutputRange' PropType is defined but prop is never used  react/no-unused-prop-types

  7:27  error  'textOpacityOutputRange' PropType is defined but prop is never used  react/no-unused-prop-types

code:

// @flow
import * as React from 'react';

type Props = {
  maxItems: number,
  itemOpacityOutputRange: Array<number>,
  textOpacityOutputRange: Array<number>,
  itemScaleOutputRange: Array<number>,
  children: Object => React.Node,
};

type State = {
  foo: number,
  bar: number,
};

function computeStateFromProps(props: Props) {
  const { maxItems, itemOpacityOutputRange, textOpacityOutputRange, itemScaleOutputRange } = props;

  if (maxItems && itemOpacityOutputRange && textOpacityOutputRange && itemScaleOutputRange) {
    return { foo: 0, bar: 0 };
  }
  return { foo: 1, bar: 0 };
}

export default class X extends React.PureComponent<Props, State> {
  constructor(props: Props) {
    super(props);
    this.state = computeStateFromProps(props);
  }

  animateItem = (index: ?number) => {
    const { itemScaleOutputRange } = this.props;
    if (itemScaleOutputRange && index) return 0;
    return 1;
  };

  render() {
    const {
      animateItem,
      props: { children },
    } = this;

    return <>{children({ animateItem })}</>;
  }
}

matthargett added a commit to matthargett/eslint-config-callstack that referenced this issue May 31, 2019
We ran into a few false positives with this rule, and it turns out there are over a dozen issues filed in eslint-plugin-react and a few unmerged PRs. Disabling it for now.

I sifted through the numerous false positive issues, and added a reduction of one of our cases that seemed to match:
jsx-eslint/eslint-plugin-react#1764
@golopot
Copy link
Contributor

golopot commented May 31, 2019

@matthargett The rule only considered props.foo as used if it is used directly inside the component. Passing the props to a function and use props.foo will not work. You can work around it by doing:

this.state = computeStateFromProps({
  maxItems: props.maxItems,
  itemOpacityOutputRange: props.itemOpacityOutputRange,
  ...
})

@ljharb
Copy link
Member

ljharb commented May 31, 2019

Passing the props or state object around is a massive antipattern; always extract the props you need directly, and then pass them separately, or as a new object, to other functions.

thymikee pushed a commit to callstack/eslint-config-callstack that referenced this issue May 31, 2019
We ran into a few false positives with this rule, and it turns out there are over a dozen issues filed in eslint-plugin-react and a few unmerged PRs. Disabling it for now.

I sifted through the numerous false positive issues, and added a reduction of one of our cases that seemed to match:
jsx-eslint/eslint-plugin-react#1764
kodiakhq bot pushed a commit to mcansh/connection that referenced this issue Jul 26, 2019
## The devDependency [eslint-plugin-react](https://github.com/yannickcr/eslint-plugin-react) was updated from `7.13.0` to `7.14.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v7.14.0</summary>

<h3>Added</h3>
<ul>
<li>Add <code>jsx-curly-newline</code> rule (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1493" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1493/hovercard">#1493</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Add support for nested destructuring to <code>prop-types</code> (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/296" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/296/hovercard">#296</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1422" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1422/hovercard">#1422</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Add support for variables defined as props to <code>prop-types</code> and <code>no-unused-prop-types</code> (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/442" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/442/hovercard">#442</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/833" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/833/hovercard">#833</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1002" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1002/hovercard">#1002</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1116" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1116/hovercard">#1116</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1257" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1257/hovercard">#1257</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1764" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1764/hovercard">#1764</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Add <code>checkFragmentShorthand</code> option to <code>jsx-key</code> (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2316" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2316/hovercard">#2316</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=19822240" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/kaykayehnn">@kaykayehnn</a>)</li>
</ul>
<h3>Fixed</h3>
<ul>
<li>Fix <code>no-did-mount-set-state</code> and <code>no-did-update-set-state</code> to handle cDU and cDM defined as class properties (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1595" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1595/hovercard">#1595</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=1413255" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/jaaberg">@jaaberg</a>)</li>
<li>Fix <code>sort-prop-types</code> cash when a shape PropType is defined in a variable (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1749" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1749/hovercard">#1749</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=93752" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/alexzherdev">@alexzherdev</a>)</li>
<li>Fix <code>no-unused-state</code> false positive when using state of non-lifecycle method (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2274" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2274/hovercard">#2274</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Fix <code>static-property-placement</code> false positive when accessing static property inside method (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/2283" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/2283/hovercard">#2283</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=20278756" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/dmason30">@dmason30</a>)</li>
<li>Fix <code>prop-type</code> detection for annotated props with default value (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/2298" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/2298/hovercard">#2298</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=13209" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/yannickcr">@yannickcr</a>)</li>
</ul>
<h3>Changed</h3>
<ul>
<li>Add ESLint 6.0.0 as valid peerDependency (<a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=13209" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/yannickcr">@yannickcr</a>)</li>
<li>Improve <code>no-render-return-value</code> performance (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2259" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2259/hovercard">#2259</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Change <code>jsx-sort-props</code> to report errors only on the identifier (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/2312" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/2312/hovercard">#2312</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=74260" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/MrHen">@MrHen</a>)</li>
<li>Change to warn only once if react version cannot be detected (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/2276" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/2276/hovercard">#2276</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=45469" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/ljharb">@ljharb</a>)</li>
<li>Documentation improvements (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2263" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2263/hovercard">#2263</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=15232461" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/dimitropoulos">@dimitropoulos</a>, <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2262" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2262/hovercard">#2262</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=473530" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/ybiquitous">@ybiquitous</a>, <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2295" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2295/hovercard">#2295</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=1921409" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/battaglr">@battaglr</a>, <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2302" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2302/hovercard">#2302</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=5185660" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/Jason-Cooke">@Jason-Cooke</a>, <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2303" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2303/hovercard">#2303</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>)</li>
<li>Code refactoring (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2265" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2265/hovercard">#2265</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2267" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2267/hovercard">#2267</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2286" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2286/hovercard">#2286</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2294" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2294/hovercard">#2294</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>, <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=45469" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/ljharb">@ljharb</a>)</li>
<li>Tests improvements (<a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/pull/2304" data-hovercard-type="pull_request" data-hovercard-url="/jsx-eslint/eslint-plugin-react/pull/2304/hovercard">#2304</a> <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/issues/1047" data-hovercard-type="issue" data-hovercard-url="/jsx-eslint/eslint-plugin-react/issues/1047/hovercard">#1047</a> <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=16285118" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/golopot">@golopot</a>, <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/hovercards?user_id=13209" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://urls.greenkeeper.io/yannickcr">@yannickcr</a>)</li>
</ul>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 68 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/dfaa92f34ac39fa8c320068501ec86fe0b9c8122"><code>dfaa92f</code></a> <code>Update CHANGELOG and bump version</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/c52b61b0d371b956b6c6fef3240839f4cea2ffa7"><code>c52b61b</code></a> <code>Merge pull request #2316 from kaykayehnn/jsx-key-fragments</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/8db631b400b242266be38202926d86c244e8d116"><code>8db631b</code></a> <code>[Fix] Fix detection of annotated props with default value</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/bbebefd2292294e892f743993f2cc778e3e36a85"><code>bbebefd</code></a> <code>[Tests] Remove AppVeyor</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/0d49f5abd1d5423a7a305d26cad9a7f5b046bc89"><code>0d49f5a</code></a> <code>[New] Add ESLint ^6.0.0 as valid peerDependency</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/0364ed210b95f1a14bda0180a3d194cf9e6b7176"><code>0364ed2</code></a> <code>Fix formatting issues</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/7c1abedb575bd97404933ec85144697a30778443"><code>7c1abed</code></a> <code>Add checkFragmentShorthand option</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/ed04c2fd48a1c1ad21daade6225229e281446c6a"><code>ed04c2f</code></a> <code>Fix tests in eslint &lt; 5</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/0d1aaf8b569e6d22bba90467fe46ee7062e5e5ba"><code>0d1aaf8</code></a> <code>Handle fragments in jsx-key</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/7d449a9e4a1330ab950df4b55348d6eff5d025af"><code>7d449a9</code></a> <code>[New] <code>jsx-sort-props</code>: Change reported range to only the identifier</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/1e102f0d3281c24cd1b99c27bdbf63c14d6bcde2"><code>1e102f0</code></a> <code>Change reported range to only the identifier</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/e6b4c33a1db4cc94c3e9223b09fb92b1dbddc00d"><code>e6b4c33</code></a> <code>Merge pull request #2301 from golopot/fix-cached-props-2</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/9a63e19460e7ddbf988f328cf9f9730f4f156e0d"><code>9a63e19</code></a> <code>Immediately destructure out propVariables rather than using it as a namespace</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/3a1a0d1d159efd6bb3c597671e4ec8afaee22018"><code>3a1a0d1</code></a> <code>Apply suggestion: replace mutation with Object.assign</code></li>
<li><a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/commit/89b8143ac18f4d3f0d45717436ad2aa344e2d494"><code>89b8143</code></a> <code>Apply suggestion: replace concat([a]) with concat(a)</code></li>
</ul>
<p>There are 68 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/yannickcr/eslint-plugin-react/compare/f39829ffb3134fb1298c7e96a4349eb835f15877...dfaa92f34ac39fa8c320068501ec86fe0b9c8122">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴
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.

5 participants