Skip to content

Commit

Permalink
Simplify Continuous Hydration Targets (#17952)
Browse files Browse the repository at this point in the history
* Simplify Continuous Hydration Targets

Let's use a constant priority for this. This helps us avoid restarting
a render when switching targets and simplifies the model.

The downside is that now we're not down-prioritizing the previous hover
target. However, we think that's ok because it'll only do one level too
much and then stop.

* Add test meant to show why it's tricky to merge both hydration levels

Having both levels co-exist works. However, if we deprioritize hydration
using a single level, we might deprioritize the wrong thing.

This adds a test that catches it if we ever try a naive deprioritization
in the future.

It also tests that we don't down-prioritize if we're changing the hover
in the middle of doing continuous priority work.
  • Loading branch information
sebmarkbage authored Feb 3, 2020
1 parent 7df32c4 commit ace9e81
Show file tree
Hide file tree
Showing 3 changed files with 205 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {createEventTarget} from 'dom-event-testing-library';
let React;
let ReactDOM;
let ReactDOMServer;
let ReactTestUtils;
let Scheduler;
let Suspense;
let usePress;
Expand Down Expand Up @@ -102,6 +103,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
React = require('react');
ReactDOM = require('react-dom');
ReactDOMServer = require('react-dom/server');
ReactTestUtils = require('react-dom/test-utils');
Scheduler = require('scheduler');
Suspense = React.Suspense;
usePress = require('react-interactions/events/press').usePress;
Expand Down Expand Up @@ -585,7 +587,7 @@ describe('ReactDOMServerSelectiveHydration', () => {
document.body.removeChild(container);
});

it('hydrates the last target as higher priority for continuous events', async () => {
it('hydrates the hovered targets as higher priority for continuous events', async () => {
let suspend = false;
let resolve;
let promise = new Promise(resolvePromise => (resolve = resolvePromise));
Expand Down Expand Up @@ -669,21 +671,107 @@ describe('ReactDOMServerSelectiveHydration', () => {

// 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.
// To simplify implementation details we hydrate both B and C at
// 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).toFlushAndYield([
'D',
'Clicked D',
'B', // Ideally this should be later.
'C',
'Hover C',
'B',
'A',
]);

document.body.removeChild(container);
});

it('hydrates the last target path first for continuous events', async () => {
let suspend = false;
let resolve;
let promise = new Promise(resolvePromise => (resolve = resolvePromise));

function Child({text}) {
if ((text === 'A' || text === 'D') && suspend) {
throw promise;
}
Scheduler.unstable_yieldValue(text);
return (
<span
onMouseEnter={e => {
e.preventDefault();
Scheduler.unstable_yieldValue('Hover ' + text);
}}>
{text}
</span>
);
}

function App() {
Scheduler.unstable_yieldValue('App');
return (
<div>
<Suspense fallback="Loading...">
<Child text="A" />
</Suspense>
<Suspense fallback="Loading...">
<div>
<Suspense fallback="Loading...">
<Child text="B" />
</Suspense>
</div>
<Child text="C" />
</Suspense>
<Suspense fallback="Loading...">
<Child text="D" />
</Suspense>
</div>
);
}

let finalHTML = ReactDOMServer.renderToString(<App />);

expect(Scheduler).toHaveYielded(['App', 'A', 'B', 'C', 'D']);

let container = document.createElement('div');
// We need this to be in the document since we'll dispatch events on it.
document.body.appendChild(container);

container.innerHTML = finalHTML;

let spanB = container.getElementsByTagName('span')[1];
let spanC = container.getElementsByTagName('span')[2];
let spanD = container.getElementsByTagName('span')[3];

suspend = true;

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

// Nothing has been hydrated so far.
expect(Scheduler).toHaveYielded([]);

// Hover over B and then C.
dispatchMouseHoverEvent(spanB, spanD);
dispatchMouseHoverEvent(spanC, spanB);

suspend = false;
resolve();
await promise;

// 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']);

document.body.removeChild(container);
});

it('hydrates the last explicitly hydrated target at higher priority', async () => {
function Child({text}) {
Scheduler.unstable_yieldValue(text);
Expand Down Expand Up @@ -731,4 +819,110 @@ describe('ReactDOMServerSelectiveHydration', () => {
// gets highest priority followed by the next added.
expect(Scheduler).toFlushAndYield(['App', 'C', 'B', 'A']);
});

it('hydrates before an update even if hydration moves away from it', async () => {
function Child({text}) {
Scheduler.unstable_yieldValue(text);
return <span>{text}</span>;
}
let ChildWithBoundary = React.memo(function({text}) {
return (
<Suspense fallback="Loading...">
<Child text={text} />
<Child text={text.toLowerCase()} />
</Suspense>
);
});

function App({a}) {
Scheduler.unstable_yieldValue('App');
React.useEffect(() => {
Scheduler.unstable_yieldValue('Commit');
});
return (
<div>
<ChildWithBoundary text={a} />
<ChildWithBoundary text="B" />
<ChildWithBoundary text="C" />
</div>
);
}

let finalHTML = ReactDOMServer.renderToString(<App a="A" />);

expect(Scheduler).toHaveYielded(['App', 'A', 'a', 'B', 'b', 'C', 'c']);

let container = document.createElement('div');
container.innerHTML = finalHTML;

// We need this to be in the document since we'll dispatch events on it.
document.body.appendChild(container);

let spanA = container.getElementsByTagName('span')[0];
let spanB = container.getElementsByTagName('span')[2];
let spanC = container.getElementsByTagName('span')[4];

let root = ReactDOM.createRoot(container, {hydrate: true});
ReactTestUtils.act(() => {
root.render(<App a="A" />);

// Hydrate the shell.
expect(Scheduler).toFlushAndYieldThrough(['App', 'Commit']);

// Render an update at Idle priority that needs to update A.
Scheduler.unstable_runWithPriority(
Scheduler.unstable_IdlePriority,
() => {
root.render(<App a="AA" />);
},
);

// Start rendering. This will force the first boundary to hydrate
// by scheduling it at one higher pri than Idle.
expect(Scheduler).toFlushAndYieldThrough(['App', 'A']);

// Hover over A which (could) schedule at one higher pri than Idle.
dispatchMouseHoverEvent(spanA, null);

// Before, we're done we now switch to hover over B.
// This is meant to test that this doesn't cause us to forget that
// we still have to hydrate A. The first boundary.
// This also tests that we don't do the -1 down-prioritization of
// continuous hover events because that would decrease its priority
// to Idle.
dispatchMouseHoverEvent(spanB, spanA);

// Also click C to prioritize that even higher which resets the
// priority levels.
dispatchClickEvent(spanC);

expect(Scheduler).toHaveYielded([
// Hydrate C first since we clicked it.
'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',
'aa',
'Commit',
]);
});

let spanA2 = container.getElementsByTagName('span')[0];
// This is supposed to have been hydrated, not replaced.
expect(spanA).toBe(spanA2);

document.body.removeChild(container);
});
});
16 changes: 3 additions & 13 deletions packages/react-reconciler/src/ReactFiberExpirationTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@ export const Never = 1;
// Idle is slightly higher priority than Never. It must completely finish in
// order to be consistent.
export const Idle = 2;
// Continuous Hydration is a moving priority. It is slightly higher than Idle
// and is used to increase priority of hover targets. It is increasing with
// each usage so that last always wins.
let ContinuousHydration = 3;
// Continuous Hydration is slightly higher than Idle and is used to increase
// priority of hover targets.
export const ContinuousHydration = 3;
export const Sync = MAX_SIGNED_31_BIT_INT;
export const Batched = Sync - 1;

Expand Down Expand Up @@ -119,15 +118,6 @@ export function computeInteractiveExpiration(currentTime: ExpirationTime) {
);
}

export function computeContinuousHydrationExpiration(
currentTime: ExpirationTime,
) {
// Each time we ask for a new one of these we increase the priority.
// This ensures that the last one always wins since we can't deprioritize
// once we've scheduled work already.
return ContinuousHydration++;
}

export function inferPriorityFromExpirationTime(
currentTime: ExpirationTime,
expirationTime: ExpirationTime,
Expand Down
9 changes: 3 additions & 6 deletions packages/react-reconciler/src/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,8 @@ import {
import {StrictMode} from './ReactTypeOfMode';
import {
Sync,
ContinuousHydration,
computeInteractiveExpiration,
computeContinuousHydrationExpiration,
} from './ReactFiberExpirationTime';
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
import {
Expand Down Expand Up @@ -384,11 +384,8 @@ export function attemptContinuousHydration(fiber: Fiber): void {
// Suspense.
return;
}
let expTime = computeContinuousHydrationExpiration(
requestCurrentTimeForUpdate(),
);
scheduleWork(fiber, expTime);
markRetryTimeIfNotHydrated(fiber, expTime);
scheduleWork(fiber, ContinuousHydration);
markRetryTimeIfNotHydrated(fiber, ContinuousHydration);
}

export function attemptHydrationAtCurrentPriority(fiber: Fiber): void {
Expand Down

0 comments on commit ace9e81

Please sign in to comment.