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.
  • Loading branch information
@michaelfaith
michaelfaith committed Jan 30, 2025
1 parent 829401d commit 5477c30
Show file tree
Hide file tree
Showing 4 changed files with 2 additions and 350 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
Expand Up @@ -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: [
Expand Down
175 changes: 0 additions & 175 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
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
Expand Up @@ -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)) {
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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' ||
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 ` +
Expand Down
Loading

0 comments on commit 5477c30

Please sign in to comment.