Replay Client Actions After Hydration #26716
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
| @@ -105,7 +111,7 @@ const discreteReplayableEvents: Array<DOMEventName> = [ | |||
| 'change', | |||
| 'contextmenu', | |||
| 'reset', | |||
| 'submit', | |||
| // 'submit', // stopPropagation blocks the replay mechanism | |||
There was a problem hiding this comment.
This is really unfortunate and I'm not sure what the correct solution is.
We currently call stopPropagation when we might have hydrated some of the tree but not all the way to the target. We do this because we want to treat the tree as if it wasn't hydrated at all. Meaning we shouldn't call any parent event handlers if they would've been stopped.
However, this is not like "before" hydration for any other more global scripts that were inserted early or by third parties.
In this case, this blocks our event replaying handler at the root so we don't call preventDefault and it just ends up going through the javascript: URL.
There was a problem hiding this comment.
What event handlers is it trying to block? Native event handlers added by components would be added by useEffect which hasn't run at this point right?
There was a problem hiding this comment.
It could've been if it's in a parent component with a Suspense/Hydration boundary around the target.
| } | ||
| return writeBootstrap(destination, responseState) && writeMore; |
There was a problem hiding this comment.
This lets us add more bootstrapping code in case we discover that something nested needs bootstrapping code later on. Which seems like a generally useful thing.
sebmarkbage
force-pushed
the
formactionsreplaying
branch
from
9d7d285 to
1ceacac
Compare
| } from './ReactDOMFizzInstructionSetShared'; | ||
|
|
||
| import {enableFormActions} from 'shared/ReactFeatureFlags'; |
There was a problem hiding this comment.
The test failures are because the hacky Fizz test runtime doesn't use the same bundling rules as we use when actually building this file so it doesn't know how to resolve the feature flags even though the prod builds do.
There was a problem hiding this comment.
For now, you could gate the tests to only run during build, like this:
// @gate !sourceThere was a problem hiding this comment.
I have an alternative strategy here #26717 that I think is probably the better solution anyway.
| return; | ||
| } | ||
| const form = event.target; | ||
| const submitter = event['submitter']; |
There was a problem hiding this comment.
do we need better closure externs? (do we even get anything out of advanced mode?)
There was a problem hiding this comment.
Yea, I added one for addEventListener already. Not sure we need advanced mode tbh.
There was a problem hiding this comment.
I think the only reason I made it use advanced was to avoid having to run Rollup on it to minify the symbols (though there's probably a better way to do that besides advanced mode) but it looks like we're probably going to have to run Rollup on this file anyway
There was a problem hiding this comment.
Ideally it would use the same build pipeline as every other file but there's a sequencing issue because our normal build pipeline doesn't have an easy way to do codegen. We'd have to run the build script twice with different arguments, maybe.
| // needs to be available before that happens so after construction it's too | ||
| // late. We use a temporary fake node for the duration of this event. | ||
| // TODO: FormData takes a second argument that it's the submitter but this | ||
| // is fairly new so not all browsers support it yet. Switch to that technique |
There was a problem hiding this comment.
looks like it's actually supported in all modern browsers as of this month…
There was a problem hiding this comment.
Yea but it'll take a while until everyone is on modern iOS versions and all the Samsung TV upgrade.
| i -= 3; | ||
| } | ||
| // Schedule a replay in case this unblocked something. | ||
| scheduleReplayQueueIfNeeded(formReplayingQueue); |
There was a problem hiding this comment.
does this only need to happen in the typeof action === 'function' case?
There was a problem hiding this comment.
This one is a little interesting because this might have been a case that was blocking another action that is further in the list already but has already been resolved.
| ); | ||
| } | ||
| } | ||
|
|
||
| export function retryIfBlockedOn( |
There was a problem hiding this comment.
So does this wait until all the components in a container are hydrated?
There was a problem hiding this comment.
Yea but this also gets called if it ends up deleted instead.
| }); | ||
|
|
||
| // It should've now been replayed | ||
| expect(foo).toBe('bar'); |
There was a problem hiding this comment.
make sure it's called just once?
make sure formData is captured at the time of submission?
| @@ -105,7 +111,7 @@ const discreteReplayableEvents: Array<DOMEventName> = [ | |||
| 'change', | |||
| 'contextmenu', | |||
| 'reset', | |||
| 'submit', | |||
| // 'submit', // stopPropagation blocks the replay mechanism | |||
There was a problem hiding this comment.
What event handlers is it trying to block? Native event handlers added by components would be added by useEffect which hasn't run at this point right?
…ates Then try to replay any action that got unblocked. The hard part comes from the it might be more than one React instance that shares this queue.
This blocks the action replaying mechanism because it calls stopPropagation.
sebmarkbage
force-pushed
the
formactionsreplaying
branch
from
1ceacac to
af1d403
Compare
We used to have Event Replaying for any kind of Discrete event where we'd track any event after hydrateRoot and before the async code/data has loaded in to hydrate the target. However, this didn't really work out because code inside event handlers are expected to be able to synchronously read the state of the world at the time they're invoked. If we replay discrete events later, the mutable state around them like selection or form state etc. may have changed. This limitation doesn't apply to Client Actions: - They're expected to be async functions that themselves work asynchronously. They're conceptually also in the "navigation" events that happen after the "submit" events so they're already not synchronously even before the first `await`. - They're expected to operate mostly on the FormData as input which we can snapshot at the time of the event. This PR adds a bit of inline script to the Fizz runtime (or external runtime) to track any early submit events on the page - but only if the action URL is our placeholder `javascript:` URL. We track a queue of these on `document.$$reactFormReplay`. Then we replay them in order as they get hydrated and we get a handle on the Client Action function. I add the runtime to the `bootstrapScripts` phase in Fizz which is really technically a little too late, because on a large page, it might take a while to get to that script even if you have displayed the form. However, that's also true for external runtime. So there's a very short window we might miss an event but it's good enough and better than risking blocking display on this script. The main thing that makes the replaying difficult to reason about is that we can have multiple instance of React using this same queue. This would be very usual but you could have two different Reacts SSR:ing different parts of the tree and using around the same version. We don't have any coordinating ids for this. We could stash something on the form perhaps but given our current structure it's more difficult to get to the form instance in the commit phase and a naive solution wouldn't preserve ordering between forms. This solution isn't 100% guaranteed to preserve ordering between different React instances neither but should be in order within one instance which is the common case. The hard part is that we don't know what instance something will belong to until it hydrates. So to solve that I keep everything in the original queue while we wait, so that ordering is preserved until we know which instance it'll go into. I ended up doing a bunch of clever tricks to make this work. These could use a lot more tests than I have right now. Another thing that's tricky is that you can update the action before it's replayed but we actually want to invoke the old action if that happens. So we have to extract it even if we can't invoke it right now just so we get the one that was there during hydration.
We used to have Event Replaying for any kind of Discrete event where we'd track any event after hydrateRoot and before the async code/data has loaded in to hydrate the target. However, this didn't really work out because code inside event handlers are expected to be able to synchronously read the state of the world at the time they're invoked. If we replay discrete events later, the mutable state around them like selection or form state etc. may have changed. This limitation doesn't apply to Client Actions: - They're expected to be async functions that themselves work asynchronously. They're conceptually also in the "navigation" events that happen after the "submit" events so they're already not synchronously even before the first `await`. - They're expected to operate mostly on the FormData as input which we can snapshot at the time of the event. This PR adds a bit of inline script to the Fizz runtime (or external runtime) to track any early submit events on the page - but only if the action URL is our placeholder `javascript:` URL. We track a queue of these on `document.$$reactFormReplay`. Then we replay them in order as they get hydrated and we get a handle on the Client Action function. I add the runtime to the `bootstrapScripts` phase in Fizz which is really technically a little too late, because on a large page, it might take a while to get to that script even if you have displayed the form. However, that's also true for external runtime. So there's a very short window we might miss an event but it's good enough and better than risking blocking display on this script. The main thing that makes the replaying difficult to reason about is that we can have multiple instance of React using this same queue. This would be very usual but you could have two different Reacts SSR:ing different parts of the tree and using around the same version. We don't have any coordinating ids for this. We could stash something on the form perhaps but given our current structure it's more difficult to get to the form instance in the commit phase and a naive solution wouldn't preserve ordering between forms. This solution isn't 100% guaranteed to preserve ordering between different React instances neither but should be in order within one instance which is the common case. The hard part is that we don't know what instance something will belong to until it hydrates. So to solve that I keep everything in the original queue while we wait, so that ordering is preserved until we know which instance it'll go into. I ended up doing a bunch of clever tricks to make this work. These could use a lot more tests than I have right now. Another thing that's tricky is that you can update the action before it's replayed but we actually want to invoke the old action if that happens. So we have to extract it even if we can't invoke it right now just so we get the one that was there during hydration. DiffTrain build for commit bf449ee.
We used to have Event Replaying for any kind of Discrete event where we'd track any event after hydrateRoot and before the async code/data has loaded in to hydrate the target. However, this didn't really work out because code inside event handlers are expected to be able to synchronously read the state of the world at the time they're invoked. If we replay discrete events later, the mutable state around them like selection or form state etc. may have changed.
This limitation doesn't apply to Client Actions:
await.This PR adds a bit of inline script to the Fizz runtime (or external runtime) to track any early submit events on the page - but only if the action URL is our placeholder
javascript:URL. We track a queue of these ondocument.$$reactFormReplay. Then we replay them in order as they get hydrated and we get a handle on the Client Action function.I add the runtime to the
bootstrapScriptsphase in Fizz which is really technically a little too late, because on a large page, it might take a while to get to that script even if you have displayed the form. However, that's also true for external runtime. So there's a very short window we might miss an event but it's good enough and better than risking blocking display on this script.The main thing that makes the replaying difficult to reason about is that we can have multiple instance of React using this same queue. This would be very usual but you could have two different Reacts SSR:ing different parts of the tree and using around the same version. We don't have any coordinating ids for this. We could stash something on the form perhaps but given our current structure it's more difficult to get to the form instance in the commit phase and a naive solution wouldn't preserve ordering between forms.
This solution isn't 100% guaranteed to preserve ordering between different React instances neither but should be in order within one instance which is the common case.
The hard part is that we don't know what instance something will belong to until it hydrates. So to solve that I keep everything in the original queue while we wait, so that ordering is preserved until we know which instance it'll go into. I ended up doing a bunch of clever tricks to make this work. These could use a lot more tests than I have right now.
Another thing that's tricky is that you can update the action before it's replayed but we actually want to invoke the old action if that happens. So we have to extract it even if we can't invoke it right now just so we get the one that was there during hydration.