-
-
Notifications
You must be signed in to change notification settings - Fork 6.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
refactor: simplify build optimizer node_env handling #14829
Conversation
|
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.
Hmm looks like we don't. All tests pass if I set it like this: const define = {
'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV || config.mode),
} I suppose we don't have a lot of test for the build optimizer + lib mode. Let me try to add a test. |
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.
It seems the condition was wrong, this looks good to me too
I tried to add a test (in trace
I'll merge it in for now then. |
Description
We can workaround the change in
optimizer/index.ts
withdefine: { 'process.env.NODE_ENV': 'process.env.NODE_ENV' }
instead. (esbuild example)Also I'm not sure if the check was wrong. The comment mentioned lib builds but we were checking generic builds. I changed the condition to check lib builds instead (same as
define.ts
)Additional context
What is the purpose of this pull request?