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

New API for top-level updates #10624

Closed
wants to merge 24 commits into from
Closed

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Sep 6, 2017

Introduces an additional, new API for performing top-level updates in React DOM:

// Existing API
ReactDOM.render(<App foo="A" />, container); // mount
ReactDOM.render(<App foo="B" />, container); // update
ReactDOM.unmountComponentAtNode(container); // unmount


// New API
const root = ReactDOM.unstable_createRoot(container);

// Simple
root.render(<App foo="A" />); // mount
root.render(<App foo="B" />); // update
root.unmount(); // unmount

// With pre-rendering
const work = root.prerender(<App foo="C" />); // Begin rendering but don't flush yet.
await work; // Wait for work to complete. Still don't flush yet.
work.commit(); // Synchronously flush changes

// Start prerendering before container is available
let container;
const root = ReactDOM.unstable_createRoot(function getContainer() {
  return container;
});
root.prerender(<App foo="A" />); // Begin rendering but don't flush yet.
container = await promiseForContainer;
work.commit(); // Synchronously flush remaining work

(Will fill in with more details later regarding motivations and use cases.)

I had thought we might be able to implement the existing API (ReactDOM.render) on top of the new API, but the semantics of the .then callback are sufficiently different that I don't think it's possible without a breaking change.

Work-in-progress:

  • ReactDOM.unstable_createRoot that returns a public root instance. Replaces hidden _reactRootContainer field.
  • Work type that represents a top-level update. Has then and commit methods.
  • work.commit() synchronously flushes all remaining work.
  • Pre-rendering in one tree should not block updates in a separate tree.
  • Completion callbacks (.then) resolve synchronously if tree is already complete
  • Committing one tree should not flush updates in a separate tree.
  • Figure out how hydration fits into this.
  • Lazy containers
    • Namespace parameter
    • Owner document parameter
  • Things that are "done" but need tests to confirm
    • Rendering before DOM container is available

Depends on expiration times PR #10426

@acdlite acdlite force-pushed the newtoplevelapi branch 2 times, most recently from 036f294 to 3a91a5d Compare September 6, 2017 22:33
var ReactDOMFiber = {
unstable_create(
Copy link
Collaborator

Choose a reason for hiding this comment

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

As much as I like the flushSync(batch) naming convention, this might be a bit too overloaded and vague.

unstable_createRoot?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Works for me. (I figure we can have a team bikeshedding meeting before we land this :D)

var ReactDOMFiber = {
unstable_create(
container: DOMContainer | (() => DOMContainer),
namespace: ?string,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this suppose to be namespace ?: string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ooh, didn't know Flow treated those differently.

@sebmarkbage
Copy link
Collaborator

Did we figure out how the hydrate API fits into this?

@acdlite
Copy link
Collaborator Author

acdlite commented Sep 7, 2017

Nope, I'll add that to the todos

@acdlite acdlite force-pushed the newtoplevelapi branch 2 times, most recently from 6420547 to 7887ceb Compare September 8, 2017 20:57
expect(root.getChildren()).toEqual([span('A')]);
});

it('resolves `then` callback synchronously if tree is already completed', () => {
Copy link
Collaborator Author

@acdlite acdlite Sep 8, 2017

Choose a reason for hiding this comment

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

Hmm, is this behavior really necessary for work that prerendered async? Or is it only relevant to sync work?

@gaearon
Copy link
Collaborator

gaearon commented Sep 8, 2017

Is it not confusing that something that looks thenable doesn't behave like a promise (zalgo)?

@acdlite acdlite force-pushed the newtoplevelapi branch 2 times, most recently from 1e7100e to 9f81d8c Compare September 9, 2017 00:22
@acdlite
Copy link
Collaborator Author

acdlite commented Sep 9, 2017

@gaearon We thought it was valuable to be able to implement the normal root.render API on top of the root.prerender API, including in sync mode. It might be confusing, but I think we can reasonably assume that prerender is an advanced, lower-level API that will be used by relatively few people, at least directly. Also, if you await a thenable, it's automatically wrapped in a promise.

@acdlite acdlite force-pushed the newtoplevelapi branch 3 times, most recently from 2e7eac6 to ad1dc28 Compare September 14, 2017 18:17
acdlite added a commit to acdlite/react that referenced this pull request Sep 14, 2017
Prevents a push/pop mismatch when bailing out on HostRoots. This is
currently unobservable, because HostRoots never bail out on low
priority, but this does happen with prerendering.

I found this when rebasing facebook#10624 on top of master.
@acdlite acdlite force-pushed the newtoplevelapi branch 2 times, most recently from 8422a91 to f31b7ab Compare September 14, 2017 18:34
acdlite added a commit that referenced this pull request Sep 14, 2017
Prevents a push/pop mismatch when bailing out on HostRoots. This is
currently unobservable, because HostRoots never bail out on low
priority, but this does happen with prerendering.

I found this when rebasing #10624 on top of master.
@acdlite acdlite mentioned this pull request Sep 15, 2017
@acdlite acdlite changed the title [Work-in-progress] New API for top-level updates New API for top-level updates Sep 29, 2017
@reactjs-bot
Copy link

Deploy preview ready!

Built with commit 24b7272

https://deploy-preview-10624--reactjs.netlify.com

@reactjs-bot
Copy link

Deploy preview ready!

Built with commit f59a531

https://deploy-preview-10624--reactjs.netlify.com

acdlite added 12 commits October 9, 2017 10:03
An expiration time represents a time in the future by which an update
should flush. The priority of the update is related to the difference
between the current clock time and the expiration time. This has the
effect of increasing the priority of updates as time progresses, to
prevent starvation.

This lays the initial groundwork for expiration times without changing
any behavior. Future commits will replace work priority with
expiration times.
Instead of a priority, a fiber has an expiration time that represents
a point in the future by which it should render.

Pending updates still have priorities so that they can be coalesced.

We use a host config method to read the current time. This commit
implements everything except that method, which currently returns a
constant value. So this just proves that expiration times work the same
as priorities when time is frozen. Subsequent commits will show the
effect of advancing time.
shouldComponentUpdate was removed from functional components.

Running the demo shows, now that expiration is enabled, the demo does
not starve. (Still won't run smoothly until we add back the ability to
resume interrupted work.)
There are a few cases related to sync mode where we need to distinguish
between work that is scheduled as task and work that is treated like
task because it expires. For example, batchedUpdates. We don't want to
perform any work until the end of the batch, regardless of how much
time has elapsed.
- Rename Done -> NoWork
- Use max int32 instead of max safe int
- Use bitwise operations instead of Math functions
Refactors the complete phase to add support for blocking a tree from
committing. Also adds a basic version of "resuming" for HostRoots.

Neither of these features are actually implemented in this commit; will
come later.
We can perform work in the render and complete phases even before we
have access to the DOM container. We only need the namespace.

Once we get to the commit phase, throw if we don't have a DOM container.
acdlite added 12 commits October 9, 2017 11:16
We'll use an UpdateQueue to store top-level completion callbacks.
`renderer.updateRoot` returns a Work object, which has methods `then`
and `commit`.

- `then` schedules a callback to fire once the update has completed. It
resolves synchronously if the tree has already completed.
- `commit` synchronously flushes all the remaining work.
This makes the types for UpdateQueue a bit more sound though it's still
not quite right. Not sure how to properly State so that it works with
partial state updates, replaceState, and updater functions.

This at least gives us more safety for non-Fiber update queues, like
FiberRoot's completionCallbacks queue.
More unit tests. These completion callbacks (as I'm calling them) have
some interesting properties.
We need to process completion callbacks in two places. The first is
intuitive: right after a root completes. It might seem like that is
sufficient. But if a completion callback is scheduled on an already
completed root, it's possible we won't complete that root again. So
we also need to process completion callbacks whenever we skip over an
already completed root.
This is the main reason Work is a thenable and not a promise.
`forceExpire` is tracked per root rather than per scheduler.
Hydration should be disabled by default. It's also incompatible with
lazy containers, since you can only hydrate a container that has
already resolved.

After considering these constraints, we came up with this API:

createRoot(container: Element, ?{hydrate?: boolean})
createLazyRoot(container: () => Element, ?{namespace?: string, ownerDocument?: Document})
scheduleWork updates the priority of the fiber and its ancestors and
schedules work to be performed.

scheduleUpdate inserts an update into a fiber's update queue and calls
then schedules work with scheduleWork.

Now we don't have to export so many things from the scheduler.
After thinking about how to implement blockers in general, I figured
out how to simplify top-level blockers, too.
@gaearon
Copy link
Collaborator

gaearon commented Jan 5, 2018

Seems stale and superseded by #11225.

@gaearon gaearon closed this Jan 5, 2018
@NE-SmallTown
Copy link
Contributor

@acdlite Is there any explanation why we drop this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants