From ba04e2dea7f02295311e990ff0d8bee9cdb2e96a Mon Sep 17 00:00:00 2001 From: Mateo Tibaquira Date: Fri, 29 Mar 2024 13:36:03 -0500 Subject: [PATCH] fix(core): expand env variables on load and unload Env variables using other variables were not unloaded from the environment and further customizations were impossible in more specific env files. --- e2e/nx/src/extras.test.ts | 86 ++++++++++-------- package.json | 4 +- packages/nx/package.json | 4 +- .../run-commands/run-commands.impl.spec.ts | 30 +++++-- .../run-commands/run-commands.impl.ts | 24 +++-- packages/nx/src/tasks-runner/task-env.ts | 89 ++++++++++++------- pnpm-lock.yaml | 36 +++++--- 7 files changed, 174 insertions(+), 99 deletions(-) diff --git a/e2e/nx/src/extras.test.ts b/e2e/nx/src/extras.test.ts index 71aec7f6dce9b7..2b9a1ad445cff4 100644 --- a/e2e/nx/src/extras.test.ts +++ b/e2e/nx/src/extras.test.ts @@ -2,7 +2,6 @@ import { parseJson } from '@nx/devkit'; import { checkFilesExist, cleanupProject, - getSelectedPackageManager, isNotWindows, newProject, readFile, @@ -293,49 +292,64 @@ describe('Extra Nx Misc Tests', () => { }); describe('Env File', () => { - it('should have the right env', () => { - const appName = uniq('app'); + const libName = uniq('lib'); + + beforeAll(() => { runCLI( - `generate @nx/react:app ${appName} --style=css --bundler=webpack --no-interactive` + `generate @nx/js:lib ${libName} --bundler=none --unitTestRunner=none --no-interactive` ); + }); + + it('should have the right env', () => { updateFile( '.env', `FIRSTNAME="firstname" - LASTNAME="lastname" - NX_USERNAME=$FIRSTNAME $LASTNAME` - ); - updateFile( - `apps/${appName}/src/app/app.tsx`, - ` - import NxWelcome from './nx-welcome'; - - export function App() { - return ( - <> - - - ); - } - - export default App; - ` +LASTNAME="lastname" +NX_USERNAME=$FIRSTNAME $LASTNAME` ); - updateFile( - `apps/${appName}/src/app/app.spec.tsx`, - `import { render } from '@testing-library/react'; - - import App from './app'; - - describe('App', () => { - it('should have a greeting as the title', () => { - const { getByText } = render(); - expect(getByText(/Welcome firstname lastname/gi)).toBeTruthy(); + updateJson(join('libs', libName, 'project.json'), (config) => { + config.targets.echo = { + command: 'echo $NX_USERNAME', + }; + return config; + }); + let result = runCLI(`run ${libName}:echo`); + expect(result).toContain('firstname lastname'); + + updateFile('.env', (content) => { + content = content.replace('firstname', 'firstname2'); + content = content.replace('lastname', 'lastname2'); + return content; }); + result = runCLI(`run ${libName}:echo`); + expect(result).toContain('firstname2 lastname2'); }); - ` - ); - const unitTestsOutput = runCLI(`test ${appName}`); - expect(unitTestsOutput).toContain('Successfully ran target test'); + + it('should work with custom env file', () => { + updateFile(`libs/${libName}/.custom1.env`, `hello="hello1"`); + updateFile(`libs/${libName}/.custom2.env`, `hello="hello2"`); + updateJson(join('libs', libName, 'project.json'), (config) => { + config.targets.hello1 = { + command: 'echo $hello', + options: { + envFile: `libs/${libName}/.custom1.env`, + }, + }; + config.targets.hello2 = { + command: 'echo $hello', + options: { + envFile: `libs/${libName}/.custom2.env`, + }, + }; + return config; + }); + let result = runCLI(`run ${libName}:hello1`); + expect(result).toContain('hello1'); + result = runCLI(`run ${libName}:hello2`); + expect(result).toContain('hello2'); + result = runCLI(`run-many --target=hello1,hello2`); + expect(result).toContain('hello1'); + expect(result).toContain('hello2'); }); }); diff --git a/package.json b/package.json index 10200d4e7cb2fb..a2bcba834d3350 100644 --- a/package.json +++ b/package.json @@ -160,8 +160,8 @@ "cz-git": "^1.4.0", "czg": "^1.4.0", "detect-port": "^1.5.1", - "dotenv": "~16.3.1", - "dotenv-expand": "^10.0.0", + "dotenv": "~16.4.5", + "dotenv-expand": "~11.0.6", "ejs": "^3.1.7", "enhanced-resolve": "^5.8.3", "esbuild": "0.19.5", diff --git a/packages/nx/package.json b/packages/nx/package.json index c6d0f179af7d7e..578bd7b9d88f3c 100644 --- a/packages/nx/package.json +++ b/packages/nx/package.json @@ -45,8 +45,8 @@ "cli-cursor": "3.1.0", "cli-spinners": "2.6.1", "cliui": "^8.0.1", - "dotenv": "~16.3.1", - "dotenv-expand": "~10.0.0", + "dotenv": "~16.4.5", + "dotenv-expand": "~11.0.6", "enquirer": "~2.3.6", "figures": "3.2.0", "flat": "^5.0.2", 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 0222bfc02d369d..a7bb1b796de8de 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 @@ -1,4 +1,4 @@ -import { readFileSync, unlinkSync, writeFileSync } from 'fs'; +import { appendFileSync, readFileSync, unlinkSync, writeFileSync } from 'fs'; import { relative } from 'path'; import { dirSync, fileSync } from 'tmp'; import runCommands, { @@ -337,8 +337,6 @@ describe('Run Commands', () => { result = res; }); - expect(readFile(f)).toEqual(''); - setTimeout(() => { expect(readFile(f)).toEqual('1'); expect(result).toBeNull(); @@ -808,7 +806,7 @@ describe('Run Commands', () => { }); it('should load the root .env file by default if there is one', async () => { - let f = fileSync().name; + const f = fileSync().name; const result = await runCommands( { commands: [ @@ -828,8 +826,8 @@ describe('Run Commands', () => { it('should load the specified .env file instead of the root one', async () => { const devEnv = fileSync().name; writeFileSync(devEnv, 'NX_SITE=https://nx.dev/'); - let f = fileSync().name; - const result = await runCommands( + const f = fileSync().name; + let result = await runCommands( { commands: [ { @@ -843,11 +841,27 @@ describe('Run Commands', () => { ); expect(result).toEqual(expect.objectContaining({ success: true })); - expect(readFile(f)).toEqual('https://nx.dev/'); + 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 error if the specified .env file does not exist', async () => { - let f = fileSync().name; + const f = fileSync().name; try { await runCommands( { 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 f0543b2cb5ec0d..1158722967a81b 100644 --- a/packages/nx/src/executors/run-commands/run-commands.impl.ts +++ b/packages/nx/src/executors/run-commands/run-commands.impl.ts @@ -10,20 +10,29 @@ import { PseudoTtyProcess, } from '../../tasks-runner/pseudo-terminal'; import { signalToCode } from '../../utils/exit-codes'; +import { + loadAndExplandDotEnvFile, + unloadDotEnvFile, +} from '../../tasks-runner/task-env'; export const LARGE_BUFFER = 1024 * 1000000; let pseudoTerminal: PseudoTerminal | null; const childProcesses = new Set(); -async function loadEnvVars(path?: string) { +function loadEnvVarsFile(path: string, env: Record = {}) { + unloadDotEnvFile(path, env); + const result = loadAndExplandDotEnvFile(path, env); + if (result.error) { + throw result.error; + } +} + +function loadEnvVars(path?: string, env: Record = {}) { if (path) { - const result = (await import('dotenv')).config({ path }); - if (result.error) { - throw result.error; - } + loadEnvVarsFile(path, env); } else { try { - (await import('dotenv')).config(); + loadEnvVarsFile('.env', env); } catch {} } } @@ -110,7 +119,8 @@ export default async function ( }> { registerProcessListener(); if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') { - await loadEnvVars(options.envFile); + options.env ??= {}; + loadEnvVars(options.envFile, options.env); } const normalized = normalizeOptions(options); diff --git a/packages/nx/src/tasks-runner/task-env.ts b/packages/nx/src/tasks-runner/task-env.ts index 2c971a59362467..655d8f19732dc7 100644 --- a/packages/nx/src/tasks-runner/task-env.ts +++ b/packages/nx/src/tasks-runner/task-env.ts @@ -21,7 +21,7 @@ export function getEnvVariablesForBatchProcess( export function getTaskSpecificEnv(task: Task) { // Unload any dot env files at the root of the workspace that were loaded on init of Nx. - const taskEnv = unloadDotEnvFiles({ ...process.env }); + const taskEnv = unloadDotEnvFiles(task, { ...process.env }); return process.env.NX_LOAD_DOT_ENV_FILES === 'true' ? loadDotEnvFilesForTask(task, taskEnv) : // If not loading dot env files, ensure env vars created by system are still loaded @@ -121,12 +121,50 @@ function getNxEnvVariablesForTask( }; } -function loadDotEnvFilesForTask( - task: Task, - environmentVariables: NodeJS.ProcessEnv +/** + * This function loads a .env file and expands the variables in it. + * It is going to override existing environmentVariables. + * @param filename + * @param environmentVariables + */ +export function loadAndExplandDotEnvFile( + filename: string, + environmentVariables: NodeJS.ProcessEnv, + override = false +) { + const myEnv = loadDotEnvFile({ + path: filename, + processEnv: environmentVariables, + override, + }); + return expand({ + ...myEnv, + processEnv: environmentVariables, + }); +} + +/** + * This funciton unloads a .env file and removes the variables in it from the environmentVariables. + * @param filename + * @param environmentVariables + */ +export function unloadDotEnvFile( + filename: string, + environmentVariables: NodeJS.ProcessEnv, + override = false ) { + const parsedDotEnvFile: NodeJS.ProcessEnv = {}; + loadAndExplandDotEnvFile(filename, parsedDotEnvFile, override); + Object.keys(parsedDotEnvFile).forEach((envVarKey) => { + if (environmentVariables[envVarKey] === parsedDotEnvFile[envVarKey]) { + delete environmentVariables[envVarKey]; + } + }); +} + +function getEnvFilesForTask(task: Task): string[] { // Collect dot env files that may pertain to a task - const dotEnvFiles = [ + return [ // Load DotEnv Files for a configuration in the project root ...(task.target.configuration ? [ @@ -175,39 +213,26 @@ function loadDotEnvFilesForTask( `.env.local`, `.env`, ]; +} +function loadDotEnvFilesForTask( + task: Task, + environmentVariables: NodeJS.ProcessEnv +) { + const dotEnvFiles = getEnvFilesForTask(task); for (const file of dotEnvFiles) { - const myEnv = loadDotEnvFile({ - path: file, - processEnv: environmentVariables, - // Do not override existing env variables as we load - override: false, - }); - environmentVariables = { - ...expand({ - ...myEnv, - ignoreProcessEnv: true, // Do not override existing env variables as we load - }).parsed, - ...environmentVariables, - }; + loadAndExplandDotEnvFile(file, environmentVariables); } - return environmentVariables; } -function unloadDotEnvFiles(environmentVariables: NodeJS.ProcessEnv) { - const unloadDotEnvFile = (filename: string) => { - let parsedDotEnvFile: NodeJS.ProcessEnv = {}; - loadDotEnvFile({ path: filename, processEnv: parsedDotEnvFile }); - Object.keys(parsedDotEnvFile).forEach((envVarKey) => { - if (environmentVariables[envVarKey] === parsedDotEnvFile[envVarKey]) { - delete environmentVariables[envVarKey]; - } - }); - }; - - for (const file of ['.env', '.local.env', '.env.local']) { - unloadDotEnvFile(file); +function unloadDotEnvFiles( + task: Task, + environmentVariables: NodeJS.ProcessEnv +) { + const dotEnvFiles = getEnvFilesForTask(task); + for (const file of dotEnvFiles) { + unloadDotEnvFile(file, environmentVariables); } return environmentVariables; } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index c62ab79ec01a67..d510637825b132 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -555,11 +555,11 @@ devDependencies: specifier: ^1.5.1 version: 1.5.1 dotenv: - specifier: ~16.3.1 - version: 16.3.1 + specifier: ~16.4.5 + version: 16.4.5 dotenv-expand: - specifier: ^10.0.0 - version: 10.0.0 + specifier: ~11.0.6 + version: 11.0.6 ejs: specifier: ^3.1.7 version: 3.1.8 @@ -9874,7 +9874,7 @@ packages: create-require: 1.1.1 defu: 6.1.4 destr: 2.0.2 - dotenv: 16.3.1 + dotenv: 16.4.5 git-url-parse: 13.1.1 is-docker: 3.0.0 jiti: 1.21.0 @@ -11981,7 +11981,7 @@ packages: chalk: 4.1.2 chokidar: 3.5.3 cross-spawn: 7.0.3 - dotenv: 16.3.1 + dotenv: 16.4.5 es-module-lexer: 1.4.1 esbuild: 0.17.6 esbuild-plugins-node-modules-polyfill: 1.6.1(esbuild@0.17.6) @@ -17903,7 +17903,7 @@ packages: dependencies: chokidar: 3.5.3 defu: 6.1.4 - dotenv: 16.3.1 + dotenv: 16.4.5 giget: 1.2.1 jiti: 1.21.0 mlly: 1.5.0 @@ -20090,8 +20090,20 @@ packages: engines: {node: '>=12'} dev: true - /dotenv@16.3.1: - resolution: {integrity: sha512-IPzF4w4/Rd94bA9imS68tZBaYyBWSCE47V1RGuMrB94iyTOIEwRmVL2x/4An+6mETpLrKJ5hQkB8W4kFAadeIQ==} + /dotenv-expand@11.0.6: + resolution: {integrity: sha512-8NHi73otpWsZGBSZwwknTXS5pqMOrk9+Ssrna8xCaxkzEpU9OTf9R5ArQGVw03//Zmk9MOwLPng9WwndvpAJ5g==} + engines: {node: '>=12'} + dependencies: + dotenv: 16.4.5 + dev: true + + /dotenv@16.3.2: + resolution: {integrity: sha512-HTlk5nmhkm8F6JcdXvHIzaorzCoziNQT9mGxLPVXW8wJF1TiGSL60ZGB4gHWabHOaMmWmhvk2/lPHfnBiT78AQ==} + engines: {node: '>=12'} + dev: true + + /dotenv@16.4.5: + resolution: {integrity: sha512-ZmdL2rui+eB2YwhsWzjInR8LldtZHGDoQ1ugH85ppHKwpUHL7j7rN0Ti9NCnGiQbhaZ11FpR+7ao1dNsmduNUg==} engines: {node: '>=12'} dev: true @@ -25118,7 +25130,7 @@ packages: engines: {node: '>=14.0.0'} dependencies: app-root-dir: 1.0.2 - dotenv: 16.3.1 + dotenv: 16.4.5 dotenv-expand: 10.0.0 dev: true @@ -27704,7 +27716,7 @@ packages: cli-cursor: 3.1.0 cli-spinners: 2.6.1 cliui: 8.0.1 - dotenv: 16.3.1 + dotenv: 16.3.2 dotenv-expand: 10.0.0 enquirer: 2.3.6 figures: 3.2.0 @@ -27769,7 +27781,7 @@ packages: cli-cursor: 3.1.0 cli-spinners: 2.6.1 cliui: 8.0.1 - dotenv: 16.3.1 + dotenv: 16.3.2 dotenv-expand: 10.0.0 enquirer: 2.3.6 figures: 3.2.0