From b5e86bfd8bf8244a926eceacc8b097cf5b7ef951 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Sun, 17 Jun 2018 23:35:33 +0300 Subject: [PATCH 1/5] Added `no-unsafe` rule --- README.md | 2 + docs/rules/no-unsafe.md | 46 +++++++++++++++ index.js | 2 + lib/rules/no-unsafe.js | 93 ++++++++++++++++++++++++++++++ tests/lib/rules/no-unsafe.js | 107 +++++++++++++++++++++++++++++++++++ 5 files changed, 250 insertions(+) create mode 100644 docs/rules/no-unsafe.md create mode 100644 lib/rules/no-unsafe.js create mode 100644 tests/lib/rules/no-unsafe.js diff --git a/README.md b/README.md index 067fe20ca0..6f83a2d78c 100644 --- a/README.md +++ b/README.md @@ -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` @@ -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) diff --git a/docs/rules/no-unsafe.md b/docs/rules/no-unsafe.md new file mode 100644 index 0000000000..f3237ca3b6 --- /dev/null +++ b/docs/rules/no-unsafe.md @@ -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() {} +}); +``` diff --git a/index.js b/index.js index e311f3549a..af7da134b7 100644 --- a/index.js +++ b/index.js @@ -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'), @@ -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, 'react/prop-types': 2, 'react/react-in-jsx-scope': 2, 'react/require-render-return': 2 diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js new file mode 100644 index 0000000000..c533d1597c --- /dev/null +++ b/lib/rules/no-unsafe.js @@ -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, + 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}` + }); + } + + /** + * 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 + }; + }) +}; diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js new file mode 100644 index 0000000000..baf6af5259 --- /dev/null +++ b/tests/lib/rules/no-unsafe.js @@ -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' + }, + { + 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' + } + ] + } + ] +}); From 5b08a1e7392ea2b4d03ac615c14a71723fc615f2 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 19 Jun 2018 03:34:28 +0300 Subject: [PATCH 2/5] Adjusted `no-unsafe` rule --- docs/rules/no-unsafe.md | 3 +- lib/rules/no-unsafe.js | 6 ++-- tests/lib/rules/no-unsafe.js | 66 ++++++++++++++++++++++++++++++------ 3 files changed, 62 insertions(+), 13 deletions(-) diff --git a/docs/rules/no-unsafe.md b/docs/rules/no-unsafe.md index f3237ca3b6..87d30d7c90 100644 --- a/docs/rules/no-unsafe.md +++ b/docs/rules/no-unsafe.md @@ -1,7 +1,8 @@ # 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]. +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 diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js index c533d1597c..2ee63e167b 100644 --- a/lib/rules/no-unsafe.js +++ b/lib/rules/no-unsafe.js @@ -8,6 +8,7 @@ const Components = require('../util/Components'); const astUtil = require('../util/ast'); const docsUrl = require('../util/docsUrl'); +const versionUtil = require('../util/version'); // ------------------------------------------------------------------------------ // Rule Definition @@ -44,7 +45,8 @@ module.exports = { */ function isUnsafe(method) { const unsafeMethods = getUnsafeMethods(); - return unsafeMethods.indexOf(method) !== -1; + const isApplicable = versionUtil.testReactVersion(context, '16.3.0'); + return unsafeMethods.indexOf(method) !== -1 && isApplicable; } /** @@ -59,7 +61,7 @@ module.exports = { context.report({ node: node, - message: `Do not use ${method}` + message: `${method} is unsafe for use in async rendering, see https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html` }); } diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js index baf6af5259..c9dcf8cfce 100644 --- a/tests/lib/rules/no-unsafe.js +++ b/tests/lib/rules/no-unsafe.js @@ -19,6 +19,10 @@ const parserOptions = { } }; +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 // ------------------------------------------------------------------------------ @@ -32,7 +36,8 @@ ruleTester.run('no-unsafe', rule, { componentDidUpdate() {} render() {} } - ` + `, + settings: {react: {version: '16.4.0'}} }, { code: ` @@ -40,7 +45,8 @@ ruleTester.run('no-unsafe', rule, { componentDidUpdate: function() {}, render: function() {} }); - ` + `, + settings: {react: {version: '16.4.0'}} }, { code: ` @@ -49,7 +55,8 @@ ruleTester.run('no-unsafe', rule, { UNSAFE_componentWillReceiveProps() {} UNSAFE_componentWillUpdate() {} } - ` + `, + settings: {react: {version: '16.4.0'}} }, { code: ` @@ -58,7 +65,28 @@ ruleTester.run('no-unsafe', rule, { 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'}} } ], @@ -71,15 +99,28 @@ ruleTester.run('no-unsafe', rule, { UNSAFE_componentWillUpdate() {} } `, + settings: {react: {version: '16.3.0'}}, errors: [ { - message: 'Do not use UNSAFE_componentWillMount' + message: errorMessage('UNSAFE_componentWillMount'), + line: 2, + column: 7, + endLine: 6, + endColumn: 8 }, { - message: 'Do not use UNSAFE_componentWillReceiveProps' + message: errorMessage('UNSAFE_componentWillReceiveProps'), + line: 2, + column: 7, + endLine: 6, + endColumn: 8 }, { - message: 'Do not use UNSAFE_componentWillUpdate' + message: errorMessage('UNSAFE_componentWillUpdate'), + line: 2, + column: 7, + endLine: 6, + endColumn: 8 } ] }, @@ -91,15 +132,20 @@ ruleTester.run('no-unsafe', rule, { UNSAFE_componentWillUpdate: function() {}, }); `, + settings: {react: {version: '16.3.0'}}, errors: [ { - message: 'Do not use UNSAFE_componentWillMount' + message: errorMessage('UNSAFE_componentWillMount'), + line: 2, + column: 38, + endLine: 6, + endColumn: 10 }, { - message: 'Do not use UNSAFE_componentWillReceiveProps' + message: errorMessage('UNSAFE_componentWillReceiveProps') }, { - message: 'Do not use UNSAFE_componentWillUpdate' + message: errorMessage('UNSAFE_componentWillUpdate') } ] } From 5e17159ec68959f651cfb4610607dbacb2073214 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 19 Jun 2018 03:56:25 +0300 Subject: [PATCH 3/5] Added missed position details for `no-unsafe` --- tests/lib/rules/no-unsafe.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js index c9dcf8cfce..e2b019fbf9 100644 --- a/tests/lib/rules/no-unsafe.js +++ b/tests/lib/rules/no-unsafe.js @@ -142,10 +142,18 @@ ruleTester.run('no-unsafe', rule, { endColumn: 10 }, { - message: errorMessage('UNSAFE_componentWillReceiveProps') + message: errorMessage('UNSAFE_componentWillReceiveProps'), + line: 2, + column: 38, + endLine: 6, + endColumn: 10 }, { - message: errorMessage('UNSAFE_componentWillUpdate') + message: errorMessage('UNSAFE_componentWillUpdate'), + line: 2, + column: 38, + endLine: 6, + endColumn: 10 } ] } From e41500ae5afac96dd35fa1735b8854859988a094 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Wed, 20 Jun 2018 02:35:08 +0300 Subject: [PATCH 4/5] Added early termination for no-unsafe, adjusted tests to support ESLint3 --- lib/rules/no-unsafe.js | 8 ++++++-- tests/lib/rules/no-unsafe.js | 18 ++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js index 2ee63e167b..d168d61491 100644 --- a/lib/rules/no-unsafe.js +++ b/lib/rules/no-unsafe.js @@ -26,6 +26,11 @@ module.exports = { }, create: Components.detect((context, components, utils) => { + const isApplicable = versionUtil.testReactVersion(context, '16.3.0'); + if (!isApplicable) { + return {}; + } + /** * Returns a list of unsafe methods * @returns {Array} A list of unsafe methods @@ -45,8 +50,7 @@ module.exports = { */ function isUnsafe(method) { const unsafeMethods = getUnsafeMethods(); - const isApplicable = versionUtil.testReactVersion(context, '16.3.0'); - return unsafeMethods.indexOf(method) !== -1 && isApplicable; + return unsafeMethods.indexOf(method) !== -1; } /** diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js index e2b019fbf9..c5e2606852 100644 --- a/tests/lib/rules/no-unsafe.js +++ b/tests/lib/rules/no-unsafe.js @@ -105,22 +105,19 @@ ruleTester.run('no-unsafe', rule, { message: errorMessage('UNSAFE_componentWillMount'), line: 2, column: 7, - endLine: 6, - endColumn: 8 + type: 'ClassDeclaration' }, { message: errorMessage('UNSAFE_componentWillReceiveProps'), line: 2, column: 7, - endLine: 6, - endColumn: 8 + type: 'ClassDeclaration' }, { message: errorMessage('UNSAFE_componentWillUpdate'), line: 2, column: 7, - endLine: 6, - endColumn: 8 + type: 'ClassDeclaration' } ] }, @@ -138,22 +135,19 @@ ruleTester.run('no-unsafe', rule, { message: errorMessage('UNSAFE_componentWillMount'), line: 2, column: 38, - endLine: 6, - endColumn: 10 + type: 'ObjectExpression' }, { message: errorMessage('UNSAFE_componentWillReceiveProps'), line: 2, column: 38, - endLine: 6, - endColumn: 10 + type: 'ObjectExpression' }, { message: errorMessage('UNSAFE_componentWillUpdate'), line: 2, column: 38, - endLine: 6, - endColumn: 10 + type: 'ObjectExpression' } ] } From 0285eef378bf139582d70dd4a8622fcdec94fa13 Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Thu, 21 Jun 2018 02:30:16 +0300 Subject: [PATCH 5/5] Excluded `no-unsafe` from the recommended for now to avoid breaking changes --- README.md | 1 - index.js | 2 +- lib/rules/no-unsafe.js | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 6f83a2d78c..5b787b1afc 100644 --- a/README.md +++ b/README.md @@ -209,7 +209,6 @@ 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) diff --git a/index.js b/index.js index af7da134b7..45a96d7da3 100644 --- a/index.js +++ b/index.js @@ -140,7 +140,7 @@ module.exports = { 'react/no-string-refs': 2, 'react/no-unescaped-entities': 2, 'react/no-unknown-property': 2, - 'react/no-unsafe': 2, + 'react/no-unsafe': 0, 'react/prop-types': 2, 'react/react-in-jsx-scope': 2, 'react/require-render-return': 2 diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js index d168d61491..006e5596fb 100644 --- a/lib/rules/no-unsafe.js +++ b/lib/rules/no-unsafe.js @@ -19,7 +19,7 @@ module.exports = { docs: { description: 'Prevent usage of UNSAFE_ methods', category: 'Best Practices', - recommended: true, + recommended: false, url: docsUrl('no-unsafe') }, schema: []