Skip to content

Commit

Permalink
Add JSX support to react-hooks/exhaustive-deps (prototype)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrstnv authored and Kristina Volosiuk committed Sep 20, 2020
1 parent 6291255 commit 7a458e2
Show file tree
Hide file tree
Showing 2 changed files with 296 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,138 @@ const tests = {
}
`,
},
/**
* JSX-dependencies support tests
*/
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => <Component />
}), [Component]);
return memoized.render();
}
`,
},
// HTML-elements are not React components and
// must not be in the dependencies list
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => (
<div>
<Component />
</div>
)
}), [Component]);
return memoized.render();
}
`,
},
// If JSX-component is in the return statement,
// it must be in the dependencies list also
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => {
return <Component />;
}
}), [Component]);
return memoized.render();
}
`,
},
// If the Component is used twice or more,
// it must be declared as a dependency only once
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => {
return (
<div>
<Component />
<Component />
</div>
);
}
}), [Component]);
return memoized.render();
}
`,
},
// React Fragments are not hook dependencies
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => (
<Fragment>
<Component />
</Fragment>
)
}), [Component]);
return memoized.render();
}
`,
},
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => (
<>
<div />
<Component />
</>
)
}), [Component]);
return memoized.render();
}
`,
},
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => (
<React.Fragment>
<Component />
</React.Fragment>
)
}), [Component]);
return memoized.render();
}
`,
},
// Also check Fragments in the return statement
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => {
return (
<>
<div />
<Component />
</>
);
}
}), [Component]);
return memoized.render();
}
`,
},
],
invalid: [
{
Expand Down Expand Up @@ -6967,6 +7099,73 @@ const tests = {
},
],
},
/**
* JSX-dependencies support tests
*/
{
code: normalizeIndent`
function Example({component: UnknownComponent}) {
const memoized = useMemo(() => ({
render: () => <UnknownComponent/>
}), []);
return memoized.render();
}
`,
errors: [
{
message:
"React Hook useMemo has a missing dependency: 'UnknownComponent'. " +
'Either include it or remove the dependency array.',
suggestions: [
{
desc: 'Update the dependencies array to be: [UnknownComponent]',
output: normalizeIndent`
function Example({component: UnknownComponent}) {
const memoized = useMemo(() => ({
render: () => <UnknownComponent/>
}), [UnknownComponent]);
return memoized.render();
}
`,
},
],
},
],
},
{
code: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => null
}), [Component]);
return memoized.render();
}
`,
errors: [
{
message:
"React Hook useMemo has an unnecessary dependency: 'Component'. " +
'Either exclude it or remove the dependency array.',
suggestions: [
{
desc: 'Update the dependencies array to be: []',
output: normalizeIndent`
function Example({component: Component}) {
const memoized = useMemo(() => ({
render: () => null
}), []);
return memoized.render();
}
`,
},
],
},
],
},
],
};

Expand Down
98 changes: 97 additions & 1 deletion packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -576,11 +576,103 @@ export default {
}
}

gatherJSXDependencies(currentScope);

for (const childScope of currentScope.childScopes) {
gatherDependenciesRecursively(childScope);
}
}

function gatherJSXDependencies(currentScope) {
if (currentScope.type !== 'function') return;

const {body} = currentScope.block;
const JSXComponents = [];

// check JSXElement/JSXFragment for arrow functions
// and BlockStatement for traditional functions
switch (body.type) {
case 'JSXElement':
case 'JSXFragment':
findJSXComponents(body);
break;
case 'BlockStatement':
const bodyElements = [].concat(body.body);

bodyElements.forEach(element => {
if (
element.type === 'ReturnStatement' &&
(element.argument.type === 'JSXElement' ||
isFragment(element.argument))
) {
findJSXComponents(element.argument);
}
});
break;
default:
break;
}

function isHTMLElement(name) {
return /^[a-z]/.test(name);
}

function isFragment(element) {
// <> </>
if (element.type === 'JSXFragment') return true;

if (element.type !== 'JSXElement') return false;

const name = element.openingElement.name;

// <Fragment> </Fragment>
return (
name.name === 'Fragment' ||
// <React.Fragment> </React.Fragment>
(name.type === 'JSXMemberExpression' &&
name.object.name === 'React' &&
name.object.type === 'JSXIdentifier' &&
name.property.name === 'Fragment' &&
name.property.type === 'JSXIdentifier')
);
}

// check JSX elements tree
function findJSXComponents(element) {
const elements = [element];

while (elements.length) {
const current = elements.shift();

if (!isFragment(current)) {
const {openingElement} = current;
const elementName = openingElement.name.name;

if (
!isHTMLElement(elementName) &&
!JSXComponents.includes(elementName)
) {
JSXComponents.push(elementName);
}
}

const {children} = current;

children.forEach(child => {
if (child.type === 'JSXElement') {
elements.push(child);
}
});
}
}

JSXComponents.forEach(component =>
dependencies.set(component, {
references: [],
}),
);
}

// Warn about accessing .current in cleanup effects.
currentRefsInEffectCleanup.forEach(
({reference, dependencyNode}, dependency) => {
Expand Down Expand Up @@ -669,6 +761,7 @@ export default {
if (setStateInsideEffectWithoutDeps) {
return;
}

references.forEach(reference => {
if (setStateInsideEffectWithoutDeps) {
return;
Expand Down Expand Up @@ -1051,7 +1144,10 @@ export default {
// Is this a variable from top scope?
const topScopeRef = componentScope.set.get(missingDep);
const usedDep = dependencies.get(missingDep);
if (usedDep.references[0].resolved !== topScopeRef) {
if (
usedDep.references.length === 0 ||
usedDep.references[0].resolved !== topScopeRef
) {
return;
}
// Is this a destructured prop?
Expand Down

0 comments on commit 7a458e2

Please sign in to comment.