Skip to content

Commit

Permalink
fix(core): make sure env vars specified in run-commands envFile optio…
Browse files Browse the repository at this point in the history
…n take priority over other loaded env files (#27583)

<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
When you specify the `envFile` property on a `nx:run-commands` executor,
values in that file don't override values from other loaded env files
(like `.env` and others that are loaded by the task runner).

## Expected Behavior
`envFile` contents should take precedence. The properties specified in
the `env` option should still override this.

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #

(cherry picked from commit 3fbaf7f)
  • Loading branch information
MaxKless authored and FrozenPandaz committed Aug 28, 2024
1 parent 90ebb22 commit 9d13a7a
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 6 deletions.
48 changes: 48 additions & 0 deletions packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,7 @@ describe('Run Commands', () => {
expect(result).toEqual(expect.objectContaining({ success: true }));
expect(readFile(f)).toEqual('my-value');
});

it('should prioritize env setting over local dotenv files', async () => {
writeFileSync('.env', 'MY_ENV_VAR=from-dotenv');
const root = dirSync().name;
Expand All @@ -825,6 +826,32 @@ describe('Run Commands', () => {
expect(result).toEqual(expect.objectContaining({ success: true }));
expect(readFile(f)).toEqual('from-options');
});

it('should prioritize env setting over dotenv file from envFile option', async () => {
const devEnv = fileSync().name;
writeFileSync(devEnv, 'MY_ENV_VAR=from-dotenv');
const root = dirSync().name;
const f = fileSync().name;
const result = await runCommands(
{
commands: [
{
command: `echo "$MY_ENV_VAR" >> ${f}`,
},
],
env: {
MY_ENV_VAR: 'from-options',
envFile: devEnv,
},
parallel: true,
__unparsed__: [],
},
{ root } as any
);

expect(result).toEqual(expect.objectContaining({ success: true }));
expect(readFile(f)).toEqual('from-options');
});
});

describe('dotenv', () => {
Expand Down Expand Up @@ -896,6 +923,27 @@ describe('Run Commands', () => {
expect(readFile(f)).toContain('https://nx.dev/');
});

it('should override environment variables that are present in both the specified .env file and other loaded ones', async () => {
const devEnv = fileSync().name;
writeFileSync(devEnv, 'NRWL_SITE=https://nrwl.io/override');
const f = fileSync().name;
let result = await runCommands(
{
commands: [
{
command: `echo $NRWL_SITE >> ${f}`,
},
],
envFile: devEnv,
__unparsed__: [],
},
context
);

expect(result).toEqual(expect.objectContaining({ success: true }));
expect(readFile(f)).toContain('https://nrwl.io/override');
});

it('should error if the specified .env file does not exist', async () => {
const f = fileSync().name;
try {
Expand Down
11 changes: 8 additions & 3 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const childProcesses = new Set<ChildProcess | PseudoTtyProcess>();

function loadEnvVarsFile(path: string, env: Record<string, string> = {}) {
unloadDotEnvFile(path, env);
const result = loadAndExpandDotEnvFile(path, env);
const result = loadAndExpandDotEnvFile(path, env, true);
if (result.error) {
throw result.error;
}
Expand Down Expand Up @@ -484,14 +484,19 @@ function processEnv(
envFile?: string
) {
const localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() });
const res = {
let res = {
...process.env,
...localEnv,
...env,
};
// env file from envFile option takes priority over process env
if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
loadEnvVars(envFile, res);
}
// env variables from env option takes priority over everything else
res = {
...res,
...env,
};
// need to override PATH to make sure we are using the local node_modules
if (localEnv.PATH) res.PATH = localEnv.PATH; // UNIX-like
if (localEnv.Path) res.Path = localEnv.Path; // Windows
Expand Down
6 changes: 3 additions & 3 deletions packages/nx/src/tasks-runner/task-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ function getNxEnvVariablesForTask(

/**
* This function loads a .env file and expands the variables in it.
* It is going to override existing environmentVariables.
* @param filename
* @param environmentVariables
* @param filename the .env file to load
* @param environmentVariables the object to load environment variables into
* @param override whether to override existing environment variables
*/
export function loadAndExpandDotEnvFile(
filename: string,
Expand Down

0 comments on commit 9d13a7a

Please sign in to comment.