[eslint] Do not allow useEffectEvent fns to be called in arbitrary closures #33544
Conversation
…osures Summary: useEffectEvent is meant to be used specifically in combination with useEffect, and using the feature in arbitrary closures can lead to surprising reactivity semantics. In order to minimize risk in the experimental rollout, we are going to restrict its usage to being called directly inside an effect or another useEffectEvent, effectively enforcing the function coloring statically. Without an effect system this is the best we can do.
|
@rickhanlonii: I'm torn on the approach here and could use your perspective. We can't let the non-reactivity of useEffectEvent enter memoization dep arrays* (must be included in useCallback/useMemo) if we plan to let these functions be used in arbitrary closures. The two fixes for that problem are either:
useEffectEvent fns are unstable, so if we include it in the [?] dep array then this refactor is a breaking change (re-runs on every render). If we exclude it in the [?] dep array then we aren't using the instability to invalidate memoization, breaking its purpose. I put this version up because I think it is the better way forward, especially in the experimental release. Introducing differences in what you include in dep arrays depending on the API feels like a non-starter |
| if ( | ||
| lastEffect == null && | ||
| useEffectEventFunctions.has(node) && | ||
| node.parent.type !== 'CallExpression' |
There was a problem hiding this comment.
The meaningful change here is that we error regardless of if the parent node is a call expression when we are not in an effect and the identifier is a uEE function. We tweak the error message based on whetehr it was called or not
Summary:
useEffectEvent is meant to be used specifically in combination with useEffect, and using
the feature in arbitrary closures can lead to surprising reactivity semantics. In order to
minimize risk in the experimental rollout, we are going to restrict its usage to being
called directly inside an effect or another useEffectEvent, effectively enforcing the function
coloring statically. Without an effect system this is the best we can do.