Event Replaying #16725
Event Replaying #16725
Conversation
sebmarkbage
requested review from
pieterv,
gaearon,
trueadm,
acdlite and
alexmckenley
Details of bundled changes.Comparing: 70754f1...0b322c4 react-dom
react-art
react-test-renderer
react-reconciler
react-native-renderer
|
Agreed. I have a bunch of ideas of how we might do this, but we can talk about them in our meeting later this week. |
Yea. I originally tried that but there's some quirks:
So basically, I don't know what the easy fix is here. |
sebmarkbage
force-pushed
the
eventreplaying
branch
2 times, most recently
from
fc70581
to
8845c1f
Compare
|
I'm interested in how this works across suspense boundaries, is the following true? If we receive a discrete event for boundary A (1), the for boundary B (2) then another for A (3). Once A hydrates (1) will be replayed, and (3) will be left in the queue behind (2). If a user clicks in the hydrated boundary A, the event (4) will be queued behind (3) and only re played once boundary B is unblocked. So we could be have situations where if any one section is slow to hydrate it could switch the page back to being unresponsive if someone interacts with it? I wonder if we need the full app to have the same queue, maybe we want to allow areas to indicate they are isolated i.e. a navigation bar (that said nav events most likely could be handled via the browser) or page columns. |
|
@pieterv That's correct. It's difficult to predict how the events might correlate which is why React even in concurrent mode must flush one event after another. For the case where a nav is handled by the browser, that will interrupt the replaying even of previous events in the current approach. Meaning that even other queued events won't replay if I think this should be mostly fine because if you're able to interact with two completely disconnected trees that's already very unlikely to have happened very fast since even a single interaction takes a while. We should also be reasonably fast to hydrate after the first interaction with the prioritization, and if not, then this approach kind of falls apart. Additionally, there's an additional fail safe that I plan to add. Currently, there's no timeout associated with the replaying. Things gets blocked indefinitely because the normal expiration doesn't apply to hydration. I plan on adding a timeout when a boundary should give up. If the section for boundary B is slow to load, it'll replace it with a fallback and drop that event (replay without a target) which will then unblock the next events. My current thinking was that this would be when we're I/O bound though, not if the section is CPU bound. |
|
I do think we'll need more "features" like allowing events to preventDefault before replaying or other advanced things using some annotation in the DOM but that would be a new API on a per event level. This could also be used to opt-out of discrete ordering potentially. But that would be on a per-event basis rather than a whole subtree. |
@sebmarkbage Have you put any thought into if/how React users will be able to customize this sort of behavior? For example, it might be more desirable to keep the dehydrated content visible but render a loading modal or something. |
|
@devknoll Suspense is becoming a default mechanism for handling all kinds of scenarios like this in React so you're expected to have to have a reasonable boundary. In the case of an I/O bound timeout, it's effectively the same as attempting to render a lazy loaded component during an update and it suspending causing an update to trigger the fallback. In that case there's no reasonable alternative since there's no way to render a consistent tree while having some it show old data. We long ago gave up on the idea of allowing partial tearing of a tree. In theory, replaying could have an alternative such as setting state elsewhere but then that just opens the question, what if that alternate state hasn't loaded yet? Also, if you're already having to model the fallback loading state, who is actually going to design another loading state for this special edge case that really should never happen. If we're not able to replay events quickly enough in cases other than edge cases, we've really lost. One thing we could do though is for example switch the cursor to Eventually we'll have to drop it and in that case the fallback is quite nice, because a) it already exists anyway so there's nothing extra to design and download. b) it indicates why the event was dropped. It's similar to clicking one element and then quickly another where the first one deletes the second one. It's not that confusing that the event was dropped in that scenario. |
I was originally looking at touch events but there's not really an "over" event - only moves against the started target. This doesn't track every move but only when it changes target. This use case is really more for the hover case anyway which is only modeled using pointer events. |
sebmarkbage
force-pushed
the
eventreplaying
branch
5 times, most recently
from
c0d0678
to
bdacb27
Compare
sebmarkbage
force-pushed
the
eventreplaying
branch
2 times, most recently
from
ce9d60d
to
c6d5872
Compare
sebmarkbage
force-pushed
the
eventreplaying
branch
3 times, most recently
from
be1f29f
to
cfb0988
Compare
| @@ -14,3 +14,4 @@ export const RESPONDER_EVENT_SYSTEM = 1 << 1; | |||
| export const IS_PASSIVE = 1 << 2; | |||
| export const IS_ACTIVE = 1 << 3; | |||
| export const PASSIVE_NOT_SUPPORTED = 1 << 4; | |||
| export const IS_REPLAYED = 1 << 5; | |||
| @@ -100,6 +101,7 @@ function _receiveRootNodeIDEvent( | |||
| batchedUpdates(function() { | |||
| runExtractedPluginEventsInBatch( | |||
| topLevelType, | |||
| RESPONDER_EVENT_SYSTEM, | |||
There was a problem hiding this comment.
Why are we passing RESPONDER_EVENT_SYSTEM? That flag was meant for the new event system.
There was a problem hiding this comment.
😯🙃Nice catch! This was an oversight. It should be passing the plugin event system flag.
| @@ -313,6 +314,7 @@ const run = function(config, hierarchyConfig, nativeEventConfig) { | |||
| // Trigger the event | |||
| const extractedEvents = ResponderEventPlugin.extractEvents( | |||
| nativeEventConfig.topLevelType, | |||
| RESPONDER_EVENT_SYSTEM, | |||
There was a problem hiding this comment.
Why are we passing RESPONDER_EVENT_SYSTEM? That flag was meant for the new event system. We should be using the PLUGIN_EVENT_SYSTEM surely?
These tests were written with replaying in mind and now we can properly enable them.
These events only replay their last target if the target is not yet hydrated. That way we don't have to wait for a previously hovered boundary before invoking the current target.
That way we can check if this is a replay and therefore needs a special case. One such special case is "mouseover" where we check the relatedTarget.
To minimize breakages in a minor, I only do this for the new root APIs since replaying only matters there anyway. Only if hydrating. For Flare, I have to attach all active listeners since the current system has one DOM listener for each. In a follow up I plan on optimizing that by only attaching one if there's at least one active listener which would allow us to start with only passive and then upgrade.
We need to check if the "relatedTarget" is mounted due to how the old event system dispatches from the "out" event.
This is a follow up to facebook#16673 which didn't have a test because it wasn't observable yet. This shows that it had a bug.
sebmarkbage
force-pushed
the
eventreplaying
branch
from
d9cb249
to
0b322c4
Compare
There's a gap between when we call
createRootorcreateSyncRootand when we actually commit the hydrated tree. E.g. it can take a while before the user callsroot.render(). In batched mode, there's a gap before the scheduled callback actually starts rendering. In concurrent mode, there's a gap every time we yield. Additionally, for Suspense boundaries, we use Partial Hydration and there's a gap between rendering each new level.In between these gaps, there can be events issued on dehydrated DOM nodes that are currently dropped. This adds the basic infra for replaying those events.
It is not behind a feature flag or any mode because it should not be possible to get into this mode in the legacy
ReactDOM.hydrate(...)API since it doesn't leave any gaps. So if there is a reason this has any behavior changes, that's a bug and we'd like to find it.Principle
The constraint here is that we can't wait for the actual components to load/render before showing the server rendered HTML. We also can't load much in terms of even the event system. We're limited to as much script we can reasonably inline into the HTML page which isn't much.
Some interactions don't make sense to do any other way than handling once we do have the code. Such a click on a rich UI. These needs to be prioritized.
Other interactions can't be replayed later. They have to be dealt with by the browser right now. Such as text input.
Many small interactions such as hovering or scrolling a bit also is fairly harmless.
Some events are not really user driven and can be inferred by a hydrating component (such as image "load").
There are cases where the dehydrated tree needs to be dropped and we can no longer replay that event. Such as if it gets deleted by a previous event or forced client-rendered. We could use this mechanism to force hydration even synchronously such as before invoking a discrete event. However, the issue with doing that is that there are so many discrete events that happen just by navigating around such as keypresses or touch to scroll. This would also not work before the code has loaded so we'd have to always wait to show the SSR content until the code has loaded. This wouldn't work with the plain HTML option.
My hypothesis here is that we can get far by letting the browser do most of what it does by default. Server rendered components would know to be somewhat resilient to this. Then we simply replay those events as "passive" events later to let JS observed what happened. This inherently is a somewhat degrated experience.
Event Types
I've categorized events as "discrete", "continuous" or "other".
The "discrete" category are similar to Discrete priority events. It's events that needs to be replayed in the order they were issued and each one needs to be replayed.
The "continuous" category are things like mouseover, focus, selectionchange. It's expected that there are a lot of these while the user is simply navigating the page. However, the previous ones are not really relevant and can be collapsed. Only the last state for each type needs to be replayed. There's only one "hover target" and one "active element" once we hydrate. The intermediate steps are not relevant.
The "other" category don't need to be replayed. Either because it can be replayed by the Host component hydrating itself or it's not a critical event.
In this PR:
Follow up PRs:
beforeunloadfires. This means a link or form successfully navigated. This avoids double side-effects by the browser navigation and JS both triggering.ReactDOM.createRoot(..., {replayEvents: ...}). This allows an early light script to track events that happened before even React loaded onto the page.preventDefaultbehavior if that succeeds. This is useful for handling links with JS.ReactDOM.hydrate(node)to increase the priority of that boundary to Normal pri or whatever scheduler priority context we're running in.In future PRs we can improve the actual replaying mechanism based on special cases. I'm sure there's a long tail.
In particular the integration with Flare has some flaws right now. We can also evaluate having Flare replaying happen at other levels such as in the synthetic layer.