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.

Sign up for GitHub

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

Jump to bottom

Add withSuspenseConfig API #15593

Merged
merged 14 commits into from
May 16, 2019
Merged

Add withSuspenseConfig API #15593

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
dae328f
Add suspendIfNeeded API and a global scope to track it
sebmarkbage May 8, 2019
53f94f9
Add the suspenseConfig to all updates created during this scope
sebmarkbage May 8, 2019
58671af
Compute expiration time based on the timeout of the suspense config
sebmarkbage May 8, 2019
aee7a4a
Track if there was a processed suspenseConfig this render pass
sebmarkbage May 8, 2019
f80e403
Mark suspended states that should be avoided as a separate flag
sebmarkbage May 8, 2019
b9e467a
Suspend until the full expiration time if something asked to suspend
sebmarkbage May 8, 2019
df782f6
Reenable an old test that we can now repro again
sebmarkbage May 9, 2019
8660f24
Suspend the commit even if it is complete if there is a minimum delay
sebmarkbage May 9, 2019
044af3c
Default timeoutMs to low pri expiration if not provided
sebmarkbage May 9, 2019
0c902e4
Rename to withSuspenseConfig and drop the default config
sebmarkbage May 16, 2019
367354d
Track the largest suspense config expiration separately
sebmarkbage May 16, 2019
1f889b6
Add basic tests for functionality using each update mechanism
sebmarkbage May 16, 2019
d937e45
Fix issue when newly created avoided boundary doesn't suspend with delay
sebmarkbage May 16, 2019
4347de6
Add test for loading indicator with minLoadingDurationMs option
sebmarkbage May 16, 2019
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
Prev Previous commit
Next Next commit
Track if there was a processed suspenseConfig this render pass 
We'll use this info to suspend a commit for longer when necessary.
  • Loading branch information
@sebmarkbage
sebmarkbage committed May 16, 2019
commit aee7a4ac4bdca9bdd7c591004d2f8636a71044c5
7 changes: 5 additions & 2 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,10 @@ import {
enableSuspenseServerRenderer,
enableEventAPI,
} from 'shared/ReactFeatureFlags';
import {markRenderEventTime, renderDidSuspend} from './ReactFiberScheduler';
import {
markRenderEventTimeAndConfig,
renderDidSuspend,
} from './ReactFiberScheduler';
import {getEventComponentHostChildrenCount} from './ReactFiberEvents';
import getComponentName from 'shared/getComponentName';
import warning from 'shared/warning';
Expand Down Expand Up @@ -698,7 +701,7 @@ function completeWork(
// was given a normal pri expiration time at the time it was shown.
const fallbackExpirationTime: ExpirationTime =
prevState.fallbackExpirationTime;
markRenderEventTime(fallbackExpirationTime);
markRenderEventTimeAndConfig(fallbackExpirationTime, null);

// Delete the fallback.
// TODO: Would it be better to store the fallback fragment on
Expand Down
7 changes: 5 additions & 2 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import {
flushPassiveEffects,
requestCurrentTime,
warnIfNotCurrentlyActingUpdatesInDev,
markRenderEventTime,
markRenderEventTimeAndConfig,
} from './ReactFiberScheduler';

import invariant from 'shared/invariant';
Expand Down Expand Up @@ -731,7 +731,10 @@ function updateReducer<S, I, A>(
// TODO: We should skip this update if it was already committed but currently
// we have no way of detecting the difference between a committed and suspended
// update here.
markRenderEventTime(updateExpirationTime);
markRenderEventTimeAndConfig(
updateExpirationTime,
update.suspenseConfig,
);

// Process this update.
if (update.eagerReducer === reducer) {
Expand Down
29 changes: 21 additions & 8 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,8 @@ let workInProgressRootExitStatus: RootExitStatus = RootIncomplete;
// This is conceptually a time stamp but expressed in terms of an ExpirationTime
// because we deal mostly with expiration times in the hot path, so this avoids
// the conversion happening in the hot path.
let workInProgressRootMostRecentEventTime: ExpirationTime = Sync;
let workInProgressRootLatestProcessedExpirationTime: ExpirationTime = Sync;
let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null;

let nextEffect: Fiber | null = null;
let hasUncaughtError = false;
Expand Down Expand Up @@ -731,7 +732,8 @@ function prepareFreshStack(root, expirationTime) {
workInProgress = createWorkInProgress(root.current, null, expirationTime);
renderExpirationTime = expirationTime;
workInProgressRootExitStatus = RootIncomplete;
workInProgressRootMostRecentEventTime = Sync;
workInProgressRootLatestProcessedExpirationTime = Sync;
workInProgressRootCanSuspendUsingConfig = null;

if (__DEV__) {
ReactStrictModeWarnings.discardPendingWarnings();
Expand Down Expand Up @@ -937,12 +939,15 @@ function renderRoot(
// at that level.
return renderRoot.bind(null, root, lastPendingTime);
}
// If workInProgressRootMostRecentEventTime is Sync, that means we didn't
// If workInProgressRootLatestProcessedExpirationTime is Sync, that means we didn't
// track any event times. That can happen if we retried but nothing switched
// from fallback to content. There's no reason to delay doing no work.
if (workInProgressRootMostRecentEventTime !== Sync) {
if (workInProgressRootLatestProcessedExpirationTime !== Sync) {
if (workInProgressRootCanSuspendUsingConfig !== null) {
// TODO: If needed, suspend here up until workInProgressRootLatestProcessedExpirationTime.
}
let msUntilTimeout = computeMsUntilTimeout(
workInProgressRootMostRecentEventTime,
workInProgressRootLatestProcessedExpirationTime,
expirationTime,
);
// Don't bother with a very short suspense time.
Expand Down Expand Up @@ -971,12 +976,20 @@ function renderRoot(
}
}

export function markRenderEventTime(expirationTime: ExpirationTime): void {
export function markRenderEventTimeAndConfig(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Naming suggestion: markBatchConfig

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

markExpirationAndSuspenseConfig?

Several callers must supply the expiration but don't supply a config, and that config likely wouldn't be the "whole batch's config" but specifically it is where you pass an object or null that is the interesting flag here.

expirationTime: ExpirationTime,
suspenseConfig: null | SuspenseConfig,
): void {
if (
expirationTime < workInProgressRootMostRecentEventTime &&
expirationTime < workInProgressRootLatestProcessedExpirationTime &&
expirationTime > Never
) {
workInProgressRootMostRecentEventTime = expirationTime;
workInProgressRootLatestProcessedExpirationTime = expirationTime;
}
if (suspenseConfig !== null) {
// For simplicity we pick any config for the minDuration.
// Most of the time we only have one config and getting wrong is not bad.
workInProgressRootCanSuspendUsingConfig = suspenseConfig;
}
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react-reconciler/src/ReactUpdateQueue.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ import {
} from 'shared/ReactFeatureFlags';

import {StrictMode} from './ReactTypeOfMode';
import {markRenderEventTime} from './ReactFiberScheduler';
import {markRenderEventTimeAndConfig} from './ReactFiberScheduler';

import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
Expand Down Expand Up @@ -469,7 +469,7 @@ export function processUpdateQueue<State>(
// TODO: We should skip this update if it was already committed but currently
// we have no way of detecting the difference between a committed and suspended
// update here.
markRenderEventTime(updateExpirationTime);
markRenderEventTimeAndConfig(updateExpirationTime, update.suspenseConfig);

// Process it and compute a new result.
resultState = getStateFromUpdate(
Expand Down