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

Revert "Fix:- Improve HOC support and state preservation in React Refresh" #32214

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gaearon
Copy link
Collaborator

@gaearon gaearon commented Jan 24, 2025

Reverts #30660

I don’t feel confident in the approach. This part of code is supposed to rely on the module bundler behaving as expected. Maybe this is correct but I need to review it closer — it was intentionally not implemented this way originally.

I’ll try to take a closer look some time this week. We don’t have to merge this revert right now but just flagging that I don’t understand the thinking behind the new approach and don’t have confidence in it.

@react-sizebot
Copy link

Comparing: b65afdd...e5d6858

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.js = 6.68 kB 6.68 kB +0.05% 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js = 514.24 kB 514.24 kB = 91.74 kB 91.73 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB +0.05% 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js = 557.28 kB 557.28 kB = 98.97 kB 98.97 kB
facebook-www/ReactDOM-prod.classic.js = 595.79 kB 595.79 kB = 104.85 kB 104.85 kB
facebook-www/ReactDOM-prod.modern.js = 586.21 kB 586.21 kB = 103.30 kB 103.30 kB
facebook-www/ReactFreshRuntime-dev.classic.js = 13.82 kB 12.37 kB = 3.19 kB 2.99 kB
facebook-www/ReactFreshRuntime-dev.modern.js = 13.82 kB 12.37 kB = 3.19 kB 2.99 kB
oss-experimental/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB
oss-stable-semver/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB
oss-stable/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
facebook-www/ReactFreshRuntime-dev.classic.js = 13.82 kB 12.37 kB = 3.19 kB 2.99 kB
facebook-www/ReactFreshRuntime-dev.modern.js = 13.82 kB 12.37 kB = 3.19 kB 2.99 kB
oss-experimental/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB
oss-stable-semver/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB
oss-stable/react-refresh/cjs/react-refresh-runtime.development.js = 13.80 kB 12.36 kB = 3.18 kB 2.98 kB

Generated by 🚫 dangerJS against 76a1e72

Copy link
Member

@rickhanlonii rickhanlonii left a comment

Choose a reason for hiding this comment

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

sgtm

@Biki-das
Copy link
Contributor

thanks @gaearon for the follow up, happy to collaborate if we are looking forward to revert the changes and understand if its necessary!

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.

5 participants