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

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,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/developer-guide/working-with-rules#providing-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 />
));

var Component = (props) => (
<div />
);
```

## When not to use

If you don't want to enforce that components using `forwardRef` utilize the forwarded ref.
94 changes: 94 additions & 0 deletions lib/rules/forward-ref-uses-ref.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
/**
* @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
// ------------------------------------------------------------------------------

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();
/**
* @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)))
);
}

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];

Check warning on line 72 in lib/rules/forward-ref-uses-ref.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/forward-ref-uses-ref.js#L71-L72

Added lines #L71 - L72 were not covered by tests
// 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 ? ')' : ''}`)

Check warning on line 77 in lib/rules/forward-ref-uses-ref.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/forward-ref-uses-ref.js#L74-L77

Added lines #L74 - L77 were not covered by tests
);
},
}
),
Object.assign(
getMessageData('removeForwardRef', messages.removeForwardRef),
{
fix: (fixer) => fixer.replaceText(node.parent, sourceCode.getText(node)),

Check warning on line 85 in lib/rules/forward-ref-uses-ref.js

View check run for this annotation

Codecov / codecov/patch

lib/rules/forward-ref-uses-ref.js#L85

Added line #L85 was not covered by tests
}
),
],
});
}
},
};
},
};
1 change: 1 addition & 0 deletions lib/rules/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,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