[react/primitive] React 18 – Flush discrete CustomEvent dispatch calls

[andy-hook]

Fixes #1287
Supersedes #1292

Big thanks to @Andarist for providing great insight into a resolution. This change is mostly equivalent though with the additional goal of providing a clear heuristic for when this is necessary and applying the change to all applicable code.

Context

One of the core changes in React 18 is the introduction of consistent batching across all event handlers, this differs from 17 where only certain events would be subject to batching. As a result, certain state updates are now batched where before they were not, creating issues like those seen in #1287

Event priorities

React assigns priorities to updates based on the type of user interaction (discrete, continuous, default), discrete is an important distinction as it allows updates from those handlers to be made immediately. As stated in #1292 (comment) the way that React determines this priority is by usage of window.event.

Because custom event types are unable to be assessed in this way, they fall through to default. In most cases this wouldn’t be a problem, but in the case of ContextMenu our code structure is such that there is an implicit expectation for one update to commit before another.


Why this can be problematic

In the case of ContextMenu, the incorrect behaviour is caused by the unexpected batching of updates in pointerdown and contextmenu, while the execution order remains correct (pointerdown -> contextmenu), the resulting state change is batched in 18, resulting in no update to open. As the effect cleanup is no longer cycled by moving through false -> true pointer-events: none; does not get removed and our handler in contextmenu is blocked.

You can see and compare the behaviour in this example.



A heuristic for consistency

Given our understanding of how React splits priorities for updates, and that the intention for immediate user interaction i.e. discrete events is to not defer, dispatchDiscreteCustomEvent should be used whenever a custom event type is dispatched from a discrete handler. This is the recommendation from the core team.

Internal p
riorities for these types can be found here

Use of Event vs CustomEvent

I’ve changed every usage of the the Event constructor to be CustomEvent when a custom type is provided, this is a style change as either constructor is susceptible (it’s important to remember priority is inferred by the type passed, not which constructor used) the hope being that staying consistent will make deciding when to use dispatchDiscreteCustomEvent easier.



e.g.


// 

dispatching a click


target.dispatchEvent(new Event(‘click’))





// 



dispatching a custom type


target.dispatchEvent(new CustomEvent(‘customType’))





// 





dispatching a custom type within a discrete handler


onPointerDown={(e) => dispatchDiscreteCustomEvent(e.target, new CustomEvent(‘customType’))}

[benoitgrelard]

Will have to re-review these with the change of package so I'm skipping for now.

[benoitgrelard]

♥️ the description btw!

[Andarist]

A great level of detail has been added here in the description and in the comment in the code 👍