From ceb73fe7a5d34e10c1f4b19b1e2063c6b54bc32a Mon Sep 17 00:00:00 2001 From: Tiger Oakes Date: Fri, 15 Dec 2023 10:33:24 -0800 Subject: [PATCH] [New] add `forward-ref-uses-ref` rule for checking ref parameter --- CHANGELOG.md | 2 + README.md | 1 + docs/rules/forward-ref-uses-ref.md | 45 ++++ lib/rules/forward-ref-uses-ref.js | 99 +++++++++ lib/rules/index.js | 1 + tests/lib/rules/forward-ref-uses-ref.js | 265 ++++++++++++++++++++++++ 6 files changed, 413 insertions(+) create mode 100644 docs/rules/forward-ref-uses-ref.md create mode 100644 lib/rules/forward-ref-uses-ref.js create mode 100644 tests/lib/rules/forward-ref-uses-ref.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d08f1f0a2..13e52b855a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange ### Added * [`no-string-refs`]: allow this.refs in > 18.3.0 ([#3807][] @henryqdineen) * [`jsx-no-literals`] Add `elementOverrides` option and the ability to ignore this rule on specific elements ([#3812][] @Pearce-Ropion) +* [`forward-ref-uses-ref`]: add rule for checking ref parameter is added ([#3667][] @NotWoods) ### Fixed * [`function-component-definition`], [`boolean-prop-naming`], [`jsx-first-prop-new-line`], [`jsx-props-no-multi-spaces`], `propTypes`: use type args ([#3629][] @HenryBrown0) @@ -27,6 +28,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange [#3812]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3812 [#3731]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3731 +[#3694]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3667 [#3629]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3629 [#3817]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3817 [#3807]: https://github.com/jsx-eslint/eslint-plugin-react/pull/3807 diff --git a/README.md b/README.md index e77a809efe..3df80eb2da 100644 --- a/README.md +++ b/README.md @@ -301,6 +301,7 @@ module.exports = [ | [forbid-elements](docs/rules/forbid-elements.md) | Disallow certain elements | | | | | | | [forbid-foreign-prop-types](docs/rules/forbid-foreign-prop-types.md) | Disallow using another component's propTypes | | | | | | | [forbid-prop-types](docs/rules/forbid-prop-types.md) | Disallow certain propTypes | | | | | | +| [forward-ref-uses-ref](docs/rules/forward-ref-uses-ref.md) | Require all forwardRef components include a ref parameter | | | | 💡 | | | [function-component-definition](docs/rules/function-component-definition.md) | Enforce a specific function type for function components | | | 🔧 | | | | [hook-use-state](docs/rules/hook-use-state.md) | Ensure destructuring and symmetric naming of useState hook value and setter variables | | | | 💡 | | | [iframe-missing-sandbox](docs/rules/iframe-missing-sandbox.md) | Enforce sandbox attribute on iframe elements | | | | | | diff --git a/docs/rules/forward-ref-uses-ref.md b/docs/rules/forward-ref-uses-ref.md new file mode 100644 index 0000000000..68891a091b --- /dev/null +++ b/docs/rules/forward-ref-uses-ref.md @@ -0,0 +1,45 @@ +# Require all forwardRef components include a ref parameter (`react/forward-ref-uses-ref`) + +💡 This rule is manually fixable by [editor suggestions](https://eslint.org/docs/latest/use/core-concepts#rule-suggestions). + + + +Requires that components wrapped with `forwardRef` must have a `ref` parameter. Omitting the `ref` argument is usually a bug, and components not using `ref` don't need to be wrapped by `forwardRef`. + +See + +## Rule Details + +This rule checks all React components using `forwardRef` and verifies that there is a second parameter. + +The following patterns are considered warnings: + +```jsx +var React = require('react'); + +var Component = React.forwardRef((props) => ( +
+)); +``` + +The following patterns are **not** considered warnings: + +```jsx +var React = require('react'); + +var Component = React.forwardRef((props, ref) => ( +
+)); + +var Component = React.forwardRef((props, ref) => ( +
+)); + +function Component(props) { + return
; +}; +``` + +## When not to use + +If you don't want to enforce that components using `forwardRef` utilize the forwarded ref. diff --git a/lib/rules/forward-ref-uses-ref.js b/lib/rules/forward-ref-uses-ref.js new file mode 100644 index 0000000000..aeedeb82df --- /dev/null +++ b/lib/rules/forward-ref-uses-ref.js @@ -0,0 +1,99 @@ +/** + * @fileoverview Require all forwardRef components include a ref parameter + */ + +'use strict'; + +const isParenthesized = require('../util/ast').isParenthesized; +const docsUrl = require('../util/docsUrl'); +const report = require('../util/report'); +const getMessageData = require('../util/message'); + +// ------------------------------------------------------------------------------ +// Rule Definition +// ------------------------------------------------------------------------------ + +/** + * @param {ASTNode} node + * @returns {boolean} If the node represents the identifier `forwardRef`. + */ +function isForwardRefIdentifier(node) { + return node.type === 'Identifier' && node.name === 'forwardRef'; +} + +/** + * @param {ASTNode} node + * @returns {boolean} If the node represents a function call `forwardRef()` or `React.forwardRef()`. + */ +function isForwardRefCall(node) { + return ( + node.type === 'CallExpression' + && ( + isForwardRefIdentifier(node.callee) + || (node.callee.type === 'MemberExpression' && isForwardRefIdentifier(node.callee.property)) + ) + ); +} + +const messages = { + missingRefParameter: 'forwardRef is used with this component but no ref parameter is set', + addRefParameter: 'Add a ref parameter', + removeForwardRef: 'Remove forwardRef wrapper', +}; + +module.exports = { + meta: { + docs: { + description: 'Require all forwardRef components include a ref parameter', + category: 'Possible Errors', + recommended: false, + url: docsUrl('forward-ref-uses-ref'), + }, + messages, + schema: [], + type: 'suggestion', + hasSuggestions: true, + }, + + create(context) { + const sourceCode = context.getSourceCode(); + + return { + 'FunctionExpression, ArrowFunctionExpression'(node) { + if (!isForwardRefCall(node.parent)) { + return; + } + + if (node.params.length === 1) { + report(context, messages.missingRefParameter, 'missingRefParameter', { + node, + suggest: [ + Object.assign( + getMessageData('addRefParameter', messages.addRefParameter), + { + fix(fixer) { + const param = node.params[0]; + // If using shorthand arrow function syntax, add parentheses around the new parameter pair + const shouldAddParentheses = node.type === 'ArrowFunctionExpression' && !isParenthesized(context, param); + return [].concat( + shouldAddParentheses ? fixer.insertTextBefore(param, '(') : [], + fixer.insertTextAfter(param, `, ref${shouldAddParentheses ? ')' : ''}`) + ); + }, + } + ), + Object.assign( + getMessageData('removeForwardRef', messages.removeForwardRef), + { + fix(fixer) { + return fixer.replaceText(node.parent, sourceCode.getText(node)); + }, + } + ), + ], + }); + } + }, + }; + }, +}; diff --git a/lib/rules/index.js b/lib/rules/index.js index 11a4475ba2..1e010b677b 100644 --- a/lib/rules/index.js +++ b/lib/rules/index.js @@ -15,6 +15,7 @@ module.exports = { 'forbid-elements': require('./forbid-elements'), 'forbid-foreign-prop-types': require('./forbid-foreign-prop-types'), 'forbid-prop-types': require('./forbid-prop-types'), + 'forward-ref-uses-ref': require('./forward-ref-uses-ref'), 'function-component-definition': require('./function-component-definition'), 'hook-use-state': require('./hook-use-state'), 'iframe-missing-sandbox': require('./iframe-missing-sandbox'), diff --git a/tests/lib/rules/forward-ref-uses-ref.js b/tests/lib/rules/forward-ref-uses-ref.js new file mode 100644 index 0000000000..906baaca1e --- /dev/null +++ b/tests/lib/rules/forward-ref-uses-ref.js @@ -0,0 +1,265 @@ +/** + * @fileoverview Require all forwardRef components include a ref parameter + * @author Tiger Oakes + */ + +'use strict'; + +// ------------------------------------------------------------------------------ +// Requirements +// ------------------------------------------------------------------------------ + +const RuleTester = require('../../helpers/ruleTester'); +const rule = require('../../../lib/rules/forward-ref-uses-ref'); +const parsers = require('../../helpers/parsers'); + +// ------------------------------------------------------------------------------ +// Tests +// ------------------------------------------------------------------------------ + +const ruleTester = new RuleTester({ + parserOptions: { + ecmaVersion: 2018, + sourceType: 'module', + }, +}); + +ruleTester.run('forward-ref-uses-ref', rule, { + valid: parsers.all([ + { + code: ` + import { forwardRef } from 'react' + forwardRef((props, ref) => { + return null; + }); + `, + }, + { + code: ` + import { forwardRef } from 'react' + forwardRef((props, ref) => null); + `, + }, + { + code: ` + import { forwardRef } from 'react' + forwardRef(function (props, ref) { + return null; + }); + `, + }, + { + code: ` + import { forwardRef } from 'react' + forwardRef(function Component(props, ref) { + return null; + }); + `, + }, + { + code: ` + import * as React from 'react' + React.forwardRef((props, ref) => { + return null; + }); + `, + }, + { + code: ` + import * as React from 'react' + React.forwardRef((props, ref) => null); + `, + }, + { + code: ` + import * as React from 'react' + React.forwardRef(function (props, ref) { + return null; + }); + `, + }, + { + code: ` + import * as React from 'react' + React.forwardRef(function Component(props, ref) { + return null; + }); + `, + }, + { + code: ` + import * as React from 'react' + function Component(props) { + return null; + }; + `, + }, + { + code: ` + import * as React from 'react' + (props) => null; + `, + }, + ]), + invalid: parsers.all([ + { + code: ` + import { forwardRef } from 'react' + forwardRef((props) => { + return null; + }); + `, + errors: [ + { + message: 'forwardRef is used with this component but no ref parameter is set', + suggestions: [ + { + messageId: 'addRefParameter', + output: ` + import { forwardRef } from 'react' + forwardRef((props, ref) => { + return null; + }); + `, + }, + { + messageId: 'removeForwardRef', + output: ` + import { forwardRef } from 'react' + (props) => { + return null; + }; + `, + }, + ], + }, + ], + }, + { + code: ` + import { forwardRef } from 'react' + forwardRef(props => { + return null; + }); + `, + errors: [ + { + message: 'forwardRef is used with this component but no ref parameter is set', + suggestions: [ + { + messageId: 'addRefParameter', + output: ` + import { forwardRef } from 'react' + forwardRef((props, ref) => { + return null; + }); + `, + }, + { + messageId: 'removeForwardRef', + output: ` + import { forwardRef } from 'react' + props => { + return null; + }; + `, + }, + ], + }, + ], + }, + { + code: ` + import * as React from 'react' + React.forwardRef((props) => null); + `, + errors: [ + { + message: 'forwardRef is used with this component but no ref parameter is set', + suggestions: [ + { + messageId: 'addRefParameter', + output: ` + import * as React from 'react' + React.forwardRef((props, ref) => null); + `, + }, + { + messageId: 'removeForwardRef', + output: ` + import * as React from 'react' + (props) => null; + `, + }, + ], + }, + ], + }, + { + code: ` + import { forwardRef } from 'react' + const Component = forwardRef(function (props) { + return null; + }); + `, + errors: [ + { + message: 'forwardRef is used with this component but no ref parameter is set', + suggestions: [ + { + messageId: 'addRefParameter', + output: ` + import { forwardRef } from 'react' + const Component = forwardRef(function (props, ref) { + return null; + }); + `, + }, + { + messageId: 'removeForwardRef', + output: ` + import { forwardRef } from 'react' + const Component = function (props) { + return null; + }; + `, + }, + ], + }, + ], + }, + { + code: ` + import * as React from 'react' + React.forwardRef(function Component(props) { + return null; + }); + `, + errors: [ + { + message: 'forwardRef is used with this component but no ref parameter is set', + suggestions: [ + { + messageId: 'addRefParameter', + output: ` + import * as React from 'react' + React.forwardRef(function Component(props, ref) { + return null; + }); + `, + }, + { + messageId: 'removeForwardRef', + output: ` + import * as React from 'react' + function Component(props) { + return null; + }; + `, + }, + ], + }, + ], + }, + ]), +});