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

API for prerendering a top-level update and deferring the commit #11232

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions src/renderers/dom/fiber/ReactDOMFiberEntry.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
'use strict';

import type {ReactNodeList} from 'ReactTypes';
import type {ExpirationTime} from 'ReactFiberExpirationTime';

require('checkReact');
var DOMNamespaces = require('DOMNamespaces');
Expand Down Expand Up @@ -757,8 +758,27 @@ function createPortal(
return ReactPortal.createPortal(children, container, null, key);
}

type WorkNode = {
commit(): void,

_reactRootContainer: *,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we use an existential type here instead of something explicit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because I was lazy and didn't want to import it :D

Copy link

Choose a reason for hiding this comment

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

:D

_expirationTime: ExpirationTime,
};

function Work(root: *, expirationTime: ExpirationTime) {
this._reactRootContainer = root;
this._expirationTime = expirationTime;
}
Work.prototype.commit = function() {
const root = this._reactRootContainer;
const expirationTime = this._expirationTime;
DOMRenderer.unblockRoot(root, expirationTime);
DOMRenderer.flushRoot(root, expirationTime);
};

type ReactRootNode = {
render(children: ReactNodeList, callback: ?() => mixed): void,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this new API really should return a Thenable since that's what people have been asking for with these callback APIs. I noticed because the prerender API doesn't have it.

prerender(children: ReactNodeList): WorkNode,
unmount(callback: ?() => mixed): void,

_reactRootContainer: *,
Expand All @@ -779,6 +799,11 @@ ReactRoot.prototype.render = function(
const root = this._reactRootContainer;
DOMRenderer.updateContainer(children, root, null, callback);
};
ReactRoot.prototype.prerender = function(children: ReactNodeList): WorkNode {
const root = this._reactRootContainer;
const expirationTime = DOMRenderer.updateRoot(children, root, null, null);
return new Work(root, expirationTime);
};
ReactRoot.prototype.unmount = function(callback) {
const root = this._reactRootContainer;
DOMRenderer.updateContainer(null, root, null, callback);
Expand Down
30 changes: 30 additions & 0 deletions src/renderers/dom/shared/__tests__/ReactDOMRoot-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,34 @@ describe('ReactDOMRoot', () => {
root.render(<div><span>d</span><span>c</span></div>);
expect(container.textContent).toEqual('abdc');
});

it('can defer commit using prerender', () => {
const root = ReactDOM.createRoot(container);
const work = root.prerender(<div>Hi</div>);
// Hasn't updated yet
expect(container.textContent).toEqual('');
// Flush work
work.commit();
expect(container.textContent).toEqual('Hi');
});

it("does not restart a blocked root that wasn't updated", () => {
let ops = [];
function Foo(props) {
ops.push('Foo');
return props.children;
}
const root = ReactDOM.createRoot(container);
const work = root.prerender(<Foo>Hi</Foo>);
expect(ops).toEqual(['Foo']);
// Hasn't updated yet
expect(container.textContent).toEqual('');

ops = [];

// Flush work. Shouldn't re-render Foo.
work.commit();
expect(ops).toEqual([]);
expect(container.textContent).toEqual('Hi');
});
});
13 changes: 11 additions & 2 deletions src/renderers/shared/fiber/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
}

function updateHostRoot(current, workInProgress, renderExpirationTime) {
const root: FiberRoot = workInProgress.stateNode;
pushHostRootContext(workInProgress);
const updateQueue = workInProgress.updateQueue;
if (updateQueue !== null) {
Expand All @@ -331,14 +332,23 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
null,
renderExpirationTime,
);
memoizeState(workInProgress, state);
if (root.completedAt === renderExpirationTime) {
// The root is already complete. Bail out and commit.
// TODO: This is a limited version of resuming that only applies to
// the root, to account for the pathological case where a completed
// root must be completely restarted before it can commit. Once we
// implement resuming for real, this special branch shouldn't
// be neccessary.
return null;
}
if (prevState === state) {
// If the state is the same as before, that's a bailout because we had
// no work that expires at this time.
resetHydrationState();
return bailoutOnAlreadyFinishedWork(current, workInProgress);
}
const element = state.element;
const root: FiberRoot = workInProgress.stateNode;
if (
(current === null || current.child === null) &&
root.hydrate &&
Expand Down Expand Up @@ -370,7 +380,6 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
resetHydrationState();
reconcileChildren(current, workInProgress, element);
}
memoizeState(workInProgress, state);
return workInProgress.child;
}
resetHydrationState();
Expand Down
7 changes: 7 additions & 0 deletions src/renderers/shared/fiber/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
config: HostConfig<T, P, I, TI, PI, C, CX, PL>,
hostContext: HostContext<C, CX>,
hydrationContext: HydrationContext<C, CX>,
blockCurrentlyRenderingWork: () => void,
) {
const {
createInstance,
Expand Down Expand Up @@ -219,6 +220,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
// TODO: Delete this when we delete isMounted and findDOMNode.
workInProgress.effectTag &= ~Placement;
}

const memoizedState = workInProgress.memoizedState;
if (memoizedState !== null && memoizedState.isBlocked) {
// Root is blocked by a top-level update.
blockCurrentlyRenderingWork();
}
return null;
}
case HostComponent: {
Expand Down
73 changes: 69 additions & 4 deletions src/renderers/shared/fiber/ReactFiberReconciler.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import type {Fiber} from 'ReactFiber';
import type {FiberRoot} from 'ReactFiberRoot';
import type {ReactNodeList} from 'ReactTypes';
import type {ExpirationTime} from 'ReactFiberExpirationTime';

var ReactFeatureFlags = require('ReactFeatureFlags');
var {
Expand Down Expand Up @@ -226,6 +227,14 @@ export type Reconciler<C, I, TI> = {
parentComponent: ?React$Component<any, any>,
callback: ?Function,
): void,
updateRoot(
element: ReactNodeList,
container: OpaqueRoot,
parentComponent: ?React$Component<any, any>,
callback: ?Function,
): ExpirationTime,
unblockRoot(root: OpaqueRoot, expirationTime: ExpirationTime): void,
flushRoot(root: OpaqueRoot, expirationTime: ExpirationTime): void,
batchedUpdates<A>(fn: () => A): A,
unbatchedUpdates<A>(fn: () => A): A,
flushSync<A>(fn: () => A): A,
Expand Down Expand Up @@ -266,6 +275,7 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
computeAsyncExpiration,
computeExpirationForFiber,
scheduleWork,
expireWork,
batchedUpdates,
unbatchedUpdates,
flushSync,
Expand All @@ -275,8 +285,9 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
function scheduleTopLevelUpdate(
current: Fiber,
element: ReactNodeList,
isBlocked: boolean,
callback: ?Function,
) {
): ExpirationTime {
if (__DEV__) {
if (
ReactDebugCurrentFiber.phase === 'render' &&
Expand Down Expand Up @@ -323,15 +334,15 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(

const update = {
expirationTime,
partialState: {element},
partialState: {element, isBlocked},
callback,
isReplace: false,
isForced: false,
nextCallback: null,
next: null,
};
insertUpdateIntoFiber(current, update);
scheduleWork(current, expirationTime);
return expirationTime;
}

return {
Expand Down Expand Up @@ -367,7 +378,61 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
container.pendingContext = context;
}

scheduleTopLevelUpdate(current, element, callback);
scheduleTopLevelUpdate(current, element, false, callback);
},

// Like updateContainer, but blocks the root from committing. Returns an
// expiration time.
// TODO: Can this be unified with updateContainer? Or is it incompatible
// with the existing semantics of ReactDOM.render?
updateRoot(
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need to be stricter with naming of these things. I'm not sure what makes most sense here since Root is otherwise just an internal concept. I think this still make sense to refer to Container here. From the outside perspective there is no root.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Especially since the comment eve says that this might not even be a root.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Didn't we just add an API called createRoot? :D

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also I think that comment is outdated post-portals, but I wasn't sure enough to delete it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We have the createRoot for persistent updates, HostRoot fiber and createRoot in the ReactDOM renderer. Neither of which are the same and neither is this.

element: ReactNodeList,
container: OpaqueRoot,
parentComponent: ?React$Component<any, any>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is an new API we don't have to support legacy apis like renderSubtreeIntoContainer.

callback: ?Function,
): ExpirationTime {
// TODO: If this is a nested container, this won't be the root.
const current = container.current;

if (__DEV__) {
if (ReactFiberInstrumentation.debugTool) {
if (current.alternate === null) {
ReactFiberInstrumentation.debugTool.onMountContainer(container);
} else if (element === null) {
ReactFiberInstrumentation.debugTool.onUnmountContainer(container);
} else {
ReactFiberInstrumentation.debugTool.onUpdateContainer(container);
}
}
}

const context = getContextForSubtree(parentComponent);
if (container.context === null) {
container.context = context;
} else {
container.pendingContext = context;
}

return scheduleTopLevelUpdate(current, element, true, callback);
},

unblockRoot(root: OpaqueRoot, expirationTime: ExpirationTime) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we need these to be separate? This is inherent complexity to support where as the current API only lets you do both at once.

I think we should try again to simply defer all commits until explicitly called since we have so many cases where that seems to matter. And the legacy mode is simply a wrapper that does that eagerly. In that case that's the normal mode so the concept of being blocked doesn't make sense and unblocking doesn't as well.

const current = root.current;
const partialState = {isBlocked: false};
const update = {
expirationTime,
partialState,
callback: null,
isReplace: false,
isForced: false,
next: null,
};
insertUpdateIntoFiber(current, update);
scheduleWork(current, expirationTime);
},

flushRoot(root: OpaqueRoot, expirationTime: ExpirationTime) {
expireWork(root, expirationTime);
},

batchedUpdates,
Expand Down
11 changes: 11 additions & 0 deletions src/renderers/shared/fiber/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@
'use strict';

import type {Fiber} from 'ReactFiber';
import type {ExpirationTime} from 'ReactFiberExpirationTime';

const {createHostRootFiber} = require('ReactFiber');
const {NoWork} = require('ReactFiberExpirationTime');

export type FiberRoot = {
// Any additional information from the host associated with this root.
Expand All @@ -21,6 +23,12 @@ export type FiberRoot = {
current: Fiber,
// Determines if this root has already been added to the schedule for work.
isScheduled: boolean,
// Determines if this root was blocked from committing.
isBlocked: boolean,
// The time at which this root completed.
// TODO: Remove once we add back resuming.
completedAt: ExpirationTime,
forceExpire: ExpirationTime,
// The work schedule is a linked list.
nextScheduledRoot: FiberRoot | null,
// Top context object, used by renderSubtreeIntoContainer
Expand All @@ -41,6 +49,9 @@ exports.createFiberRoot = function(
current: uninitializedFiber,
containerInfo: containerInfo,
isScheduled: false,
isBlocked: false,
completedAt: NoWork,
forceExpire: NoWork,
nextScheduledRoot: null,
context: null,
pendingContext: null,
Expand Down
Loading