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.

Sign up for GitHub

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

Jump to bottom

Added no-unsafe rule #1831

Merged
merged 5 commits into from
Jun 21, 2018
Merged

Added no-unsafe rule #1831

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b5e86bf
Added `no-unsafe` rule
sergei-startsev Jun 17, 2018
5b08a1e
Adjusted `no-unsafe` rule
sergei-startsev Jun 19, 2018
5e17159
Added missed position details for `no-unsafe`
sergei-startsev Jun 19, 2018
e41500a
Added early termination for no-unsafe, adjusted tests to support ESLint3
sergei-startsev Jun 19, 2018
0285eef
Excluded `no-unsafe` from the recommended for now to avoid breaking c…
sergei-startsev Jun 20, 2018
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
3 changes: 2 additions & 1 deletion docs/rules/no-unsafe.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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

Expand Down
6 changes: 4 additions & 2 deletions lib/rules/no-unsafe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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');
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;
}

/**
Expand All @@ -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`
});
}

Expand Down
66 changes: 56 additions & 10 deletions tests/lib/rules/no-unsafe.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
// ------------------------------------------------------------------------------
Expand All @@ -32,15 +36,17 @@ ruleTester.run('no-unsafe', rule, {
componentDidUpdate() {}
render() {}
}
`
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
const Foo = createReactClass({
componentDidUpdate: function() {},
render: function() {}
});
`
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
Expand All @@ -49,7 +55,8 @@ ruleTester.run('no-unsafe', rule, {
UNSAFE_componentWillReceiveProps() {}
UNSAFE_componentWillUpdate() {}
}
`
`,
settings: {react: {version: '16.4.0'}}
},
{
code: `
Expand All @@ -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'}}
}
],

Expand All @@ -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
}
]
},
Expand All @@ -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')
}
]
}
Expand Down