Skip to content

Rename createSyncRoot to createBlockingRoot#17165

Merged
gaearon merged 2 commits into
react:masterfrom
gaearon:rename-modes
Oct 23, 2019
Merged

Rename createSyncRoot to createBlockingRoot#17165
gaearon merged 2 commits into
react:masterfrom
gaearon:rename-modes

Conversation

@gaearon

@gaearon gaearon commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

"Sync" is easy to confuse with the Legacy mode. It also might sound too positive.

While I'm renaming createSyncRoot to createBlockingRoot, I also did some find-and-replace in comments and internal naming. Let's standardize on:

  • Legacy Mode: The old one.
  • Blocking Mode: createBlockingRoot
  • Concurrent Mode: createRoot

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

Can we consider passing an option to createRoot instead, like {disableConcurrentRendering: true}? (This options object already exists for enabling hydration.) I think the original reason we went with a separate API was because we didn't want to expose createBatch to the non-concurrent version. But createBatch no longer exists.

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

I'm just not convinced "blocking" is a thing people will intuitively understand and I'm worried the additional terminology is more confusing than it's worth.

@gaearon

gaearon commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator Author

Concurrent Mode with "disabled concurrent rendering" isn't that much easier to explain IMO though — and we need to refer to this concept somehow when discussing adoption strategy.

@sizebot

sizebot commented Oct 21, 2019

Copy link
Copy Markdown

Size changes (experimental)

No significant bundle size changes to report.

Generated by 🚫 dangerJS against be0ce36

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

But like what is the difference between "blocking" and "concurrent"? Are they mutually exclusive? Can you have one without the other? Technically legacy is also blocking. Why not createNonConcurrentRoot? I don't get what "blocking" buys us as an additional terminology.

@sizebot

sizebot commented Oct 21, 2019

Copy link
Copy Markdown

Size changes (stable)

No significant bundle size changes to report.

Generated by 🚫 dangerJS against be0ce36

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

To clarify, I have two separate objections:

  • Do we need two separate methods, instead of using an option? Given that the end game is to get rid of "blocking" mode and move everyone to concurrent mode.
  • Is it worth adding a new term "blocking" when most people won't know what that means without an extra explanation, at which point we might as well keep using "concurrent."

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

OTOH I don't think it's any worse than what we already have, so I don't mind if we merge this for now. I would prefer to bikeshed it more before the actual release, though.

@sebmarkbage

Copy link
Copy Markdown
Contributor

There are a number of considerations for an options object:

  • Future additions to the api (such as adding createBatch back) that are mode specific.
  • Tree shaking, or clearly defined separate modules (like /legacy), makes it easier to create separate builds for it in the future.
  • It encourages dynamic usage in wrappers. In fact that’s exactly what we do with hydration at FB.

That said, these arguments mostly apply to hydration too and we already said we didn’t want a combinatorial explosion.

@sebmarkbage

Copy link
Copy Markdown
Contributor

I mostly suggested it because Sync looked bad in the column. I’m all for bikeshedding this further before release.

However we should decide the name of the column now “Legacy Mode”, “??? Mode”, “Concurrent Mode”.

@acdlite

acdlite commented Oct 21, 2019

Copy link
Copy Markdown
Collaborator

What I like about "legacy" is that describes the target use case as opposed to the technical characteristics. Maybe we could do the same here. "Compatibility Mode"?

@sebmarkbage

Copy link
Copy Markdown
Contributor

Tests failing.

@gaearon

gaearon commented Oct 23, 2019

Copy link
Copy Markdown
Collaborator Author

Oops. Pushed

@gaearon
gaearon merged commit f6b8d31 into react:master Oct 23, 2019
@gaearon
gaearon deleted the rename-modes branch October 23, 2019 22:04
ecreeth added a commit to ecreeth/concurrent-react-notes that referenced this pull request Oct 24, 2019
- **createSyncRoot** was renamed to **createSyncRoot** in [#17165](react/react#17165)
- **unstable_** is not necessary in experimental releases. See [#17146](react/react#17146) and [#17108](react/react#17108)
@gaearon gaearon mentioned this pull request Mar 29, 2022
oraclecode318 pushed a commit to oraclecode318/brain that referenced this pull request Mar 25, 2025
- **createSyncRoot** was renamed to **createSyncRoot** in [#17165](react/react#17165)
- **unstable_** is not necessary in experimental releases. See [#17146](react/react#17146) and [#17108](react/react#17108)
mrizwanashiq pushed a commit to mrizwanashiq/react that referenced this pull request Jun 25, 2026
* Rename createSyncRoot to createBlockingRoot

* Fix up
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.

5 participants