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

ReactDOM.createRoot creates an async root #11769

Merged
merged 1 commit into from
Dec 4, 2017

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Dec 4, 2017

Makes createRoot the opt-in API for async updates. Now we don't have to check the top-level element to see if it's an async container.

Makes createRoot the opt-in API for async updates. Now we don't have
to check the top-level element to see if it's an async container.
@acdlite
Copy link
Collaborator Author

acdlite commented Dec 4, 2017

@bvaughn React Native doesn't have createRoot. How should we expose this?

@bvaughn
Copy link
Contributor

bvaughn commented Dec 4, 2017

@bvaughn React Native doesn't have createRoot. How should we expose this?

Unclear. We could hook into something like AppRegistry.registerComponent with a new optional config option (in a similar way to how ReactDOM.createRoot works). I'll chat with some other folks like @grabbou about this.

For the time being, we can continue to use AsyncComponent.

Copy link
Contributor

@bvaughn bvaughn left a comment

Choose a reason for hiding this comment

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

btw this looks fine to me.

@@ -1300,7 +1301,7 @@ if (enableCreateRoot) {
options?: RootOptions,
): ReactRoot {
const hydrate = options != null && options.hydrate === true;
return new ReactRoot(container, hydrate);
return new ReactRoot(container, true, hydrate);
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why async is hard-coded to true here vs being opt-in like hydrate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

createRoot is only useful in async mode, so the rationale is that if you're not async resilient, you should keep using the legacy API.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops. Sorry. Had written this comment before we chatted in person and forgot to remove it before submitting the LGTM.

@acdlite acdlite merged commit 4d0e8fc into facebook:master Dec 4, 2017
@acdlite acdlite deleted the asynccreateroot branch December 4, 2017 22:35
@grabbou
Copy link

grabbou commented Dec 20, 2017

@bvaughn I am not sure if we ever had a chance to talk about this change (I remember being CC'ed, but have been traveling a lot).

If any of these are still relevant matters that you'd like to discuss, I am happy to chat. Been digging into registerComponent a bit recently.

@bvaughn
Copy link
Contributor

bvaughn commented Dec 20, 2017

Hey Mike. No worries. I sent you a message on Slack about it, but no, we didn't get the chance to talk. I have chatted with a few others about this approach, like Leland from AirBnb though. So far it seems like AppRegistry.registerComponent is probably a reasonable place for this logic to be added.

@NE-SmallTown
Copy link
Contributor

@bvaughn Is there any public slack channel I can join? :)

@bvaughn
Copy link
Contributor

bvaughn commented Jan 21, 2018

I'm not aware of a public Slack channel. There's Reactiflux on Discord though.

NMinhNguyen referenced this pull request in enzymejs/react-shallow-renderer Jan 29, 2020
Makes createRoot the opt-in API for async updates. Now we don't have
to check the top-level element to see if it's an async container.
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.

None yet

5 participants