Skip to content

Commit 54a5072

Browse files
sebmarkbagesebmarkbage
authored
[Fiber] Replay events between commits (facebook#33130)
Stacked on facebook#33129. Flagged behind `enableHydrationChangeEvent`. If you type into a controlled input before hydration and something else rerenders like a setState in an effect, then the controlled input will reset to whatever React thought it was. Even with event replaying that this is stacked on, if the second render happens before event replaying has fired in a separate task. We don't want to flush inside the commit phase because then things like flushSync in these events wouldn't work since they're inside the commit stack. This flushes all event replaying between renders by flushing it at the end of `flushSpawned` work. We've already committed at that point and is about to either do subsequent renders or yield to event loop for passive effects which could have these events fired anyway. This just ensures that they've already happened by the time subsequent renders fire. This means that there's now a type of event that fire between sync render passes.
1 parent 587cb8f commit 54a5072

File tree

7 files changed

+124
-7
lines changed

7 files changed

+124
-7
lines changed

‎fixtures/ssr/src/components/Page.js

+12-1
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,17 @@ const autofocusedInputs = [
1111
];
1212

1313
export default class Page extends Component {
14-
state = {active: false};
14+
state = {active: false, value: ''};
1515
handleClick = e => {
1616
this.setState({active: true});
1717
};
18+
handleChange = e => {
19+
this.setState({value: e.target.value});
20+
};
21+
componentDidMount() {
22+
// Rerender on mount
23+
this.setState({mounted: true});
24+
}
1825
render() {
1926
const link = (
2027
<a className="link" onClick={this.handleClick}>
@@ -30,6 +37,10 @@ export default class Page extends Component {
3037
<p>Autofocus on page load: {autofocusedInputs}</p>
3138
<p>{!this.state.active ? link : 'Thanks!'}</p>
3239
{this.state.active && <p>Autofocus on update: {autofocusedInputs}</p>}
40+
<p>
41+
Controlled input:{' '}
42+
<input value={this.state.value} onChange={this.handleChange} />
43+
</p>
3344
</Suspend>
3445
</div>
3546
);

‎packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

+10-1
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ import {
9999
DOCUMENT_FRAGMENT_NODE,
100100
} from './HTMLNodeType';
101101

102-
import {retryIfBlockedOn} from '../events/ReactDOMEventReplaying';
102+
import {
103+
flushEventReplaying,
104+
retryIfBlockedOn,
105+
} from '../events/ReactDOMEventReplaying';
103106

104107
import {
105108
enableCreateEventHandleAPI,
@@ -3655,6 +3658,12 @@ export function commitHydratedSuspenseInstance(
36553658
retryIfBlockedOn(suspenseInstance);
36563659
}
36573660
3661+
export function flushHydrationEvents(): void {
3662+
if (enableHydrationChangeEvent) {
3663+
flushEventReplaying();
3664+
}
3665+
}
3666+
36583667
export function shouldDeleteUnhydratedTailInstances(
36593668
parentType: string,
36603669
): boolean {

‎packages/react-dom-bindings/src/events/ReactDOMEventReplaying.js

+14-5
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,19 @@ function replayUnblockedEvents() {
472472
}
473473
}
474474

475+
export function flushEventReplaying(): void {
476+
// Synchronously flush any event replaying so that it gets observed before
477+
// any new updates are applied.
478+
if (hasScheduledReplayAttempt) {
479+
replayUnblockedEvents();
480+
}
481+
}
482+
475483
export function queueChangeEvent(target: EventTarget): void {
476484
if (enableHydrationChangeEvent) {
477485
queuedChangeEventTargets.push(target);
478486
if (!hasScheduledReplayAttempt) {
479487
hasScheduledReplayAttempt = true;
480-
scheduleCallback(NormalPriority, replayUnblockedEvents);
481488
}
482489
}
483490
}
@@ -490,10 +497,12 @@ function scheduleCallbackIfUnblocked(
490497
queuedEvent.blockedOn = null;
491498
if (!hasScheduledReplayAttempt) {
492499
hasScheduledReplayAttempt = true;
493-
// Schedule a callback to attempt replaying as many events as are
494-
// now unblocked. This first might not actually be unblocked yet.
495-
// We could check it early to avoid scheduling an unnecessary callback.
496-
scheduleCallback(NormalPriority, replayUnblockedEvents);
500+
if (!enableHydrationChangeEvent) {
501+
// Schedule a callback to attempt replaying as many events as are
502+
// now unblocked. This first might not actually be unblocked yet.
503+
// We could check it early to avoid scheduling an unnecessary callback.
504+
scheduleCallback(NormalPriority, replayUnblockedEvents);
505+
}
497506
}
498507
}
499508
}

‎packages/react-dom/src/__tests__/ReactDOMServerIntegrationUserInteraction-test.js

+79
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ describe('ReactDOMServerIntegrationUserInteraction', () => {
5252
}
5353
this.setState({value: event.target.value});
5454
}
55+
componentDidMount() {
56+
if (this.props.cascade) {
57+
// Trigger a cascading render immediately upon hydration which rerenders the input.
58+
this.setState({cascade: true});
59+
}
60+
}
5561
render() {
5662
return (
5763
<input
@@ -73,6 +79,12 @@ describe('ReactDOMServerIntegrationUserInteraction', () => {
7379
}
7480
this.setState({value: event.target.value});
7581
}
82+
componentDidMount() {
83+
if (this.props.cascade) {
84+
// Trigger a cascading render immediately upon hydration which rerenders the textarea.
85+
this.setState({cascade: true});
86+
}
87+
}
7688
render() {
7789
return (
7890
<textarea
@@ -93,6 +105,12 @@ describe('ReactDOMServerIntegrationUserInteraction', () => {
93105
}
94106
this.setState({value: event.target.checked});
95107
}
108+
componentDidMount() {
109+
if (this.props.cascade) {
110+
// Trigger a cascading render immediately upon hydration which rerenders the checkbox.
111+
this.setState({cascade: true});
112+
}
113+
}
96114
render() {
97115
return (
98116
<input
@@ -114,6 +132,12 @@ describe('ReactDOMServerIntegrationUserInteraction', () => {
114132
}
115133
this.setState({value: event.target.value});
116134
}
135+
componentDidMount() {
136+
if (this.props.cascade) {
137+
// Trigger a cascading render immediately upon hydration which rerenders the select.
138+
this.setState({cascade: true});
139+
}
140+
}
117141
render() {
118142
return (
119143
<select
@@ -361,5 +385,60 @@ describe('ReactDOMServerIntegrationUserInteraction', () => {
361385
gate(flags => flags.enableHydrationChangeEvent) ? 1 : 0,
362386
);
363387
});
388+
389+
// @gate enableHydrationChangeEvent
390+
it('should not blow away user-entered text cascading hydration to a controlled input', async () => {
391+
let changeCount = 0;
392+
await testUserInteractionBeforeClientRender(
393+
<ControlledInput onChange={() => changeCount++} cascade={true} />,
394+
);
395+
expect(changeCount).toBe(1);
396+
});
397+
398+
// @gate enableHydrationChangeEvent
399+
it('should not blow away user-interaction cascading hydration to a controlled range input', async () => {
400+
let changeCount = 0;
401+
await testUserInteractionBeforeClientRender(
402+
<ControlledInput
403+
type="range"
404+
initialValue="0.25"
405+
onChange={() => changeCount++}
406+
cascade={true}
407+
/>,
408+
'0.25',
409+
'1',
410+
);
411+
expect(changeCount).toBe(1);
412+
});
413+
414+
// @gate enableHydrationChangeEvent
415+
it('should not blow away user-entered text cascading hydration to a controlled checkbox', async () => {
416+
let changeCount = 0;
417+
await testUserInteractionBeforeClientRender(
418+
<ControlledCheckbox onChange={() => changeCount++} cascade={true} />,
419+
true,
420+
false,
421+
'checked',
422+
);
423+
expect(changeCount).toBe(1);
424+
});
425+
426+
// @gate enableHydrationChangeEvent
427+
it('should not blow away user-entered text cascading hydration to a controlled textarea', async () => {
428+
let changeCount = 0;
429+
await testUserInteractionBeforeClientRender(
430+
<ControlledTextArea onChange={() => changeCount++} cascade={true} />,
431+
);
432+
expect(changeCount).toBe(1);
433+
});
434+
435+
// @gate enableHydrationChangeEvent
436+
it('should not blow away user-selected value cascading hydration to an controlled select', async () => {
437+
let changeCount = 0;
438+
await testUserInteractionBeforeClientRender(
439+
<ControlledSelect onChange={() => changeCount++} cascade={true} />,
440+
);
441+
expect(changeCount).toBe(1);
442+
});
364443
});
365444
});

‎packages/react-reconciler/src/ReactFiberConfigWithNoHydration.js

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ export const commitHydratedInstance = shim;
5050
export const commitHydratedContainer = shim;
5151
export const commitHydratedActivityInstance = shim;
5252
export const commitHydratedSuspenseInstance = shim;
53+
export const flushHydrationEvents = shim;
5354
export const clearActivityBoundary = shim;
5455
export const clearSuspenseBoundary = shim;
5556
export const clearActivityBoundaryFromContainer = shim;

‎packages/react-reconciler/src/ReactFiberWorkLoop.js

+7
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ import {
109109
startGestureTransition,
110110
stopViewTransition,
111111
createViewTransitionInstance,
112+
flushHydrationEvents,
112113
} from './ReactFiberConfig';
113114

114115
import {createWorkInProgress, resetWorkInProgress} from './ReactFiber';
@@ -3859,6 +3860,12 @@ function flushSpawnedWork(): void {
38593860
}
38603861
}
38613862

3863+
// Eagerly flush any event replaying that we unblocked within this commit.
3864+
// This ensures that those are observed before we render any new changes.
3865+
if (supportsHydration) {
3866+
flushHydrationEvents();
3867+
}
3868+
38623869
// If layout work was scheduled, flush it now.
38633870
flushSyncWorkOnAllRoots();
38643871

‎packages/react-reconciler/src/forks/ReactFiberConfig.custom.js

+1
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,7 @@ export const commitHydratedActivityInstance =
228228
export const commitHydratedSuspenseInstance =
229229
$$$config.commitHydratedSuspenseInstance;
230230
export const finalizeHydratedChildren = $$$config.finalizeHydratedChildren;
231+
export const flushHydrationEvents = $$$config.flushHydrationEvents;
231232
export const clearActivityBoundary = $$$config.clearActivityBoundary;
232233
export const clearSuspenseBoundary = $$$config.clearSuspenseBoundary;
233234
export const clearActivityBoundaryFromContainer =

0 commit comments

Comments
 (0)