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.

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

[New] forward-ref-uses-ref: add rule for checking ref parameter is added #3667

Merged
merged 1 commit into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | | | | | |
Expand Down
45 changes: 45 additions & 0 deletions docs/rules/forward-ref-uses-ref.md
Original file line number Diff line number Diff line change
@@ -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).

<!-- end auto-generated rule header -->

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 <https://react.dev/reference/react/forwardRef>

## 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) => (
<div />
));
```

The following patterns are **not** considered warnings:

```jsx
var React = require('react');

var Component = React.forwardRef((props, ref) => (
<div ref={ref} />
));

var Component = React.forwardRef((props, ref) => (
<div />
));

function Component(props) {
return <div />;
};
```

## When not to use

If you don't want to enforce that components using `forwardRef` utilize the forwarded ref.
99 changes: 99 additions & 0 deletions lib/rules/forward-ref-uses-ref.js
Original file line number Diff line number Diff line change
@@ -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));
},
}
),
],
});
}
},
};
},
};
1 change: 1 addition & 0 deletions lib/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
Loading