From 326ae0cd1a908968b507cd11130abf9c2e531c5f 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 | 56 ++++------- package.json | 2 +- packages/nx/package.json | 2 +- .../run-commands/run-commands.impl.spec.ts | 27 ++++-- .../run-commands/run-commands.impl.ts | 17 +++- packages/nx/src/tasks-runner/task-env.ts | 95 +++++++++++++------ pnpm-lock.yaml | 34 ++++--- 7 files changed, 143 insertions(+), 90 deletions(-) diff --git a/e2e/nx/src/extras.test.ts b/e2e/nx/src/extras.test.ts index 71aec7f6dce9b7..7f8528cb66ea28 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, @@ -294,48 +293,33 @@ describe('Extra Nx Misc Tests', () => { describe('Env File', () => { it('should have the right env', () => { - const appName = uniq('app'); + const libName = uniq('lib'); runCLI( - `generate @nx/react:app ${appName} --style=css --bundler=webpack --no-interactive` + `generate @nx/js:lib ${libName} --bundler=none --unitTestRunner=none --no-interactive` ); updateFile( '.env', `FIRSTNAME="firstname" - LASTNAME="lastname" - NX_USERNAME=$FIRSTNAME $LASTNAME` +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; - ` - ); - 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; }); - }); - ` - ); - const unitTestsOutput = runCLI(`test ${appName}`); - expect(unitTestsOutput).toContain('Successfully ran target test'); + + 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'); }); }); diff --git a/package.json b/package.json index e4898adf97feaf..9eb1a46b01af74 100644 --- a/package.json +++ b/package.json @@ -161,7 +161,7 @@ "czg": "^1.4.0", "detect-port": "^1.5.1", "dotenv": "~16.3.1", - "dotenv-expand": "^10.0.0", + "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..7b01676966a5c9 100644 --- a/packages/nx/package.json +++ b/packages/nx/package.json @@ -46,7 +46,7 @@ "cli-spinners": "2.6.1", "cliui": "^8.0.1", "dotenv": "~16.3.1", - "dotenv-expand": "~10.0.0", + "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..467ca2348ede4c 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, { @@ -802,7 +802,6 @@ describe('Run Commands', () => { delete process.env.NRWL_SITE; delete process.env.NX_SITE; }); - afterAll(() => { unlinkSync('.env'); }); @@ -829,7 +828,7 @@ describe('Run Commands', () => { const devEnv = fileSync().name; writeFileSync(devEnv, 'NX_SITE=https://nx.dev/'); let f = fileSync().name; - const result = await runCommands( + let result = await runCommands( { commands: [ { @@ -843,17 +842,33 @@ 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}`, + }, + ], + cwd: process.cwd(), + 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; try { await runCommands( { commands: [ { - command: `echo $NX_SITE >> ${f} && echo $NRWL_SITE >> ${f}`, + command: `echo $NX_SITE && echo $NRWL_SITE`, }, ], 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 f0543b2cb5ec0d..116254a4a6f2f2 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 loadEnvVars(path?: string) { if (path) { - const result = (await import('dotenv')).config({ path }); + unloadDotEnvFile(path, process.env); + const result = loadAndExplandDotEnvFile(path, process.env); if (result.error) { throw result.error; } } else { try { - (await import('dotenv')).config(); + unloadDotEnvFile('.env', process.env); + const result = loadAndExplandDotEnvFile('.env', process.env); + if (result.error) { + throw result.error; + } } catch {} } } @@ -110,7 +119,7 @@ export default async function ( }> { registerProcessListener(); if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') { - await loadEnvVars(options.envFile); + loadEnvVars(options.envFile); } 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..28822d4224db78 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,58 @@ function getNxEnvVariablesForTask( }; } -function loadDotEnvFilesForTask( - task: Task, +/** + * 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 +) { + const myEnv = loadDotEnvFile({ + path: filename, + processEnv: environmentVariables, + // Do not override existing env variables as we load + override: false, + }); + 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 ) { + let parsedDotEnvFile: NodeJS.ProcessEnv = {}; + const myEnv = loadDotEnvFile({ + path: filename, + processEnv: parsedDotEnvFile, + // Do not override existing env variables as we load + override: false, + }); + expand({ + ...myEnv, + processEnv: parsedDotEnvFile, + }); + 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 +221,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 46eb4262dc48d2..49168ced68a306 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -556,10 +556,10 @@ devDependencies: version: 1.5.1 dotenv: specifier: ~16.3.1 - version: 16.3.1 + version: 16.3.2 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.3.2 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.3.2 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.3.2 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.3.2 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