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 · 326ae0c · 326ae0c
1 parent a3322f7
commit 326ae0c
Show file tree

Hide file tree

Showing 7 changed files with 143 additions and 90 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,
@@ -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 (
-          <>
-            <NxWelcome title={process.env.NX_USERNAME} />
-          </>
-        );
-      }
-  
-      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(<App />);
-        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
@@ -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
@@ -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
@@ -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
@@ -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<ChildProcess | PseudoTtyProcess>();
 
-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
@@ -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;
 }