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

Minify DEV bundles with closure #29809

Merged
merged 1 commit into from
Jun 8, 2024
Merged

Conversation

sebmarkbage
Copy link
Collaborator

The goal is to improve speed of the development by inlining and DCE unused branches.

We have the ability to preserve some variable names and pretty print in the production version so might as well do the same with DEV.

@sebmarkbage sebmarkbage requested a review from acdlite June 7, 2024 20:39
Copy link

vercel bot commented Jun 7, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-compiler-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 7, 2024 9:35pm

@react-sizebot
Copy link

react-sizebot commented Jun 7, 2024

The size diff is too large to display in a single comment. The CircleCI job contains an artifact called 'sizebot-message.md' with the full message.

Generated by 🚫 dangerJS against 433ba94

The goal is to improve speed of the development by inlining and DCE unused
branches.

We have the ability to preserve some variable names and pretty print
in the production version so might as well do the same with DEV.
@@ -43,6 +43,7 @@ function willCoercionThrow(value: mixed): boolean {
}
}

/** @noinline */
Copy link
Collaborator Author

@sebmarkbage sebmarkbage Jun 7, 2024

Choose a reason for hiding this comment

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

This is a fun one. Closure doesn't think '' + can throw so it DCE this whole set of warnings. We'll have to watch out for stuff like that. Avoiding the inlining fixes that.

Luckily this was caught by build tests.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does Closure keep this comment? I remember we had a similar one in warning() but not sure if it’s gone now.

@sebmarkbage sebmarkbage merged commit c2ae9e2 into facebook:main Jun 8, 2024
44 checks passed
github-actions bot pushed a commit that referenced this pull request Jun 8, 2024
The goal is to improve speed of the development by inlining and DCE
unused branches.

We have the ability to preserve some variable names and pretty print in
the production version so might as well do the same with DEV.

DiffTrain build for commit c2ae9e2.
github-actions bot pushed a commit that referenced this pull request Jun 8, 2024
The goal is to improve speed of the development by inlining and DCE
unused branches.

We have the ability to preserve some variable names and pretty print in
the production version so might as well do the same with DEV.

DiffTrain build for [c2ae9e2](c2ae9e2)
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