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

Clean up partial renderer entry points #21682

Merged
merged 2 commits into from Jun 14, 2021

Conversation

sebmarkbage
Copy link
Collaborator

I made a mistake by leaving server.browser.stable in which is the partial renderer for the browser build of stable. That should use the legacy fizz one.

Since the only usage of the partial renderer now is at FB and we don't use it with Node, I removed the Node build of partial renderer too.

@sebmarkbage sebmarkbage requested a review from gaearon June 14, 2021 22:28
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Jun 14, 2021
@sebmarkbage sebmarkbage changed the title [Clean up partial renderer entry points Clean up partial renderer entry points Jun 14, 2021
@sizebot
Copy link

sizebot commented Jun 14, 2021

Comparing: bd45ad0...e4e5909

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 = 127.08 kB 127.08 kB = 40.74 kB 40.74 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 129.89 kB 129.89 kB = 41.67 kB 41.67 kB
facebook-www/ReactDOM-prod.classic.js = 405.00 kB 405.00 kB = 74.92 kB 74.92 kB
facebook-www/ReactDOM-prod.modern.js = 393.35 kB 393.35 kB = 73.10 kB 73.10 kB
facebook-www/ReactDOMForked-prod.classic.js = 405.00 kB 405.00 kB = 74.92 kB 74.92 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +56.83% 20.48 kB 32.12 kB +39.64% 7.60 kB 10.61 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +56.83% 20.48 kB 32.12 kB +39.64% 7.60 kB 10.61 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +56.32% 20.66 kB 32.30 kB +39.96% 7.68 kB 10.75 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +56.32% 20.66 kB 32.30 kB +39.96% 7.68 kB 10.75 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +52.35% 143.24 kB 218.24 kB +38.02% 37.67 kB 51.99 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +52.35% 143.24 kB 218.24 kB +38.02% 37.67 kB 51.99 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +52.01% 150.89 kB 229.36 kB +37.90% 38.15 kB 52.60 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +52.01% 150.89 kB 229.36 kB +37.90% 38.15 kB 52.60 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable-semver/react-dom/cjs/react-dom-server.browser.production.min.js +56.83% 20.48 kB 32.12 kB +39.64% 7.60 kB 10.61 kB
oss-stable/react-dom/cjs/react-dom-server.browser.production.min.js +56.83% 20.48 kB 32.12 kB +39.64% 7.60 kB 10.61 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.production.min.js +56.32% 20.66 kB 32.30 kB +39.96% 7.68 kB 10.75 kB
oss-stable/react-dom/umd/react-dom-server.browser.production.min.js +56.32% 20.66 kB 32.30 kB +39.96% 7.68 kB 10.75 kB
oss-stable-semver/react-dom/cjs/react-dom-server.browser.development.js +52.35% 143.24 kB 218.24 kB +38.02% 37.67 kB 51.99 kB
oss-stable/react-dom/cjs/react-dom-server.browser.development.js +52.35% 143.24 kB 218.24 kB +38.02% 37.67 kB 51.99 kB
oss-stable-semver/react-dom/umd/react-dom-server.browser.development.js +52.01% 150.89 kB 229.36 kB +37.90% 38.15 kB 52.60 kB
oss-stable/react-dom/umd/react-dom-server.browser.development.js +52.01% 150.89 kB 229.36 kB +37.90% 38.15 kB 52.60 kB

Generated by 🚫 dangerJS against e4e5909

I made a mistake by leaving server.browser.stable in which is the partial
renderer for the browser build of stable. That should use the legacy fizz
one.

Since the only usage of the partial renderer now is at FB and we don't use
it with Node, I removed the Node build of partial renderer too.
No code is running this path anymore. Ideally this should be ported to
a Fizz form.
@sebmarkbage
Copy link
Collaborator Author

I deleted the GC tests since nothing runs them now. I think there's some similar cases that could be tested in Fizz since there are places where we need to switch context at the right places. Those are very different tests but should ideally exist.

@sebmarkbage sebmarkbage merged commit e601854 into facebook:master Jun 14, 2021
koto pushed a commit to koto/react that referenced this pull request Jun 15, 2021
* Clean up partial renderer entry points

I made a mistake by leaving server.browser.stable in which is the partial
renderer for the browser build of stable. That should use the legacy fizz
one.

Since the only usage of the partial renderer now is at FB and we don't use
it with Node, I removed the Node build of partial renderer too.

* Remove GC test

No code is running this path anymore. Ideally this should be ported to
a Fizz form.
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.

None yet

4 participants