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.

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

Rename experimental useEvent to useEffectEvent #25881

Merged
merged 2 commits into from
Dec 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -7636,7 +7636,7 @@ if (__EXPERIMENTAL__) {
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEvent(() => {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
Expand All @@ -7652,7 +7652,7 @@ if (__EXPERIMENTAL__) {
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onStuff = useEvent(() => {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
Expand All @@ -7663,14 +7663,14 @@ if (__EXPERIMENTAL__) {
errors: [
{
message:
'Functions returned from `useEvent` must not be included in the dependency array. ' +
'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 = useEvent(() => {
const onStuff = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1050,9 +1050,9 @@ if (__EXPERIMENTAL__) {
...tests.valid,
{
code: normalizeIndent`
// Valid because functions created with useEvent can be called in a useEffect.
// Valid because functions created with useEffectEvent can be called in a useEffect.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
useEffect(() => {
Expand All @@ -1063,9 +1063,9 @@ if (__EXPERIMENTAL__) {
},
{
code: normalizeIndent`
// Valid because functions created with useEvent can be called in closures.
// Valid because functions created with useEffectEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
Expand All @@ -1074,9 +1074,9 @@ if (__EXPERIMENTAL__) {
},
{
code: normalizeIndent`
// Valid because functions created with useEvent can be called in closures.
// Valid because functions created with useEffectEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
const onClick2 = () => { onClick() };
Expand All @@ -1090,13 +1090,13 @@ if (__EXPERIMENTAL__) {
},
{
code: normalizeIndent`
// Valid because functions created with useEvent can be passed by reference in useEffect
// and useEvent.
// Valid because functions created with useEffectEvent can be passed by reference in useEffect
// and useEffectEvent.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
const onClick2 = useEvent(() => {
const onClick2 = useEffectEvent(() => {
debounce(onClick);
});
useEffect(() => {
Expand All @@ -1110,7 +1110,7 @@ if (__EXPERIMENTAL__) {
{
code: normalizeIndent`
const MyComponent = ({theme}) => {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
Expand All @@ -1121,10 +1121,10 @@ if (__EXPERIMENTAL__) {
code: normalizeIndent`
function MyComponent({ theme }) {
const notificationService = useNotifications();
const showNotification = useEvent((text) => {
const showNotification = useEffectEvent((text) => {
notificationService.notify(theme, text);
});
const onClick = useEvent((text) => {
const onClick = useEffectEvent((text) => {
showNotification(text);
});
return <Child onClick={(text) => onClick(text)} />
Expand All @@ -1137,7 +1137,7 @@ if (__EXPERIMENTAL__) {
useEffect(() => {
onClick();
});
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
}
Expand Down Expand Up @@ -1188,64 +1188,64 @@ if (__EXPERIMENTAL__) {
{
code: normalizeIndent`
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEventError('onClick')],
errors: [useEffectEventError('onClick')],
},
{
code: normalizeIndent`
// This should error even though it shares an identifier name with the below
function MyComponent({theme}) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme)
});
return <Child onClick={onClick} />
}

// The useEvent function shares an identifier name with the above
// The useEffectEvent function shares an identifier name with the above
function MyOtherComponent({theme}) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme)
});
return <Child onClick={() => onClick()} />
}
`,
errors: [{...useEventError('onClick'), line: 7}],
errors: [{...useEffectEventError('onClick'), line: 7}],
},
{
code: normalizeIndent`
const MyComponent = ({ theme }) => {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEventError('onClick')],
errors: [useEffectEventError('onClick')],
},
{
code: normalizeIndent`
// Invalid because onClick is being aliased to foo but not invoked
function MyComponent({ theme }) {
const onClick = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(theme);
});
let foo = onClick;
return <Bar onClick={foo} />
}
`,
errors: [{...useEventError('onClick'), line: 7}],
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 = useEvent(() => {
const onClick = useEffectEvent(() => {
showNotification(them);
});
useEffect(() => {
Expand All @@ -1254,7 +1254,7 @@ if (__EXPERIMENTAL__) {
return <Child onClick={onClick} />
}
`,
errors: [useEventError('onClick')],
errors: [useEffectEventError('onClick')],
},
{
code: normalizeIndent`
Expand Down Expand Up @@ -1360,10 +1360,10 @@ function classError(hook) {
};
}

function useEventError(fn) {
function useEffectEventError(fn) {
return {
message:
`\`${fn}\` is a function created with React Hook "useEvent", and can only be called from ` +
`\`${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.',
};
}
Expand Down
21 changes: 12 additions & 9 deletions packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export default {
const stateVariables = new WeakSet();
const stableKnownValueCache = new WeakMap();
const functionWithoutCapturedValueCache = new WeakMap();
const useEventVariables = new WeakSet();
const useEffectEventVariables = new WeakSet();
function memoizeWithWeakMap(fn, map) {
return function(arg) {
if (map.has(arg)) {
Expand Down Expand Up @@ -158,7 +158,7 @@ export default {
// ^^^ true for this reference
// const ref = useRef()
// ^^^ true for this reference
// const onStuff = useEvent(() => {})
// const onStuff = useEffectEvent(() => {})
// ^^^ true for this reference
// False for everything else.
function isStableKnownHookValue(resolved) {
Expand Down Expand Up @@ -226,13 +226,16 @@ export default {
if (name === 'useRef' && id.type === 'Identifier') {
// useRef() return value is stable.
return true;
} else if (isUseEventIdentifier(callee) && id.type === 'Identifier') {
} else if (
isUseEffectEventIdentifier(callee) &&
id.type === 'Identifier'
) {
for (const ref of resolved.references) {
if (ref !== id) {
useEventVariables.add(ref.identifier);
useEffectEventVariables.add(ref.identifier);
}
}
// useEvent() return value is always unstable.
// useEffectEvent() return value is always unstable.
return true;
} else if (name === 'useState' || name === 'useReducer') {
// Only consider second value in initializing tuple stable.
Expand Down Expand Up @@ -645,11 +648,11 @@ export default {
});
return;
}
if (useEventVariables.has(declaredDependencyNode)) {
if (useEffectEventVariables.has(declaredDependencyNode)) {
reportProblem({
node: declaredDependencyNode,
message:
'Functions returned from `useEvent` must not be included in the dependency array. ' +
'Functions returned from `useEffectEvent` must not be included in the dependency array. ' +
`Remove \`${context.getSource(
declaredDependencyNode,
)}\` from the list.`,
Expand Down Expand Up @@ -1851,9 +1854,9 @@ function isAncestorNodeOf(a, b) {
return a.range[0] <= b.range[0] && a.range[1] >= b.range[1];
}

function isUseEventIdentifier(node) {
function isUseEffectEventIdentifier(node) {
if (__EXPERIMENTAL__) {
return node.type === 'Identifier' && node.name === 'useEvent';
return node.type === 'Identifier' && node.name === 'useEffectEvent';
}
return false;
}
36 changes: 18 additions & 18 deletions packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ function isInsideComponentOrHook(node) {
return false;
}

function isUseEventIdentifier(node) {
function isUseEffectEventIdentifier(node) {
if (__EXPERIMENTAL__) {
return node.type === 'Identifier' && node.name === 'useEvent';
return node.type === 'Identifier' && node.name === 'useEffectEvent';
}
return false;
}
Expand All @@ -130,24 +130,24 @@ export default {
let lastEffect = null;
const codePathReactHooksMapStack = [];
const codePathSegmentStack = [];
const useEventFunctions = new WeakSet();
const useEffectEventFunctions = new WeakSet();

// For a given scope, iterate through the references and add all useEvent definitions. We can
// do this in non-Program nodes because we can rely on the assumption that useEvent functions
// 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 recordAllUseEventFunctions(scope) {
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 &&
isUseEventIdentifier(parent.init.callee)
isUseEffectEventIdentifier(parent.init.callee)
) {
for (const ref of reference.resolved.references) {
if (ref !== reference) {
useEventFunctions.add(ref.identifier);
useEffectEventFunctions.add(ref.identifier);
}
}
}
Expand Down Expand Up @@ -571,12 +571,12 @@ export default {
reactHooks.push(node.callee);
}

// useEvent: useEvent functions can be passed by reference within useEffect as well as in
// another useEvent
// 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' ||
isUseEventIdentifier(node.callee)) &&
isUseEffectEventIdentifier(node.callee)) &&
node.arguments.length > 0
) {
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
Expand All @@ -586,19 +586,19 @@ export default {
},

Identifier(node) {
// This identifier resolves to a useEvent function, but isn't being referenced in an
// 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 &&
useEventFunctions.has(node) &&
useEffectEventFunctions.has(node) &&
node.parent.type !== 'CallExpression'
) {
context.report({
node,
message:
`\`${context.getSource(
node,
)}\` is a function created with React Hook "useEvent", and can only be called from ` +
)}\` 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.',
});
}
Expand All @@ -611,16 +611,16 @@ export default {
},

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

ArrowFunctionExpression(node) {
// const MyComponent = () => { const onClick = useEvent(...) }
// const MyComponent = () => { const onClick = useEffectEvent(...) }
if (isInsideComponentOrHook(node)) {
recordAllUseEventFunctions(context.getScope());
recordAllUseEffectEventFunctions(context.getScope());
}
},
};
Expand Down
Loading