From 6a91cebda511066625202f52f34ef7f60b9cd53b Mon Sep 17 00:00:00 2001 From: Emily Xiong Date: Fri, 27 Sep 2024 15:38:55 -0400 Subject: [PATCH] fix(core): fix env override run command (#28156) ## Current Behavior ## Expected Behavior ## Related Issue(s) Fixes https://github.com/nrwl/nx/issues/27821 (cherry picked from commit caae4cf0ffb02ba3a3ed885f8ef98ca95b504c70) --- .../run-commands/run-commands.impl.spec.ts | 127 ++++++------------ .../run-commands/run-commands.impl.ts | 37 +++-- 2 files changed, 55 insertions(+), 109 deletions(-) diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts index a5e5715455515..438852a3bcad5 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.spec.ts @@ -783,12 +783,11 @@ describe('Run Commands', () => { describe('env', () => { afterAll(() => { delete process.env.MY_ENV_VAR; - unlinkSync('.env'); }); - it('should add the env to the command', async () => { - const root = dirSync().name; + it('should use value from process.env', async () => { const f = fileSync().name; + process.env.MY_ENV_VAR = 'from-env'; const result = await runCommands( { commands: [ @@ -796,22 +795,18 @@ describe('Run Commands', () => { command: `echo "$MY_ENV_VAR" >> ${f}`, }, ], - env: { - MY_ENV_VAR: 'my-value', - }, parallel: true, __unparsed__: [], }, - { root } as any + context ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toEqual('my-value'); + expect(result.success).toEqual(true); + expect(readFile(f)).toContain('from-env'); }); - it('should prioritize env setting over local dotenv files', async () => { - writeFileSync('.env', 'MY_ENV_VAR=from-dotenv'); - const root = dirSync().name; + it('should add the env to the command', async () => { + process.env.MY_ENV_VAR = 'from-env'; const f = fileSync().name; const result = await runCommands( { @@ -821,22 +816,20 @@ describe('Run Commands', () => { }, ], env: { - MY_ENV_VAR: 'from-options', + MY_ENV_VAR: 'my-value', }, parallel: true, __unparsed__: [], }, - { root } as any + context ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toEqual('from-options'); + expect(result.success).toEqual(true); + expect(readFile(f)).toContain('my-value'); }); - 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; + it('should prioritize env setting over local process.env', async () => { + process.env.MY_ENV_VAR = 'from-env'; const f = fileSync().name; const result = await runCommands( { @@ -847,117 +840,75 @@ describe('Run Commands', () => { ], 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', () => { - beforeAll(() => { - writeFileSync('.env', 'NRWL_SITE=https://nrwl.io/'); - }); - - beforeEach(() => { - delete process.env.NRWL_SITE; - delete process.env.NX_SITE; - }); - - afterAll(() => { - unlinkSync('.env'); - }); - - it('should load the root .env file by default if there is one', async () => { - const f = fileSync().name; - const result = await runCommands( - { - commands: [ - { - command: `echo $NRWL_SITE >> ${f}`, - }, - ], - __unparsed__: [], - }, context ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toEqual('https://nrwl.io/'); + expect(result.success).toEqual(true); + expect(readFile(f)).toContain('from-options'); }); - it('should load the specified .env file instead of the root one', async () => { + it('should prioritize process.env over envFile option', async () => { + process.env.MY_ENV_VAR = 'from-env'; const devEnv = fileSync().name; - writeFileSync(devEnv, 'NX_SITE=https://nx.dev/'); + writeFileSync(devEnv, 'MY_ENV_VAR=from-dotenv'); const f = fileSync().name; - let result = await runCommands( + const result = await runCommands( { commands: [ { - command: `echo $NX_SITE >> ${f} && echo $NRWL_SITE >> ${f}`, + command: `echo "$MY_ENV_VAR" >> ${f}`, }, ], - envFile: devEnv, + env: { + envFile: devEnv, + }, + parallel: true, __unparsed__: [], }, context ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toContain('https://nx.dev/'); - - appendFileSync(devEnv, 'NX_TEST=$NX_SITE'); - await runCommands( - { - commands: [ - { - command: `echo $NX_TEST >> ${f}`, - }, - ], - envFile: devEnv, - __unparsed__: [], - }, - context - ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toContain('https://nx.dev/'); + expect(result.success).toEqual(true); + expect(readFile(f)).toContain('from-env'); }); - it('should override environment variables that are present in both the specified .env file and other loaded ones', async () => { + it('should prioritize env setting over dotenv file from envFile option', async () => { + process.env.MY_ENV_VAR = 'from-env'; const devEnv = fileSync().name; - writeFileSync(devEnv, 'NRWL_SITE=https://nrwl.io/override'); + writeFileSync(devEnv, 'MY_ENV_VAR=from-dotenv'); const f = fileSync().name; - let result = await runCommands( + const result = await runCommands( { commands: [ { - command: `echo $NRWL_SITE >> ${f}`, + command: `echo "$MY_ENV_VAR" >> ${f}`, }, ], - envFile: devEnv, + env: { + MY_ENV_VAR: 'from-options', + envFile: devEnv, + }, + parallel: true, __unparsed__: [], }, context ); - expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toContain('https://nrwl.io/override'); + expect(result.success).toEqual(true); + expect(readFile(f)).toContain('from-options'); }); it('should error if the specified .env file does not exist', async () => { - const f = fileSync().name; try { await runCommands( { commands: [ { - command: `echo $NX_SITE >> ${f} && echo $NRWL_SITE >> ${f}`, + command: `echo $MY_ENV_VAR`, }, ], envFile: '/somePath/.fakeEnv', diff --git a/packages/nx/src/executors/run-commands/run-commands.impl.ts b/packages/nx/src/executors/run-commands/run-commands.impl.ts index 414c7a8d942e1..f0e9b37722691 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.ts @@ -21,22 +21,12 @@ const childProcesses = new Set(); function loadEnvVarsFile(path: string, env: Record = {}) { unloadDotEnvFile(path, env); - const result = loadAndExpandDotEnvFile(path, env, true); + const result = loadAndExpandDotEnvFile(path, env); if (result.error) { throw result.error; } } -function loadEnvVars(path?: string, env: Record = {}) { - if (path) { - loadEnvVarsFile(path, env); - } else { - try { - loadEnvVarsFile('.env', env); - } catch {} - } -} - export type Json = { [k: string]: any; }; @@ -478,25 +468,30 @@ function calculateCwd( return path.join(context.root, cwd); } +/** + * Env variables are processed in the following order: + * - env option from executor options + * - env file from envFile option if provided + * - local env variables + */ function processEnv( color: boolean, cwd: string, - env: Record, + envOptionFromExecutor: Record, envFile?: string ) { - const localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() }); - let res = { + let localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() }); + localEnv = { ...process.env, ...localEnv, }; - // env file from envFile option takes priority over process env - if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') { - loadEnvVars(envFile, res); + + if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false' && envFile) { + loadEnvVarsFile(envFile, localEnv); } - // env variables from env option takes priority over everything else - res = { - ...res, - ...env, + let res: Record = { + ...localEnv, + ...envOptionFromExecutor, }; // need to override PATH to make sure we are using the local node_modules if (localEnv.PATH) res.PATH = localEnv.PATH; // UNIX-like