The prop-types rule fails when presented with destructuring:
package.json
{
"dependencies": {
"eslint": "^4.6.1",
"eslint-plugin-react": "^7.3.0",
"proptypes": "^1.1.0",
"react": "^15.6.1"
}
}
.eslintrc
{
"env": {
"es6": true,
"node": true
},
"parserOptions": {
"sourceType": "module"
},
"plugins": ["eslint-plugin-react"],
"rules": {
"react/prop-types": 2
}
}
Examples.jsx
import React from 'react';
import PropTypes from 'proptypes';
// FAIL: Should error that `bar` is missing in propTypes
class DestructuredDeepMissing extends React.PureComponent {
render () {
const {
foo: {
bar,
},
} = this.props;
const it = bar;
}
}
DestructuredDeepMissing.propTypes = {
foo: PropTypes.shape({}).isRequired,
};
// Interestingly, non-deep destructuring works just fine
// PASS: Should error
class DestructuredMissing extends React.PureComponent {
render () {
const {
foo,
} = this.props;
const it = foo;
}
}
DestructuredMissing.propTypes = {};
Related: #296
Possibly related: #1346
Also possibly related is that no-unused-prop-types (without using Flow) fails on destructuring as well, which is behavior I was able to replicate (though there seem to be a plethora of tickets related to this, especially in the Flow context).
Shape props have known bugs; do you have skipShapeProps enabled?
@ljharb no - we use Relay, so basically all of our props are within a shape (viewer.x, or deeper). Enabling that option would be akin to turning this rule off for us. I know that no-unused-prop-types has issues with shapes since it mentions that in the description, but there is no such warning in the prop-types rule (though makes sense that the two rules would share some issues).
We have this test case:
code: [
'class Hello extends React.Component {',
' render() {',
' this.props.a.b',
' return <div>Hello</div>;',
' }',
'}',
'Hello.propTypes = {',
' a: PropTypes.shape({',
' })',
'};'
].join('\n'),
errors: [{
message: '\'a.b\' is missing in props validation'
}]
Judging from that, it seems some basic shapes support exists and the example should actually be supported.
Indeed when I try the following snippet I actually get an error. But the destructuring doesn't work with the shapes...
class DestructuredDeepMissing extends React.PureComponent {
render () {
this.props.foo.bar;
}
}
DestructuredDeepMissing.propTypes = {
foo: PropTypes.shape({}).isRequired,
};
I have sort of the same error, but not with any PropTypes.shape, but with a rest a destructuring.
class MyComponent extends React.Component {
static displayName = 'MyComponent';
static propTypes = {
name: PropTypes.string
}
render() {
const { name, title, ...rest } = this.props;
return (
<div {...rest}>
<h1>{name}</h1>
</div>
);
}
}
Here it fails to detect title as not defined in the propTypes.
I have narrowed it down to the spread on the JSX element: <div {...rest}>. When this is enabled the detection turns off. This is event if the spread happens in a separate context (ie. another method) like this:
class MyComponent extends React.Component {
static displayName = 'MyComponent';
static propTypes = {
name: PropTypes.string
}
renderName() {
const { name, ...rest } = this.props;
return <h1 {...rest}>{name}</h1>;
}
render() {
const { title } = this.props;
return (
<div>
{title}
{this.renderName()}
</div>
);
}
}
In this example, if I remove the {...rest} from <h1 {...rest}> the title is correctly being marked as "not defined in prop-types".
That seems like an improvement to be made.
@ljharb looked into this tonight locally with latest master, it actually looks like this has been fixed in a commit between last release and today. The code above is erroring correctly. I believe this should be fixed when you publish a new version. Can you confirm?
https://github.com/yannickcr/eslint-plugin-react/issues/1958#issuecomment-421091965
Thanks
oh, great! Could you open a PR with these test cases, and then we can close this issue?
Sure! @ljharb
@ljharb https://github.com/yannickcr/eslint-plugin-react/pull/2029. When youre ready.
Let me know if you have any feedback.
Most helpful comment
@ljharb looked into this tonight locally with latest master, it actually looks like this has been fixed in a commit between last release and today. The code above is erroring correctly. I believe this should be fixed when you publish a new version. Can you confirm?
https://github.com/yannickcr/eslint-plugin-react/issues/1958#issuecomment-421091965
Thanks