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

react/no-used-prop-types error thrown when a prop is only used in nextProps #1142

Closed
mjilugu opened this issue Apr 7, 2017 · 11 comments
Closed
Labels

Comments

@mjilugu
Copy link

mjilugu commented Apr 7, 2017

When we use a prop only with nextProps to set the state in the componentWillRecieveProps(nextProps) method and not elsewhere we get react/no-used-prop-types error for the prop in the propTypes block.

Same as #801

Here is an excerpt of my code:

class MyComponent extends React.Component {

  static propTypes = {
    isLoading: React.PropTypes.bool, // eslint-disable-line react/no-unused-prop-types
    error: React.PropTypes.bool, // eslint-disable-line react/no-unused-prop-types
  }

  static defaultProps = {
    isLoading: true,
    error: false,
  };

  constructor(props) {
    super(props);

    this.state = {
      isLoading: true,
      error: false,
    };
  }

  componentWillReceiveProps(props) {
    if (props.error) {
      this.setState({ ...this.state, error: props.error });
    } else {
      this.setState({
        ...this.state,
        isLoading: props.isLoading,
        error: false,
      });
    }
  }

  // the rest of the code
}

Notice I have to use // eslint-disable-line react/no-unused-prop-types to avoid the warnings

@ljharb ljharb added the bug label Apr 7, 2017
@borwahs
Copy link

borwahs commented Apr 25, 2017

@mjilugu & @ljharb -

Just as an FYI, I found that if you destructure nextProps, the linter does not throw an error in this scenario. If you use dot notation, it fails.

I also confirmed others have seen similar results in this thread: #884 (comment)

@joetidee
Copy link

Please can you fix this - having the same issue :(

@jseminck
Copy link
Contributor

I could try and take a look at this. Not sure how far I will get.

I see that the same issue exists with shouldComponentUpdate and nextProps: #1213

@jseminck
Copy link
Contributor

jseminck commented May 29, 2017

Hi,

This issue seems fixed in #1106 - E.g. it should work on the 7.0.0 release (which was released after this issue was created.). @joetidee can you please let me know what version you use and what is failing for you?

Just to be clear, the example code from the original issue works. See below for a more concise example:

The following example passes eslint-plugin-react test cases so it should work

class Hello extends Component {
  static propTypes = {
    foo: PropTypes.string
    bar: PropTypes.string
  };

  componentWillReceiveProps (nextProps) {
    if (nextProps.foo) {
      return true;
    }
  }

  render() {
    return (<div>{this.props.bar}</div>);
  }
}

The following example does NOT pass eslint-plugin-react test cases so it should NOT work

class Hello extends Component {
  static propTypes = {
    foo: PropTypes.string
    bar: PropTypes.string
  };

  shouldComponentUpdate (nextProps) { // replace componentWIllReceiveProps with shouldComponentUpdate
    if (nextProps.foo) {
      return true;
    }
  }

  render() {
    return (<div>{this.props.bar}</div>);
  }
}

I identified a few issues:

  • The parameter has to be named either props or nextProps. Calling it foo will not work. (not sure if there is a rule for that)
  • We only support nextProps for componentWillReceiveProps, but not for other React lifecycle methods (e.g. shouldComponentUpdate(nextProps) and componentDidUpdate(nextProps)
  • We also do not support componentDidUpdate(prevProps) due to a combination of both points above.

I'll get one or a few PR-s out for this within the next few days. As I believe we might as well fix all three points but it may be better to handle them one issue at a time. :)

@joetidee
Copy link

Excellent - thanks - have upgraded to 7.0.0 and now fixed.

@ljharb
Copy link
Member

ljharb commented Jun 2, 2017

#1232 hopefully has solved this; we can reopen after the next release if not.

@ljharb ljharb closed this as completed Jun 2, 2017
@ljharb
Copy link
Member

ljharb commented Jan 8, 2018

@mqklin could you file a new issue about that?

@mqklin
Copy link

mqklin commented Jan 8, 2018

@ljharb there is already: #814

@pkhodaveissi
Copy link

pkhodaveissi commented May 25, 2019

still not working here
I have componentWillReceiveProps but I don't use the prop there, I pass nextProps to a function call inside the lifecycle method and then in there I go ahead and access the prop via dot notation or object destruction, but next to the prop declaration inside PropsType(using flow) I see 'helperAppStatus' PropType is defined but prop is never used eslint(react/no-unused-prop-types)

using 7.12.4

@ljharb
Copy link
Member

ljharb commented May 25, 2019

@pkhodaveissi if you pass a props or state object around, you’ve both defeated all static analysis and also, you’ve given up control of a mutable object that your component depends on, a bad practice.

Always extract every prop/state from the objects immediately, and pass those around.

@pkhodaveissi
Copy link

pkhodaveissi commented May 25, 2019 via email

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

7 participants