define process.env.NODE_ENV works differently

[sapphi-red]

process.env.NODE_ENV = 'test'
const foo = process.env.NODE_ENV

foo.bar = 'foo bar'
const bar = foo.bar

is transformed to

"node_env" = "test"; // process.env.NODE_ENV is replaced even if it's a lvalue
const foo = "node_env";
foo.bar = "foo bar"; // foo.bar is not replaced
const bar = foo.bar;

with --define:process.env.NODE_ENV=\"node_env\" --define:foo.bar=\"foo_bar\" (repl).

The generated code is not a valid code. A string cannot be a lvalue.

This happens with 0.11.23, 0.12.29, 0.13.15, 0.14.54, 0.15.18, 0.16.1.

[evanw]

I just looked into this. There isn't anything special about process.env.NODE_ENV here. In your example the define for foo.bar doesn't apply because the define feature only replaces global variable references and foo is a local variable in this case. If you add let process to the code then process.env.NODE_ENV won't be replaced either, since then process will also be a local variable.

[sapphi-red]

Oh, you are right. There aren't anything special with process.env.NODE_ENV. It's special when it includes a ..

foo_bar = 'foo bar'
const bar = foo_bar

is transformed to

foo_bar = "foo bar";
const bar = "foo_bar";

[evanw]

I discovered that Webpack's DefinePlugin has the same behavior here (being able to replace an assignment target with a non-identifier). I can change esbuild to avoid generating code with a syntax error by avoiding replacements for assignment targets, but the result still won't make much sense since it means the load will be substituted but the store won't be.

Edit: Maybe that means it's a good candidate for a warning. I should probably add one for this case (define for something in assignment position).

[sapphi-red]

I agree the result won't make sense. This is something I found while digging around #2718.
I think it's nice to have a warning.