Skip to content

Commit

Permalink
[Fix] require-default-props: report when required props have defaul…
Browse files Browse the repository at this point in the history
…t value
  • Loading branch information
akulsr0 authored and ljharb committed Jul 18, 2024
1 parent a08cb93 commit a4b0bbc
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 8 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
### Fixed
* [`no-invalid-html-attribute`]: substitute placeholders in suggestion messages ([#3759][] @mdjermanovic)
* [`sort-prop-types`]: single line type ending without semicolon ([#3784][] @akulsr0)
* [`require-default-props`]: report when required props have default value ([#3785][] @akulsr0)

### Changed
* [Refactor] `variableUtil`: Avoid creating a single flat variable scope for each lookup ([#3782][] @DanielRosenwasser)

[#3785]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3785
[#3784]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3784
[#3782]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3782
[#3777]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3777
Expand Down
31 changes: 23 additions & 8 deletions lib/rules/require-default-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ const messages = {
destructureInSignature: 'Must destructure props in the function signature to initialize an optional prop.',
};

function isPropWithNoDefaulVal(prop) {
if (prop.type === 'RestElement' || prop.type === 'ExperimentalRestProperty') {
return false;

Check warning on line 29 in lib/rules/require-default-props.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/require-default-props.js#L29

Added line #L29 was not covered by tests
}
return prop.value.type !== 'AssignmentPattern';
}

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
meta: {
Expand Down Expand Up @@ -134,15 +141,23 @@ module.exports = {
});
}
} else if (props.type === 'ObjectPattern') {
// Filter required props with default value and report error
props.properties.filter((prop) => {
if (prop.type === 'RestElement' || prop.type === 'ExperimentalRestProperty') {
return false;
}
const propType = propTypes[prop.key.name];
if (!propType || propType.isRequired) {
return false;
}
return prop.value.type !== 'AssignmentPattern';
const propName = prop && prop.key && prop.key.name;
const isPropRequired = propTypes[propName] && propTypes[propName].isRequired;
return propTypes[propName] && isPropRequired && !isPropWithNoDefaulVal(prop);
}).forEach((prop) => {
report(context, messages.noDefaultWithRequired, 'noDefaultWithRequired', {
node: prop,
data: { name: prop.key.name },
});
});

// Filter non required props with no default value and report error
props.properties.filter((prop) => {
const propName = prop && prop.key && prop.key.name;
const isPropRequired = propTypes[propName] && propTypes[propName].isRequired;
return propTypes[propName] && !isPropRequired && isPropWithNoDefaulVal(prop);
}).forEach((prop) => {
report(context, messages.shouldAssignObjectDefault, 'shouldAssignObjectDefault', {
node: prop,
Expand Down
42 changes: 42 additions & 0 deletions tests/lib/rules/require-default-props.js
Original file line number Diff line number Diff line change
Expand Up @@ -3042,5 +3042,47 @@ ruleTester.run('require-default-props', rule, {
propWrapperFunctions: ['forbidExtraProps'],
},
},
{
code: `
function MyStatelessComponent({ foo = 'foo' }) {
return <div>{foo}{bar}</div>;
}
MyStatelessComponent.propTypes = {
foo: PropTypes.string.isRequired,
};
`,
options: [{ functions: 'defaultArguments' }],
errors: [
{
messageId: 'noDefaultWithRequired',
data: { name: 'foo' },
line: 2,
},
],
},
{
code: `
function MyStatelessComponent({ foo = 'foo', bar }) {
return <div>{foo}{bar}</div>;
}
MyStatelessComponent.propTypes = {
foo: PropTypes.string.isRequired,
bar: PropTypes.string
};
`,
options: [{ functions: 'defaultArguments' }],
errors: [
{
messageId: 'noDefaultWithRequired',
data: { name: 'foo' },
line: 2,
},
{
messageId: 'shouldAssignObjectDefault',
data: { name: 'bar' },
line: 2,
},
],
},
]),
});

0 comments on commit a4b0bbc

Please sign in to comment.