diff --git a/CHANGELOG.md b/CHANGELOG.md index 233604f584..9a165f7e0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index d1b27eefe1..c186279cd4 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -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; + } + return prop.value.type !== 'AssignmentPattern'; +} + /** @type {import('eslint').Rule.RuleModule} */ module.exports = { meta: { @@ -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, diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index dd73a712e7..928cf4ba1d 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -3042,5 +3042,47 @@ ruleTester.run('require-default-props', rule, { propWrapperFunctions: ['forbidExtraProps'], }, }, + { + code: ` + function MyStatelessComponent({ foo = 'foo' }) { + return
{foo}{bar}
; + } + MyStatelessComponent.propTypes = { + foo: PropTypes.string.isRequired, + }; + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'noDefaultWithRequired', + data: { name: 'foo' }, + line: 2, + }, + ], + }, + { + code: ` + function MyStatelessComponent({ foo = 'foo', bar }) { + return
{foo}{bar}
; + } + 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, + }, + ], + }, ]), });