diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c04094309..d12e36fcf1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange * [`jsx-tag-spacing`]: Add `multiline-always` option ([#3260][] @Nokel81) * [`function-component-definition`]: replace `var` by `const` in certain situations ([#3248][] @JohnBerd @SimeonC) * add [`jsx-no-leaked-render`] ([#3203][] @Belco90) +* [`require-default-props`]: add option `functions` ([#3249][] @nix6839) ### Fixed * [`hook-use-state`]: Allow UPPERCASE setState setter prefixes ([#3244][] @duncanbeevers) @@ -41,11 +42,12 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange [#3258]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3258 [#3254]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3254 [#3251]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3251 -[#3203]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3203 +[#3249]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3249 [#3248]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3248 [#3244]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3244 [#3235]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3235 [#3230]: https://github.com/jsx-eslint/eslint-plugin-react/issues/3230 +[#3203]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3203 ## [7.29.4] - 2022.03.13 diff --git a/docs/rules/require-default-props.md b/docs/rules/require-default-props.md index 7857710c71..bacb416454 100644 --- a/docs/rules/require-default-props.md +++ b/docs/rules/require-default-props.md @@ -36,7 +36,7 @@ HelloWorld.defaultProps = { // Logs: // Invalid prop `name` of type `string` supplied to `HelloWorld`, expected `object`. -ReactDOM.render(, document.getElementById('app')); +ReactDOM.render(, document.getElementById('app')); ``` Without `defaultProps`: @@ -55,7 +55,7 @@ HelloWorld.propTypes = { // Nothing is logged, renders: // "Hello,!" -ReactDOM.render(, document.getElementById('app')); +ReactDOM.render(, document.getElementById('app')); ``` ## Rule Details @@ -197,13 +197,21 @@ NotAComponent.propTypes = { ```js ... -"react/require-default-props": [, { forbidDefaultForRequired: , ignoreFunctionalComponents: }] +"react/require-default-props": [, { + "forbidDefaultForRequired": , + "classes": "defaultProps | "ignore", + "functions": "defaultProps" | "defaultArguments" | "ignore" + // @deprecated Use `functions` option instead. + "ignoreFunctionalComponents": , +}] ... ``` -* `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. -* `forbidDefaultForRequired`: optional boolean to forbid prop default for a required prop. Defaults to false. -* `ignoreFunctionalComponents`: optional boolean to ignore this rule for functional components. Defaults to false. +- `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. +- `forbidDefaultForRequired`: optional boolean to forbid prop default for a required prop. Defaults to false. +- `classes`: optional string to determine which strategy a class component uses for defaulting props. Defaults to "defaultProps". +- `functions`: optional string to determine which strategy a functional component uses for defaulting props. Defaults to "defaultProps". +- `ignoreFunctionalComponents`: optional boolean to ignore this rule for functional components. Defaults to false. Deprecated, use `functions` instead. ### `forbidDefaultForRequired` @@ -279,9 +287,129 @@ MyStatelessComponent.propTypes = { }; ``` +### `classes` + +- "defaultProps": Use `.defaultProps`. It's default. +- "ignore": Ignore this rule for class components. + +Examples of **incorrect** code for this rule, when set to `defaultProps`: + +```jsx +class Greeting extends React.Component { + render() { + return ( +

Hello, {this.props.foo} {this.props.bar}

+ ); + } + + static propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired + }; +} +``` + +Examples of **correct** code for this rule, when set to `defaultProps`: + +```jsx +class Greeting extends React.Component { + render() { + return ( +

Hello, {this.props.foo} {this.props.bar}

+ ); + } + + static propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired + }; + + static defaultProps = { + foo: "foo" + }; +} +``` + +### `functions` + +- "defaultProps": Use `.defaultProps`. It's default. +- "defaultArguments": Use default arguments in the function signature. +- "ignore": Ignore this rule for functional components. + +Examples of **incorrect** code for this rule, when set to `defaultArguments`: + +```jsx +function Hello({ foo }) { + return
{foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string +}; +Hello.defaultProps = { + foo: 'foo' +} +``` + +```jsx +function Hello({ foo = 'foo' }) { + return
{foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string +}; +Hello.defaultProps = { + foo: 'foo' +} +``` + +```jsx +function Hello(props) { + return
{props.foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string +}; +``` + +Examples of **correct** code for this rule, when set to `defaultArguments`: + +```jsx +function Hello({ foo = 'foo' }) { + return
{foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string +}; +``` + +```jsx +function Hello({ foo }) { + return
{foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string.isRequired +}; +``` + +```jsx +function Hello(props) { + return
{props.foo}
; +} + +Hello.propTypes = { + foo: PropTypes.string.isRequired +}; +``` + ### `ignoreFunctionalComponents` When set to `true`, ignores this rule for all functional components. +**Deprecated**, use `functions` instead. Examples of **incorrect** code for this rule: @@ -345,6 +473,7 @@ MyStatelessComponent.propTypes = { If you don't care about using `defaultProps` for your component's props that are not required, you can disable this rule. # Resources + - [Official React documentation on defaultProps](https://facebook.github.io/react/docs/typechecking-with-proptypes.html#default-prop-values) [PropTypes]: https://reactjs.org/docs/typechecking-with-proptypes.html diff --git a/lib/rules/require-default-props.js b/lib/rules/require-default-props.js index 8067e25cc5..c779b9293c 100644 --- a/lib/rules/require-default-props.js +++ b/lib/rules/require-default-props.js @@ -5,6 +5,8 @@ 'use strict'; +const entries = require('object.entries'); +const values = require('object.values'); const Components = require('../util/Components'); const docsUrl = require('../util/docsUrl'); const astUtil = require('../util/ast'); @@ -17,6 +19,9 @@ const report = require('../util/report'); const messages = { noDefaultWithRequired: 'propType "{{name}}" is required and should not have a defaultProps declaration.', shouldHaveDefault: 'propType "{{name}}" is not required, but has no corresponding defaultProps declaration.', + noDefaultPropsWithFunction: 'Don’t use defaultProps with function components.', + shouldAssignObjectDefault: 'propType "{{name}}" is not required, but has no corresponding default argument value.', + destructureInSignature: 'Must destructure props in the function signature to initialize an optional prop.', }; module.exports = { @@ -35,6 +40,19 @@ module.exports = { forbidDefaultForRequired: { type: 'boolean', }, + classes: { + allow: { + enum: ['defaultProps', 'ignore'], + }, + }, + functions: { + allow: { + enum: ['defaultArguments', 'defaultProps', 'ignore'], + }, + }, + /** + * @deprecated + */ ignoreFunctionalComponents: { type: 'boolean', }, @@ -46,7 +64,15 @@ module.exports = { create: Components.detect((context, components) => { const configuration = context.options[0] || {}; const forbidDefaultForRequired = configuration.forbidDefaultForRequired || false; - const ignoreFunctionalComponents = configuration.ignoreFunctionalComponents || false; + const classes = configuration.classes || 'defaultProps'; + /** + * @todo + * - Remove ignoreFunctionalComponents + * - Change default to 'defaultArguments' + */ + const functions = configuration.ignoreFunctionalComponents + ? 'ignore' + : configuration.functions || 'defaultProps'; /** * Reports all propTypes passed in that don't have a defaultProps counterpart. @@ -55,14 +81,10 @@ module.exports = { * @return {void} */ function reportPropTypesWithoutDefault(propTypes, defaultProps) { - // If this defaultProps is "unresolved", then we should ignore this component and not report - // any errors for it, to avoid false-positives with e.g. external defaultProps declarations or spread operators. - if (defaultProps === 'unresolved') { - return; - } + entries(propTypes).forEach((propType) => { + const propName = propType[0]; + const prop = propType[1]; - Object.keys(propTypes).forEach((propName) => { - const prop = propTypes[propName]; if (!prop.node) { return; } @@ -87,6 +109,48 @@ module.exports = { }); } + /** + * If functions option is 'defaultArguments', reports defaultProps is used and all params that doesn't initialized. + * @param {Object} componentNode Node of component. + * @param {Object[]} declaredPropTypes List of propTypes to check `isRequired`. + * @param {Object} defaultProps Object of defaultProps to check used. + */ + function reportFunctionComponent(componentNode, declaredPropTypes, defaultProps) { + if (defaultProps) { + report(context, messages.noDefaultPropsWithFunction, 'noDefaultPropsWithFunction', { + node: componentNode, + }); + } + + const props = componentNode.params[0]; + const propTypes = declaredPropTypes; + + if (props.type === 'Identifier') { + const hasOptionalProp = values(propTypes).some((propType) => !propType.isRequired); + if (hasOptionalProp) { + report(context, messages.destructureInSignature, 'destructureInSignature', { + node: props, + }); + } + } else if (props.type === 'ObjectPattern') { + 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'; + }).forEach((prop) => { + report(context, messages.shouldAssignObjectDefault, 'shouldAssignObjectDefault', { + node: prop, + data: { name: prop.key.name }, + }); + }); + } + } + // -------------------------------------------------------------------------- // Public API // -------------------------------------------------------------------------- @@ -95,17 +159,33 @@ module.exports = { 'Program:exit'() { const list = components.list(); - Object.keys(list).filter((component) => { - if (ignoreFunctionalComponents - && (astUtil.isFunction(list[component].node) || astUtil.isFunctionLikeExpression(list[component].node))) { + values(list).filter((component) => { + if (functions === 'ignore' && astUtil.isFunctionLike(component.node)) { return false; } - return list[component].declaredPropTypes; + if (classes === 'ignore' && astUtil.isClass(component.node)) { + return false; + } + + // If this defaultProps is "unresolved", then we should ignore this component and not report + // any errors for it, to avoid false-positives with e.g. external defaultProps declarations or spread operators. + if (component.defaultProps === 'unresolved') { + return false; + } + return component.declaredPropTypes !== undefined; }).forEach((component) => { - reportPropTypesWithoutDefault( - list[component].declaredPropTypes, - list[component].defaultProps || {} - ); + if (functions === 'defaultArguments' && astUtil.isFunctionLike(component.node)) { + reportFunctionComponent( + component.node, + component.declaredPropTypes, + component.defaultProps + ); + } else { + reportPropTypesWithoutDefault( + component.declaredPropTypes, + component.defaultProps || {} + ); + } }); }, }; diff --git a/tests/lib/rules/require-default-props.js b/tests/lib/rules/require-default-props.js index 137c685678..972df97930 100644 --- a/tests/lib/rules/require-default-props.js +++ b/tests/lib/rules/require-default-props.js @@ -211,6 +211,84 @@ ruleTester.run('require-default-props', rule, { `, options: [{ ignoreFunctionalComponents: true }], }, + { + code: ` + function Hello(props) { + return
Hello {props.foo}
; + } + Hello.propTypes = { + foo: PropTypes.string + }; + Hello.defaultProps = { + foo: 'bar;' + } + `, + options: [{ forbidDefaultForRequired: true }], + }, + { + code: ` + function Hello({ foo = 'asdf', bar }) { + return
Hello {foo} and {bar}
; + } + Hello.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired + }; + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + function Hello({ foo = 'asdf', bar = 'qwer' }) { + return
Hello {foo} and {bar}
; + } + Hello.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string + }; + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + function Hello(props) { + return
Hello {foo} and {bar}
; + } + Hello.propTypes = { + foo: PropTypes.string.isRequired, + bar: PropTypes.string.isRequired, + }; + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + function MyStatelessComponent({ foo, bar = 'asdf', ...props }) { + return
{foo}{bar}
; + } + MyStatelessComponent.propTypes = { + foo: PropTypes.string.isRequired, + bar: PropTypes.string, + hello: PropTypes.string.isRequired, + world: PropTypes.string.isRequired + } + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + function MyStatelessComponent({ foo, bar, ...props }) { + return
{foo}{bar}
; + } + MyStatelessComponent.propTypes = { + foo: PropTypes.string.isRequired, + bar: PropTypes.string.isRequired, + hello: PropTypes.string.isRequired, + world: PropTypes.string.isRequired + } + `, + options: [{ functions: 'defaultArguments' }], + }, // stateless components as function expressions { @@ -247,6 +325,42 @@ ruleTester.run('require-default-props', rule, { `, options: [{ ignoreFunctionalComponents: true }], }, + { + code: ` + const Hello = function({ foo = 'asdf', bar = 'qwer' }) { + return
Hello {foo} and {bar}
; + } + Hello.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string + }; + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + const NoPropsComponent = function () { + return
Hello, world!
; + } + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + function NoPropsComponent() { + return
Hello, world!
; + } + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + const NoPropsComponent = () => { + return
Hello, world!
; + } + `, + options: [{ functions: 'defaultArguments' }], + }, // stateless components as arrow function expressions { @@ -285,6 +399,18 @@ ruleTester.run('require-default-props', rule, { `, options: [{ ignoreFunctionalComponents: true }], }, + { + code: ` + const Hello = ({ foo = 'asdf', bar = 'qwer' }) => { + return
Hello {foo} and {bar}
; + } + Hello.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string + }; + `, + options: [{ functions: 'defaultArguments' }], + }, // createReactClass components { @@ -366,6 +492,26 @@ ruleTester.run('require-default-props', rule, { `, options: [{ ignoreFunctionalComponents: true }], }, + { + code: ` + var Greeting = createReactClass({ + render: function() { + return
Hello {this.props.foo} {this.props.bar}
; + }, + propTypes: { + foo: PropTypes.string, + bar: PropTypes.string + }, + getDefaultProps: function() { + return { + foo: "foo", + bar: "bar" + }; + } + }); + `, + options: [{ functions: 'defaultArguments' }], + }, // ES6 class component { @@ -484,6 +630,75 @@ ruleTester.run('require-default-props', rule, { `, options: [{ ignoreFunctionalComponents: true }], }, + { + code: ` + class Greeting extends React.Component { + render() { + return ( +

Hello, {this.props.foo} {this.props.bar}

+ ); + } + } + Greeting.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired + }; + Greeting.defaultProps = { + foo: "foo" + }; + `, + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + class Greeting extends React.Component { + render() { + return ( +

Hello, {this.props.foo} {this.props.bar}

+ ); + } + } + Greeting.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired + }; + `, + options: [{ classes: 'ignore' }], + }, + { + code: ` + class Greeting extends React.Component { + render() { + return ( +

Hello, {this.props.foo} {this.props.bar}

+ ); + } + } + Greeting.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string + }; + Greeting.defaultProps = { + foo: "foo" + }; + `, + options: [{ classes: 'ignore' }], + }, + { + code: ` + class Hello extends React.Component { + static get propTypes() { + return { + name: PropTypes.string + }; + } + render() { + return
Hello {this.props.name}
; + } + } + `, + options: [{ classes: 'ignore' }], + }, // edge cases @@ -511,6 +726,20 @@ ruleTester.run('require-default-props', rule, { }; `, }, + { + code: ` + let Greetings = {}; + Greetings.Hello = class extends React.Component { + render () { + return
Hello {this.props.foo}
; + } + } + Greetings.Hello.propTypes = { + foo: PropTypes.string + }; + `, + options: [{ classes: 'ignore' }], + }, // external references { code: ` @@ -624,6 +853,18 @@ ruleTester.run('require-default-props', rule, { } `, }, + { + code: ` + MyStatelessComponent.propTypes = { + ...stuff, + foo: PropTypes.string + }; + function MyStatelessComponent({ foo = "foo", bar }) { + return
{foo}{bar}
; + } + `, + options: [{ functions: 'defaultArguments' }], + }, { code: ` class Greeting extends React.Component { @@ -952,6 +1193,32 @@ ruleTester.run('require-default-props', rule, { features: ['flow'], options: [{ forbidDefaultForRequired: true }], }, + { + code: ` + type Props = { + foo?: string, + bar?: string + } + function Hello({ foo = 'asdf', bar = 'qwer' }: Props) { + return
Hello {foo} and {bar}
; + } + `, + features: ['types'], + options: [{ functions: 'defaultArguments' }], + }, + { + code: ` + type Props = { + foo?: string, + bar: string + } + function Hello({ foo = 'asdf', bar }: Props) { + return
Hello {foo} and {bar}
; + } + `, + features: ['types'], + options: [{ functions: 'defaultArguments' }], + }, { code: ` type Props = { @@ -982,6 +1249,19 @@ ruleTester.run('require-default-props', rule, { `, features: ['ts'], }, + { + code: ` + interface Props { + foo?: string, + bar?: string + } + const Hello: React.FC = ({ foo = 'asdf', bar = 'qwer' }) => { + return
Hello {foo} and {bar}
; + } + `, + features: ['ts'], + options: [{ functions: 'defaultArguments' }], + }, ]), invalid: parsers.all([ @@ -1143,6 +1423,68 @@ ruleTester.run('require-default-props', rule, { }, ], }, + { + code: ` + const types = { + foo: PropTypes.string, + bar: PropTypes.string + }; + function MyStatelessComponent({ foo, bar = "asdf" }) { + return
{foo}{bar}
; + } + MyStatelessComponent.propTypes = types; + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'shouldAssignObjectDefault', + data: { name: 'foo' }, + line: 6, + column: 41, + }, + ], + }, + { + code: ` + const types = { + foo: PropTypes.string, + }; + function MyStatelessComponent(props) { + return
{props.foo}
; + } + MyStatelessComponent.propTypes = types; + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'destructureInSignature', + line: 5, + column: 39, + }, + ], + }, + { + code: ` + function MyStatelessComponent({ foo, bar, ...props }) { + return
{foo}{bar}
; + } + MyStatelessComponent.propTypes = { + foo: PropTypes.string, + bar: PropTypes.string.isRequired, + hello: PropTypes.string.isRequired, + world: PropTypes.string.isRequired + } + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'shouldAssignObjectDefault', + data: { name: 'foo' }, + line: 2, + column: 41, + }, + ], + }, // createReactClass components { @@ -2440,6 +2782,43 @@ ruleTester.run('require-default-props', rule, { }, ], }, + { + code: ` + function Hello(props) { + return
Hello {props.foo}
; + } + Hello.propTypes = { + foo: PropTypes.string.isRequired + }; + Hello.defaultProps = { + foo: 'bar' + }; + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'noDefaultPropsWithFunction', + }, + ], + }, + { + code: ` + function Hello(props) { + return
Hello {props.foo} and {props.bar}
; + } + Hello.propTypes = { + foo: PropTypes.string.isRequired, + bar: PropTypes.string + }; + `, + options: [{ functions: 'defaultArguments' }], + errors: [ + { + messageId: 'destructureInSignature', + }, + ], + }, + // test support for React PropTypes as Component's class generic { code: ` @@ -2601,7 +2980,7 @@ ruleTester.run('require-default-props', rule, { }, }, }; - + export const Daily = ({ history, match: { params: { date = moment().toISOString(),