fix(core): expand env variables on load and unload (#26459)

xiongemi · matheo · FrozenPandaz · commit ef3d1e227cbb · 2024-06-26T12:17:17.000-04:00
This pr is meant to replace #22585 and #20524 Env variables using other variables were not unloaded from the environment and further customizations were impossible in more specific env files.     ## Current Behavior  ## Expected Behavior  ## Related Issue(s)  Fixes #23581 Co-authored-by: Mateo Tibaquira <nestormateo@gmail.com> (cherry picked from commit 88fd03b)
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 {}
   }
 }
@@ -109,9 +118,6 @@ export default async function (
   terminalOutput: string;
 }> {
   registerProcessListener();
-  if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
-    await loadEnvVars(options.envFile);
-  }
   const normalized = normalizeOptions(options);
 
   if (normalized.readyWhenStatus.length && !normalized.parallel) {
@@ -159,7 +165,8 @@ async function runInParallel(
       true,
       options.usePty,
       options.streamOutput,
-      options.tty
+      options.tty,
+      options.envFile
     ).then((result: { success: boolean; terminalOutput: string }) => ({
       result,
       command: c.command,
@@ -287,11 +294,12 @@ async function runSerially(
         [],
         options.color,
         calculateCwd(options.cwd, context),
-        options.env ?? {},
+        options.processEnv ?? options.env ?? {},
         false,
         options.usePty,
         options.streamOutput,
-        options.tty
+        options.tty,
+        options.envFile
       );
     terminalOutput += result.terminalOutput;
     if (!result.success) {
@@ -321,9 +329,10 @@ async function createProcess(
   isParallel: boolean,
   usePty: boolean = true,
   streamOutput: boolean = true,
-  tty: boolean
+  tty: boolean,
+  envFile?: string
 ): Promise<{ success: boolean; terminalOutput: string }> {
-  env = processEnv(color, cwd, env);
+  env = processEnv(color, cwd, env, envFile);
   // The rust runCommand is always a tty, so it will not look nice in parallel and if we need prefixes
   // currently does not work properly in windows
   if (
@@ -462,13 +471,21 @@ function calculateCwd(
   return path.join(context.root, cwd);
 }
 
-function processEnv(color: boolean, cwd: string, env: Record<string, string>) {
+function processEnv(
+  color: boolean,
+  cwd: string,
+  env: Record<string, string>,
+  envFile?: string
+) {
   const localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() });
   const res = {
     ...process.env,
     ...localEnv,
     ...env,
   };
+  if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
+    loadEnvVars(envFile, res);
+  }
   // need to override PATH to make sure we are using the local node_modules
   if (localEnv.PATH) res.PATH = localEnv.PATH; // UNIX-like
   if (localEnv.Path) res.Path = localEnv.Path; // Windows
diff --git a/packages/nx/src/tasks-runner/task-env.ts b/packages/nx/src/tasks-runner/task-env.ts
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
