Skip to content

Commit

Permalink
src: add --optional-env-file flag
Browse files Browse the repository at this point in the history
Fixes: nodejs#50993
Refs: nodejs#51451

test: remove unnecessary comment

src: conform to style guidelines

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

test: revert automatic linter changes

doc: fix typos

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

src: refactor `env_file_data` and `GetEnvFileDataFromArgs`

test: clean up tests

src: print error when file not found

test: remove unnecessary extras
  • Loading branch information
BoscoDomingo committed Aug 25, 2024
1 parent 43f699d commit 35b5823
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 27 deletions.
12 changes: 12 additions & 0 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,8 @@ in the file, the value from the environment takes precedence.
You can pass multiple `--env-file` arguments. Subsequent files override
pre-existing variables defined in previous files.

An error is thrown if the file does not exist.

```bash
node --env-file=.env --env-file=.development.env index.js
```
Expand Down Expand Up @@ -863,6 +865,9 @@ Export keyword before a key is ignored:
export USERNAME="nodejs" # will result in `nodejs` as the value.
```

If you want to load environment variables from a file that may not exist, you
can use the [`--env-file-if-exists`][] flag instead.

### `-e`, `--eval "script"`

<!-- YAML
Expand Down Expand Up @@ -1754,6 +1759,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-if-exists=config`

Behavior is the same as [`--env-file`][], but an error is not thrown if the file
does not exist.

### `--pending-deprecation`

<!-- YAML
Expand Down Expand Up @@ -3537,6 +3547,8 @@ node --stack-trace-limit=12 -p -e "Error.stackTraceLimit" # prints 12
[`--build-snapshot`]: #--build-snapshot
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--env-file-if-exists`]: #--env-file-if-existsconfig
[`--env-file`]: #--env-fileconfig
[`--experimental-default-type=module`]: #--experimental-default-typetype
[`--experimental-sea-config`]: single-executable-applications.md#generating-single-executable-preparation-blobs
[`--experimental-strip-types`]: #--experimental-strip-types
Expand Down
18 changes: 12 additions & 6 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -854,20 +854,26 @@ static ExitCode InitializeNodeWithArgsInternal(
HandleEnvOptions(per_process::cli_options->per_isolate->per_env);

std::string node_options;
auto file_paths = node::Dotenv::GetPathFromArgs(*argv);
auto env_files = node::Dotenv::GetDataFromArgs(*argv);

if (!file_paths.empty()) {
if (!env_files.empty()) {
CHECK(!per_process::v8_initialized);

for (const auto& file_path : file_paths) {
switch (per_process::dotenv_file.ParsePath(file_path)) {
for (const auto& file_data : env_files) {
switch (per_process::dotenv_file.ParsePath(file_data.path)) {
case Dotenv::ParseResult::Valid:
break;
case Dotenv::ParseResult::InvalidContent:
errors->push_back(file_path + ": invalid format");
errors->push_back(file_data.path + ": invalid format");
break;
case Dotenv::ParseResult::FileError:
errors->push_back(file_path + ": not found");
if (file_data.is_optional) {
fprintf(stderr,
"%s not found. Continuing without it.\n",
file_data.path.c_str());
continue;
}
errors->push_back(file_data.path + ": not found");
break;
default:
UNREACHABLE();
Expand Down
51 changes: 36 additions & 15 deletions src/node_dotenv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,36 +11,57 @@ using v8::NewStringType;
using v8::Object;
using v8::String;

std::vector<std::string> Dotenv::GetPathFromArgs(
std::vector<Dotenv::env_file_data> Dotenv::GetDataFromArgs(
const std::vector<std::string>& args) {
const std::string_view optional_env_file_flag = "--env-file-if-exists";

const auto find_match = [](const std::string& arg) {
return arg == "--" || arg == "--env-file" || arg.starts_with("--env-file=");
return arg == "--" ||
arg == "--env-file" ||
arg.starts_with("--env-file=") ||
arg == "--optional-env-file" ||
arg.starts_with("--optional-env-file=");
};
std::vector<std::string> paths;
auto path = std::find_if(args.begin(), args.end(), find_match);

while (path != args.end()) {
if (*path == "--") {
return paths;
std::vector<Dotenv::env_file_data> env_files;
// This will be an iterator, pointing to args.end() if no matches are found
auto matched_arg = std::find_if(args.begin(), args.end(), find_match);

while (matched_arg != args.end()) {
if (*matched_arg == "--") {
return env_files;
}
auto equal_char = path->find('=');

auto equal_char = matched_arg->find('=');

if (equal_char != std::string::npos) {
paths.push_back(path->substr(equal_char + 1));
// `--env-file=path`
auto flag = matched_arg->substr(0, equal_char);
auto file_path = matched_arg->substr(equal_char + 1);
struct env_file_data env_file_data = {
file_path,
flag.starts_with(optional_env_file_flag)
};
env_files.push_back(env_file_data);
} else {
auto next_path = std::next(path);
// `--env-file path`
auto file_path = std::next(matched_arg);

if (next_path == args.end()) {
return paths;
if (file_path == args.end()) {
return env_files;
}

paths.push_back(*next_path);
struct env_file_data env_file_data = {
*file_path,
matched_arg->starts_with(optional_env_file_flag)
};
env_files.push_back(env_file_data);
}

path = std::find_if(++path, args.end(), find_match);
matched_arg = std::find_if(++matched_arg, args.end(), find_match);
}

return paths;
return env_files;
}

void Dotenv::SetEnvironment(node::Environment* env) {
Expand Down
6 changes: 5 additions & 1 deletion src/node_dotenv.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ namespace node {
class Dotenv {
public:
enum ParseResult { Valid, FileError, InvalidContent };
struct env_file_data {
std::string path;
bool is_optional;
};

Dotenv() = default;
Dotenv(const Dotenv& d) = delete;
Expand All @@ -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> GetDataFromArgs(
const std::vector<std::string>& args);

private:
Expand Down
4 changes: 4 additions & 0 deletions src/node_options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
"set environment variables from supplied file",
&EnvironmentOptions::env_file);
Implies("--env-file", "[has_env_file_string]");
AddOption("--env-file-if-exists",
"set environment variables from supplied file",
&EnvironmentOptions::optional_env_file);
Implies("--env-file-if-exists", "[has_env_file_string]");
AddOption("--test",
"launch test runner on startup",
&EnvironmentOptions::test_runner);
Expand Down
1 change: 1 addition & 0 deletions src/node_options.h
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ class EnvironmentOptions : public Options {
std::string redirect_warnings;
std::string diagnostic_dir;
std::string env_file;
std::string optional_env_file;
bool has_env_file_string = false;
bool test_runner = false;
uint64_t test_runner_concurrency = 0;
Expand Down
21 changes: 16 additions & 5 deletions test/parallel/test-dotenv-edge-cases.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,15 @@ const validEnvFilePath = '../fixtures/dotenv/valid.env';
const nodeOptionsEnvFilePath = '../fixtures/dotenv/node-options.env';

describe('.env supports edge cases', () => {

it('supports multiple declarations', async () => {
// process.env.BASIC is equal to `basic` because the second .env file overrides it.
it('supports multiple declarations, including optional ones', async () => {
const code = `
const assert = require('assert');
assert.strictEqual(process.env.BASIC, 'basic');
assert.strictEqual(process.env.NODE_NO_WARNINGS, '1');
`.trim();
const child = await common.spawnPromisified(
process.execPath,
[ `--env-file=${nodeOptionsEnvFilePath}`, `--env-file=${validEnvFilePath}`, '--eval', code ],
[ `--env-file=${nodeOptionsEnvFilePath}`, `--env-file-if-exists=${validEnvFilePath}`, '--eval', code ],
{ cwd: __dirname },
);
assert.strictEqual(child.stderr, '');
Expand Down Expand Up @@ -52,6 +50,19 @@ describe('.env supports edge cases', () => {
assert.strictEqual(child.code, 9);
});

it('should handle non-existent optional .env file', async () => {
const code = `
require('assert').strictEqual(1,1);
`.trim();
const child = await common.spawnPromisified(
process.execPath,
['--env-file-if-exists=.env', '--eval', code],
{ cwd: __dirname },
);
assert.notStrictEqual(child.stderr, '');
assert.strictEqual(child.code, 0);
});

it('should not override existing environment variables but introduce new vars', async () => {
const code = `
require('assert').strictEqual(process.env.BASIC, 'existing');
Expand Down Expand Up @@ -106,7 +117,7 @@ describe('.env supports edge cases', () => {
'--eval', `require('assert').strictEqual(process.env.BASIC, undefined);`,
'--', '--env-file', validEnvFilePath,
],
{ cwd: fixtures.path('dotenv') },
{ cwd: __dirname },
);
assert.strictEqual(child.stdout, '');
assert.strictEqual(child.stderr, '');
Expand Down

0 comments on commit 35b5823

Please sign in to comment.