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.

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

[New] Add no-arrow-function-lifecycle rule #1980

Merged
merged 1 commit into from Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -7,8 +7,10 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

### Added
* [`no-unused-class-component-methods`]: Handle unused class component methods ([#2166][] @jakeleventhal @pawelnvk)
* add [`no-arrow-function-lifecycle`] ([#1980][] @ngtan)

[#2166]: https://github.com/yannickcr/eslint-plugin-react/pull/2166
[#1980]: https://github.com/yannickcr/eslint-plugin-react/pull/1980

## [7.26.1] - 2021.09.29

Expand Down Expand Up @@ -3465,6 +3467,7 @@ If you're still not using React 15 you can keep the old behavior by setting the
[`no-access-state-in-setstate`]: docs/rules/no-access-state-in-setstate.md
[`no-adjacent-inline-elements`]: docs/rules/no-adjacent-inline-elements.md
[`no-array-index-key`]: docs/rules/no-array-index-key.md
[`no-arrow-function-lifecycle`]: docs/rules/no-arrow-function-lifecycle.md
[`no-children-prop`]: docs/rules/no-children-prop.md
[`no-comment-textnodes`]: docs/rules/jsx-no-comment-textnodes.md
[`no-danger-with-children`]: docs/rules/no-danger-with-children.md
Expand Down
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -135,6 +135,7 @@ Enable the rules that you would like to use.
| | | [react/no-access-state-in-setstate](docs/rules/no-access-state-in-setstate.md) | Reports when this.state is accessed within setState |
| | | [react/no-adjacent-inline-elements](docs/rules/no-adjacent-inline-elements.md) | Prevent adjacent inline elements not separated by whitespace. |
| | | [react/no-array-index-key](docs/rules/no-array-index-key.md) | Prevent usage of Array index in keys |
| | 🔧 | [react/no-arrow-function-lifecycle](docs/rules/no-arrow-function-lifecycle.md) | Lifecycle methods should be methods on the prototype, not class fields |
| ✔ | | [react/no-children-prop](docs/rules/no-children-prop.md) | Prevent passing of children as props. |
| | | [react/no-danger](docs/rules/no-danger.md) | Prevent usage of dangerous JSX props |
| ✔ | | [react/no-danger-with-children](docs/rules/no-danger-with-children.md) | Report when a DOM element is using both children and dangerouslySetInnerHTML |
Expand Down
41 changes: 41 additions & 0 deletions docs/rules/no-arrow-function-lifecycle.md
@@ -0,0 +1,41 @@
# Lifecycle methods should be methods on the prototype, not class fields (react/no-arrow-function-lifecycle)

It is not neccessary to use arrow function for lifecycle methods. This makes things harder to test, conceptually less performant (although in practice, performance will not be affected, since most engines will optimize efficiently), and can break hot reloading patterns.

## Rule Details

The following patterns are considered warnings:

```jsx
class Hello extends React.Component {
render = () => {
return <div />;
}
}

var AnotherHello = createReactClass({
render: () => {
return <div />;
},
});
```

The following patterns are **not** considered warnings:

```jsx
class Hello extends React.Component {
render() {
return <div />;
}
}

var AnotherHello = createReactClass({
render() {
return <div />;
},
});

```
## When Not To Use It

If you don't care about performance of your application or conceptual correctness of class property placement, you can disable this rule.
1 change: 1 addition & 0 deletions index.js
Expand Up @@ -57,6 +57,7 @@ const allRules = {
'no-access-state-in-setstate': require('./lib/rules/no-access-state-in-setstate'),
'no-adjacent-inline-elements': require('./lib/rules/no-adjacent-inline-elements'),
'no-array-index-key': require('./lib/rules/no-array-index-key'),
'no-arrow-function-lifecycle': require('./lib/rules/no-arrow-function-lifecycle'),
'no-children-prop': require('./lib/rules/no-children-prop'),
'no-danger': require('./lib/rules/no-danger'),
'no-danger-with-children': require('./lib/rules/no-danger-with-children'),
Expand Down
80 changes: 80 additions & 0 deletions lib/rules/no-arrow-function-lifecycle.js
@@ -0,0 +1,80 @@
/**
* @fileoverview Lifecycle methods should be methods on the prototype, not class fields
* @author Tan Nguyen
*/

'use strict';

const values = require('object.values');

const Components = require('../util/Components');
const astUtil = require('../util/ast');
const docsUrl = require('../util/docsUrl');
const lifecycleMethods = require('../util/lifecycleMethods');
ngtan marked this conversation as resolved.
Show resolved Hide resolved

module.exports = {
meta: {
docs: {
description: 'Lifecycle methods should be methods on the prototype, not class fields',
category: 'Best Practices',
recommended: false,
url: docsUrl('no-arrow-function-lifecycle')
},
schema: [],
fixable: 'code'
},

create: Components.detect((context, components, utils) => {
function getText(node) {
const params = node.value.params.map((p) => p.name);

if (node.type === 'Property') {
return `: function(${params.join(', ')}) `;
}

if (node.type === 'ClassProperty') {
return `(${params.join(', ')}) `;
}

return null;
}

/**
* @param {Array} properties list of component properties
*/
function reportNoArrowFunctionLifecycle(properties) {
properties.forEach((node) => {
const propertyName = astUtil.getPropertyName(node);
const nodeType = node.value.type;
const isLifecycleMethod = (
node.static && !utils.isES5Component(node)
? lifecycleMethods.static
: lifecycleMethods.instance
).indexOf(propertyName) > -1;

if (nodeType === 'ArrowFunctionExpression' && isLifecycleMethod) {
const range = [node.key.range[1], node.value.body.range[0]];
const text = getText(node);

context.report({
node,
message: '{{propertyName}} is a React lifecycle method, and should not be an arrow function or in a class field. Use an instance method instead.',
data: {
propertyName
},
fix: (fixer) => fixer.replaceTextRange(range, text)
});
}
});
}

return {
'Program:exit'() {
values(components.list()).forEach((component) => {
const properties = astUtil.getComponentProperties(component.node);
reportNoArrowFunctionLifecycle(properties);
});
}
};
})
};
22 changes: 3 additions & 19 deletions lib/rules/no-typos.js
Expand Up @@ -8,29 +8,13 @@ const PROP_TYPES = Object.keys(require('prop-types'));
const Components = require('../util/Components');
const docsUrl = require('../util/docsUrl');
const report = require('../util/report');
const lifecycleMethods = require('../util/lifecycleMethods');

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

const STATIC_CLASS_PROPERTIES = ['propTypes', 'contextTypes', 'childContextTypes', 'defaultProps'];
const STATIC_LIFECYCLE_METHODS = ['getDerivedStateFromProps'];
const LIFECYCLE_METHODS = [
'getDerivedStateFromProps',
'componentWillMount',
'UNSAFE_componentWillMount',
'componentDidMount',
'componentWillReceiveProps',
'UNSAFE_componentWillReceiveProps',
'shouldComponentUpdate',
'componentWillUpdate',
'UNSAFE_componentWillUpdate',
'getSnapshotBeforeUpdate',
'componentDidUpdate',
'componentDidCatch',
'componentWillUnmount',
'render'
];

const messages = {
typoPropTypeChain: 'Typo in prop type chain qualifier: {{name}}',
Expand Down Expand Up @@ -167,7 +151,7 @@ module.exports = {
return;
}

STATIC_LIFECYCLE_METHODS.forEach((method) => {
lifecycleMethods.static.forEach((method) => {
if (!node.static && nodeKeyName.toLowerCase() === method.toLowerCase()) {
report(context, messages.staticLifecycleMethod, 'staticLifecycleMethod', {
node,
Expand All @@ -178,7 +162,7 @@ module.exports = {
}
});

LIFECYCLE_METHODS.forEach((method) => {
lifecycleMethods.instance.concat(lifecycleMethods.static).forEach((method) => {
if (method.toLowerCase() === nodeKeyName.toLowerCase() && method !== nodeKeyName) {
report(context, messages.typoLifecycleMethod, 'typoLifecycleMethod', {
node,
Expand Down
30 changes: 30 additions & 0 deletions lib/util/lifecycleMethods.js
@@ -0,0 +1,30 @@
/**
* @fileoverview lifecycle methods
* @author Tan Nguyen
*/

'use strict';

module.exports = {
instance: [
'getDefaultProps',
'getInitialState',
'getChildContext',
'componentWillMount',
'UNSAFE_componentWillMount',
'componentDidMount',
'componentWillReceiveProps',
'UNSAFE_componentWillReceiveProps',
'shouldComponentUpdate',
'componentWillUpdate',
'UNSAFE_componentWillUpdate',
'getSnapshotBeforeUpdate',
'componentDidUpdate',
'componentDidCatch',
'componentWillUnmount',
'render'
],
static: [
'getDerivedStateFromProps'
]
};