Use ESM build in modern browser (<script type="module" ...>)

[mmulet]

Current Behavior

babel-plugin-dev-expression replaces all instances of __DEV__ with process.env.NODE_ENV !== 'production'. As the process variable may not be available outside of nodejs, it introduces an incompatibility with modern browsers.

Suggested solution(s)

There are several different ways we could approach fixing this:

1. Add an option to tsdx.config.js that allows users to opt out of babel-plugin-dev-expression, or replace with a different plugin.
2. Replace babel-plugin-dev-expression with another plugin that works in both browser and nodejs environments. I made a quick fork of babel-plugin-dev-expression that does just that.

Additional context

Affects these issues:
immer
an open issue in
babel-plugin-dev-expression

[agilgur5]

Ok, without the context you added in the immer issue I would have just said "use the UMD build" -- why you'd like to use ESM in a browser should be added to your issue description.

Unfortunately it's currently non-trivial to swap out Babel plugins, even if we move to a partial preset, you can't remove plugins in a preset either 😕

Per the issue in babel-plugin-dev-expression, I don't think that alternative or your fork would get compiled out by a bundler like webpack (I'm not sure), so it wouldn't quite work for both. Would have to think about how to support this without having to redo the ESM compilation. It's possible there's a way to rewrite the alternative so it works for both

[mmulet]

Hmmmm. I guess the simplest way would be to add a polyfill at the start of the esm out.

if(typeof window === "object" && typeof process === "undefined" ){
    window.process = {env: {NODE_ENV: "production"}};
}

[mmulet]

Of course, we probably don't want to mess with the global variables in window.

[mmulet]

Here, we go. Now we define a module scoped variable process, that will mirror node's process in node, but be a polyfill variable in an esm module

var process2 = process
var process = typeof process === "undefined" ? {env: {NODE_ENV: "production"}} : process;

edit: Nevermind. Doesn't work in nodejs

[mmulet]

Now it does work with we shadow the process variable in function scope

//Works in node and in esm modules
(function(process) {
    console.log(process.env.NODE_ENV)
})(
  typeof process === "undefined" ? { env: { NODE_ENV: "production" } } : process
);

But esm modules don't support exporting from inside a function, so you would either have to wrap function definitions or play around with the function prototype. Either way, it's not longer a "simple" solution, so probably isn't worth it.

[agilgur5]

edit: Nevermind. Doesn't work in nodejs

Why doesn't this work in Node? Seems to work ok on my local Node version (v10.16.0). Can also simplify to just process || {env: {NODE_ENV: "production"}}.

Buuut stubbing process may cause unexpected errors further downstream if there were any checks for it etc. process?.env?.NODE_ENV is much safer with that regard

Giving it some thought, I also don't think this solution is optimal for browser ESM support, because the files aren't minified. If we make a production, minified ESM build, that would support this feature.
As a workaround, I think if you run tsdx build --env production --minify --format esm, that's what you'd get. But there's no way to include that in package.json, afaik there is no field specific to browser ESM. See also #140 / #141 / #142 on why TSDX doesn't ship dev/prod ESM bundles (tl;dr you can't do conditional imports like you can with CJS)
(Also --env and --minify aren't officially supported (#508 , #310), I think they just happen to work because the options happen to get passed all the way through.)

[mmulet]

1. I agree, I think a separate production production, minified ESM build would be better than stubbing things in. I'll close this issue.
2. For completion's sake:

//test.js
var process = typeof process === "undefined" ? {env: {NODE_ENV: "production"}} : process;
console.log(process );

node test.js
#Outputs {env: {NODE_ENV: "production"}}

What's happening, is that the process referred to in typeof process is the variable: var process. (This will actually throw an error if you use use let process or const process). So, it actually looks like this due to hoisting:

var process;
process = typeof process === "undefined" ? {env: {NODE_ENV: "production"}} : process;

As you can see, process will always be undefined.
I mistakenly thought it worked in the browser because typeof process == undefined was the desired outcome.
Finally: I did get a solution to work. In both node and browsers.

const globalScope = typeof global === "undefined" ? window : global;
const process = globalScope.process || {env: {NODE_ENV: "production"}};
//or in the future:
const process = globalScope.process ?? {env: {NODE_ENV: "production"}};

[agilgur5]

React is currently having the same issue with conditionally importing dev/prod ESM builds: https://twitter.com/sebmarkbage/status/1250444550197264387

And: https://twitter.com/sebmarkbage/status/1250453658904326144?s=19

And: nodejs/node#32869

• this one is particularly of interest, browser is distinct from dev/prod which sounds potentially problematic

[agilgur5]

The Twitter thread didn't seem to get any new updates but followed the GitHub threads upstream a few times and found:

nodejs/node#32869 was superseded by nodejs/node#33171 was superseded by nodejs/node#34637 and that was merged and backported to Node 12 as well.

The React issue which has has a key comment from Webpack's author/lead maintainer: facebook/react#11503 (comment) . That was written before the above was merged, so I'm not sure what the current state of this is.

If a clear standard emerges and becomes well-tested, I'd like to support that, but it seems like it's still too early to tell. And once it emerges, it'll go through a feedback phase that I think is important before TSDX pushes it to wider adoption, as too early could spur too much breakage or poor feedback.

In the meantime, reduxjs/redux-toolkit#658 created a workaround for this too (but requires manually importing a dist path in the browser due to above) that abuses the system format to achieve this (though arguably, system is meant to represent the browser, so maybe that's not quite abuse).