Skip to content

Commit

Permalink
Check that useEvent is locally called
Browse files Browse the repository at this point in the history
This update to the lint rule checks that functions created with
`useEvent` can only be invoked in a `useEffect`callback or closure.
They can't be passed down directly as a reference to child components.
  • Loading branch information
poteto committed Sep 22, 2022
1 parent ec0e8e8 commit 237e38e
Show file tree
Hide file tree
Showing 3 changed files with 237 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,72 @@ const tests = {
const [myState, setMyState] = useState(null);
}
`,
`
// Valid because functions created with useEvent can be called in a useEffect.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
onClick();
});
}
`,
`
// Valid because functions created with useEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
}
`,
`
// Valid because functions created with useEvent can be called in closures.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
const onClick2 = () => { onClick() };
const onClick3 = useCallback(() => onClick(), []);
return <>
<Child onClick={onClick2}></Child>
<Child onClick={onClick3}></Child>
</>;
}
`,
`
// Valid because functions created with useEvent can be passed by reference in useEffect.
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
useEffect(() => {
let id = setInterval(onClick, 100);
return () => clearInterval(onClick);
}, []);
}
`,
`
const MyComponent = ({theme}) => {
const onClick = useEvent(() => {
showNotification(theme);
});
return <Child onClick={() => onClick()}></Child>;
};
`,
`
function MyComponent({ theme }) {
const notificationService = useNotifications();
const showNotification = useEvent((text) => {
notificationService.notify(theme, text);
});
const onClick = useEvent((text) => {
showNotification(text);
});
return <Child onClick={(text) => onClick(text)} />
}
`,
],
invalid: [
{
Expand Down Expand Up @@ -449,7 +515,7 @@ const tests = {
},
{
code: `
// This is a false positive (it's valid) that unfortunately
// This is a false positive (it's valid) that unfortunately
// we cannot avoid. Prefer to rename it to not start with "use"
class Foo extends Component {
render() {
Expand Down Expand Up @@ -971,6 +1037,64 @@ const tests = {
`,
errors: [classError('useState')],
},
{
code: `
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEventError('onClick')],
},
{
code: `
// This should error even though it shares an identifier name with the below
function MyComponent({theme}) {
const onClick = useEvent(() => {
showNotification(theme)
});
return <Child onClick={onClick} />
}
// The useEvent function shares an identifier name with the above
function MyOtherComponent({theme}) {
const onClick = useEvent(() => {
showNotification(theme)
});
return <Child onClick={() => onClick()} />
}
`,
errors: [{...useEventError('onClick'), line: 4}],
},
{
code: `
const MyComponent = ({ theme }) => {
const onClick = useEvent(() => {
showNotification(theme);
});
return <Child onClick={onClick}></Child>;
}
`,
errors: [useEventError('onClick')],
},
{
code: `
// Invalid because onClick is being aliased to foo but not invoked
function MyComponent({ theme }) {
const onClick = useEvent(() => {
showNotification(theme);
});
let foo;
useEffect(() => {
foo = onClick;
});
return <Bar onClick={foo} />
}
`,
errors: [useEventError('onClick')],
},
],
};

Expand Down Expand Up @@ -1031,6 +1155,14 @@ function classError(hook) {
};
}

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

// For easier local testing
if (!process.env.CI) {
let only = [];
Expand Down
5 changes: 4 additions & 1 deletion packages/eslint-plugin-react-hooks/src/ExhaustiveDeps.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,10 @@ export default {
if (name === 'useRef' && id.type === 'Identifier') {
// useRef() return value is stable.
return true;
} else if (name === 'useEvent' && id.type === 'Identifier') {
} else if (
(name === 'experimental_useEvent' || name === 'useEvent') &&
id.type === 'Identifier'
) {
// useEvent() return value is stable.
return true;
} else if (name === 'useState' || name === 'useReducer') {
Expand Down
100 changes: 100 additions & 0 deletions packages/eslint-plugin-react-hooks/src/RulesOfHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,13 @@ function isInsideComponentOrHook(node) {
return false;
}

function isUseEventIdentifier(node) {
return (
node.type === 'Identifier' &&
(node.name === 'useEvent' || node.name === 'experimental_useEvent')
);
}

export default {
meta: {
type: 'problem',
Expand All @@ -110,8 +117,45 @@ export default {
},
},
create(context) {
let lastEffect = null;
const codePathReactHooksMapStack = [];
const codePathSegmentStack = [];
const useEventViolations = new Set();

// For a given AST node, iterate through the top level statements and add all useEvent
// definitions. We can do this in non-Program nodes because we can rely on the assumption that
// useEvent functions can only be declared within a component or hook at its top level.
function addAllUseEventViolations(node) {
if (node.body.type !== 'BlockStatement') return;
for (const statement of node.body.body) {
if (statement.type !== 'VariableDeclaration') continue;
for (const declaration of statement.declarations) {
if (
declaration.type === 'VariableDeclarator' &&
declaration.init &&
declaration.init.type === 'CallExpression' &&
declaration.init.callee &&
isUseEventIdentifier(declaration.init.callee)
) {
useEventViolations.add(declaration.id);
}
}
}
}

// Resolve a useEvent violation, ie the useEvent created function was called.
function resolveUseEventViolation(scope, ident) {
if (scope.references == null || useEventViolations.size === 0) return;
for (const ref of scope.references) {
if (ref.resolved == null) continue;
const [useEventFunctionIdentifier] = ref.resolved.identifiers;
if (ident.name === useEventFunctionIdentifier.name) {
useEventViolations.delete(useEventFunctionIdentifier);
break;
}
}
}

return {
// Maintain code segment path stack as we traverse.
onCodePathSegmentStart: segment => codePathSegmentStack.push(segment),
Expand Down Expand Up @@ -522,6 +566,62 @@ export default {
}
reactHooks.push(node.callee);
}

const scope = context.getScope();
// useEvent: Resolve a function created with useEvent that is invoked locally at least once.
// OK - onClick();
resolveUseEventViolation(scope, node.callee);

// useEvent: useEvent functions can be passed by reference within useEffect
if (
node.callee.type === 'Identifier' &&
node.callee.name === 'useEffect' &&
node.arguments.length > 0
) {
// Denote that we have traversed into a useEffect call, and stash the CallExpr for
// comparison later when we exit
lastEffect = node;
}
},

Identifier(node) {
// OK - useEffect(() => { setInterval(onClick, ...) }, []);
if (lastEffect != null && node.parent.type === 'CallExpression') {
resolveUseEventViolation(context.getScope(), node);
}
},

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

FunctionDeclaration(node) {
// function MyComponent() { const onClick = useEvent(...) }
if (isInsideComponentOrHook(node)) {
addAllUseEventViolations(node);
}
},

ArrowFunctionExpression(node) {
// const MyComponent = () => { const onClick = useEvent(...) }
if (isInsideComponentOrHook(node)) {
addAllUseEventViolations(node);
}
},

'Program:exit'(_node) {
for (const node of useEventViolations.values()) {
context.report({
node,
message:
`\`${context.getSource(
node,
)}\` is a function created with React Hook "useEvent", and can only be called from ` +
'the same component. They cannot be assigned to variables or passed down.',
});
}
},
};
},
Expand Down

0 comments on commit 237e38e

Please sign in to comment.