Bug: useEffect Timing changes depending on if Portal was rendered

[jquense]

This is a weird one. Basically, if you add an event listener to the document in an effect that was triggered by an event. e.g. click toggles some state, which triggers an effect, which adds a click handler to the document. In the normal case the new event handler will "miss" the triggering event, e.g. the added click handler won't respond to the click event that triggered it being added (omg).

HOWEVER, if you render a portal first, the timing changes slightly and the added event handler will see the current event.

React version: 17

Steps To Reproduce

https://codesandbox.io/s/react-playground-forked-cyt0f?file=/index.js

1. Click the "show Message" button to see a message toggle in and out
2. Click the "Render Portal" button (see a portal rendered into the body)
3. Click the "show Message" button again and notice nothing happens

The reason for the final behavior is the click event both opens and closes the message, (calls set state twice)

Link to code example:

https://codesandbox.io/s/react-playground-forked-cyt0f?file=/index.js

The current behavior

The expected behavior

That they be consistent

[gaearon]

17 only?

[jquense]

17 only! downstream issue react-bootstrap/react-bootstrap#5409

[jquense]

well tbh maybe this was present but not observable since before 17 you couldn't attach an event listener "higher up" so there wasn't any way to get in front of the current event

[gaearon]

What sounds like a bug here is that useEffect is called synchronously at all. It should normally be deferred — which would avoid this issue. So it sounds like there's an unexpected effect flush. Would be nice to dig into why.

[gaearon]

I'm sooo confused. I can't reproduce this anymore. The only thing I did was to update Chrome...

[gaearon]

Can you still repro it?

[jquense]

bah sorry @gaearon i was goofing around trying to find a workaround and didn't realize it was saving instead of forking. It should be back to being reproducible

[gaearon]

Ok at least I can still sleep at night

[gaearon]

What happens here is that the native event bubbles:

• On the root container
• On the body (portal target)

This is why we get into dispatchEvent twice. Root container pass schedules an effect, and body pass flushes it.

Seems like a bug that we flush the effect in this case.

[gaearon]

To follow up on this — it does seem like a bug but we can't commit to prioritizing it at the moment. There will be a bunch of work done to clean up and simplify related parts of the code in the coming months so we'll likely address it then. Have you found any userland workaround, or is this a hard blocker for you?