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

Sync hydrate discrete events in capture phase and dont replay discrete events #22448

Merged
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
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
Original file line number Diff line number Diff line change
Expand Up @@ -1905,8 +1905,15 @@ describe('ReactDOMServerPartialHydration', () => {
resolve();
await promise;
});
expect(clicks).toBe(1);
// Clicks aren't replayed
salazarm marked this conversation as resolved.
Show resolved Hide resolved
expect(clicks).toBe(0);
expect(container.textContent).toBe('Click meHello');

await act(async () => {
a.click();
});
// Now click went through
expect(clicks).toBe(1);
expect(container.textContent).toBe('Hello');

document.body.removeChild(container);
Expand Down Expand Up @@ -1991,8 +1998,13 @@ describe('ReactDOMServerPartialHydration', () => {
await promise;
});

expect(onEvent).toHaveBeenCalledTimes(2);
// Clicks are not replayed
expect(onEvent).toHaveBeenCalledTimes(0);

await act(async () => {
a.click();
});
expect(onEvent).toHaveBeenCalledTimes(1);
document.body.removeChild(container);
});

Expand Down Expand Up @@ -2072,7 +2084,13 @@ describe('ReactDOMServerPartialHydration', () => {
resolve();
await promise;
});
expect(clicks).toBe(2);
// clicks are not replayed
salazarm marked this conversation as resolved.
Show resolved Hide resolved
expect(clicks).toBe(0);

await act(async () => {
a.click();
});
expect(clicks).toBe(1);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -2158,7 +2176,16 @@ describe('ReactDOMServerPartialHydration', () => {
resolve();
await promise;
});
expect(onEvent).toHaveBeenCalledTimes(2);

// Stays 0 because we don't replay clicks
expect(onEvent).toHaveBeenCalledTimes(0);

await act(async () => {
a.click();
});

// Clicks now go through after resolving
expect(onEvent).toHaveBeenCalledTimes(1);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -2231,6 +2258,13 @@ describe('ReactDOMServerPartialHydration', () => {
await promise;
});

// Stays 0 because we don't replay click events
expect(clicksOnChild).toBe(0);

await act(async () => {
span.click();
});
// Now click events go through
expect(clicksOnChild).toBe(1);
// This will be zero due to the stopPropagation.
expect(clicksOnParent).toBe(0);
Expand Down Expand Up @@ -2309,8 +2343,12 @@ describe('ReactDOMServerPartialHydration', () => {
await promise;
});

// We're now full hydrated.
// We're now full hydrated but we don't replay clicks
expect(clicks).toBe(0);

await act(async () => {
a.click();
});
expect(clicks).toBe(1);

document.body.removeChild(parentContainer);
Expand Down Expand Up @@ -2351,7 +2389,7 @@ describe('ReactDOMServerPartialHydration', () => {
onMouseLeave={() => ops.push('Mouse Leave First')}
/>
{/* We suspend after to test what happens when we eager
attach the listener. */}
attach the listener. */}
<First />
</Suspense>
<Suspense fallback="Loading Second...">
Expand Down Expand Up @@ -2396,9 +2434,11 @@ describe('ReactDOMServerPartialHydration', () => {
expect(ops).toEqual([]);

// Resolving the second promise so that rendering can complete.
suspend2 = false;
resolve2();
await promise2;
await act(async () => {
suspend2 = false;
resolve2();
await promise2;
});

Scheduler.unstable_flushAll();
jest.runAllTimers();
Expand All @@ -2407,10 +2447,12 @@ describe('ReactDOMServerPartialHydration', () => {
// able to replay it now.
expect(ops).toEqual(['Mouse Enter Second']);

// Resolving the first promise has no effect now.
suspend1 = false;
resolve1();
await promise1;
await act(async () => {
// Resolving the first promise has no effect now.
suspend1 = false;
resolve1();
await promise1;
});

Scheduler.unstable_flushAll();
jest.runAllTimers();
Expand Down Expand Up @@ -2580,6 +2622,18 @@ describe('ReactDOMServerPartialHydration', () => {
await promise;
});

// Submit event is not replayed
salazarm marked this conversation as resolved.
Show resolved Hide resolved
expect(submits).toBe(0);
expect(container.textContent).toBe('Click meHello');

await act(async () => {
form.dispatchEvent(
new Event('submit', {
bubbles: true,
}),
);
});
// Submit event has now gone through
expect(submits).toBe(1);
expect(container.textContent).toBe('Hello');
document.body.removeChild(container);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,10 @@ describe('ReactDOMServerSelectiveHydration', () => {
Scheduler.unstable_yieldValue(text);
return (
<span
onClickCapture={() => {
Scheduler.unstable_yieldValue('Capture Clicked ' + text);
}}
onClick={e => {
e.preventDefault();
Scheduler.unstable_yieldValue('Clicked ' + text);
}}>
{text}
Expand Down Expand Up @@ -172,13 +174,15 @@ describe('ReactDOMServerSelectiveHydration', () => {

// This should synchronously hydrate the root App and the second suspense
// boundary.
const result = dispatchClickEvent(span);

// The event should have been canceled because we called preventDefault.
expect(result).toBe(false);
dispatchClickEvent(span);

// We rendered App, B and then invoked the event without rendering A.
expect(Scheduler).toHaveYielded(['App', 'B', 'Clicked B']);
expect(Scheduler).toHaveYielded([
'App',
'B',
'Capture Clicked B',
'Clicked B',
]);

// After continuing the scheduler, we finally hydrate A.
expect(Scheduler).toFlushAndYield(['A']);
Expand Down Expand Up @@ -256,7 +260,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
});
expect(Scheduler).toHaveYielded([
'App',
// Continuing rendering will render B next.
// B and C don't suspense so they are rendered immediately
'B',
'C',
]);
Expand All @@ -266,9 +270,9 @@ describe('ReactDOMServerSelectiveHydration', () => {
resolve();
await promise;
});
// After the click, we should prioritize D and the Click first,
// After the click, we should prioritize hydrating D
salazarm marked this conversation as resolved.
Show resolved Hide resolved
// and only after that render A and C.
expect(Scheduler).toHaveYielded(['D', 'Clicked D', 'A']);
expect(Scheduler).toHaveYielded(['D', 'A']);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -338,28 +342,25 @@ describe('ReactDOMServerSelectiveHydration', () => {
// Nothing has been hydrated so far.
expect(Scheduler).toHaveYielded([]);

// This click target cannot be hydrated yet because the first is Suspended.
// A and D cannot be hydrated yet because they Suspended.
dispatchClickEvent(spanA);
dispatchClickEvent(spanC);
dispatchClickEvent(spanD);

expect(Scheduler).toHaveYielded(['App']);
// C can be immediately hydrated in capture phase in time for it to be clicked
dispatchClickEvent(spanC);
expect(Scheduler).toHaveYielded(['App', 'C', 'Clicked C']);

await act(async () => {
suspend = false;
resolve();
await promise;
});

// We should prioritize hydrating A, C and D first since we clicked in
// We should prioritize hydrating A then D first since we clicked in
// them. Only after they're done will we hydrate B.
expect(Scheduler).toHaveYielded([
'A',
'Clicked A',
'C',
'Clicked C',
'D',
'Clicked D',
// B should render last since it wasn't clicked.
'B',
]);
Expand Down Expand Up @@ -512,14 +513,15 @@ describe('ReactDOMServerSelectiveHydration', () => {
});
expect(Scheduler).toHaveYielded(['App', 'B', 'C']);

// After the click, we should prioritize D and the Click first,
// and only after that render A and C.
// After the click, we should prioritize D,
// and only after that render A
await act(async () => {
suspend = false;
resolve();
await promise;
});
expect(Scheduler).toHaveYielded(['D', 'Clicked D', 'A']);
// No click is yielded since we don't replay clicks
expect(Scheduler).toHaveYielded(['D', 'A']);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -596,10 +598,12 @@ describe('ReactDOMServerSelectiveHydration', () => {

// This click target cannot be hydrated yet because the first is Suspended.
createEventTarget(spanA).virtualclick();
createEventTarget(spanC).virtualclick();
createEventTarget(spanD).virtualclick();

expect(Scheduler).toHaveYielded(['App']);
// C can be immediately hydrated in capture phase in time for click
createEventTarget(spanC).virtualclick();

expect(Scheduler).toHaveYielded(['App', 'C', 'Clicked C']);

await act(async () => {
suspend = false;
Expand All @@ -611,11 +615,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
// them. Only after they're done will we hydrate B.
expect(Scheduler).toHaveYielded([
'A',
'Clicked A',
'C',
'Clicked C',
'D',
'Clicked D',
// B should render last since it wasn't clicked.
'B',
]);
Expand Down Expand Up @@ -699,7 +699,8 @@ describe('ReactDOMServerSelectiveHydration', () => {
dispatchMouseHoverEvent(spanB, spanD);
dispatchMouseHoverEvent(spanC, spanB);

expect(Scheduler).toHaveYielded(['App']);
// We can hydrate B and C now.
expect(Scheduler).toHaveYielded(['App', 'B', 'C']);

await act(async () => {
suspend = false;
Expand All @@ -713,14 +714,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
// the same time since B was already scheduled.
// This is ok because it will at least not continue for nested
// boundary. See the next test below.
expect(Scheduler).toHaveYielded([
'D',
'Clicked D',
'B', // Ideally this should be later.
'C',
'Hover C',
'A',
]);
expect(Scheduler).toHaveYielded(['D', 'A']);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -784,10 +778,8 @@ describe('ReactDOMServerSelectiveHydration', () => {

suspend = true;

// A and D will be suspended. We'll click on D which should take
// priority, after we unsuspend.
const root = ReactDOM.createRoot(container, {hydrate: true});
root.render(<App />);
// A and D will be suspended.
ReactDOM.hydrateRoot(container, <App />);

// Nothing has been hydrated so far.
expect(Scheduler).toHaveYielded([]);
Expand All @@ -796,16 +788,18 @@ describe('ReactDOMServerSelectiveHydration', () => {
dispatchMouseHoverEvent(spanB, spanD);
dispatchMouseHoverEvent(spanC, spanB);

suspend = false;
resolve();
await promise;
// C renders before B since its the current hover target
// B renders because its priority was increased when it was hovered over
expect(Scheduler).toHaveYielded(['App', 'C', 'B', 'Hover C']);

// We should prioritize hydrating D first because we clicked it.
// Next we should hydrate C since that's the current hover target.
// Next it doesn't matter if we hydrate A or B first but as an
// implementation detail we're currently hydrating B first since
// we at one point hovered over it and we never deprioritized it.
expect(Scheduler).toFlushAndYield(['App', 'C', 'Hover C', 'A', 'B', 'D']);
await act(async () => {
suspend = false;
resolve();
await promise;
});

// Finally D and A render
expect(Scheduler).toHaveYielded(['D', 'A']);

document.body.removeChild(container);
});
Expand Down Expand Up @@ -942,20 +936,17 @@ describe('ReactDOMServerSelectiveHydration', () => {
dispatchClickEvent(spanC);

expect(Scheduler).toHaveYielded([
// Hydrate C first since we clicked it.
// Hydrate A and B since we hovered
// then Hydrate C since we clicked it.
'A',
'a',
'B',
'b',
'C',
'c',
]);

expect(Scheduler).toFlushAndYield([
// Finish hydration of A since we forced it to hydrate.
'A',
'a',
// Also, hydrate B since we hovered over it.
// It's not important which one comes first. A or B.
// As long as they both happen before the Idle update.
'B',
'b',
// Begin the Idle update again.
'App',
'AA',
Expand Down
Loading