From b60148dba09c0596d32bd7034fba28571453a4ef Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 4 Oct 2021 13:12:37 -0400 Subject: [PATCH 1/8] test that discrete events that arent hydratable do not propagate --- ...MServerSelectiveHydration-test.internal.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index bb04b46df1c40..6660ae8fad5ce 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1130,4 +1130,69 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + + it('does not propagate discrete event if it cannot be synchronously hydrated', async () => { + let triggeredParent = false; + let triggeredChild = false; + let suspend = false; + let promise = new Promise(() => {}); + function Child() { + if (suspend) { + throw promise; + } + Scheduler.unstable_yieldValue('Child'); + return ( + { + e.stopPropagation(); + triggeredChild = true; + }}> + Click me + + ); + } + function App() { + const ref = React.useRef(); + const onClick = () => { + triggeredParent = true; + }; + React.useLayoutEffect(() => { + if (!ref.current) { + return; + } + ref.current.onClick = onClick; + }, []); + Scheduler.unstable_yieldValue('App'); + return ( +
+ + + +
+ ); + } + let finalHTML; + expect(() => { + finalHTML = ReactDOMServer.renderToString(); + }).toErrorDev(['useLayoutEffect does nothing on the server']); + + expect(Scheduler).toHaveYielded(['App', 'Child']); + + const container = document.createElement('div'); + document.body.appendChild(container); + container.innerHTML = finalHTML; + + ReactDOM.hydrateRoot(container, ); + // Nothing has been hydrated so far. + expect(Scheduler).toHaveYielded([]); + + suspend = true; + + const span = container.getElementsByTagName('span')[0]; + dispatchClickEvent(span); + + expect(Scheduler).toHaveYielded(['App']); + expect(triggeredParent).toBe(false); + expect(triggeredChild).toBe(false); + }); }); From 8734075a31c90144003e33ce16f15c1e3bdcb6dd Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 4 Oct 2021 13:23:11 -0400 Subject: [PATCH 2/8] lint --- .../__tests__/ReactDOMServerSelectiveHydration-test.internal.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 6660ae8fad5ce..0643f194c0fbd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1135,7 +1135,7 @@ describe('ReactDOMServerSelectiveHydration', () => { let triggeredParent = false; let triggeredChild = false; let suspend = false; - let promise = new Promise(() => {}); + const promise = new Promise(() => {}); function Child() { if (suspend) { throw promise; From bce932b345ea5edd6313a8e8cc1fe5461a061953 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 4 Oct 2021 13:43:34 -0400 Subject: [PATCH 3/8] feedback --- packages/react-dom/src/events/ReactDOMEventListener.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index 6467421d4f635..a7f3f95980bc3 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -251,6 +251,10 @@ export function dispatchEvent( } blockedOn = nextBlockedOn; } + if (blockedOn) { + nativeEvent.stopPropagation(); + return; + } } // This is not replayable so we'll invoke it but without a target, From 5d2cb371bdbf7ab8026504f916b6bdb9d1042f2f Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 4 Oct 2021 13:47:16 -0400 Subject: [PATCH 4/8] feedback --- .../react-dom/src/events/ReactDOMEventListener.js | 11 +++++------ .../react-dom/src/events/ReactDOMEventReplaying.js | 9 +-------- 2 files changed, 6 insertions(+), 14 deletions(-) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index a7f3f95980bc3..cdf646f7c0342 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -16,13 +16,12 @@ import { enableSelectiveHydration, } from 'shared/ReactFeatureFlags'; import { - isReplayableDiscreteEvent, + isDiscreteEvent, queueDiscreteEvent, hasQueuedDiscreteEvents, clearIfContinuousEvent, queueIfContinuousEvent, attemptSynchronousHydration, - isCapturePhaseSynchronouslyHydratableEvent, } from './ReactDOMEventReplaying'; import { getNearestMountedFiber, @@ -169,7 +168,7 @@ export function dispatchEvent( if ( allowReplay && hasQueuedDiscreteEvents() && - isReplayableDiscreteEvent(domEventName) + isDiscreteEvent(domEventName) ) { // If we already have a queue of discrete events, and this is another discrete // event, then we can't dispatch it regardless of its target, since they @@ -202,7 +201,7 @@ export function dispatchEvent( if (allowReplay) { if ( !enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay && - isReplayableDiscreteEvent(domEventName) + isDiscreteEvent(domEventName) ) { // This this to be replayed later once the target is available. queueDiscreteEvent( @@ -232,8 +231,8 @@ export function dispatchEvent( if ( enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay && - enableSelectiveHydration && - isCapturePhaseSynchronouslyHydratableEvent(domEventName) + eventSystemFlags & IS_CAPTURE_PHASE && + isDiscreteEvent(domEventName) ) { while (blockedOn !== null) { const fiber = getInstanceFromNode(blockedOn); diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index c363721bcd848..f4dd4b5b5e217 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -160,7 +160,7 @@ const discreteReplayableEvents: Array = [ 'submit', ]; -export function isReplayableDiscreteEvent(eventType: DOMEventName): boolean { +export function isDiscreteEvent(eventType: DOMEventName): boolean { return discreteReplayableEvents.indexOf(eventType) > -1; } @@ -300,13 +300,6 @@ function accumulateOrCreateContinuousQueuedReplayableEvent( return existingQueuedEvent; } -export function isCapturePhaseSynchronouslyHydratableEvent( - eventName: DOMEventName, -) { - // TODO: maybe include more events - return isReplayableDiscreteEvent(eventName); -} - export function queueIfContinuousEvent( blockedOn: null | Container | SuspenseInstance, domEventName: DOMEventName, From 0422e3d9d50f9f073c320ae0d339cb62448db4be Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Mon, 4 Oct 2021 13:55:18 -0400 Subject: [PATCH 5/8] lint --- packages/react-dom/src/events/ReactDOMEventListener.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index cdf646f7c0342..ac837c6db1029 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -11,10 +11,7 @@ import type {AnyNativeEvent} from '../events/PluginModuleType'; import type {FiberRoot} from 'react-reconciler/src/ReactInternalTypes'; import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig'; import type {DOMEventName} from '../events/DOMEventNames'; -import { - enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay, - enableSelectiveHydration, -} from 'shared/ReactFeatureFlags'; +import {enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay} from 'shared/ReactFeatureFlags'; import { isDiscreteEvent, queueDiscreteEvent, From 0b18fae01d29ca2ca942c32ee2fbc39695445dfd Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 5 Oct 2021 17:16:02 -0400 Subject: [PATCH 6/8] better test --- .../ReactDOMServerSelectiveHydration-test.internal.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 0643f194c0fbd..5cf7546483038 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1131,6 +1131,7 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.removeChild(container); }); + // @gate enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay it('does not propagate discrete event if it cannot be synchronously hydrated', async () => { let triggeredParent = false; let triggeredChild = false; @@ -1160,7 +1161,7 @@ describe('ReactDOMServerSelectiveHydration', () => { if (!ref.current) { return; } - ref.current.onClick = onClick; + ref.current.onclick = onClick; }, []); Scheduler.unstable_yieldValue('App'); return ( @@ -1192,6 +1193,9 @@ describe('ReactDOMServerSelectiveHydration', () => { dispatchClickEvent(span); expect(Scheduler).toHaveYielded(['App']); + + dispatchClickEvent(span); + expect(triggeredParent).toBe(false); expect(triggeredChild).toBe(false); }); From e20dfc8fbe703985725ad512d6ad20469b4cb72d Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Tue, 5 Oct 2021 17:45:46 -0400 Subject: [PATCH 7/8] nits --- ...MServerSelectiveHydration-test.internal.js | 21 +++++++------------ .../src/events/ReactDOMEventListener.js | 8 +++---- .../src/events/ReactDOMEventReplaying.js | 4 +++- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 5cf7546483038..3dcdcb60dc9c0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1157,25 +1157,20 @@ describe('ReactDOMServerSelectiveHydration', () => { const onClick = () => { triggeredParent = true; }; - React.useLayoutEffect(() => { - if (!ref.current) { - return; - } - ref.current.onclick = onClick; - }, []); Scheduler.unstable_yieldValue('App'); return ( -
+
{ + if (n) n.onclick = onClick; + }} + onClick={onClick}>
); } - let finalHTML; - expect(() => { - finalHTML = ReactDOMServer.renderToString(); - }).toErrorDev(['useLayoutEffect does nothing on the server']); + const finalHTML = ReactDOMServer.renderToString(); expect(Scheduler).toHaveYielded(['App', 'Child']); @@ -1183,12 +1178,12 @@ describe('ReactDOMServerSelectiveHydration', () => { document.body.appendChild(container); container.innerHTML = finalHTML; + suspend = true; + ReactDOM.hydrateRoot(container, ); // Nothing has been hydrated so far. expect(Scheduler).toHaveYielded([]); - suspend = true; - const span = container.getElementsByTagName('span')[0]; dispatchClickEvent(span); diff --git a/packages/react-dom/src/events/ReactDOMEventListener.js b/packages/react-dom/src/events/ReactDOMEventListener.js index ac837c6db1029..86924e7c2673d 100644 --- a/packages/react-dom/src/events/ReactDOMEventListener.js +++ b/packages/react-dom/src/events/ReactDOMEventListener.js @@ -13,7 +13,7 @@ import type {Container, SuspenseInstance} from '../client/ReactDOMHostConfig'; import type {DOMEventName} from '../events/DOMEventNames'; import {enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay} from 'shared/ReactFeatureFlags'; import { - isDiscreteEvent, + isDiscreteEventThatRequiresHydration, queueDiscreteEvent, hasQueuedDiscreteEvents, clearIfContinuousEvent, @@ -165,7 +165,7 @@ export function dispatchEvent( if ( allowReplay && hasQueuedDiscreteEvents() && - isDiscreteEvent(domEventName) + isDiscreteEventThatRequiresHydration(domEventName) ) { // If we already have a queue of discrete events, and this is another discrete // event, then we can't dispatch it regardless of its target, since they @@ -198,7 +198,7 @@ export function dispatchEvent( if (allowReplay) { if ( !enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay && - isDiscreteEvent(domEventName) + isDiscreteEventThatRequiresHydration(domEventName) ) { // This this to be replayed later once the target is available. queueDiscreteEvent( @@ -229,7 +229,7 @@ export function dispatchEvent( if ( enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay && eventSystemFlags & IS_CAPTURE_PHASE && - isDiscreteEvent(domEventName) + isDiscreteEventThatRequiresHydration(domEventName) ) { while (blockedOn !== null) { const fiber = getInstanceFromNode(blockedOn); diff --git a/packages/react-dom/src/events/ReactDOMEventReplaying.js b/packages/react-dom/src/events/ReactDOMEventReplaying.js index f4dd4b5b5e217..ed31f20e51185 100644 --- a/packages/react-dom/src/events/ReactDOMEventReplaying.js +++ b/packages/react-dom/src/events/ReactDOMEventReplaying.js @@ -160,7 +160,9 @@ const discreteReplayableEvents: Array = [ 'submit', ]; -export function isDiscreteEvent(eventType: DOMEventName): boolean { +export function isDiscreteEventThatRequiresHydration( + eventType: DOMEventName, +): boolean { return discreteReplayableEvents.indexOf(eventType) > -1; } From 08c833b85bf24bd2611b7af27f420ee31c88fae0 Mon Sep 17 00:00:00 2001 From: Marco Salazar Date: Wed, 6 Oct 2021 08:22:26 -0400 Subject: [PATCH 8/8] lint --- .../__tests__/ReactDOMServerSelectiveHydration-test.internal.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 3dcdcb60dc9c0..5c29513cf6bfd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1153,7 +1153,6 @@ describe('ReactDOMServerSelectiveHydration', () => { ); } function App() { - const ref = React.useRef(); const onClick = () => { triggeredParent = true; };