Skip to content

Commit

Permalink
fix(eslint-plugin-react-hooks): remove experimental checks involving …
Browse files Browse the repository at this point in the history 
…useEffectEvent

Contributing to facebook#32240, this change removes the experimental `useEffectEvent` checks in the two react hooks eslint rule.  As the api is refined, and the compiler rule(s) are incorporated into this plugin, these checks will be covered then.
@michaelfaith
michaelfaith committed Jan 31, 2025
1 parent 829401d commit 2a91f47
Showing 4 changed files with 2 additions and 358 deletions.
56 changes: 0 additions & 56 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
View file
Original file line number Diff line number Diff line change
@@ -7675,62 +7675,6 @@ const tests = {
],
};

if (__EXPERIMENTAL__) {
tests.valid = [
...tests.valid,
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, []);
}
`,
},
];

tests.invalid = [
...tests.invalid,
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, [onStuff]);
}
`,
errors: [
{
message:
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
'Remove `onStuff` from the list.',
suggestions: [
{
desc: 'Remove the dependency `onStuff`',
output: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
onStuff();
}, []);
}
`,
},
],
},
],
},
];
}

// Tests that are only valid/invalid across parsers supporting Flow
const testsFlow = {
valid: [
183 changes: 0 additions & 183 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
View file
Original file line number Diff line number Diff line change
@@ -1286,181 +1286,6 @@ const tests = {
],
};

if (__EXPERIMENTAL__) {
tests.valid = [
...tests.valid,
{
code: normalizeIndent`
// Valid because functions created with useEffectEvent can be called in a useEffect.
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
onClick();
});
}
`,
},
{
code: normalizeIndent`
// Valid because functions created with useEffectEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
}
`,
},
{
code: normalizeIndent`
// Valid because functions created with useEffectEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
const onClick2 = () => { onClick() };
const onClick3 = useCallback(() => onClick(), []);
return <>
<Child onClick={onClick2}></Child>
<Child onClick={onClick3}></Child>
</>;
}
`,
},
{
code: normalizeIndent`
// Valid because functions created with useEffectEvent can be passed by reference in useEffect
// and useEffectEvent.
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
const onClick2 = useEffectEvent(() => {
debounce(onClick);
});
useEffect(() => {
let id = setInterval(onClick, 100);
return () => clearInterval(onClick);
}, []);
return <Child onClick={() => onClick2()} />
}
`,
},
{
code: normalizeIndent`
const MyComponent = ({theme}) => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
};
`,
},
{
code: normalizeIndent`
function MyComponent({ theme }) {
const notificationService = useNotifications();
const showNotification = useEffectEvent((text) => {
notificationService.notify(theme, text);
});
const onClick = useEffectEvent((text) => {
showNotification(text);
});
return <Child onClick={(text) => onClick(text)} />
}
`,
},
{
code: normalizeIndent`
function MyComponent({ theme }) {
useEffect(() => {
onClick();
});
const onClick = useEffectEvent(() => {
showNotification(theme);
});
}
`,
},
];
tests.invalid = [
...tests.invalid,
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEffectEventError('onClick')],
},
{
code: normalizeIndent`
// This should error even though it shares an identifier name with the below
function MyComponent({theme}) {
const onClick = useEffectEvent(() => {
showNotification(theme)
});
return <Child onClick={onClick} />
}
// The useEffectEvent function shares an identifier name with the above
function MyOtherComponent({theme}) {
const onClick = useEffectEvent(() => {
showNotification(theme)
});
return <Child onClick={() => onClick()} />
}
`,
errors: [{...useEffectEventError('onClick'), line: 7}],
},
{
code: normalizeIndent`
const MyComponent = ({ theme }) => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEffectEventError('onClick')],
},
{
code: normalizeIndent`
// Invalid because onClick is being aliased to foo but not invoked
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
let foo = onClick;
return <Bar onClick={foo} />
}
`,
errors: [{...useEffectEventError('onClick'), line: 7}],
},
{
code: normalizeIndent`
// Should error because it's being passed down to JSX, although it's been referenced once
// in an effect
function MyComponent({ theme }) {
const onClick = useEffectEvent(() => {
showNotification(them);
});
useEffect(() => {
setTimeout(onClick, 100);
});
return <Child onClick={onClick} />
}
`,
errors: [useEffectEventError('onClick')],
},
];
}

function conditionalError(hook, hasPreviousFinalizer = false) {
return {
message:
@@ -1518,14 +1343,6 @@ function classError(hook) {
};
}

function useEffectEventError(fn) {
return {
message:
`\`${fn}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
'the same component. They cannot be assigned to variables or passed down.',
};
}

function asyncComponentHookError(fn) {
return {
message: `React Hook "${fn}" cannot be called in an async function.`,
42 changes: 1 addition & 41 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
View file
Original file line number Diff line number Diff line change
@@ -97,7 +97,6 @@ export default {
const stateVariables = new WeakSet();
const stableKnownValueCache = new WeakMap();
const functionWithoutCapturedValueCache = new WeakMap();
const useEffectEventVariables = new WeakSet();
function memoizeWithWeakMap(fn, map) {
return function (arg) {
if (map.has(arg)) {
@@ -183,8 +182,6 @@ export default {
// ^^^ true for this reference
// const ref = useRef()
// ^^^ true for this reference
// const onStuff = useEffectEvent(() => {})
// ^^^ true for this reference
// False for everything else.
function isStableKnownHookValue(resolved) {
if (!isArray(resolved.defs)) {
@@ -251,17 +248,6 @@ export default {
if (name === 'useRef' && id.type === 'Identifier') {
// useRef() return value is stable.
return true;
} else if (
isUseEffectEventIdentifier(callee) &&
id.type === 'Identifier'
) {
for (const ref of resolved.references) {
if (ref !== id) {
useEffectEventVariables.add(ref.identifier);
}
}
// useEffectEvent() return value is always unstable.
return true;
} else if (
name === 'useState' ||
name === 'useReducer' ||
@@ -687,26 +673,7 @@ export default {
});
return;
}
if (useEffectEventVariables.has(declaredDependencyNode)) {
reportProblem({
node: declaredDependencyNode,
message:
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
`Remove \`${getSource(
declaredDependencyNode,
)}\` from the list.`,
suggest: [
{
desc: `Remove the dependency \`${getSource(
declaredDependencyNode,
)}\``,
fix(fixer) {
return fixer.removeRange(declaredDependencyNode.range);
},
},
],
});
}

// Try to normalize the declared dependency. If we can't then an error
// will be thrown. We will catch that error and report an error.
let declaredDependency;
@@ -1910,13 +1877,6 @@ function isAncestorNodeOf(a, b) {
return a.range[0] <= b.range[0] && a.range[1] >= b.range[1];
}

function isUseEffectEventIdentifier(node) {
if (__EXPERIMENTAL__) {
return node.type === 'Identifier' && node.name === 'useEffectEvent';
}
return false;
}

function getUnknownDependenciesMessage(reactiveHookName) {
return (
`React Hook ${reactiveHookName} received a function whose dependencies ` +
79 changes: 1 addition & 78 deletions packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
View file
Original file line number Diff line number Diff line change
@@ -110,13 +110,6 @@ function isInsideDoWhileLoop(node) {
return false;
}

function isUseEffectEventIdentifier(node) {
if (__EXPERIMENTAL__) {
return node.type === 'Identifier' && node.name === 'useEffectEvent';
}
return false;
}

function isUseIdentifier(node) {
return isReactFunction(node, 'use');
}
@@ -134,29 +127,6 @@ export default {
let lastEffect = null;
const codePathReactHooksMapStack = [];
const codePathSegmentStack = [];
const useEffectEventFunctions = new WeakSet();

// For a given scope, iterate through the references and add all useEffectEvent definitions. We can
// do this in non-Program nodes because we can rely on the assumption that useEffectEvent functions
// can only be declared within a component or hook at its top level.
function recordAllUseEffectEventFunctions(scope) {
for (const reference of scope.references) {
const parent = reference.identifier.parent;
if (
parent.type === 'VariableDeclarator' &&
parent.init &&
parent.init.type === 'CallExpression' &&
parent.init.callee &&
isUseEffectEventIdentifier(parent.init.callee)
) {
for (const ref of reference.resolved.references) {
if (ref !== reference) {
useEffectEventFunctions.add(ref.identifier);
}
}
}
}
}

/**
* SourceCode#getText that also works down to ESLint 3.0.0
@@ -169,17 +139,6 @@ export default {
: node => {
return context.sourceCode.getText(node);
};
/**
* SourceCode#getScope that also works down to ESLint 3.0.0
*/
const getScope =
typeof context.getScope === 'function'
? () => {
return context.getScope();
}
: node => {
return context.sourceCode.getScope(node);
};

return {
// Maintain code segment path stack as we traverse.
@@ -613,12 +572,9 @@ export default {
reactHooks.push(node.callee);
}

// useEffectEvent: useEffectEvent functions can be passed by reference within useEffect as well as in
// another useEffectEvent
if (
node.callee.type === 'Identifier' &&
(node.callee.name === 'useEffect' ||
isUseEffectEventIdentifier(node.callee)) &&
node.callee.name === 'useEffect' &&
node.arguments.length > 0
) {
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
@@ -627,44 +583,11 @@ export default {
}
},

Identifier(node) {
// This identifier resolves to a useEffectEvent function, but isn't being referenced in an
// effect or another event function. It isn't being called either.
if (
lastEffect == null &&
useEffectEventFunctions.has(node) &&
node.parent.type !== 'CallExpression'
) {
context.report({
node,
message:
`\`${getSource(
node,
)}\` is a function created with React Hook "useEffectEvent", and can only be called from ` +
'the same component. They cannot be assigned to variables or passed down.',
});
}
},

'CallExpression:exit'(node) {
if (node === lastEffect) {
lastEffect = null;
}
},

FunctionDeclaration(node) {
// function MyComponent() { const onClick = useEffectEvent(...) }
if (isInsideComponentOrHook(node)) {
recordAllUseEffectEventFunctions(getScope(node));
}
},

ArrowFunctionExpression(node) {
// const MyComponent = () => { const onClick = useEffectEvent(...) }
if (isInsideComponentOrHook(node)) {
recordAllUseEffectEventFunctions(getScope(node));
}
},
};
},
};

0 comments on commit 2a91f47

Please sign in to comment.