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

Added no-unsafe rule #1831

Merged
merged 5 commits into from
Jun 21, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ Enable the rules that you would like to use.
* [react/no-this-in-sfc](docs/rules/no-this-in-sfc.md): Prevent using `this` in stateless functional components
* [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md): Prevent invalid characters from appearing in markup
* [react/no-unknown-property](docs/rules/no-unknown-property.md): Prevent usage of unknown DOM property (fixable)
* [react/no-unsafe](docs/rules/no-unsafe.md): Prevent usage of `UNSAFE_` methods
* [react/no-unused-prop-types](docs/rules/no-unused-prop-types.md): Prevent definitions of unused prop types
* [react/no-unused-state](docs/rules/no-unused-state.md): Prevent definitions of unused state properties
* [react/no-will-update-set-state](docs/rules/no-will-update-set-state.md): Prevent usage of `setState` in `componentWillUpdate`
Expand Down Expand Up @@ -208,6 +209,7 @@ The rules enabled in this configuration are:
* [react/no-string-refs](docs/rules/no-string-refs.md)
* [react/no-unescaped-entities](docs/rules/no-unescaped-entities.md)
* [react/no-unknown-property](docs/rules/no-unknown-property.md)
* [react/no-unsafe](docs/rules/no-unsafe.md)
* [react/prop-types](docs/rules/prop-types.md)
* [react/react-in-jsx-scope](docs/rules/react-in-jsx-scope.md)
* [react/require-render-return](docs/rules/require-render-return.md)
Expand Down
47 changes: 47 additions & 0 deletions docs/rules/no-unsafe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Prevent usage of `UNSAFE_` methods (react/no-unsafe)

Certain legacy lifecycle methods are [unsafe for use in async React applications][async_rendering] 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].

[async_rendering]: https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html
[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() {}
});
```
2 changes: 2 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const allRules = {
'no-typos': require('./lib/rules/no-typos'),
'no-unescaped-entities': require('./lib/rules/no-unescaped-entities'),
'no-unknown-property': require('./lib/rules/no-unknown-property'),
'no-unsafe': require('./lib/rules/no-unsafe'),
'no-unused-prop-types': require('./lib/rules/no-unused-prop-types'),
'no-unused-state': require('./lib/rules/no-unused-state'),
'no-will-update-set-state': require('./lib/rules/no-will-update-set-state'),
Expand Down Expand Up @@ -139,6 +140,7 @@ module.exports = {
'react/no-string-refs': 2,
'react/no-unescaped-entities': 2,
'react/no-unknown-property': 2,
'react/no-unsafe': 2,
Copy link
Member

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.

'react/prop-types': 2,
'react/react-in-jsx-scope': 2,
'react/require-render-return': 2
Expand Down
95 changes: 95 additions & 0 deletions lib/rules/no-unsafe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* @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');
const versionUtil = require('../util/version');

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

module.exports = {
meta: {
docs: {
description: 'Prevent usage of UNSAFE_ methods',
category: 'Best Practices',
recommended: true,
Copy link
Member

Choose a reason for hiding this comment

The 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();
const isApplicable = versionUtil.testReactVersion(context, '16.3.0');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a check we could do earlier, and prevent even returning any visitors in the first place?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added early termination for the rule.

return unsafeMethods.indexOf(method) !== -1 && isApplicable;
}

/**
* 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: `${method} is unsafe for use in async rendering, see https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html`
});
}

/**
* 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
};
})
};
153 changes: 153 additions & 0 deletions tests/lib/rules/no-unsafe.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
/**
* @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
}
};

function errorMessage(method) {
return `${method} is unsafe for use in async rendering, see https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html`;
}

// ------------------------------------------------------------------------------
// Tests
// ------------------------------------------------------------------------------

const ruleTester = new RuleTester({parserOptions});
ruleTester.run('no-unsafe', rule, {
valid: [
{
code: `
class Foo extends React.Component {
componentDidUpdate() {}
render() {}
}
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
const Foo = createReactClass({
componentDidUpdate: function() {},
render: function() {}
});
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
class Foo extends Bar {
UNSAFE_componentWillMount() {}
UNSAFE_componentWillReceiveProps() {}
UNSAFE_componentWillUpdate() {}
}
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
const Foo = bar({
UNSAFE_componentWillMount: function() {},
UNSAFE_componentWillReceiveProps: function() {},
UNSAFE_componentWillUpdate: function() {},
});
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
class Foo extends React.Component {
UNSAFE_componentWillMount() {}
UNSAFE_componentWillReceiveProps() {}
UNSAFE_componentWillUpdate() {}
}
`,
settings: {react: {version: '16.2.0'}}
},
{
code: `
const Foo = createReactClass({
UNSAFE_componentWillMount: function() {},
UNSAFE_componentWillReceiveProps: function() {},
UNSAFE_componentWillUpdate: function() {},
});
`,
settings: {react: {version: '16.2.0'}}
}
],

invalid: [
{
code: `
class Foo extends React.Component {
UNSAFE_componentWillMount() {}
UNSAFE_componentWillReceiveProps() {}
UNSAFE_componentWillUpdate() {}
}
`,
settings: {react: {version: '16.3.0'}},
errors: [
{
message: errorMessage('UNSAFE_componentWillMount'),
line: 2,
column: 7,
endLine: 6,
endColumn: 8
},
{
message: errorMessage('UNSAFE_componentWillReceiveProps'),
line: 2,
column: 7,
endLine: 6,
endColumn: 8
},
{
message: errorMessage('UNSAFE_componentWillUpdate'),
line: 2,
column: 7,
endLine: 6,
endColumn: 8
}
]
},
{
code: `
const Foo = createReactClass({
UNSAFE_componentWillMount: function() {},
UNSAFE_componentWillReceiveProps: function() {},
UNSAFE_componentWillUpdate: function() {},
});
`,
settings: {react: {version: '16.3.0'}},
errors: [
{
message: errorMessage('UNSAFE_componentWillMount'),
line: 2,
column: 38,
endLine: 6,
endColumn: 10
},
{
message: errorMessage('UNSAFE_componentWillReceiveProps')
},
{
message: errorMessage('UNSAFE_componentWillUpdate')
}
]
}
]
});