Should the polyfill for useEffectEvent return a stable reference?

[lixiaoyan]

According to what the React docs claim, useEffectEvent intentionally returns a different reference on every render — see the note here. Maybe this useCallback should be removed?

[snowystinger]

You're welcome to try it, we ran into some difficulties with the real one #9590

I'll try to get a more precise answer of what this meant:

the real useEffectEvent is causing issues

However, if we don't have a stable function returned, I'm not sure how we'd keep our effects from calling stale functions in the closure. We aren't react, so we don't have access to internals. We would be dependent on our effects updating in order to call the most recent function. This negates the benefit of what useEffectEvent is supposed to be for, which is not causing the effect to run ever render and always being up to date regardless.

[lixiaoyan]

I didn't quite catch what you meant. What I was trying to say is: we're currently wrapping the return value in useCallback with [] as deps, so in a sense the returned function is a stable reference. I'm not sure whether removing this useCallback would cause other issues.

React's official approach leans on ESLint to prevent developers from putting a useEffectEvent's return value into a dependency array, which would otherwise cause the effect to re-run on every update. This does break the original design intent of deps to some extent, but per the docs it's considered an acceptable trade-off.

The official ESLint rule appears to match purely by function name, so a polyfilled useEffectEvent would be covered by the same rule. In theory, this should be safe.

[snowystinger]

Ah, I see what you're saying.

We'll need to get more info about this:

the real useEffectEvent is causing issues

Though maybe we should back up a bit, are you using this (our polyfill) function or is it causing a bug in one of our components?

If it's the first, we don't consider this to a be a public API polyfill, it's a teeny file, so you're welcome to copy it and do with it as you will. If it's the second, can you provide more information about what is breaking?

Otherwise, it looks like the rationale for it changing every render is just to show if an effect is running every render. That only happens if you pass it to other components/hooks/files where eslint can't follow, which we aren't doing at the moment to the best of my knowledge, and lint should catch the other instances.

What I'm getting at is that, unless it's causing a bug or problem, this would be pretty low on our priorities.

[lixiaoyan]

I haven't actually run into any issues myself — I just came across this recently while reading the React documentation. We already have a polyfill in our project with a very similar implementation, since we're still on React 18. That said, I'm a bit concerned about whether we'll be able to smoothly migrate to the official hook once we upgrade to React 19 down the road. So I wanted to hear your thoughts on this, and whether any of you have run into related issues. This isn't really a bug report — just a question. If you could share any issues you've run into, I'd really appreciate it.

[snowystinger]

Ok, our paper trail was here #9095 (comment) so it wasn't that we observed any issues, it was that some tests were failing.

So I think next steps would probably be, check if these are still a problem. See if updating our polyfill causes the same issues.
The end goal really being to turn back on the native implementation, the rest is just bonus.