Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replay Client Actions After Hydration #26716

Merged
merged 5 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/react-dom-bindings/src/events/ReactDOMEventListener.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,19 +225,25 @@ export function dispatchEvent(
);
}

export function findInstanceBlockingEvent(
nativeEvent: AnyNativeEvent,
): null | Container | SuspenseInstance {
const nativeEventTarget = getEventTarget(nativeEvent);
return findInstanceBlockingTarget(nativeEventTarget);
}

export let return_targetInst: null | Fiber = null;

// Returns a SuspenseInstance or Container if it's blocked.
// The return_targetInst field above is conceptually part of the return value.
export function findInstanceBlockingEvent(
nativeEvent: AnyNativeEvent,
export function findInstanceBlockingTarget(
targetNode: Node,
): null | Container | SuspenseInstance {
// TODO: Warn if _enabled is false.

return_targetInst = null;

const nativeEventTarget = getEventTarget(nativeEvent);
let targetInst = getClosestInstanceFromNode(nativeEventTarget);
let targetInst = getClosestInstanceFromNode(targetNode);

if (targetInst !== null) {
const nearestMounted = getNearestMountedFiber(targetInst);
Expand Down
139 changes: 137 additions & 2 deletions packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ import {
getContainerFromFiber,
getSuspenseInstanceFromFiber,
} from 'react-reconciler/src/ReactFiberTreeReflection';
import {findInstanceBlockingEvent} from './ReactDOMEventListener';
import {
findInstanceBlockingEvent,
findInstanceBlockingTarget,
} from './ReactDOMEventListener';
import {setReplayingEvent, resetReplayingEvent} from './CurrentReplayingEvent';
import {
getInstanceFromNode,
getClosestInstanceFromNode,
getFiberCurrentPropsFromNode,
} from '../client/ReactDOMComponentTree';
import {HostRoot, SuspenseComponent} from 'react-reconciler/src/ReactWorkTags';
import {isHigherEventPriority} from 'react-reconciler/src/ReactEventPriorities';
import {isRootDehydrated} from 'react-reconciler/src/ReactFiberShellHydration';
import {dispatchReplayedFormAction} from './plugins/FormActionEventPlugin';

import {
attemptContinuousHydration,
Expand All @@ -41,6 +46,7 @@ import {
runWithPriority as attemptHydrationAtPriority,
getCurrentUpdatePriority,
} from 'react-reconciler/src/ReactEventPriorities';
import {enableFormActions} from 'shared/ReactFeatureFlags';

// TODO: Upgrade this definition once we're on a newer version of Flow that
// has this definition built-in.
Expand Down Expand Up @@ -105,7 +111,7 @@ const discreteReplayableEvents: Array<DOMEventName> = [
'change',
'contextmenu',
'reset',
'submit',
// 'submit', // stopPropagation blocks the replay mechanism
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could've been if it's in a parent component with a Suspense/Hydration boundary around the target.

];

export function isDiscreteEventThatRequiresHydration(
Expand Down Expand Up @@ -430,6 +436,67 @@ function scheduleCallbackIfUnblocked(
}
}

type FormAction = FormData => void | Promise<void>;

type FormReplayingQueue = Array<any>; // [form, submitter or action, formData...]

let lastScheduledReplayQueue: null | FormReplayingQueue = null;

function replayUnblockedFormActions(formReplayingQueue: FormReplayingQueue) {
if (lastScheduledReplayQueue === formReplayingQueue) {
lastScheduledReplayQueue = null;
}
for (let i = 0; i < formReplayingQueue.length; i += 3) {
const form: HTMLFormElement = formReplayingQueue[i];
const submitterOrAction:
| null
| HTMLInputElement
| HTMLButtonElement
| FormAction = formReplayingQueue[i + 1];
const formData: FormData = formReplayingQueue[i + 2];
if (typeof submitterOrAction !== 'function') {
// This action is not hydrated yet. This might be because it's blocked on
// a different React instance or higher up our tree.
const blockedOn = findInstanceBlockingTarget(submitterOrAction || form);
if (blockedOn === null) {
// We're not blocked but we don't have an action. This must mean that
// this is in another React instance. We'll just skip past it.
continue;
} else {
// We're blocked on something in this React instance. We'll retry later.
break;
}
}
const formInst = getInstanceFromNode(form);
if (formInst !== null) {
// This is part of our instance.
// We're ready to replay this. Let's delete it from the queue.
formReplayingQueue.splice(i, 3);
i -= 3;
dispatchReplayedFormAction(formInst, submitterOrAction, formData);
// Continue without incrementing the index.
continue;
}
// This form must've been part of a different React instance.
// If we want to preserve ordering between React instances on the same root
// we'd need some way for the other instance to ping us when it's done.
// We'll just skip this and let the other instance execute it.
}
}

function scheduleReplayQueueIfNeeded(formReplayingQueue: FormReplayingQueue) {
// Schedule a callback to execute any unblocked form actions in.
// We only keep track of the last queue which means that if multiple React oscillate
// commits, we could schedule more callbacks than necessary but it's not a big deal
// and we only really except one instance.
if (lastScheduledReplayQueue !== formReplayingQueue) {
lastScheduledReplayQueue = formReplayingQueue;
scheduleCallback(NormalPriority, () =>
replayUnblockedFormActions(formReplayingQueue),
);
}
}

export function retryIfBlockedOn(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So does this wait until all the components in a container are hydrated?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea but this also gets called if it ends up deleted instead.

unblocked: Container | SuspenseInstance,
): void {
Expand Down Expand Up @@ -467,4 +534,72 @@ export function retryIfBlockedOn(
}
}
}

if (enableFormActions) {
// Check the document if there are any queued form actions.
const root = unblocked.getRootNode();
const formReplayingQueue: void | FormReplayingQueue = (root: any)
.$$reactFormReplay;
if (formReplayingQueue != null) {
for (let i = 0; i < formReplayingQueue.length; i += 3) {
const form: HTMLFormElement = formReplayingQueue[i];
const submitterOrAction:
| null
| HTMLInputElement
| HTMLButtonElement
| FormAction = formReplayingQueue[i + 1];
const formProps = getFiberCurrentPropsFromNode(form);
if (typeof submitterOrAction === 'function') {
// This action has already resolved. We're just waiting to dispatch it.
if (!formProps) {
// This was not part of this React instance. It might have been recently
// unblocking us from dispatching our events. So let's make sure we schedule
// a retry.
scheduleReplayQueueIfNeeded(formReplayingQueue);
}
continue;
}
let target: Node = form;
if (formProps) {
// This form belongs to this React instance but the submitter might
// not be done yet.
let action: null | FormAction = null;
const submitter = submitterOrAction;
if (submitter && submitter.hasAttribute('formAction')) {
// The submitter is the one that is responsible for the action.
target = submitter;
const submitterProps = getFiberCurrentPropsFromNode(submitter);
if (submitterProps) {
// The submitter is part of this instance.
action = (submitterProps: any).formAction;
} else {
const blockedOn = findInstanceBlockingTarget(target);
if (blockedOn !== null) {
// The submitter is not hydrated yet. We'll wait for it.
continue;
}
// The submitter must have been a part of a different React instance.
// Except the form isn't. We don't dispatch actions in this scenario.
}
} else {
action = (formProps: any).action;
}
if (typeof action === 'function') {
formReplayingQueue[i + 1] = action;
} else {
// Something went wrong so let's just delete this action.
formReplayingQueue.splice(i, 3);
i -= 3;
}
// Schedule a replay in case this unblocked something.
scheduleReplayQueueIfNeeded(formReplayingQueue);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this only need to happen in the typeof action === 'function' case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

continue;
}
// Something above this target is still blocked so we can't continue yet.
// We're not sure if this target is actually part of this React instance
// yet. It could be a different React as a child but at least some parent is.
// We must continue for any further queued actions.
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -114,3 +114,11 @@ function extractEvents(
}

export {extractEvents};

export function dispatchReplayedFormAction(
formInst: Fiber,
action: FormData => void | Promise<void>,
formData: FormData,
): void {
startHostTransition(formInst, action, formData);
}
65 changes: 53 additions & 12 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
completeBoundary as completeBoundaryFunction,
completeBoundaryWithStyles as styleInsertionFunction,
completeSegment as completeSegmentFunction,
formReplaying as formReplayingRuntime,
} from './fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings';

import {
Expand Down Expand Up @@ -104,11 +105,12 @@ const ScriptStreamingFormat: StreamingFormat = 0;
const DataStreamingFormat: StreamingFormat = 1;

export type InstructionState = number;
const NothingSent /* */ = 0b0000;
const SentCompleteSegmentFunction /* */ = 0b0001;
const SentCompleteBoundaryFunction /* */ = 0b0010;
const SentClientRenderFunction /* */ = 0b0100;
const SentStyleInsertionFunction /* */ = 0b1000;
const NothingSent /* */ = 0b00000;
const SentCompleteSegmentFunction /* */ = 0b00001;
const SentCompleteBoundaryFunction /* */ = 0b00010;
const SentClientRenderFunction /* */ = 0b00100;
const SentStyleInsertionFunction /* */ = 0b01000;
const SentFormReplayingRuntime /* */ = 0b10000;

// Per response, global state that is not contextual to the rendering subtree.
export type ResponseState = {
Expand Down Expand Up @@ -637,6 +639,7 @@ const actionJavaScriptURL = stringToPrecomputedChunk(

function pushFormActionAttribute(
target: Array<Chunk | PrecomputedChunk>,
responseState: ResponseState,
formAction: any,
formEncType: any,
formMethod: any,
Expand Down Expand Up @@ -683,6 +686,7 @@ function pushFormActionAttribute(
actionJavaScriptURL,
attributeEnd,
);
injectFormReplayingRuntime(responseState);
} else {
// Plain form actions support all the properties, so we have to emit them.
if (name !== null) {
Expand Down Expand Up @@ -1256,9 +1260,30 @@ function pushStartOption(
return children;
}

const formReplayingRuntimeScript =
stringToPrecomputedChunk(formReplayingRuntime);

function injectFormReplayingRuntime(responseState: ResponseState): void {
// If we haven't sent it yet, inject the runtime that tracks submitted JS actions
// for later replaying by Fiber. If we use an external runtime, we don't need
// to emit anything. It's always used.
if (
(responseState.instructions & SentFormReplayingRuntime) === NothingSent &&
(!enableFizzExternalRuntime || !responseState.externalRuntimeConfig)
) {
responseState.instructions |= SentFormReplayingRuntime;
responseState.bootstrapChunks.unshift(
responseState.startInlineScript,
formReplayingRuntimeScript,
endInlineScript,
);
}
}

function pushStartForm(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
): ReactNodeList {
target.push(startChunkForTag('form'));

Expand Down Expand Up @@ -1335,6 +1360,7 @@ function pushStartForm(
actionJavaScriptURL,
attributeEnd,
);
injectFormReplayingRuntime(responseState);
} else {
// Plain form actions support all the properties, so we have to emit them.
if (formAction !== null) {
Expand Down Expand Up @@ -1365,6 +1391,7 @@ function pushStartForm(
function pushInput(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
): ReactNodeList {
if (__DEV__) {
checkControlledValueProps('input', props);
Expand Down Expand Up @@ -1445,6 +1472,7 @@ function pushInput(

pushFormActionAttribute(
target,
responseState,
formAction,
formEncType,
formMethod,
Expand Down Expand Up @@ -1499,6 +1527,7 @@ function pushInput(
function pushStartButton(
target: Array<Chunk | PrecomputedChunk>,
props: Object,
responseState: ResponseState,
): ReactNodeList {
target.push(startChunkForTag('button'));

Expand Down Expand Up @@ -1561,6 +1590,7 @@ function pushStartButton(

pushFormActionAttribute(
target,
responseState,
formAction,
formEncType,
formMethod,
Expand Down Expand Up @@ -2947,11 +2977,11 @@ export function pushStartInstance(
case 'textarea':
return pushStartTextArea(target, props);
case 'input':
return pushInput(target, props);
return pushInput(target, props, responseState);
case 'button':
return pushStartButton(target, props);
return pushStartButton(target, props, responseState);
case 'form':
return pushStartForm(target, props);
return pushStartForm(target, props, responseState);
case 'menuitem':
return pushStartMenuItem(target, props);
case 'title':
Expand Down Expand Up @@ -3127,7 +3157,7 @@ export function pushEndInstance(
target.push(endTag1, stringToChunk(type), endTag2);
}

export function writeCompletedRoot(
function writeBootstrap(
destination: Destination,
responseState: ResponseState,
): boolean {
Expand All @@ -3137,11 +3167,20 @@ export function writeCompletedRoot(
writeChunk(destination, bootstrapChunks[i]);
}
if (i < bootstrapChunks.length) {
return writeChunkAndReturn(destination, bootstrapChunks[i]);
const lastChunk = bootstrapChunks[i];
bootstrapChunks.length = 0;
return writeChunkAndReturn(destination, lastChunk);
}
return true;
}

export function writeCompletedRoot(
destination: Destination,
responseState: ResponseState,
): boolean {
return writeBootstrap(destination, responseState);
}

// Structural Nodes

// A placeholder is a node inside a hidden partial tree that can be filled in later, but before
Expand Down Expand Up @@ -3599,11 +3638,13 @@ export function writeCompletedBoundaryInstruction(
writeChunk(destination, completeBoundaryScript3b);
}
}
let writeMore;
if (scriptFormat) {
return writeChunkAndReturn(destination, completeBoundaryScriptEnd);
writeMore = writeChunkAndReturn(destination, completeBoundaryScriptEnd);
} else {
return writeChunkAndReturn(destination, completeBoundaryDataEnd);
writeMore = writeChunkAndReturn(destination, completeBoundaryDataEnd);
}
return writeBootstrap(destination, responseState) && writeMore;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

const clientRenderScript1Full = stringToPrecomputedChunk(
Expand Down
Loading