-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
fix(remix-dev): do more tree-shaking in dev mode #3588
Conversation
The type of dead code elimination we want to do depends on the minify syntax property: evanw/esbuild#672 (comment). Without this we have dev builds that run fine in production, but ship server code / dependencies to the browser, blowing it up due to not optimizing out imports and module level blocks like `if (process.env.NODE_ENV === "test") {`. This will fix the issue seen by those trying to integrate react-three-fiber into their app, as well as enable a good vitest in-source testing story.
packages/remix-dev/compiler.ts
Outdated
@@ -442,6 +442,12 @@ function createServerBuild( | |||
format: config.serverModuleFormat, | |||
treeShaking: true, | |||
minify: options.mode === BuildMode.Production && isCloudflareRuntime, | |||
// The type of dead code elimination we want to do depends on the | |||
// minify syntax property: https://github.com/evanw/esbuild/issues/672#issuecomment-1029682369 | |||
// without this we have dev builds that run fine in production, but |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev builds that run fine in production
Did you mean "dev builds that run fine, but in production ship server code ..."?
If not, could you elaborate on what you mean by "dev builds ... in production"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bad wording. Dev builds are leaving code that should be optimized away in the bundle causing server / testing code to be shipped to the browser. These are properly optimized away in prod builds today, and this PR makes dev mode behave closer to production in terms of dead code elimination / tree shaking is concerned.
@jacob-ebey Do you have any quick and easy test cases that we could run locally to see the difference here? Something hopefully less involved than making a |
@brophdawg11 Throw a |
🤖 Hello there, We just published version Thanks! |
The type of dead code elimination we want to do depends on the minify syntax property: evanw/esbuild#672 (comment). Without this we have dev builds that run fine in production, but ship server code / dependencies to the browser, blowing it up due to not optimizing out imports and module level blocks like
if (process.env.NODE_ENV === "test") {
.This will
fix the issue seen by those trying to integrate react-three-fiber into their app, as well asenable a good vitest in-source testing story.~Edit: I remembered wrong on the react-three-fiber issue, that is the
minifyIdentifiers
flag that fixes the duplicate React identifiers.minifyIdentifiers
isn't something we want to enable in dev right now.Closes: #
Testing Strategy:
This is covered by existing integration tests.