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

Remove data-reactroot from server rendering and hydration heuristic #20996

Merged
merged 1 commit into from
May 13, 2021

Conversation

sebmarkbage
Copy link
Collaborator

@sebmarkbage sebmarkbage commented Mar 13, 2021

This was used to implicitly hydrate if you call ReactDOM.render.

We've had a warning to explicitly use ReactDOM.hydrate(...) instead of ReactDOM.render(...). We can now remove this from the generated markup. (And avoid adding it to Fizz.)

This is a little strange to do now since we're trying hard to make the root API work the same and especially since this only affects the old api that might be going away anyway.

If we kept it, we'd need to keep it in the generated output which adds unnecessary bytes. It also risks people relying on it, in the Fizz world where as this is an opportunity to create that clean state.

We could possibly only keep it in the old server rendering APIs but then that creates an implicit dependency between which server root API and which client root API that you use together. Currently you can really mix and match either way.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 13, 2021
@sizebot
Copy link

sizebot commented Mar 13, 2021

Comparing: 46491dc...f8c0827

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 126.12 kB 126.00 kB = 40.46 kB 40.41 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 128.94 kB 128.82 kB = 41.40 kB 41.35 kB
facebook-www/ReactDOM-prod.classic.js = 406.40 kB 406.09 kB = 75.19 kB 75.11 kB
facebook-www/ReactDOM-prod.modern.js = 394.46 kB 394.46 kB = 73.29 kB 73.29 kB
facebook-www/ReactDOMForked-prod.classic.js = 406.40 kB 406.09 kB = 75.19 kB 75.11 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactDOMServer-dev.classic.js = 150.58 kB 150.26 kB = 38.63 kB 38.53 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js = 142.38 kB 142.06 kB = 37.64 kB 37.55 kB
oss-stable/react-dom/cjs/react-dom-server.node.development.js = 142.07 kB 141.75 kB = 37.57 kB 37.47 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js = 141.08 kB 140.76 kB = 37.38 kB 37.28 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js = 140.77 kB 140.45 kB = 37.30 kB 37.20 kB
oss-experimental/react-dom/umd/react-dom-server.browser.development.js = 148.72 kB 148.38 kB = 37.83 kB 37.73 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js = 148.37 kB 148.03 kB = 37.75 kB 37.66 kB
oss-experimental/react-dom/umd/react-dom-server.browser.production.min.js = 20.86 kB 20.77 kB = 7.75 kB 7.72 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js = 20.74 kB 20.66 kB = 7.71 kB 7.68 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.min.js = 21.11 kB 21.02 kB = 7.85 kB 7.81 kB
oss-stable/react-dom/cjs/react-dom-server.node.production.min.js = 21.00 kB 20.91 kB = 7.81 kB 7.76 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.min.js = 20.68 kB 20.59 kB = 7.69 kB 7.64 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js = 20.57 kB 20.48 kB = 7.65 kB 7.60 kB
facebook-www/ReactDOMServer-prod.classic.js = 48.53 kB 48.21 kB = 11.30 kB 11.24 kB

Generated by 🚫 dangerJS against f8c0827

Copy link
Collaborator

@gaearon gaearon left a comment

Choose a reason for hiding this comment

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

are we merging this?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Apr 27, 2021

Yea I'm just waiting until the last minute I really need to.

This was used to implicitly hydrate if you call ReactDOM.render.

We've had a warning to explicitly use ReactDOM.hydrate(...) instead of
ReactDOM.render(...). We can now remove this from the generated markup.
(And avoid adding it to Fizz.)

This is a little strange to do now since we're trying hard to make the
root API work the same.

But if we kept it, we'd need to keep it in the generated output which adds
unnecessary bytes. It also risks people relying on it, in the Fizz world
where as this is an opportunity to create that clean state.

We could possibly only keep it in the old server rendering APIs but then
that creates an implicit dependency between which server API and which
client API that you use. Currently you can really mix and match either way.
@sebmarkbage
Copy link
Collaborator Author

I'm merging this now since we landed changes to the stable entry points which are breaking.

@sebmarkbage sebmarkbage merged commit 5890e0e into facebook:master May 13, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request May 25, 2021
Summary:
This sync includes the following changes:
- **[316943091](facebook/react@316943091 )**: Make StrictMode double rendering flag static for FB/www ([#21517](facebook/react#21517)) //<Brian Vaughn>//
- **[e0f89aa05](facebook/react@e0f89aa05 )**: Clean up Scheduler forks ([#20915](facebook/react#20915)) //<Ricky>//
- **[5890e0e69](facebook/react@5890e0e69 )**: Remove data-reactroot from server rendering and hydration heuristic ([#20996](facebook/react#20996)) //<Sebastian Markbåge>//
- **[46491dce9](facebook/react@46491dce9 )**: [Bugfix] Prevent already-committed setState callback from firing again during a rebase ([#21498](facebook/react#21498)) //<Andrew Clark>//
- **[b770f7500](facebook/react@b770f7500 )**: lint-build: Infer format from artifact filename ([#21489](facebook/react#21489)) //<Andrew Clark>//
- **[2bf4805e4](facebook/react@2bf4805e4 )**: Update entry point exports ([#21488](facebook/react#21488)) //<Brian Vaughn>//

Changelog:
[General][Changed] - React Native sync for revisions b8fda6c...ebcec3c

jest_e2e[run_all_tests]

Reviewed By: JoshuaGross

Differential Revision: D28572047

fbshipit-source-id: eb09c0358edb7fbf241333ea9c08724748906fea
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
…acebook#20996)

This was used to implicitly hydrate if you call ReactDOM.render.

We've had a warning to explicitly use ReactDOM.hydrate(...) instead of
ReactDOM.render(...). We can now remove this from the generated markup.
(And avoid adding it to Fizz.)

This is a little strange to do now since we're trying hard to make the
root API work the same.

But if we kept it, we'd need to keep it in the generated output which adds
unnecessary bytes. It also risks people relying on it, in the Fizz world
where as this is an opportunity to create that clean state.

We could possibly only keep it in the old server rendering APIs but then
that creates an implicit dependency between which server API and which
client API that you use. Currently you can really mix and match either way.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants