esm: add import.meta.env

[jkrems]

Ever since Vite introduced it, import.meta.env has been spreading to other tools - and to user code. This leads to some disruptions: E.g. running code that's built using Vite in nodejs-based unit tests is tough because of this missing API.

The import.meta.env API has been added to Vite, bun, and rspack at the very least so far.

[nodejs-github-bot]

Review requested:

• @nodejs/loaders

[ljharb]

I'm very confused here - environment variables simply aren't supposed to be module-specific, since they're for the entire environment. import.meta is only for module-specific things.

[RedYetiDev]

To add on to what @ljharb said, I also don't see a use-case.

If a user wants to access the environment (in ESM or CJS), they have process.env, why can't a user just use that? Why is this alias needed?

[guybedford]

This is only to land this as an experimental feature, I think it could be worth experimenting with, getting wider feedback, and then considering more carefully from there.

That is - landing this should not be considered an endorsement of it unflagging, but rather an endorsement to exploring the cross-platform environment variable design space.

[jkrems]

One thing I'd like to consider for a final API is mutability. E.g. is this a "live" object? Should it be frozen at some point in time? Can you modify it to set environment variables?

But I wanted to start with the most basic version that matches my understanding of the existing versions in other tools.

(And, of course, there's the base question of should this be how env access works in generic code.)

[ljharb]

Landing it flagged and experimental to iterate is always fine ofc! If this lands unflagged tho, then people will start depending on it, "experimental" or no.

[anonrig]

What's the benefit for having yet another way of doing things?

[aduh95]

IIRC the consensus is we should not add things to import.meta before it's listed on the WinterCG registry. wintercg/import-meta-registry#5 was closed without being merged, but IIUC it's because the campion decided to drop the proposal, not because the proposal was rejected, so I guess we could re-submit a similar PR.

Th echo what others have said, it would make little sense for that proposal to come from Node.js when there's already another way to get env variable (and indeed env variable are not really module specific). The mutability aspect is certainly worth discussing, could you elaborate about the use case(s) you have in mind? (maybe we should take that discussion to a separate issue)

[jkrems]

Sent a revised wintercg PR here: wintercg/import-meta-registry#7

The mutability aspect is certainly worth discussing, could you elaborate about the use case(s) you have in mind? (maybe we should take that discussion to a separate issue).

The reason why I'm interested in this addition is, funnily enough, the part that's not about environment variables. I'm looking at the equivalent of the development condition in the exports field but for inline (and easily dead code-detectable) conditionals. And the most established "generic" approach I've found so far is import.meta.env.DEV. But for that to be viable, it would have to actually exist across tools and runtimes and have consistent behavior. Or at least behavior that is consistent enough to have a meaningful common denominator.

There's definitely also other use cases around environmental settings that may only be visible to specific modules but that's something for a more in-depth discussion, e.g. in the context of wintercg maybe? Having a placeholder in nodejs means that some prototype/demo building could happen in nodejs. But it's definitely not a blocker since bun has shipped this already.

[mcollina]

LGTM, this can land behind a flag. I would recommend @jkrems to restart the WinterCG discussions on this.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62914/

[jsumners-nr]

And the most established "generic" approach I've found so far is import.meta.env.DEV.

process.env.NODE_ENV with another name 😒

[jkrems]

process.env.NODE_ENV with another name 😒

The difference is that it's a boolean and always set when the development condition is set in exports. Which isn't true for NODE_ENV which has a history of inconsistent usage. But yes, the evolutionary connection to NODE_ENV is still present in the naming of these. :)

[jsumners]

The difference is that it's a boolean and always set when the development condition is set in exports. Which isn't true for NODE_ENV which has a history of inconsistent usage. But yes, the evolutionary connection to NODE_ENV is still present in the naming of these. :)

Putting my 2¢ out there:

1. I have argued in the past that the project should not block itself on what some other committee does or does not decide. So if the project wants to add this, then it should.
2. As mentioned above, by spec, import.meta is for metadata related to the loaded module. I suppose an argument could be made that the environment the module is running under is metadata for the module, but I'd call that a weak argument at the least.
3. I am completely against codifying any flavor of NODE_ENV. Code should not be dependent on the "type of environment" (prod, dev, etc.) it is running under.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/62921/

[jkrems]

I am completely against codifying any flavor of NODE_ENV. Code should not be dependent on the "type of environment" (prod, dev, etc.) it is running under.

That's a valid philosophy and there's absolutely ways to abuse such a feature. At the same time, it's a common practice to strip debug code during builds and I don't think it's possible to just ignore the constraints that lead to that practice. Code shouldn't depend on the environment it's running - but not at any (size/performance) cost.

I agree that this feature requires more discussion (and hopefully changes to the API shape & semantics) before it could be unflagged. This implementation is meant for experimental use only. I personally don't believe we should add this to node in the end if the semantics really are "just expose env vars".

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.41%. Comparing base (89a2f56) to head (fe3801b).
Report is 32 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55202      +/-   ##
==========================================
+ Coverage   88.39%   88.41%   +0.01%     
==========================================
  Files         652      652              
  Lines      186565   186596      +31     
  Branches    36046    36055       +9     
==========================================
+ Hits       164916   164975      +59     
+ Misses      14908    14888      -20     
+ Partials     6741     6733       -8     

Files with missing lines	Coverage Δ
lib/internal/modules/esm/initialize_import_meta.js	100.00% <100.00%> (ø)
src/node_options.cc	87.42% <100.00%> (+0.01%)	⬆️
src/node_options.h	98.24% <100.00%> (+0.01%)	⬆️

... and 41 files with indirect coverage changes

[mcollina]

I think providing better access to NODE_ENV should really not be a goal. Frankly, NODE_ENV was a mistake and caused many problems down the line.

[jkrems]

I think providing better access to NODE_ENV should really not be a goal. Frankly, NODE_ENV was a mistake and caused many problems down the line.

That sounds like a very surprising way to paraphrase the stated goals. But there is a kernel of truth to it, of course. There is indeed overlap with NODE_ENV even if the actual stated goal is to reduce use of NODE_ENV by providing a viable alternative to some of the use cases.

In the end, it's fair to say that this doesn't need to exist in node. People who want this are likely to already use a build tool. And they already have a solution that they use today: apply that build tool to their code before node sees it.

So as long as the final solution errs on the side of "not dev by default" (which I sure I hope it will, given the hard lessons from NODE_ENV's mistakes), it's as easy as saying "if you want to enable debug assertions in node, use a build tool."

With that in mind, I'll close the PR. Thanks for the feedback!