diff --git a/packages/eslint-config/rules/helpers.js b/packages/eslint-config/rules/helpers.js new file mode 100644 index 0000000..e237c84 --- /dev/null +++ b/packages/eslint-config/rules/helpers.js @@ -0,0 +1,70 @@ +// The next function is taken form here +// https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js +/** + * Gets the static name of a function AST node. For function declarations it is + * easy. For anonymous function expressions it is much harder. If you search for + * `IsAnonymousFunctionDefinition()` in the ECMAScript spec you'll find places + * where JS gives anonymous function expressions names. We roughly detect the + * same AST nodes with some exceptions to better fit our use case. + */ + +function getFunctionName(node) { + if (node.type === 'FunctionDeclaration' || (node.type === 'FunctionExpression' && node.id)) { + // function useHook() {} + // const whatever = function useHook() {}; + // + // Function declaration or function expression names win over any + // assignment statements or other renames. + return node.id; + } + + if (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') { + if (node.parent.type === 'VariableDeclarator' && node.parent.init === node) { + // const useHook = () => {}; + return node.parent.id; + } + + if (node.parent.type === 'AssignmentExpression' && node.parent.right === node && node.parent.operator === '=') { + // useHook = () => {}; + return node.parent.left; + } + + if (node.parent.type === 'Property' && node.parent.value === node && !node.parent.computed) { + // {useHook: () => {}} + // {useHook() {}} + return node.parent.key; + + // NOTE: We could also support `ClassProperty` and `MethodDefinition` + // here to be pedantic. However, hooks in a class are an anti-pattern. So + // we don't allow it to error early. + // + // class {useHook = () => {}} + // class {useHook() {}} + } + + if (node.parent.type === 'AssignmentPattern' && node.parent.right === node && !node.parent.computed) { + // const {useHook = () => {}} = {}; + // ({useHook = () => {}} = {}); + // + // Kinda clowny, but we'd said we'd follow spec convention for + // `IsAnonymousFunctionDefinition()` usage. + return node.parent.left; + } + + return undefined; + } + + return undefined; +} + +/** + * Checks if the node is a React component name. React component names must + * always start with an uppercase letter. + */ + +function isComponentName(node) { + return node.type === 'Identifier' && /^[A-Z]/.test(node.name); +} + +exports.isComponentName = isComponentName; +exports.getFunctionName = getFunctionName; diff --git a/packages/eslint-config/rules/index.js b/packages/eslint-config/rules/index.js index 5f6c2a9..7ac6bc2 100644 --- a/packages/eslint-config/rules/index.js +++ b/packages/eslint-config/rules/index.js @@ -3,7 +3,9 @@ const projectName = 'salute-rules'; const fs = require('fs'); const path = require('path'); -const ruleFiles = fs.readdirSync(__dirname).filter((file) => file !== 'index.js' && !file.endsWith('test.js')); +const ruleFiles = fs + .readdirSync(__dirname) + .filter((file) => file !== 'index.js' && file !== 'helpers.js' && !file.endsWith('test.js')); const configs = { all: { diff --git a/packages/eslint-config/rules/no-modifying-ref-on-render.js b/packages/eslint-config/rules/no-modifying-ref-on-render.js new file mode 100644 index 0000000..e102bc4 --- /dev/null +++ b/packages/eslint-config/rules/no-modifying-ref-on-render.js @@ -0,0 +1,85 @@ +const { isComponentName, getFunctionName } = require('./helpers'); +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +function reportError(context, node) { + context.report({ + node, + message: 'Do not modifying ref on render', + }); +} + +module.exports = { + meta: { + type: null, // `problem`, `suggestion`, or `layout` + docs: { + description: 'Do not modifying ref on render', + recommended: true, + url: 'https://react.dev/reference/react/useRef#caveats', + }, + fixable: null, // Or `code` or `whitespace` + schema: [], // Add a schema if the rule has options + }, + + create: (context) => { + let isInUseEffect = false; + let currentComponent; + let currentComponentRefNames = []; + + return { + CallExpression(node) { + // Checking if the invoked function is useRef + if (node.callee.type === 'Identifier' && node.callee.name === 'useRef') { + const isDeclaration = + node.parent && + node.parent.type === 'VariableDeclarator' && + node.parent.id.type === 'Identifier'; + + if (!isDeclaration) { + return; + } + + const refName = node.parent.id.name; + currentComponentRefNames.push(refName); + } + + // Checking if the invoked function is useEffect + if (node.callee.type === 'Identifier' && node.callee.name === 'useEffect') { + isInUseEffect = true; + } + }, + 'CallExpression:exit': function (node) { + if (node.callee.type === 'Identifier' && node.callee.name === 'useEffect') { + isInUseEffect = false; + } + }, + + ':function': function (node) { + const name = getFunctionName(node); + currFunctionName = name && name.name; + + if (name && isComponentName(name)) { + currentComponent = node; + } + }, + + ':function:exit': function (node) { + if (node === currentComponent) { + currentComponent = null; + currentComponentRefNames = []; + } + }, + + 'AssignmentExpression > MemberExpression': function (node) { + if ( + currentComponentRefNames.includes(node.object.name) && + node.property.name === 'current' && + !isInUseEffect + ) { + reportError(context, node); + } + }, + }; + }, +}; diff --git a/packages/eslint-config/rules/no-modifying-ref-on-render.test.js b/packages/eslint-config/rules/no-modifying-ref-on-render.test.js new file mode 100644 index 0000000..047790d --- /dev/null +++ b/packages/eslint-config/rules/no-modifying-ref-on-render.test.js @@ -0,0 +1,50 @@ +const { RuleTester } = require('eslint'); + +const rule = require('./no-modifying-ref-on-render'); + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + ecmaFeatures: { + jsx: true, + }, + }, +}); + +// Use only: true, too run one test only + +const tests = { + valid: [ + { + code: ` + const MyComponent = (props) => { + const ref = useRef(props); + + useEffect(() => { + ref.current = props; + }) + + return
text
; + }`, + }, + ], + invalid: [ + { + code: ` + const MyComponent = (props) => { + const ref = useRef(props); + ref.current = props; + + return
text
; + }`, + errors: [errorMsg()], + }, + ], +}; + +function errorMsg() { + return { message: 'Do not modifying ref on render' }; +} + +ruleTester.run('no-modifying-ref-on-render', rule, tests); diff --git a/packages/eslint-config/rules/no-redundant-commit.js b/packages/eslint-config/rules/no-redundant-commit.js index 354d1a7..142da47 100644 --- a/packages/eslint-config/rules/no-redundant-commit.js +++ b/packages/eslint-config/rules/no-redundant-commit.js @@ -1,70 +1,4 @@ -// The next function is taken form here -// https://github.com/facebook/react/blob/main/packages/eslint-plugin-react-hooks/src/RulesOfHooks.js -/** - * Gets the static name of a function AST node. For function declarations it is - * easy. For anonymous function expressions it is much harder. If you search for - * `IsAnonymousFunctionDefinition()` in the ECMAScript spec you'll find places - * where JS gives anonymous function expressions names. We roughly detect the - * same AST nodes with some exceptions to better fit our use case. - */ - -function getFunctionName(node) { - if (node.type === 'FunctionDeclaration' || (node.type === 'FunctionExpression' && node.id)) { - // function useHook() {} - // const whatever = function useHook() {}; - // - // Function declaration or function expression names win over any - // assignment statements or other renames. - return node.id; - } - - if (node.type === 'FunctionExpression' || node.type === 'ArrowFunctionExpression') { - if (node.parent.type === 'VariableDeclarator' && node.parent.init === node) { - // const useHook = () => {}; - return node.parent.id; - } - - if (node.parent.type === 'AssignmentExpression' && node.parent.right === node && node.parent.operator === '=') { - // useHook = () => {}; - return node.parent.left; - } - - if (node.parent.type === 'Property' && node.parent.value === node && !node.parent.computed) { - // {useHook: () => {}} - // {useHook() {}} - return node.parent.key; - - // NOTE: We could also support `ClassProperty` and `MethodDefinition` - // here to be pedantic. However, hooks in a class are an anti-pattern. So - // we don't allow it to error early. - // - // class {useHook = () => {}} - // class {useHook() {}} - } - - if (node.parent.type === 'AssignmentPattern' && node.parent.right === node && !node.parent.computed) { - // const {useHook = () => {}} = {}; - // ({useHook = () => {}} = {}); - // - // Kinda clowny, but we'd said we'd follow spec convention for - // `IsAnonymousFunctionDefinition()` usage. - return node.parent.left; - } - - return undefined; - } - - return undefined; -} - -/** - * Checks if the node is a React component name. React component names must - * always start with an uppercase letter. - */ - -function isComponentName(node) { - return node.type === 'Identifier' && /^[A-Z]/.test(node.name); -} +const { isComponentName, getFunctionName } = require('./helpers'); // taken from https://github.com/eslint-community/eslint-plugin-promise/tree/main function hasPromiseCallback(node) { @@ -111,20 +45,19 @@ module.exports = { currentComponentStateSettersNames.includes(node.callee.name) && !isInAsync ) { - if ( isInUseEffect && - callExpressionNestingLevel == 2 && // function is called on the level of useEffect(()=> {'around here and not deeper' }, []) + callExpressionNestingLevel == 2 && // function is called on the level of useEffect(()=> {'around here and not deeper' }, []) UseEffectFunctionsNestingLevel == 1 - ) { + ) { context.report({ node, message: 'Avoid using synchronous state setters within effects', }); - } else { // call is inside new function + } else { + // call is inside new function currentComponentStateSettersNames.push(currFunctionName); } - } // Checking if the invoked function is useEffect