Skip to content

fix(eslint-plugin-react-hooks): remove experimental checks involving useEffectEvent #32276

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
+2 −358
Closed

fix(eslint-plugin-react-hooks): remove experimental checks involving useEffectEvent #32276

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 0 additions & 56 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
View file
Open in desktop
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
183 changes: 0 additions & 183 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
View file
Open in desktop
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 Expand Up @@ -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.`,
Expand Down
42 changes: 1 addition & 41 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
View file
Open in desktop
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
Loading