nodejs / TSC

The Node.js Technical Steering Committee
591 stars 134 forks source link

Propose vote to unblock `--env-file` PRs #1577

Closed aduh95 closed 3 months ago

aduh95 commented 3 months ago

Refs: https://github.com/nodejs/TSC/issues/1575#issuecomment-2166976221

aduh95 commented 3 months ago

/cc @anonrig @BoscoDomingo as the authors of the blocked PRs, in case you want to review the proposed options

benjamingr commented 3 months ago

Note none of the candidates are the behavior userland libraries have and we're effectively "dictating by committee" an API that's different from the one the ecosystem gravitated towards which is "ignore missing env file, warn if debug:true is passed".

For example here is dotenv with this behavior, python (and when they removed the warning at https://github.com/theskumar/python-dotenv/pull/57 ) and go.

I am happy to provide more examples (this is the behavior in every package I looked at).

I really dislike the project landing a feature it's adopting because it reached consensus in userland and then changing its behavior by committee even though all userland libraries (at least popular ones I found) have the same behavior (which is different from what we're proposing).

anonrig commented 3 months ago

I really dislike the project landing a feature it's adopting because it reached consensus in userland and then changing its behavior by committee even though all userland libraries (at least popular ones I found) have the same behavior (which is different from what we're proposing).

Note: I agree with @benjamingr 100%.

BoscoDomingo commented 3 months ago

I really dislike the project landing a feature it's adopting because it reached consensus in userland and then changing its behavior by committee even though all userland libraries (at least popular ones I found) have the same behavior (which is different from what we're proposing).

Note: I agree with @benjamingr 100%.

I agree as well, we just each approached the problem from a different angle. In the end we both want missing .env files to not throw, I just made it a decision to be taken by the user, whereas you prefer sticking to other tools' existing behaviour, and that's 100% fine too đŸ‘đŸŒ

Also, the proposal LGTM (save a small change in the title), thanks @aduh95

benjamingr commented 3 months ago

Basically my stance isn’t warn, ignore or error - it’s “do whatever userland does”.

GeoffreyBooth commented 3 months ago

Basically my stance isn’t warn, ignore or error - it’s “do whatever userland does”.

According to what you’ve written, though, dotenv has a default behavior that can be overridden by its config file. We don’t have config file support, though, so at the moment we can’t really mimic dotenv‘s behavior, right?

I guess that could be another option, though: add support for a general Node config file, and then --env-file has the warn behavior by default and it can be set to either throw or be silent via the config file. That would exactly match dotenv, I think?

benjamingr commented 3 months ago

@GeoffreyBooth the equivalent would be "do not warn on missing .env file, add an option to process.loadEnvFile to warn/throw on a missing env file

GeoffreyBooth commented 3 months ago

the equivalent would be “do not warn on missing .env file, add an option to process.loadEnvFile to warn/throw on a missing env file

That’s not equivalent to --env-file-optional or --env-file-required because by the time you can run process.loadEnvFile or util.parseEnv, Node is already running and therefore any NODE_OPTIONS within the .env file has no effect. Part of the existing use case for --env-file is as a way to define Node configuration via NODE_OPTIONS.

benjamingr commented 3 months ago

That’s not equivalent to --env-file-optional or --env-file-required because by the time you can run process.loadEnvFile or util.parseEnv, Node is already running and therefore any NODE_OPTIONS within the .env file has no effect.

Correct, I was only explaining what userland does and was suggesting we keep the same existing behavior that reached consensus in our ecosystem over many years rather than change it based on how a technical committee believes things should work.