src: add `--env-file-if-exists` flag #53060

BoscoDomingo · 2024-05-19T18:43:33Z

Add a new flag, --env-file-if-exists that allows loading .env files without throwing an error if they don't exist.

Achieved by returning an env_file_data struct instead of just the paths of the .env files. This tells the loader if the file itself is required or not, thus allowing to not throw an error if the file doesn't exist and is optional, without breaking existing behaviours with --env-file (we could maybe show a warning if an optional file is missing?).

Usage:

node --env-file A.env --env-file-if-exists B.env

If B.env exists, it will load its environment variables and overwrite those that conflict with A.env. If it doesn't, node will continue as normal.

Fixes: #50993
Refs: #51451

nodejs-github-bot · 2024-05-19T18:43:38Z

Review requested:

@nodejs/startup

anonrig

I think we shouldn't introduce a new flag. We should make the default --env-file not throw and revert the change.

BoscoDomingo · 2024-05-19T18:50:31Z

I think we shouldn't introduce a new flag. We should make the default --env-file not throw and revert the change.

@anonrig Are you sure? I am personally fine with #50588, and at this stage, we might be breaking people's CI processes and whatnot by allowing processes to run that will be missing environment variables. Multiple people in #49148 agree with this stance, too. I'd like to understand your reasoning

anonrig · 2024-05-19T18:54:15Z

@anonrig Are you sure? I am personally fine with #50588, and at this stage, we might be breaking people's CI processes and whatnot by allowing processes to run that will be missing environment variables. Multiple people in #49148 agree with this stance, too. I'd like to understand your reasoning

This is an "active development" non stable feature. therefore, breaking changes are expected.

IMHO, Instead of throwing an error, we can print a warning. I don't see why we should create a new flag rather than changing the current one.

BoscoDomingo · 2024-05-19T20:00:39Z

@anonrig Are you sure? I am personally fine with #50588, and at this stage, we might be breaking people's CI processes and whatnot by allowing processes to run that will be missing environment variables. Multiple people in #49148 agree with this stance, too. I'd like to understand your reasoning

This is an "active development" non stable feature. therefore, breaking changes are expected.

IMHO, Instead of throwing an error, we can print a warning. I don't see why we should create a new flag rather than changing the current one.

@anonrig I do see value in having a "hard" and a "soft" version, to allow users to decide what behaviour they desire. Plus the underlying implementation will be the same and thus barely any complexity is added while greatly improving UX/DX.

In my case, there are multiple instances where I'd want the node process to be aborted if the .env file is not present.

Is there any team we should tag to get their input on this as well?

anonrig · 2024-05-19T20:21:37Z

Is there any team we should tag to get their input on this as well?

There is no specific team responsible for dotenv. Originally, I implemented the dotenv parser and but we can always ask @nodejs/tsc for their opinion.

I think #50588 was a mistake. @mcollina and several others have expressed their opinion on this as well, but it never hurts to ask for their opinion one more time :-)

FYI, I didn't block this pull-request, any collaborator can come in and approve your pull-request, and land this pull-request.

BoscoDomingo · 2024-05-19T22:51:41Z

Is there any team we should tag to get their input on this as well?

There is no specific team responsible for dotenv. Originally, I implemented the dotenv parser and but we can always ask @nodejs/tsc for their opinion.

I think #50588 was a mistake. @mcollina and several others have expressed their opinion on this as well, but it never hurts to ask for their opinion one more time :-)

FYI, I didn't block this pull-request, any collaborator can come in and approve your pull-request, and land this pull-request.

Agree on asking others. I'm not an expert on this matter either, so happy to hear other POVs. I do err on the side of keeping --env-file as is for now, though

GeoffreyBooth · 2024-05-20T03:27:18Z

If we add this, the flag name needs to start with env-file like the others.

I feel pretty strongly that the current behavior is correct. We should not be changing --env-file. I'm also against adding this new flag but I feel less strongly about that if there's a reasonable need for it. I don't consider "it's annoying to make my command understand the difference between dev and prod" a persuasive enough reason.

mbrevda · 2024-05-20T05:53:16Z

I'm also against adding this new flag

There seems to be a consensus that:

No new flags should be added
There should be a mechanism to signal not to error if the env file isn't found

Is there any precedent for loader-style arguments? Perhaps prefixing (or suffixing) the path with a symbol (say, ?) can meet all the above requirements?

E.g. --env-file=?.optional.env

mcollina

lgtm

BoscoDomingo · 2024-05-20T08:09:58Z

@GeoffreyBooth Fair enough. You can see in #51451 the different workarounds we currently need to do. I believe for this to compete with dotenv, bun or other alternatives out there, the UX has to be at least matching, if not better. They all currently march on when the .env file is not found. We can do better.

In terms of changing the flag, good on my side 👍🏼

@mbrevda I feel like that's too much complexity both in use (I'd never seen such syntax) as in implementation (what happens with --env-file? path/to/file). It'd be a first AFAIK and I'm not sure such a small flag warrants such a divergence from the norm, but again, open to change my mind!

BoscoDomingo · 2024-05-21T22:15:35Z

Updated code according to the failed CI jobs. Should all be good now

@GeoffreyBooth Updated flag to start with --env-file

anonrig

I'm -1 on this change. It makes the existing implementation harder to maintain for almost no gain. We should not throw on env-file instead.

GeoffreyBooth · 2024-05-21T23:29:47Z

I will block on changing --env-file. We should not be silently ignoring failing to load a specified file, even if other tools do.

I'm also fine with no solution landing for this problem. I'm not convinced this is something that Node should solve, as opposed to users better managing their commands for production versus development.

anonrig · 2024-05-21T23:35:25Z

I will block on changing --env-file. We should not be silently ignoring failing to load a specified file, even if other tools do.

We can always convert it into a warning.

BoscoDomingo · 2024-05-22T10:59:10Z

I agree with @GeoffreyBooth on this one. I'd much rather Node doesn't allow the program to run than to start it with the wrong assumptions.

In my experience, startup warnings are rarely read in production environments, and usually not even properly marked as such by log services. It's far worse to let systems crash in prod due to missing files than to not start them to begin with.

However, the pain for users having to manage multiple scripts for whether they want to load x file or not will be a major pain (imagine all the possible combinations) when it's an easy fix for us. Therein lies the value IMHO.

Edit: @anonrig I like the idea of a warning for missing optional files

lewismoten · 2024-05-27T19:39:22Z

I would opt to have the flag not fail when the file is missing, have an option to say that the file is optional, or simply detect that a .env file exists without specifying a flag. With .env files, the industry standard is to proceed when its missing because environment variables may already exist. The main issue I see is with build servers that don't have the .env file. You end up adding extra steps in your build to create the file with each build in order to support projects that use it when environment variables are already available on the build server.

Shudrum · 2024-05-27T20:07:03Z

Hello there.

Basically, both of the solutions are going to achieve the same result. But I would also prefer to have the flag to not fail.

I think every people using .env files are currently using motdotla/dotenv, and its behavior is to not fail. Like said @lewismoten it's kind of an industry standard. The reasons I think of are:

It is strongly discouraged to commit .env files. So should not be mandatory.
It's named "environment" because well ... it should depends of the environment. And it should allow developpers to set their environments locally.
Many teams use environments to define default values for their applications, having a default one commited to their repositories. Having it missing is not frequent, and should be tested because...
...they frequently defines empty values, for the database for example. Throwing an error will not ensure of a proper configuration.
Having two dot files for overriding is frequent, making one of both optionnal.
I don't have the real statistics, but I'm sure at least 80% of deployed applications have their productions values defined from the running environment, like with Kube, AWS Fargate / ECS, GCP Run, Heroku, etc...
Secrets are rarely wrote into an env file.

I know it is still possible to use dot files for, maybe, all my examples, but with more boilerplates.

Personnaly, I never used dot env files outside of local developments. With this solution, for example, I cannot run the tests with the same npm script on my CI and locally. That's why I stay with dotenv.

I love to minimize libraries on my microservices, I really hope it will stop to throw.

BTW thank you for the contribution and all my thanks to the team!

BoscoDomingo · 2024-06-02T16:47:50Z

Just for reference: this issue will be discussed in the next TSC meeting since a consensus has not been reached. More info in the counterpart PR by @anonrig

Thank you all for the input, and we'll see how this one goes!

GeoffreyBooth · 2024-06-03T22:20:05Z

I assume the intended usage of this PR is:

node --env-file=maybe-exists.env --env-file-optional app.js

What if instead it’s:

node --env-file-optional=maybe-exists.env app.js

And if both --env-file and --env-file-optional are passed, Node throws an error that they can’t be used together.

BoscoDomingo · 2024-06-05T13:29:18Z

I assume the intended usage of this PR is:
node --env-file=maybe-exists.env --env-file-optional app.js
What if instead it’s:
node --env-file-optional=maybe-exists.env app.js
And if both --env-file and --env-file-optional are passed, Node throws an error that they can’t be used together.

@GeoffreyBooth No, the idea is to have --env-file-optional behave exactly like --env-file, but not throw an error when the file isn't found. E.g.

node --env-file this_exists.env --env-file-optional this_may_not.env

The flag was --optional-env-file originally, but I was asked to change it (so both show up together in the docs and to maintain consistency I assume)

aduh95 · 2024-06-05T14:26:31Z

I would be in favor of either non-boolean flag (e.g. --missing-env-file=throw), or a more explicit one (e.g. --throw-on-missing-env-file); my idea is that it would be good to have a flag to which we can flip the default while keeping it readable (because --no-env-file-optional is not very clear, --no-throw-on-missing-env-file is much better, and --missing-env-file=warn is shorter).
(I don't know if we are going to flip the default, but it's good to keep that option open)

BoscoDomingo · 2024-06-06T07:45:11Z

I would be in favor of either non-boolean flag (e.g. --missing-env-file=throw), or a more explicit one (e.g. --throw-on-missing-env-file); my idea is that it would be good to have a flag to which we can flip the default while keeping it readable (because --no-env-file-optional is not very clear, --no-throw-on-missing-env-file is much better, and --missing-env-file=warn is shorter). (I don't know if we are going to flip the default, but it's good to keep that option open)

@aduh95 I personally am not a big fan of flags that affect others, plus it doesn't really solve any of the issues why I proposed this change to begin with. One would still need multiple scripts for files that may or may not exist, and need to be aware of the context in which each is run to decide if the file will exist.

doc/api/cli.md

tniessen · 2024-06-08T15:41:15Z

src/node.cc

        case Dotenv::ParseResult::FileError:
-          errors->push_back(file_path + ": not found");
+          if (!file_data.is_required) continue;
+          errors->push_back(file_data.path + ": not found");


While this is a pre-existing bug, this really should not print not found if the file could not be opened due to an error other than ENOENT. (Unfortunately, ParsePath() just returns a generic FileError value regardless of the error code.)

Good catch. How'd you suggest we handle this, simply not print anything? Improving the check as to why it errored in the first place? I do believe it should still error if the flag used was --env-file

I think it's fine to leave it as it is for this PR since it's a pre-existing bug. Eventually, the code should check if the error code was ENOENT, and if it's not that, then print the appropriate error message for the error code.

mcollina

I'm -1 too.

Dismissing per the result of the related TSC vote https://github.com/nodejs/TSC/blob/main/votes/2024-06-25-0.json

aduh95 · 2024-06-27T12:50:13Z

@BoscoDomingo can you make the flag emit a warning when the file doesn't exist? Also consider renaming the flag to --env-file-if-exists which seems to have consensus.

BoscoDomingo · 2024-06-27T13:35:24Z

@BoscoDomingo can you make the flag emit a warning when the file doesn't exist? Also consider renaming the flag to --env-file-if-exists which seems to have consensus.

Sure thing. I'll take care of it this weekend 👍🏼

anonrig · 2024-06-04T17:57:31Z

src/node_dotenv.h

@@ -13,6 +13,10 @@ namespace node {
 class Dotenv {
 public:
  enum ParseResult { Valid, FileError, InvalidContent };
+  struct env_file_data {
+    std::string path;
+    bool is_required;


I believe this should be the other way around:

Suggested change

bool is_required;

bool is_optional;

anonrig · 2024-06-04T17:59:49Z

src/node_dotenv.cc

-      paths.push_back(*next_path);
+      struct env_file_data env_file_data = {
+        *file_path,
+        strncmp(matched_arg->c_str(),


anonrig · 2024-06-27T14:36:46Z

src/node_dotenv.cc

-    return strncmp(arg.c_str(), flag.data(), flag.size()) == 0;
+    const std::string_view env_file_flag = "--env-file";
+    const std::string_view optional_env_file_flag = "--env-file-optional";
+    return strncmp(arg.c_str(), env_file_flag.data(), env_file_flag.size()) ==


All strncmp calls can be replaced with arg.start_with()

anonrig · 2024-06-27T14:37:22Z

src/node_dotenv.cc

+      auto flag = matched_arg->substr(0, equal_char);
+      struct env_file_data env_file_data = {
+          matched_arg->substr(equal_char + 1),
+          strncmp(flag.c_str(),


This can be replaced with starts_with()

anonrig · 2024-06-27T14:38:31Z

src/node_dotenv.h

@@ -13,6 +13,10 @@ namespace node {
 class Dotenv {
 public:
  enum ParseResult { Valid, FileError, InvalidContent };
+  struct env_file_data {
+    std::string path;


No need to make an additional copy, let's use std::string_view and trust the lifecycle of the owned string:

Suggested change

std::string path;

std::string_view path;

anonrig · 2024-06-27T14:39:17Z

src/node_dotenv.h

@@ -27,7 +31,7 @@ class Dotenv {
  void SetEnvironment(Environment* env);
  v8::Local<v8::Object> ToObject(Environment* env) const;

-  static std::vector<std::string> GetPathFromArgs(
+  static std::vector<env_file_data> GetEnvFileDataFromArgs(


We are inside Dotenv class, we don't need EnvFile naming in here:

Dotenv::GetEnvFileDataFromArgs()
Dotenv::GetDataFromArgs()

Suggested change

static std::vector<env_file_data> GetEnvFileDataFromArgs(

static std::vector<env_file_data> GetDataFromArgs(

anonrig · 2024-06-27T14:39:44Z

src/node_options.cc

@@ -616,6 +616,11 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
            "set environment variables from supplied file",
            &EnvironmentOptions::env_file);
  Implies("--env-file", "[has_env_file_string]");
+  AddOption("--env-file-optional",
+            "set environment variables from supplied file, "
+            "but won't fail if the file doesn't exist",


We don't need this extra description in here.

Suggested change

"but won't fail if the file doesn't exist",

test/parallel/test-dotenv-edge-cases.js

BoscoDomingo · 2024-06-28T11:48:31Z

@anonrig Thanks for the tips! Not used to C++ so I'll implement the changes requested

Fixes: nodejs#50993 Refs: nodejs#51451

aduh95 · 2024-06-30T13:17:52Z

doc/api/cli.md

@@ -1692,6 +1697,11 @@ is being linked to Node.js. Sharing the OpenSSL configuration may have unwanted
 implications and it is recommended to use a configuration section specific to
 Node.js which is `nodejs_conf` and is default when this option is not used.

+### `--env-file-optional=config`


IMO --env-file-if-exists is a clearer name.

Suggested change

### `--env-file-optional=config`

### `--env-file-if-exists=config`

Yup, currently working on it. Haven't pushed changes yet, only updated branch with upstream changes 👍🏼

BoscoDomingo · 2024-06-30T13:54:34Z

@aduh95 Not sure how to make the flag return a warning. When the file options are processed (InitializeNodeWithArgsInternal) no warnings are available, only errors, which if there are any, process is stopped.

Is there a way to do this without adding an extra parameter to InitializeNodeWithArgsInternal?

Edit: Wait, am I that dumb and all it takes is a simple fprintf()?

BoscoDomingo · 2024-06-30T18:06:35Z

Stuck on this:

I've changed the path to be a string_view, but I can't seem to get it to correctly print out. Using c_str() yields weird results. However not using it doesn't compile.
Should I just use std::string and call it a day? Let someone else do the string_view handling (I'm extremely inexperienced with C++, so this is 100% skill issue)

anonrig · 2024-06-30T18:36:46Z

@BoscoDomingo You should use file_data.path.data() or std::string(file_data.path).c_str() if you want to copy it.

BoscoDomingo · 2024-06-30T21:52:49Z

@BoscoDomingo You should use file_data.path.data() or std::string(file_data.path).c_str() if you want to copy it.

@anonrig That's what I've been using to no avail, hence why it was so surprising to me. I'll give it a go this week if I have time, but I'm going on a 2-week vacation where I won't have access to a machine to develop, so this may be stuck for a bit until I'm back.

The path printed includes a bunch of weird characters I'm not sure where they come from:
/home/bosco/repos/node/out/Release/node: #٠��U\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00nv/optional.env\x00env: not found\n

Keeping it as a string works, so I'm pushing that for now 👍🏼

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 19, 2024

BoscoDomingo mentioned this pull request May 19, 2024

.env file support issue tracker #49148

Open

8 tasks

anonrig reviewed May 19, 2024

View reviewed changes

mcollina approved these changes May 20, 2024

View reviewed changes

BoscoDomingo changed the title ~~src: add --optional-env-file flag~~ May 21, 2024

anonrig previously requested changes May 21, 2024

View reviewed changes

anonrig mentioned this pull request May 27, 2024

src: make --env-file return warning when not found #53177

Closed

BoscoDomingo commented Jun 6, 2024

View reviewed changes

doc/api/cli.md Outdated Show resolved Hide resolved

tniessen reviewed Jun 8, 2024

View reviewed changes

BoscoDomingo mentioned this pull request Jun 8, 2024

Add --env-file-if-exists #50993

Open

GeoffreyBooth mentioned this pull request Jun 10, 2024

src: add support to override local env variables option #52531

Open

2 tasks

mcollina previously requested changes Jun 11, 2024

View reviewed changes

aduh95 mentioned this pull request Jun 14, 2024

Propose vote to unblock --env-file PRs nodejs/TSC#1577

Merged

GeoffreyBooth mentioned this pull request Jun 14, 2024

Node.js Technical Steering Committee (TSC) Meeting 2024-06-12 nodejs/TSC#1575

Closed

anonrig reviewed Jun 27, 2024

View reviewed changes

BoscoDomingo added 6 commits June 30, 2024 13:04

src: add --optional-env-file flag

144d191

Fixes: nodejs#50993 Refs: nodejs#51451

test: remove unnecessary comment

ce5d236

src: conform to style guidelines

1d4ef3c

src: change flag to --env-file-optional

6533679

test: revert automatic linter changes

56627e1

doc: fix typos

410ce1e

BoscoDomingo force-pushed the feature/optional-env-file-argument branch from c44968b to 410ce1e Compare June 30, 2024 11:06

aduh95 reviewed Jun 30, 2024

View reviewed changes

src: change flag to --env-file-if-exists

da6d40d

BoscoDomingo added 4 commits July 1, 2024 00:25

src: refactor env_file_data and GetEnvFileDataFromArgs

edc32a8

test: clean up tests

6043da8

src: print error when file not found

e89f72e

test: remove unnecessary extras

8302a7c

BoscoDomingo changed the title ~~src: add --env-file-optional flag~~ Jul 1, 2024

	static std::vector<env_file_data> GetEnvFileDataFromArgs(
	static std::vector<env_file_data> GetDataFromArgs(

	### `--env-file-optional=config`
	### `--env-file-if-exists=config`

src: add --env-file-if-exists flag #53060

Are you sure you want to change the base?

src: add --env-file-if-exists flag #53060

Conversation

BoscoDomingo commented May 19, 2024 • edited Loading

nodejs-github-bot commented May 19, 2024

anonrig left a comment

Choose a reason for hiding this comment

BoscoDomingo commented May 19, 2024

anonrig commented May 19, 2024

BoscoDomingo commented May 19, 2024

anonrig commented May 19, 2024

BoscoDomingo commented May 19, 2024

GeoffreyBooth commented May 20, 2024

mbrevda commented May 20, 2024

mcollina left a comment

Choose a reason for hiding this comment

BoscoDomingo commented May 20, 2024

BoscoDomingo commented May 21, 2024 • edited Loading

anonrig left a comment

Choose a reason for hiding this comment

GeoffreyBooth commented May 21, 2024

anonrig commented May 21, 2024

BoscoDomingo commented May 22, 2024 • edited Loading

lewismoten commented May 27, 2024 • edited Loading

Shudrum commented May 27, 2024 • edited Loading

BoscoDomingo commented Jun 2, 2024

GeoffreyBooth commented Jun 3, 2024

BoscoDomingo commented Jun 5, 2024

aduh95 commented Jun 5, 2024 • edited Loading

BoscoDomingo commented Jun 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

aduh95 commented Jun 27, 2024

BoscoDomingo commented Jun 27, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BoscoDomingo commented Jun 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BoscoDomingo commented Jun 30, 2024 • edited Loading

BoscoDomingo commented Jun 30, 2024 • edited Loading

anonrig commented Jun 30, 2024

BoscoDomingo commented Jun 30, 2024 • edited Loading

src: add `--env-file-if-exists` flag #53060

src: add `--env-file-if-exists` flag #53060

BoscoDomingo commented May 19, 2024 •

edited

Loading

BoscoDomingo commented May 21, 2024 •

edited

Loading

BoscoDomingo commented May 22, 2024 •

edited

Loading

lewismoten commented May 27, 2024 •

edited

Loading

Shudrum commented May 27, 2024 •

edited

Loading

aduh95 commented Jun 5, 2024 •

edited

Loading

BoscoDomingo commented Jun 30, 2024 •

edited

Loading

BoscoDomingo commented Jun 30, 2024 •

edited

Loading

BoscoDomingo commented Jun 30, 2024 •

edited

Loading