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.
nrwl · Jun 25, 2024 · 0a72ffb · 0a72ffb
1 parent b42b6f7
commit 0a72ffb
Show file tree

Hide file tree

Showing 7 changed files with 177 additions and 98 deletions.
diff --git 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 (
-          <>
-            <NxWelcome title={process.env.NX_USERNAME} />
-          </>
-        );
-      }
-  
-      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(<App />);
-        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
@@ -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
@@ -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
@@ -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
@@ -10,20 +10,29 @@ import {
   PseudoTtyProcess,
 } from '../../tasks-runner/pseudo-terminal';
 import { signalToCode } from '../../utils/exit-codes';
+import {
+  loadAndExpandDotEnvFile,
+  unloadDotEnvFile,
+} from '../../tasks-runner/task-env';
 
 export const LARGE_BUFFER = 1024 * 1000000;
 let pseudoTerminal: PseudoTerminal | null;
 const childProcesses = new Set<ChildProcess | PseudoTtyProcess>();
 
-async function loadEnvVars(path?: string) {
+function loadEnvVarsFile(path: string, env: Record<string, string> = {}) {
+  unloadDotEnvFile(path, env);
+  const result = loadAndExpandDotEnvFile(path, env);
+  if (result.error) {
+    throw result.error;
+  }
+}
+
+function loadEnvVars(path?: string, env: Record<string, string> = {}) {
   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 {}
   }
 }
@@ -99,6 +108,7 @@ export interface NormalizedRunCommandsOptions extends RunCommandsOptions {
   };
   args?: string;
   readyWhenStatus: { stringToMatch: string; found: boolean }[];
+  processEnv?: Record<string, string>; // env variables contains process.env, options.env and options.envFile
 }
 
 export default async function (
@@ -110,7 +120,13 @@ export default async function (
 }> {
   registerProcessListener();
   if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
-    await loadEnvVars(options.envFile);
+    const env = {
+      ...process.env,
+      ...(options.env ? options.env : {}),
+    };
+    loadEnvVars(options.envFile, env);
+    options.processEnv = env;
+    loadEnvVars(options.envFile, options.env);
   }
   const normalized = normalizeOptions(options);
 
@@ -287,7 +303,7 @@ async function runSerially(
         [],
         options.color,
         calculateCwd(options.cwd, context),
-        options.env ?? {},
+        options.processEnv ?? options.env ?? {},
         false,
         options.usePty,
         options.streamOutput,

diff --git a/packages/nx/src/tasks-runner/task-env.ts b/packages/nx/src/tasks-runner/task-env.ts
@@ -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 loadAndExpandDotEnvFile(
+  filename: string,
+  environmentVariables: NodeJS.ProcessEnv,
+  override = false
+) {
+  const myEnv = loadDotEnvFile({
+    path: filename,
+    processEnv: environmentVariables,
+    override,
+  });
+  return expand({
+    ...myEnv,
+    processEnv: environmentVariables,
+  });
+}
+
+/**
+ * This function 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 = {};
+  loadAndExpandDotEnvFile(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,24 @@ 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,
-    };
+    loadAndExpandDotEnvFile(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];
-      }
-    });
-  };
-
+function unloadDotEnvFiles(
+  environmentVariables: NodeJS.ProcessEnv
+) {
   for (const file of ['.env', '.local.env', '.env.local']) {
-    unloadDotEnvFile(file);
+    unloadDotEnvFile(file, environmentVariables);
   }
   return environmentVariables;
 }