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 })}</>;
  }
}

@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.

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