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

Combined two if statements that do the same things in many places in the ReactFreshRuntime.js file #28757

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

WellHiIGuess
Copy link

@WellHiIGuess WellHiIGuess commented Apr 5, 2024

Summary

Combined an if statement in the performReactRefresh function, because they both did the same thing.

For example:

if (pendingUpdates.length === 0) {
  return null;
}
if (isPerformingRefresh) {
  return null;
}

to:

if (pendingUpdates.length === 0 || isPerformingRefresh) {
  return null;
}

This simplifies the code.

How did you test this change?

Both of these if statements return null;. Combining them and adding an || does the same thing.

@WellHiIGuess
Copy link
Author

Note: I had to make this pull request again because I had not signed the license before.

@react-sizebot
Copy link

react-sizebot commented Apr 5, 2024

Comparing: 2acfb7b...8279cbf

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 = 168.90 kB 168.90 kB = 52.69 kB 52.69 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 170.71 kB 170.71 kB = 53.24 kB 53.24 kB
facebook-www/ReactDOM-prod.classic.js = 590.23 kB 590.23 kB = 103.62 kB 103.62 kB
facebook-www/ReactDOM-prod.modern.js = 566.56 kB 566.56 kB = 99.57 kB 99.57 kB
test_utils/ReactAllWarnings.js Deleted 64.04 kB 0.00 kB Deleted 16.02 kB 0.00 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 = 22.92 kB 22.86 kB +0.11% 6.12 kB 6.12 kB
facebook-www/ReactFreshRuntime-dev.modern.js = 22.92 kB 22.86 kB +0.11% 6.12 kB 6.12 kB
oss-experimental/react-refresh/cjs/react-refresh-runtime.development.js = 20.38 kB 20.29 kB = 5.97 kB 5.97 kB
oss-stable-semver/react-refresh/cjs/react-refresh-runtime.development.js = 20.38 kB 20.29 kB = 5.97 kB 5.97 kB
oss-stable/react-refresh/cjs/react-refresh-runtime.development.js = 20.38 kB 20.29 kB = 5.97 kB 5.97 kB
test_utils/ReactAllWarnings.js Deleted 64.04 kB 0.00 kB Deleted 16.02 kB 0.00 kB

Generated by 🚫 dangerJS against 8279cbf

@RedYetiDev
Copy link

I'm looking through the file, and this redundant behavior that @WellHiIGuess is attempting to remove is present in many places, maybe implementing this throughout the file might be helpful? It could also reduce the bundle size?

@WellHiIGuess
Copy link
Author

I'm looking through the file, and this redundant behavior that @WellHiIGuess is attempting to remove is present in many places, maybe implementing this throughout the file might be helpful? It could also reduce the bundle size?

You're right I'll make a commit to remove all of these

@WellHiIGuess WellHiIGuess changed the title Combined two if statements that do the same thing in function Combined two if statements that do the same things in many places in the ReactFreshRuntime.js file Apr 5, 2024
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

4 participants