-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Added no-unsafe
rule
#1831
Added no-unsafe
rule
#1831
Changes from 1 commit
b5e86bf
5b08a1e
5e17159
e41500a
0285eef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
# Prevent usage of `UNSAFE_` methods (react/no-unsafe) | ||
|
||
Certain legacy lifecycle methods are unsafe for use in async React applications and cause warnings in [_strict mode_][strict_mode]. These also happen to be the lifecycles that cause the most [confusion within the React community][component_lifecycle_changes]. | ||
|
||
[strict_mode]: https://reactjs.org/docs/strict-mode.html#identifying-unsafe-lifecycles | ||
[component_lifecycle_changes]: https://reactjs.org/blog/2018/03/29/react-v-16-3.html#component-lifecycle-changes | ||
|
||
The rule checks the following methods: `UNSAFE_componentWillMount`, `UNSAFE_componentWillReceiveProps`, `UNSAFE_componentWillUpdate`. | ||
|
||
## Rule Details | ||
|
||
The following patterns are considered warnings: | ||
|
||
```jsx | ||
class Foo extends React.Component { | ||
UNSAFE_componentWillMount() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
UNSAFE_componentWillUpdate() {} | ||
} | ||
``` | ||
|
||
```jsx | ||
const Foo = createReactClass({ | ||
UNSAFE_componentWillMount: function() {}, | ||
UNSAFE_componentWillReceiveProps: function() {}, | ||
UNSAFE_componentWillUpdate: function() {} | ||
}); | ||
``` | ||
|
||
The following patterns are **not** considered warnings: | ||
|
||
```jsx | ||
class Foo extends Bar { | ||
UNSAFE_componentWillMount() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
UNSAFE_componentWillUpdate() {} | ||
} | ||
``` | ||
|
||
```jsx | ||
const Foo = bar({ | ||
UNSAFE_componentWillMount: function() {}, | ||
UNSAFE_componentWillReceiveProps: function() {}, | ||
UNSAFE_componentWillUpdate: function() {} | ||
}); | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
/** | ||
* @fileoverview Prevent usage of UNSAFE_ methods | ||
* @author Sergei Startsev | ||
*/ | ||
|
||
'use strict'; | ||
|
||
const Components = require('../util/Components'); | ||
const astUtil = require('../util/ast'); | ||
const docsUrl = require('../util/docsUrl'); | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Rule Definition | ||
// ------------------------------------------------------------------------------ | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
description: 'Prevent usage of UNSAFE_ methods', | ||
category: 'Best Practices', | ||
recommended: true, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
url: docsUrl('no-unsafe') | ||
}, | ||
schema: [] | ||
}, | ||
|
||
create: Components.detect((context, components, utils) => { | ||
/** | ||
* Returns a list of unsafe methods | ||
* @returns {Array} A list of unsafe methods | ||
*/ | ||
function getUnsafeMethods() { | ||
return [ | ||
'UNSAFE_componentWillMount', | ||
'UNSAFE_componentWillReceiveProps', | ||
'UNSAFE_componentWillUpdate' | ||
]; | ||
} | ||
|
||
/** | ||
* Checks if a passed method is unsafe | ||
* @param {string} method Life cycle method | ||
* @returns {boolean} Returns true for unsafe methods, otherwise returns false | ||
*/ | ||
function isUnsafe(method) { | ||
const unsafeMethods = getUnsafeMethods(); | ||
return unsafeMethods.indexOf(method) !== -1; | ||
} | ||
|
||
/** | ||
* Reports the error for an unsafe method | ||
* @param {ASTNode} node The AST node being checked | ||
* @param {string} method Life cycle method | ||
*/ | ||
function checkUnsafe(node, method) { | ||
if (!isUnsafe(method)) { | ||
return; | ||
} | ||
|
||
context.report({ | ||
node: node, | ||
message: `Do not use ${method}` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the message should explain why, and link to more info There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added details |
||
}); | ||
} | ||
|
||
/** | ||
* Returns life cycle methods if available | ||
* @param {ASTNode} node The AST node being checked. | ||
* @returns {Array} The array of methods. | ||
*/ | ||
function getLifeCycleMethods(node) { | ||
const properties = astUtil.getComponentProperties(node); | ||
return properties.map(property => astUtil.getPropertyName(property)); | ||
} | ||
|
||
/** | ||
* Checks life cycle methods | ||
* @param {ASTNode} node The AST node being checked. | ||
*/ | ||
function checkLifeCycleMethods(node) { | ||
if (utils.isES5Component(node) || utils.isES6Component(node)) { | ||
const methods = getLifeCycleMethods(node); | ||
methods.forEach(method => checkUnsafe(node, method)); | ||
} | ||
} | ||
|
||
return { | ||
ClassDeclaration: checkLifeCycleMethods, | ||
ClassExpression: checkLifeCycleMethods, | ||
ObjectExpression: checkLifeCycleMethods | ||
}; | ||
}) | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/** | ||
* @fileoverview Prevent usage of UNSAFE_ methods | ||
* @author Sergei Startsev | ||
*/ | ||
'use strict'; | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Requirements | ||
// ------------------------------------------------------------------------------ | ||
|
||
const rule = require('../../../lib/rules/no-unsafe'); | ||
const RuleTester = require('eslint').RuleTester; | ||
|
||
const parserOptions = { | ||
ecmaVersion: 2018, | ||
sourceType: 'module', | ||
ecmaFeatures: { | ||
jsx: true | ||
} | ||
}; | ||
|
||
// ------------------------------------------------------------------------------ | ||
// Tests | ||
// ------------------------------------------------------------------------------ | ||
|
||
const ruleTester = new RuleTester({parserOptions}); | ||
ruleTester.run('no-unsafe', rule, { | ||
valid: [ | ||
{ | ||
code: ` | ||
class Foo extends React.Component { | ||
componentDidUpdate() {} | ||
render() {} | ||
} | ||
` | ||
}, | ||
{ | ||
code: ` | ||
const Foo = createReactClass({ | ||
componentDidUpdate: function() {}, | ||
render: function() {} | ||
}); | ||
` | ||
}, | ||
{ | ||
code: ` | ||
class Foo extends Bar { | ||
UNSAFE_componentWillMount() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
UNSAFE_componentWillUpdate() {} | ||
} | ||
` | ||
}, | ||
{ | ||
code: ` | ||
const Foo = bar({ | ||
UNSAFE_componentWillMount: function() {}, | ||
UNSAFE_componentWillReceiveProps: function() {}, | ||
UNSAFE_componentWillUpdate: function() {}, | ||
}); | ||
` | ||
} | ||
], | ||
|
||
invalid: [ | ||
{ | ||
code: ` | ||
class Foo extends React.Component { | ||
UNSAFE_componentWillMount() {} | ||
UNSAFE_componentWillReceiveProps() {} | ||
UNSAFE_componentWillUpdate() {} | ||
} | ||
`, | ||
errors: [ | ||
{ | ||
message: 'Do not use UNSAFE_componentWillMount' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these tests should include position information, so that we can test that part as well There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a few more tests and position details for invalid cases There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems that added position details causes issues in eslint 3, see There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adjusted tests to support ESLint 3 |
||
}, | ||
{ | ||
message: 'Do not use UNSAFE_componentWillReceiveProps' | ||
}, | ||
{ | ||
message: 'Do not use UNSAFE_componentWillUpdate' | ||
} | ||
] | ||
}, | ||
{ | ||
code: ` | ||
const Foo = createReactClass({ | ||
UNSAFE_componentWillMount: function() {}, | ||
UNSAFE_componentWillReceiveProps: function() {}, | ||
UNSAFE_componentWillUpdate: function() {}, | ||
}); | ||
`, | ||
errors: [ | ||
{ | ||
message: 'Do not use UNSAFE_componentWillMount' | ||
}, | ||
{ | ||
message: 'Do not use UNSAFE_componentWillReceiveProps' | ||
}, | ||
{ | ||
message: 'Do not use UNSAFE_componentWillUpdate' | ||
} | ||
] | ||
} | ||
] | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enabling new rules in an exported config is a semver-major change; please set this to 0 for now.