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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[New] Component detection: add util.isReactHookCall #3156

Merged
merged 1 commit into from Dec 22, 2021
Merged

[New] Component detection: add util.isReactHookCall #3156

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel

### Added
* [`function-component-definition`]: support namedComponents option being an array ([#3129][] @petersendidit)
* component detection: add `util.isReactHookCall` ([#3156][] @duncanbeevers)

### Fixed
* [`jsx-indent-props`]: Reset `line.isUsingOperator` correctly after ternary ([#3146][] @tobiaswaltl)
Expand All @@ -16,6 +17,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [Tests] component detection: Add testing scaffolding ([#3149][] @duncanbeevers)
* [New] component detection: track React imports ([#3149][] @duncanbeevers)

[#3156]: https://github.com/yannickcr/eslint-plugin-react/pull/3156
[#3149]: https://github.com/yannickcr/eslint-plugin-react/pull/3149
[#3146]: https://github.com/yannickcr/eslint-plugin-react/pull/3146
[#3129]: https://github.com/yannickcr/eslint-plugin-react/pull/3129
Expand Down
81 changes: 81 additions & 0 deletions lib/util/Components.js
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@

const doctrine = require('doctrine');
const arrayIncludes = require('array-includes');
const fromEntries = require('object.fromentries');
const values = require('object.values');

const variableUtil = require('./variable');
Expand Down Expand Up @@ -46,6 +47,8 @@ function mergeUsedPropTypes(propsList, newPropsList) {
return propsList.concat(propsToAdd);
}

const USE_HOOK_PREFIX_REGEX = /^use[A-Z]/;

const Lists = new WeakMap();
const ReactImports = new WeakMap();

Expand Down Expand Up @@ -787,6 +790,84 @@ function componentRule(rule, context) {
&& !!(node.params || []).length
);
},

/**
* Identify whether a node (CallExpression) is a call to a React hook
*
* @param {ASTNode} node The AST node being searched. (expects CallExpression)
* @param {('useCallback'|'useContext'|'useDebugValue'|'useEffect'|'useImperativeHandle'|'useLayoutEffect'|'useMemo'|'useReducer'|'useRef'|'useState')[]} [expectedHookNames] React hook names to which search is limited.
ljharb marked this conversation as resolved.
Show resolved Hide resolved
* @returns {Boolean} True if the node is a call to a React hook
*/
isReactHookCall(node, expectedHookNames) {
if (node.type !== 'CallExpression') {
return false;
}

const defaultReactImports = components.getDefaultReactImports();
const namedReactImports = components.getNamedReactImports();

const defaultReactImportName = defaultReactImports
&& defaultReactImports[0]
&& defaultReactImports[0].local.name;
const reactHookImportSpecifiers = namedReactImports
&& namedReactImports.filter((specifier) => USE_HOOK_PREFIX_REGEX.test(specifier.imported.name));
const reactHookImportNames = reactHookImportSpecifiers
&& fromEntries(reactHookImportSpecifiers.map((specifier) => [specifier.local.name, specifier.imported.name]));

const isPotentialReactHookCall = defaultReactImportName
&& node.callee.type === 'MemberExpression'
&& node.callee.object.type === 'Identifier'
&& node.callee.object.name === defaultReactImportName
&& node.callee.property.type === 'Identifier'
&& node.callee.property.name.match(USE_HOOK_PREFIX_REGEX);

const isPotentialHookCall = reactHookImportNames
&& node.callee.type === 'Identifier'
&& node.callee.name.match(USE_HOOK_PREFIX_REGEX);

const scope = (isPotentialReactHookCall || isPotentialHookCall) && context.getScope();

const reactResolvedDefs = isPotentialReactHookCall
&& scope.references
&& scope.references.find(
(reference) => reference.identifier.name === defaultReactImportName
).resolved.defs;

const isReactShadowed = isPotentialReactHookCall && reactResolvedDefs
&& reactResolvedDefs.some((reactDef) => reactDef.type !== 'ImportBinding');

const potentialHookReference = isPotentialHookCall
&& scope.references
&& scope.references.find(
(reference) => reactHookImportNames[reference.identifier.name]
);

const hookResolvedDefs = potentialHookReference && potentialHookReference.resolved.defs;
const localHookName = (isPotentialReactHookCall && node.callee.property.name)
|| (isPotentialHookCall && potentialHookReference && node.callee.name);
const isHookShadowed = isPotentialHookCall
&& hookResolvedDefs
&& hookResolvedDefs.some(
(hookDef) => hookDef.name.name === localHookName
&& hookDef.type !== 'ImportBinding'
);

const isHookCall = (isPotentialReactHookCall && !isReactShadowed)
|| (isPotentialHookCall && localHookName && !isHookShadowed);

if (!isHookCall) {
return false;
}

if (!expectedHookNames) {
return true;
}

return arrayIncludes(
expectedHookNames,
(reactHookImportNames && reactHookImportNames[localHookName]) || localHookName
);
},
};

// Component detection instructions
Expand Down
222 changes: 213 additions & 9 deletions tests/util/Components.js
View file
Open in desktop
@@ -1,7 +1,9 @@
'use strict';

const assert = require('assert');
const entries = require('object.entries');
const eslint = require('eslint');
const fromEntries = require('object.fromentries');
const values = require('object.values');

const Components = require('../../lib/util/Components');
Expand All @@ -19,12 +21,32 @@ const ruleTester = new eslint.RuleTester({

describe('Components', () => {
describe('static detect', () => {
function testComponentsDetect(test, done) {
const rule = Components.detect((context, components, util) => ({
'Program:exit'() {
done(context, components, util);
},
}));
function testComponentsDetect(test, instructionsOrDone, orDone) {
const done = orDone || instructionsOrDone;
const instructions = orDone ? instructionsOrDone : instructionsOrDone;

const rule = Components.detect((_context, components, util) => {
const instructionResults = [];

const augmentedInstructions = fromEntries(
entries(instructions || {}).map((nodeTypeAndHandler) => {
const nodeType = nodeTypeAndHandler[0];
const handler = nodeTypeAndHandler[1];
return [nodeType, (node) => {
instructionResults.push({ type: nodeType, result: handler(node, context, components, util) });
Comment on lines +31 to +36
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm a bit confused about these changes; could you walk me through why they're needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the Components test scaffolding is to allow exfiltration of the Components.detect locals to a scope in the test where assert is available, but without having to write a complete rule for every test with its own Program:exit clause.

My original implementation exfiltrated the Components.detect locals, but didn't make it easy to actually test some of those locals; in particular I couldn't test utils because the utility functions expect AST nodes, and by the time Program:exit fires, I've lost the CallExpression nodes I need.

These changes are intended to aid in writing new test cases for Components functionality; specifically, for testing utils functionality whose methods expect many different AST node types besides CallExpression.

Custom instructions passed into the testComponentsDetect get augmented and then incorporated into a custom rule, and when that rule runs over code the augmented instructions do two things:

  1. Provide the Components.detect locals to the instruction callback. Ordinarily the instruction callback only receives a node as an argument and relies on having been defined in the Components.detect closure to access the special locals it provides.
  2. Accumulate a log in instructionResults as the visitor traverses the parsed AST; the log tracks which node type was traversed, and the result of that visit.

Today

Taken together, these allow creation of test cases intended to read something like:

The visitor visited a CallExpression node
The visitor had access to the util local
The util function isReactHookCall returned the following result for the visited CallExpression node

Tomorrow

Down the road, as we work to flesh out coverage for the code in Components, we can write new tests cases.

The visitor visited a ClassDeclaration
The visitor had access to the util local
The util function isES6Component returned the following result for the visited ClassDeclaration node

}];
})
);

return Object.assign({}, augmentedInstructions, {
'Program:exit'(node) {
if (augmentedInstructions['Program:exit']) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't yet exercised, but I figured if someone needed to test util functionality when handling the Program:exit instruction it should behave the same way as handling any other instruction type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a self-test section to the test/util/Components file in order to exercise the logging behavior while visiting Program:exit instructions.

augmentedInstructions['Program:exit'](node, context, components, util);
}
done(components, instructionResults);
},
});
});

const tests = {
valid: parsers.all([Object.assign({}, test, {
Expand All @@ -36,6 +58,7 @@ describe('Components', () => {
})]),
invalid: [],
};

ruleTester.run(test.code, rule, tests);
}

Expand All @@ -45,7 +68,7 @@ describe('Components', () => {
function MyStatelessComponent() {
return <React.Fragment />;
}`,
}, (_context, components) => {
}, (components) => {
assert.equal(components.length(), 1, 'MyStatelessComponent should be detected component');
values(components.list()).forEach((component) => {
assert.equal(
Expand All @@ -65,7 +88,7 @@ describe('Components', () => {
return <React.Fragment />;
}
}`,
}, (_context, components) => {
}, (components) => {
assert(components.length() === 1, 'MyClassComponent should be detected component');
values(components.list()).forEach((component) => {
assert.equal(
Expand All @@ -80,7 +103,7 @@ describe('Components', () => {
it('should detect React Imports', () => {
testComponentsDetect({
code: 'import React, { useCallback, useState } from \'react\'',
}, (_context, components) => {
}, (components) => {
assert.deepEqual(
components.getDefaultReactImports().map((specifier) => specifier.local.name),
['React'],
Expand All @@ -94,5 +117,186 @@ describe('Components', () => {
);
});
});

describe('utils', () => {
describe('isReactHookCall', () => {
it('should not identify hook-like call', () => {
testComponentsDetect({
code: `import { useRef } from 'react'
function useColor() {
return useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: false }]);
});
});

it('should identify hook call', () => {
testComponentsDetect({
code: `import { useState } from 'react'
function useColor() {
return useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should identify aliased hook call', () => {
testComponentsDetect({
code: `import { useState as useStateAlternative } from 'react'
function useColor() {
return useStateAlternative()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should identify aliased present named hook call', () => {
testComponentsDetect({
code: `import { useState as useStateAlternative } from 'react'
function useColor() {
return useStateAlternative()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node, ['useState']),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should not identify shadowed hook call', () => {
testComponentsDetect({
code: `import { useState } from 'react'
function useColor() {
function useState() {
return null
}
return useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: false }]);
});
});

it('should not identify shadowed aliased present named hook call', () => {
testComponentsDetect({
code: `import { useState as useStateAlternative } from 'react'
function useColor() {
function useStateAlternative() {
return null
}
return useStateAlternative()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node, ['useState']),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: false }]);
});
});

it('should identify React hook call', () => {
testComponentsDetect({
code: `import React from 'react'
function useColor() {
return React.useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should identify aliased React hook call', () => {
testComponentsDetect({
code: `import ReactAlternative from 'react'
function useColor() {
return ReactAlternative.useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should not identify shadowed React hook call', () => {
testComponentsDetect({
code: `import React from 'react'
function useColor() {
const React = {
useState: () => null
}
return React.useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: false }]);
});
});

it('should identify present named hook call', () => {
testComponentsDetect({
code: `import { useState } from 'react'
function useColor() {
return useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node, ['useState']),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should identify present named React hook call', () => {
testComponentsDetect({
code: `import React from 'react'
function useColor() {
return React.useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node, ['useState']),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: true }]);
});
});

it('should not identify missing named hook call', () => {
testComponentsDetect({
code: `import { useState } from 'react'
function useColor() {
return useState()
}`,
}, {
CallExpression: (node, _context, _components, util) => util.isReactHookCall(node, ['useRef']),
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'CallExpression', result: false }]);
});
});
});
});

describe('testComponentsDetect', () => {
it('should log Program:exit instruction', () => {
testComponentsDetect({
code: '',
}, {
'Program:exit': () => true,
}, (_components, instructionResults) => {
assert.deepEqual(instructionResults, [{ type: 'Program:exit', result: true }]);
});
});
});
});
});