Skip to content

Commit

Permalink
Rename experimental useEvent to useEffectEvent (#25881)
Browse files Browse the repository at this point in the history 
We originally had grand plans for using this Event concept for more but
now it's only meant to be used in combination with effects.

It's an Event in the FRP terms, that is triggered from an Effect.
Technically it can also be from another function that itself is
triggered from an existing side-effect but that's kind of an advanced
case.

The canonical case is an effect that triggers an event:

```js
const onHappened = useEffectEvent(() => ...);
useEffect(() => {
  onHappened();
}, []);
```
  • Loading branch information
@sebmarkbage
sebmarkbage authored Dec 14, 2022
1 parent 4dda96a commit 84a0a17
Show file tree
Hide file tree
Showing 26 changed files with 176 additions and 171 deletions.
8 changes: 4 additions & 4 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRuleExhaustiveDeps-test.js
View file
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
56 changes: 28 additions & 28 deletions packages/eslint-plugin-react-hooks/__tests__/ESLintRulesOfHooks-test.js
View file
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
View file
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
View file
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

0 comments on commit 84a0a17

Please sign in to comment.