From df9b00ca5f6a35be5a446297dd883e6807ac2679 Mon Sep 17 00:00:00 2001 From: Emily Xiong Date: Fri, 27 Sep 2024 00:16:46 -0400 Subject: [PATCH] fix(core): fix env override run command --- .../run-commands/run-commands.impl.spec.ts | 130 ++++-------------- .../run-commands/run-commands.impl.ts | 39 +++--- 2 files changed, 46 insertions(+), 123 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 a5e5715455515b..9c6dc7f6d74a0c 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,50 @@ 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 () => { - const devEnv = fileSync().name; - writeFileSync(devEnv, 'NX_SITE=https://nx.dev/'); - const f = fileSync().name; - let result = await runCommands( - { - commands: [ - { - command: `echo $NX_SITE >> ${f} && echo $NRWL_SITE >> ${f}`, - }, - ], - envFile: devEnv, - __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/'); - }); - - 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 414c7a8d942e1c..64fdfa305e0c15 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,32 @@ 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); + + let envFromEnvFile: Record = {}; + if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false' && envFile) { + loadEnvVarsFile(envFile, envFromEnvFile); } - // env variables from env option takes priority over everything else - res = { - ...res, - ...env, + let res: Record = { + ...localEnv, + ...envFromEnvFile, + ...envOptionFromExecutor, }; // need to override PATH to make sure we are using the local node_modules if (localEnv.PATH) res.PATH = localEnv.PATH; // UNIX-like