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 - cannot detect usage within componentWillReceiveProps #884

Closed
ctumolosus opened this issue Oct 4, 2016 · 9 comments

Comments

@ctumolosus
Copy link

ctumolosus commented Oct 4, 2016

I have a component where I'm only referencing a prop within componentWillReceiveProps(nextProps) and react/no-unused-prop-types is not picking it up.
It's probably a bad practice that I need to revisit on my end, but wondering if you guys think it's something the rule should handle.

@ljharb
Copy link
Member

ljharb commented Oct 4, 2016

Can you provide the exact code in question?

@ctumolosus
Copy link
Author

ctumolosus commented Oct 4, 2016

Below is a simplified example. We have a custom higher-order component that handles form submission, validation, etc. and it injects a hasSubmitted property (among other properties) into the wrapped component when submitted successfully. The form is not editable until the edit button is pressed and its editable state is reset when submitted. It's trivial to refactor this example to satisfy the rule, but I'm curious if the rule should handle this scenario.

import React, { Component, PropTypes } from 'react';

export default class UserForm extends Component {

  static propTypes = {
    firstName: PropTypes.string,
    lastName: PropTypes.string,
    onSubmit: PropTypes.func,
    hasSubmitted: PropTypes.bool,
  }

  static defaultProps = {
    hasSubmitted: false,
  }

  state = {
    isEditable: false,
  };

  componentWillReceiveProps(nextProps) {
    if (nextProps.hasSubmitted) {
      this.setState({ isEditable: false });
    }
  }

  handleEdit = () => {
    this.setState({ isEditable: true });
  }

  render() {
    return (
      <form onSubmit={this.props.onSubmit}>
        <label htmlFor="firstName">First Name</label>
        <input type="text" id="firstName" name="firstName" value={this.props.firstName} disabled={!this.state.isEditable} />
        <label htmlFor="lastName">Last Name</label>
        <input type="text" id="lastName" name="lastName" value={this.props.lastName} disabled={!this.state.isEditable} />
        {this.state.isEditable ? (
          <div>
            <button>Cancel</button>
            <button type="submit">Update</button>
          </div>
        ) : (
          <div>
            <button onTouchTap={this.handleEdit}>Edit</button>
          </div>
        )}
      </form>
    );
  }
}

@EvHaus
Copy link
Collaborator

EvHaus commented Oct 6, 2016

I'm unable to reproduce the issue using the code your provided. The rule currently supports prop checking for componentWillReceiveProps.

Can you confirm you're running the latest version of eslint-plugin-react (6.3.0)

@ctumolosus
Copy link
Author

ctumolosus commented Oct 6, 2016

Here is repository that replicates the issue using the latest eslint-plugin-react package: https://github.com/ctumolosus/eslint-plugin-react-issue-884

If you run npm run lint after installing all dependencies you should see the error. Please let me know if there is anything else I can do to help.

@marcgrr
Copy link

marcgrr commented Oct 14, 2016

I think I have encountered this same bug. I noticed that the bug only appears if I access the prop with a dot (nextProps.hasSubmitted). If I access the prop with destructuring (const { hasSubmitted } = nextProps), then react/no-unused-prop-types correctly recognizes the access.

I'm going to see if I can make a PR fixing this!

marcgrr added a commit to marcgrr/eslint-plugin-react that referenced this issue Oct 17, 2016
@marcgrr marcgrr mentioned this issue Oct 17, 2016
@imontiel
Copy link

imontiel commented Dec 9, 2016

It looks like you have to destructure the props from nextProps – and it has to be nextProps. It can't be any other name.

@DianaSuvorova
Copy link
Contributor

DianaSuvorova commented Jul 28, 2017

This is fixed.
@ljharb I'd be happy do it myself if you give me rights to close tickets.

Thanks :)

@ljharb
Copy link
Member

ljharb commented Jul 28, 2017

That's not up to me :-) thanks for all your triage help!

@ljharb ljharb closed this as completed Jul 28, 2017
@nischalashashidhar
Copy link

@marcgrr , Thanks.. It worked for me as well.

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

No branches or pull requests

7 participants