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.

Sign up for GitHub

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

Jump to bottom

[New] require-default-props: add option functions #3249

Merged
merged 2 commits into from Apr 27, 2022
Merged

[New] require-default-props: add option functions #3249

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5554bd4
[Refactor] add AST util `isFunctionLike()`
nix6839 Mar 19, 2022
24621e4
[New] `require-default-props`: add option `functions`
nix6839 Mar 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
141 changes: 135 additions & 6 deletions docs/rules/require-default-props.md
View file
Open in desktop
Expand Up @@ -36,7 +36,7 @@ HelloWorld.defaultProps = {

// Logs:
// Invalid prop `name` of type `string` supplied to `HelloWorld`, expected `object`.
ReactDOM.render(<HelloWorld />, document.getElementById('app'));
ReactDOM.render(<HelloWorld />, document.getElementById('app'));
```

Without `defaultProps`:
Expand All @@ -55,7 +55,7 @@ HelloWorld.propTypes = {

// Nothing is logged, renders:
// "Hello,!"
ReactDOM.render(<HelloWorld />, document.getElementById('app'));
ReactDOM.render(<HelloWorld />, document.getElementById('app'));
```

## Rule Details
Expand Down Expand Up @@ -197,13 +197,21 @@ NotAComponent.propTypes = {

```js
...
"react/require-default-props": [<enabled>, { forbidDefaultForRequired: <boolean>, ignoreFunctionalComponents: <boolean> }]
"react/require-default-props": [<enabled>, {
"forbidDefaultForRequired": <boolean>,
"classes": "defaultProps | "ignore",
"functions": "defaultProps" | "defaultArguments" | "ignore"
// @deprecated Use `functions` option instead.
"ignoreFunctionalComponents": <boolean>,
}]
...
```

* `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`

Expand Down Expand Up @@ -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 (
<h1>Hello, {this.props.foo} {this.props.bar}</h1>
);
}

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 (
<h1>Hello, {this.props.foo} {this.props.bar}</h1>
);
}

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 <div>{foo}</div>;
}

Hello.propTypes = {
foo: PropTypes.string
};
Hello.defaultProps = {
foo: 'foo'
}
```

```jsx
function Hello({ foo = 'foo' }) {
return <div>{foo}</div>;
}

Hello.propTypes = {
foo: PropTypes.string
};
Hello.defaultProps = {
foo: 'foo'
}
```

```jsx
function Hello(props) {
return <div>{props.foo}</div>;
}

Hello.propTypes = {
foo: PropTypes.string
};
```

Examples of **correct** code for this rule, when set to `defaultArguments`:

```jsx
function Hello({ foo = 'foo' }) {
return <div>{foo}</div>;
}

Hello.propTypes = {
foo: PropTypes.string
};
```

```jsx
function Hello({ foo }) {
return <div>{foo}</div>;
}

Hello.propTypes = {
foo: PropTypes.string.isRequired
};
```

```jsx
function Hello(props) {
return <div>{props.foo}</div>;
}

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:

Expand Down Expand Up @@ -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
Expand Down
112 changes: 96 additions & 16 deletions lib/rules/require-default-props.js
View file
Open in desktop
Expand Up @@ -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');
Expand All @@ -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 = {
Expand All @@ -35,6 +40,19 @@ module.exports = {
forbidDefaultForRequired: {
type: 'boolean',
},
classes: {
allow: {
enum: ['defaultProps', 'ignore'],
},
},
functions: {
allow: {
enum: ['defaultArguments', 'defaultProps', 'ignore'],
},
},
/**
* @deprecated
*/
ignoreFunctionalComponents: {
type: 'boolean',
},
Expand All @@ -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.
Expand All @@ -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;
}
Expand All @@ -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
// --------------------------------------------------------------------------
Expand All @@ -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 || {}
);
}
});
},
};
Expand Down
10 changes: 10 additions & 0 deletions lib/util/ast.js
View file
Open in desktop
Expand Up @@ -232,6 +232,15 @@ function isFunction(node) {
return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration';
}

/**
* Checks if node is a function declaration or expression or arrow function.
* @param {ASTNode} node The node to check
* @return {Boolean} true if it's a function-like
*/
function isFunctionLike(node) {
return node.type === 'FunctionDeclaration' || isFunctionLikeExpression(node);
}
nix6839 marked this conversation as resolved.
Show resolved Hide resolved

/**
* Checks if the node is a class.
* @param {ASTNode} node The node to check
Expand Down Expand Up @@ -432,6 +441,7 @@ module.exports = {
isClass,
isFunction,
isFunctionLikeExpression,
isFunctionLike,
inConstructor,
isNodeFirstInLine,
unwrapTSAsExpression,
Expand Down