Added no-unsafe rule
#1831
Added no-unsafe rule
#1831
Conversation
lib/rules/no-unsafe.js
Outdated
|
|
||
| context.report({ | ||
| node: node, | ||
| message: `Do not use ${method}` |
There was a problem hiding this comment.
the message should explain why, and link to more info
There was a problem hiding this comment.
Added details
tests/lib/rules/no-unsafe.js
Outdated
| `, | ||
| errors: [ | ||
| { | ||
| message: 'Do not use UNSAFE_componentWillMount' |
There was a problem hiding this comment.
these tests should include position information, so that we can test that part as well
There was a problem hiding this comment.
Added a few more tests and position details for invalid cases
There was a problem hiding this comment.
It seems that added position details causes issues in eslint 3, see TEST=true ESLINT=3 failed travis builds.
There was a problem hiding this comment.
Adjusted tests to support ESLint 3
|
The rule is applicable for 16.3.0+ only now, good point. |
lib/rules/no-unsafe.js
Outdated
| @@ -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'); | |||
There was a problem hiding this comment.
This seems like a check we could do earlier, and prevent even returning any visitors in the first place?
There was a problem hiding this comment.
Added early termination for the rule.
index.js
Outdated
| @@ -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, | |||
There was a problem hiding this comment.
enabling new rules in an exported config is a semver-major change; please set this to 0 for now.
lib/rules/no-unsafe.js
Outdated
| docs: { | ||
| description: 'Prevent usage of UNSAFE_ methods', | ||
| category: 'Best Practices', | ||
| recommended: true, |
|
Excluded the rule from the recommended for now. Would we like to track it somehow to turn on it in the next major release? |
|
Why not build this logic into the existing no-deprecated rule as opposed to creating a new dedicated rule? |
|
I don't consider them deprecated yet, because there's not a replacement for all of their use cases. |
This Pull Request updates dependency [eslint-plugin-react](https://github.com/yannickcr/eslint-plugin-react) from `v7.9.1` to `v7.10.0` <details> <summary>Release Notes</summary> ### [`v7.10.0`](https://github.com/yannickcr/eslint-plugin-react/blob/master/CHANGELOG.md#​7100---2018-06-24) [Compare Source](jsx-eslint/eslint-plugin-react@v7.9.1...v7.10.0) ##### Added * Allow eslint ^5 ([#​1843][] @​papandreou, @​ljharb) * [`no-unsafe`][] rule ([#​1831][], [#​1830][] @​sergei-startsev) * [`no-will-update-set-state`][]: Account for `UNSAFE_` methods ([#​1845][], [#​1844][] @​alexzherdev) ##### Fixed * [`no-typos`][]: Fix static propTypes handling ([#​1827][], [#​1677][] @​alexzherdev) * [`destructuring-assignment`][]: Allow LHS ([#​1825][], [#​1728][] @​alexzherdev) * [`no-unused-prop-types`][]: Fix crash when encountering mixed union and intersection flow types ([#​1806][] @​yannickcr) ##### Changed * Typo fixes in [`jsx-no-target-blank`][] ([#​1805][] @​ferhatelmas)) [#​1845]: `jsx-eslint/eslint-plugin-react#1845 [#​1844]: `jsx-eslint/eslint-plugin-react#1844 [#​1843]: `jsx-eslint/eslint-plugin-react#1843 [#​1831]: `jsx-eslint/eslint-plugin-react#1831 [#​1830]: `jsx-eslint/eslint-plugin-react#1830 [#​1827]: `jsx-eslint/eslint-plugin-react#1827 [#​1825]: `jsx-eslint/eslint-plugin-react#1825 [#​1806]: `jsx-eslint/eslint-plugin-react#1806 [#​1805]: `jsx-eslint/eslint-plugin-react#1805 [#​1728]: `jsx-eslint/eslint-plugin-react#1728 [#​1677]: `jsx-eslint/eslint-plugin-react#1677 --- </details> --- This PR has been generated by [Renovate Bot](https://renovatebot.com).
Added
no-unsaferule to address the corresponding issue #1830. See discussion details #1750 (comment).Fixes #1830