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

Destructuring allows me to bypass prop-types rule #104

Closed
jamischarles opened this issue Jun 10, 2015 · 9 comments
Closed

Destructuring allows me to bypass prop-types rule #104

jamischarles opened this issue Jun 10, 2015 · 9 comments

Comments

@jamischarles
Copy link
Contributor

When I use this rule:
"react/prop-types": 2,

This code

var testing = this.props.something;

yields this

  31:33  error  'something' is missing in props validation  react/prop-types

which I expect.

When I use destructuring however:

let {something} = this.props;

There is no eslint error.

@yannickcr
Copy link
Member

Destructuring is handled since v2.0.0 and prop-types should yields about the missing props validation.

With the following configuration I have the expected behavior with eslint 0.22.1 and eslint-plugin-react 2.5.0:

test.js

var Hello = React.createClass({
  render: function() {
    let {firstname} = this.props;
    return <div>Hello {firstname}</div>;
  }
});

.eslintrc

{
  "env": {
    "node": true,
    "es6": true
  },
  "ecmaFeatures": {
    "jsx": true
  },
  "plugins": [
    "react"
  ],
  "rules": {
    "react/prop-types": 2
  }
}
$ eslint --reset test.js

test.js
  3:9  error  'firstname' is missing in props validation  react/prop-types

✖ 1 problem (1 error, 0 warnings)

If you still have the problem, can you post your full source code and .eslintrc configuration ? Thanks.

@stremlenye
Copy link

Destructuring listing works fine for case with this.props

let {firstname} = this.props;

but rule can't handle case of destructuring this object:

let { props: { firstname } } = this

@yannickcr
Copy link
Member

@stremlenye

let { props: { firstname } } = this

You're right, this case is not handled right now, but this is not the case reported by @jamischarles

@davidgilbertson
Copy link

This may be related, or a new issue, but I have this problem:

eslint-plugin-react version: 2.6.2

eslint version: 0.24.0

In .eslintrc I have "react/prop-types": 2

My code: const {stores, section} = this.props.content;

And the propTypes:

Section.propTypes = {
    content: PropTypes.shape({
        section: PropTypes.object.isRequired,
        stores: PropTypes.object.isRequired
    })
};

Results in the error 'stores' is missing in props validation for Section react/prop-types (and same error for section)

If I define my propTypes like this I don't get an error, but this is not what my propTypes should look like.

Section.propTypes = {
    content: PropTypes.object,
    stores: PropTypes.object,
    section: PropTypes.object
};

Of course if I define my consts like this I also don't get the error:

const stores = this.props.content.stores;
const section = this.props.content.section;

@stremlenye
Copy link

As I see it's two different issues.
The first is about destructuring, the second is handling shape clause.
As I've got from the estree docs there are different mechanics to investigate if property was supplied to shape func or as plain object.
As for me to get such deep analysis is out of the scope of the eslint rule. It looks like a static type check.

@yannickcr
Copy link
Member

Yep, it seems there is a lot of different prop-types issues here, I'll try to clean this up:

@jamischarles I will close this issue for now, feel free to re-open it if you have more information to share about the problem you encountered and how to reproduce it.

@stremlenye
Copy link

Thx @yannickcr
Do you need a new issue to be created for my clause?

@yannickcr
Copy link
Member

Do you talk about the let { props: { firstname } } = this problem ?

If it's the case, no it is not needed since the issue was already resolved.

@davidgilbertson
Copy link

Thanks for the quick reply, guys. Created issue #136

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

4 participants