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

prop-types not assigned via object literal do not work #355

Closed
kesne opened this issue Dec 8, 2015 · 8 comments
Closed

prop-types not assigned via object literal do not work #355

kesne opened this issue Dec 8, 2015 · 8 comments

Comments

@kesne
Copy link

kesne commented Dec 8, 2015

The Airbnb styleguide recommends putting proptypes in a variable at the top of the file, and then assigning it into propTypes. This causes the linting of propTypes to fail.

The following does not cause any linting errors, despite not including the proptypes validation.

const propTypes = {};

class Test extends React.Component {
  render() {
    return (
      <div>{this.props.name}</div>
    );
  }
}

Test.propTypes = propTypes;
@ljharb
Copy link
Member

ljharb commented Dec 8, 2015

Ideally either the rule should require inline propTypes, or should be able to determine what they are when assigned via a variable.

@yannickcr
Copy link
Member

This pattern is difficult to handle, and currently if you're using it the rule will totally ignore props validation for this component.

@ljharb
Copy link
Member

ljharb commented Dec 20, 2015

@yannickcr a silent failure is the worst possible way to handle it imo - if the rule can't handle it, then can it please raise an error, saying propTypes are unable to be validated?

If the rule can't handle this case, then it's useless, and I'd rather disable it completely than have it give false confidence :-/

(I recognize that if the object is imported or something it'd be difficult, but in this use case it's an object literal elsewhere in the file, so i'd think the info would be in the AST that esprima generates)

@yannickcr
Copy link
Member

@ljharb since the patterns used in the React community are very diversified it is hard (impossible?) to cover everything. And I prefer to have certain errors silenced than having some false-positive cases that can break a build because the linter does not understand the used pattern (which does not indicate that the code is not correct). But maybe this can be a configurable setting?

Also about this current issue, after investigation it seems not so difficult to do after all (at least for simple cases like in @kesne example). So i'll try to submit a patch for this :)

@ljharb
Copy link
Member

ljharb commented Dec 20, 2015

Awesome! Our use case at airbnb is just a straight object literal, that's referenced higher in the file, so hopefully that's achievable :-)

For me, a linter is a gate - i consider a pattern the linter doesn't understand to be forbidden, until the linter can be adapted to handle it (by allowing it or recognizing it). An option to enable this behavior would be great too!

@ljharb
Copy link
Member

ljharb commented Dec 20, 2015

Thanks, this is excellent! Please let me know when it's released :-D

@ljharb
Copy link
Member

ljharb commented Dec 27, 2015

Looks like this was out in 3.13.0 2 days ago. Thanks!

@yannickcr
Copy link
Member

Right, it was released in v3.13.0, sorry for not having informed you earlier.

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

No branches or pull requests

3 participants