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

Change to process?.env?.NODE_ENV? #18

Open
FezVrasta opened this issue Jan 13, 2020 · 6 comments
Open

Change to process?.env?.NODE_ENV? #18

FezVrasta opened this issue Jan 13, 2020 · 6 comments

Comments

@FezVrasta
Copy link

In order to make the ESM builds work on modern browsers, without additional bundlers involved, would you consider change the way process.env.NODE_ENV is accessed to gracefully handle a missing process?

process?.env?.NODE_ENV or maybe process?.env.NODE_ENV should be enough.

@taion
Copy link
Contributor

taion commented Jan 13, 2020

Wouldn't that defeat the point, though? The idea is for the check to be compiled out, such that we end up with the code removed through DCE. If process.env.NODE_ENV isn't getting compiled out, then you're better off not using this plugin.

@mmulet
Copy link

mmulet commented Mar 21, 2020

I'm running into this issue as well, but I agree with @taion. I think this may be better solved with a different, but similar plugin.

@mmulet
Copy link

mmulet commented Mar 21, 2020

Made a fork for it: babel-plugin-web-dev-expression

@taion
Copy link
Contributor

taion commented Mar 21, 2020

Well, hold on a sec. I feel like I'm missing something here.

If you look at e.g., the point of something like https://github.com/4Catalyzer/babel-plugin-dev-expression#invariant is to result in better production code after going through a minifier that does dead code elimination.

If you're distributing code as-is, then it seems like you wouldn't want this.

It seems a little odd to run code that has gone through this transform in the browser without an additional DCE step, but I'm probably just missing something?

@mmulet
Copy link

mmulet commented Mar 21, 2020 via email

@agilgur5
Copy link

Yea I'll agree with @taion that the browser ESM use-case should use a production minified bundle (like I mentioned in jaredpalmer/tsdx#631 (comment)), which would have process.env.NODE_ENV replaced and then DCE'd. The browser ESM use-case is quite different from the Node/bundler use-case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants