From 65116d6a681ccde37552050315adf236d4d3cf6b Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 4 Dec 2018 02:33:58 +0300 Subject: [PATCH 1/2] Adjust `no-unsafe` rule to handle all unsafe life-cycle methods - Adjust `no-unsafe` rule to handle all unsafe life-cycle methods including their aliases. - Add instructions on updating components to be consistent with React runtime warnings. --- README.md | 2 +- docs/rules/no-unsafe.md | 31 ++++++- lib/rules/no-unsafe.js | 33 +++++-- tests/lib/rules/no-unsafe.js | 170 +++++++++++++++++++++++++++++++++-- 4 files changed, 216 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 4e000f5989..a526a08ef8 100644 --- a/README.md +++ b/README.md @@ -119,7 +119,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-unsafe](docs/rules/no-unsafe.md): Prevent usage of unsafe lifecycle 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` diff --git a/docs/rules/no-unsafe.md b/docs/rules/no-unsafe.md index 87d30d7c90..9b5ca65c73 100644 --- a/docs/rules/no-unsafe.md +++ b/docs/rules/no-unsafe.md @@ -1,4 +1,4 @@ -# Prevent usage of `UNSAFE_` methods (react/no-unsafe) +# Prevent usage of unsafe lifecycle 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]. @@ -6,13 +6,22 @@ Certain legacy lifecycle methods are [unsafe for use in async React applications [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`. +The rule checks the following methods: +- `componentWillMount` (and `UNSAFE_componentWillMount` alias) +- `componentWillReceiveProps` (and `UNSAFE_componentWillReceiveProps` alias) +- `componentWillUpdate` (and `UNSAFE_componentWillUpdate` alias) ## Rule Details The following patterns are considered warnings: ```jsx +class Foo extends React.Component { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} +} +// or class Foo extends React.Component { UNSAFE_componentWillMount() {} UNSAFE_componentWillReceiveProps() {} @@ -21,6 +30,12 @@ class Foo extends React.Component { ``` ```jsx +const Foo = createReactClass({ + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {} +}); +// or const Foo = createReactClass({ UNSAFE_componentWillMount: function() {}, UNSAFE_componentWillReceiveProps: function() {}, @@ -31,6 +46,12 @@ const Foo = createReactClass({ The following patterns are **not** considered warnings: ```jsx +class Foo extends Bar { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} +} +// or class Foo extends Bar { UNSAFE_componentWillMount() {} UNSAFE_componentWillReceiveProps() {} @@ -39,6 +60,12 @@ class Foo extends Bar { ``` ```jsx +const Foo = bar({ + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {} +}); +// or const Foo = bar({ UNSAFE_componentWillMount: function() {}, UNSAFE_componentWillReceiveProps: function() {}, diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js index 006e5596fb..c79244d832 100644 --- a/lib/rules/no-unsafe.js +++ b/lib/rules/no-unsafe.js @@ -1,5 +1,5 @@ /** - * @fileoverview Prevent usage of UNSAFE_ methods + * @fileoverview Prevent usage of unsafe lifecycle methods * @author Sergei Startsev */ @@ -17,7 +17,7 @@ const versionUtil = require('../util/version'); module.exports = { meta: { docs: { - description: 'Prevent usage of UNSAFE_ methods', + description: 'Prevent usage of unsafe lifecycle methods', category: 'Best Practices', recommended: false, url: docsUrl('no-unsafe') @@ -31,16 +31,29 @@ module.exports = { return {}; } + const unsafe = {}; + unsafe.componentWillMount = unsafe.UNSAFE_componentWillMount = { + newMethod: 'componentDidMount', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + }; + unsafe.componentWillReceiveProps = unsafe.UNSAFE_componentWillReceiveProps = { + newMethod: 'getDerivedStateFromProps', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + }; + unsafe.componentWillUpdate = unsafe.UNSAFE_componentWillUpdate = { + newMethod: 'componentDidUpdate', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + }; + /** * Returns a list of unsafe methods * @returns {Array} A list of unsafe methods */ function getUnsafeMethods() { - return [ - 'UNSAFE_componentWillMount', - 'UNSAFE_componentWillReceiveProps', - 'UNSAFE_componentWillUpdate' - ]; + return Object.keys(unsafe); } /** @@ -63,9 +76,13 @@ module.exports = { return; } + const meta = unsafe[method]; + const newMethod = meta.newMethod; + const details = meta.details; + 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` + message: `${method} is unsafe for use in async rendering. Update the component to use ${newMethod} instead. ${details}` }); } diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js index c5e2606852..cd2e088eb1 100644 --- a/tests/lib/rules/no-unsafe.js +++ b/tests/lib/rules/no-unsafe.js @@ -1,5 +1,5 @@ /** - * @fileoverview Prevent usage of UNSAFE_ methods + * @fileoverview Prevent usage of unsafe lifecycle methods * @author Sergei Startsev */ 'use strict'; @@ -19,8 +19,8 @@ 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`; +function errorMessage(method, newMethod, details) { + return `${method} is unsafe for use in async rendering. Update the component to use ${newMethod} instead. ${details}`; } // ------------------------------------------------------------------------------ @@ -48,6 +48,16 @@ ruleTester.run('no-unsafe', rule, { `, settings: {react: {version: '16.4.0'}} }, + { + code: ` + class Foo extends Bar { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} + } + `, + settings: {react: {version: '16.4.0'}} + }, { code: ` class Foo extends Bar { @@ -58,6 +68,16 @@ ruleTester.run('no-unsafe', rule, { `, settings: {react: {version: '16.4.0'}} }, + { + code: ` + const Foo = bar({ + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {}, + }); + `, + settings: {react: {version: '16.4.0'}} + }, { code: ` const Foo = bar({ @@ -68,6 +88,17 @@ ruleTester.run('no-unsafe', rule, { `, settings: {react: {version: '16.4.0'}} }, + // React.Component + { + code: ` + class Foo extends React.Component { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} + } + `, + settings: {react: {version: '16.2.0'}} + }, { code: ` class Foo extends React.Component { @@ -78,6 +109,17 @@ ruleTester.run('no-unsafe', rule, { `, settings: {react: {version: '16.2.0'}} }, + // createReactClass + { + code: ` + const Foo = createReactClass({ + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {}, + }); + `, + settings: {react: {version: '16.2.0'}} + }, { code: ` const Foo = createReactClass({ @@ -91,6 +133,49 @@ ruleTester.run('no-unsafe', rule, { ], invalid: [ + // React.Component + { + code: ` + class Foo extends React.Component { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} + } + `, + settings: {react: {version: '16.3.0'}}, + errors: [ + { + message: errorMessage( + 'componentWillMount', + 'componentDidMount', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 9, + type: 'ClassDeclaration' + }, + { + message: errorMessage( + 'componentWillReceiveProps', + 'getDerivedStateFromProps', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 9, + type: 'ClassDeclaration' + }, + { + message: errorMessage( + 'componentWillUpdate', + 'componentDidUpdate', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 9, + type: 'ClassDeclaration' + } + ] + }, { code: ` class Foo extends React.Component { @@ -102,25 +187,80 @@ ruleTester.run('no-unsafe', rule, { settings: {react: {version: '16.3.0'}}, errors: [ { - message: errorMessage('UNSAFE_componentWillMount'), + message: errorMessage( + 'UNSAFE_componentWillMount', + 'componentDidMount', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 7, type: 'ClassDeclaration' }, { - message: errorMessage('UNSAFE_componentWillReceiveProps'), + message: errorMessage( + 'UNSAFE_componentWillReceiveProps', + 'getDerivedStateFromProps', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 7, type: 'ClassDeclaration' }, { - message: errorMessage('UNSAFE_componentWillUpdate'), + message: errorMessage( + 'UNSAFE_componentWillUpdate', + 'componentDidUpdate', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 7, type: 'ClassDeclaration' } ] }, + // createReactClass + { + code: ` + const Foo = createReactClass({ + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {}, + }); + `, + settings: {react: {version: '16.3.0'}}, + errors: [ + { + message: errorMessage( + 'componentWillMount', + 'componentDidMount', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 40, + type: 'ObjectExpression' + }, + { + message: errorMessage( + 'componentWillReceiveProps', + 'getDerivedStateFromProps', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 40, + type: 'ObjectExpression' + }, + { + message: errorMessage( + 'componentWillUpdate', + 'componentDidUpdate', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), + line: 2, + column: 40, + type: 'ObjectExpression' + } + ] + }, { code: ` const Foo = createReactClass({ @@ -132,19 +272,31 @@ ruleTester.run('no-unsafe', rule, { settings: {react: {version: '16.3.0'}}, errors: [ { - message: errorMessage('UNSAFE_componentWillMount'), + message: errorMessage( + 'UNSAFE_componentWillMount', + 'componentDidMount', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 38, type: 'ObjectExpression' }, { - message: errorMessage('UNSAFE_componentWillReceiveProps'), + message: errorMessage( + 'UNSAFE_componentWillReceiveProps', + 'getDerivedStateFromProps', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 38, type: 'ObjectExpression' }, { - message: errorMessage('UNSAFE_componentWillUpdate'), + message: errorMessage( + 'UNSAFE_componentWillUpdate', + 'componentDidUpdate', + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + ), line: 2, column: 38, type: 'ObjectExpression' From 8d2c7a9831caac98be84037f3c9f95fcb77a832f Mon Sep 17 00:00:00 2001 From: Sergei Startsev Date: Tue, 11 Dec 2018 01:12:11 +0300 Subject: [PATCH 2/2] Add `aliases` option to check aliases of unsafe methods Avoid breaking changes by adding the non-prefixed methods behind the option --- docs/rules/no-unsafe.md | 69 ++++++++++++++++++++++++------------ lib/rules/no-unsafe.js | 52 ++++++++++++++++++--------- tests/lib/rules/no-unsafe.js | 8 +++-- 3 files changed, 87 insertions(+), 42 deletions(-) diff --git a/docs/rules/no-unsafe.md b/docs/rules/no-unsafe.md index 9b5ca65c73..ffdb3400d4 100644 --- a/docs/rules/no-unsafe.md +++ b/docs/rules/no-unsafe.md @@ -16,12 +16,6 @@ The rule checks the following methods: The following patterns are considered warnings: ```jsx -class Foo extends React.Component { - componentWillMount() {} - componentWillReceiveProps() {} - componentWillUpdate() {} -} -// or class Foo extends React.Component { UNSAFE_componentWillMount() {} UNSAFE_componentWillReceiveProps() {} @@ -30,12 +24,6 @@ class Foo extends React.Component { ``` ```jsx -const Foo = createReactClass({ - componentWillMount: function() {}, - componentWillReceiveProps: function() {}, - componentWillUpdate: function() {} -}); -// or const Foo = createReactClass({ UNSAFE_componentWillMount: function() {}, UNSAFE_componentWillReceiveProps: function() {}, @@ -46,12 +34,6 @@ const Foo = createReactClass({ The following patterns are **not** considered warnings: ```jsx -class Foo extends Bar { - componentWillMount() {} - componentWillReceiveProps() {} - componentWillUpdate() {} -} -// or class Foo extends Bar { UNSAFE_componentWillMount() {} UNSAFE_componentWillReceiveProps() {} @@ -61,14 +43,55 @@ class Foo extends Bar { ```jsx const Foo = bar({ + UNSAFE_componentWillMount: function() {}, + UNSAFE_componentWillReceiveProps: function() {}, + UNSAFE_componentWillUpdate: function() {} +}); +``` + +## Rule Options +```json +... +"react/no-unsafe": [, { "checkAliases": }] +... +``` + +### `checkAliases` (default: `false`) + +When `true` the rule will also check aliases of unsafe methods: `componentWillMount`, `componentWillReceiveProps`, `componentWillUpdate`. + +The following patterns are considered warnings: + +```jsx +class Foo extends React.Component { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} +} +``` + +```jsx +const Foo = createReactClass({ componentWillMount: function() {}, componentWillReceiveProps: function() {}, componentWillUpdate: function() {} }); -// or +``` + +The following patterns are **not** considered warnings: + +```jsx +class Foo extends Bar { + componentWillMount() {} + componentWillReceiveProps() {} + componentWillUpdate() {} +} +``` + +```jsx const Foo = bar({ - UNSAFE_componentWillMount: function() {}, - UNSAFE_componentWillReceiveProps: function() {}, - UNSAFE_componentWillUpdate: function() {} + componentWillMount: function() {}, + componentWillReceiveProps: function() {}, + componentWillUpdate: function() {} }); -``` +``` \ No newline at end of file diff --git a/lib/rules/no-unsafe.js b/lib/rules/no-unsafe.js index c79244d832..704e004a65 100644 --- a/lib/rules/no-unsafe.js +++ b/lib/rules/no-unsafe.js @@ -22,31 +22,51 @@ module.exports = { recommended: false, url: docsUrl('no-unsafe') }, - schema: [] + schema: [ + { + type: 'object', + properties: { + checkAliases: { + default: false, + type: 'boolean' + } + }, + additionalProperties: false + } + ] }, create: Components.detect((context, components, utils) => { + const config = context.options[0] || {}; + const checkAliases = config.checkAliases || false; + const isApplicable = versionUtil.testReactVersion(context, '16.3.0'); if (!isApplicable) { return {}; } - const unsafe = {}; - unsafe.componentWillMount = unsafe.UNSAFE_componentWillMount = { - newMethod: 'componentDidMount', - details: - 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' - }; - unsafe.componentWillReceiveProps = unsafe.UNSAFE_componentWillReceiveProps = { - newMethod: 'getDerivedStateFromProps', - details: - 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' - }; - unsafe.componentWillUpdate = unsafe.UNSAFE_componentWillUpdate = { - newMethod: 'componentDidUpdate', - details: - 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + const unsafe = { + UNSAFE_componentWillMount: { + newMethod: 'componentDidMount', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + }, + UNSAFE_componentWillReceiveProps: { + newMethod: 'getDerivedStateFromProps', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + }, + UNSAFE_componentWillUpdate: { + newMethod: 'componentDidUpdate', + details: + 'See https://reactjs.org/blog/2018/03/27/update-on-async-rendering.html.' + } }; + if (checkAliases) { + unsafe.componentWillMount = unsafe.UNSAFE_componentWillMount; + unsafe.componentWillReceiveProps = unsafe.UNSAFE_componentWillReceiveProps; + unsafe.componentWillUpdate = unsafe.UNSAFE_componentWillUpdate; + } /** * Returns a list of unsafe methods diff --git a/tests/lib/rules/no-unsafe.js b/tests/lib/rules/no-unsafe.js index cd2e088eb1..b3184ba0e4 100644 --- a/tests/lib/rules/no-unsafe.js +++ b/tests/lib/rules/no-unsafe.js @@ -97,7 +97,7 @@ ruleTester.run('no-unsafe', rule, { componentWillUpdate() {} } `, - settings: {react: {version: '16.2.0'}} + settings: {react: {version: '16.4.0'}} }, { code: ` @@ -118,7 +118,7 @@ ruleTester.run('no-unsafe', rule, { componentWillUpdate: function() {}, }); `, - settings: {react: {version: '16.2.0'}} + settings: {react: {version: '16.4.0'}} }, { code: ` @@ -142,7 +142,8 @@ ruleTester.run('no-unsafe', rule, { componentWillUpdate() {} } `, - settings: {react: {version: '16.3.0'}}, + options: [{checkAliases: true}], + settings: {react: {version: '16.4.0'}}, errors: [ { message: errorMessage( @@ -227,6 +228,7 @@ ruleTester.run('no-unsafe', rule, { componentWillUpdate: function() {}, }); `, + options: [{checkAliases: true}], settings: {react: {version: '16.3.0'}}, errors: [ {