Skip to content

Commit

Permalink
fix(core): expand env variables on load and unload
Browse files Browse the repository at this point in the history
Env variables using other variables were not unloaded from the environment
and further customizations were impossible in more specific env files.
  • Loading branch information
matheo authored and xiongemi committed Jun 25, 2024
1 parent d356f61 commit ba04e2d
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 99 deletions.
86 changes: 50 additions & 36 deletions e2e/nx/src/extras.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { parseJson } from '@nx/devkit';
import {
checkFilesExist,
cleanupProject,
getSelectedPackageManager,
isNotWindows,
newProject,
readFile,
Expand Down Expand Up @@ -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');
});
});

Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions packages/nx/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
30 changes: 22 additions & 8 deletions packages/nx/src/executors/run-commands/run-commands.impl.spec.ts
Original file line number Diff line number Diff line change
@@ -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, {
Expand Down Expand Up @@ -337,8 +337,6 @@ describe('Run Commands', () => {
result = res;
});

expect(readFile(f)).toEqual('');

setTimeout(() => {
expect(readFile(f)).toEqual('1');
expect(result).toBeNull();
Expand Down Expand Up @@ -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: [
Expand All @@ -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: [
{
Expand All @@ -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(
{
Expand Down
24 changes: 17 additions & 7 deletions packages/nx/src/executors/run-commands/run-commands.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 loadEnvVarsFile(path: string, env: Record<string, string> = {}) {
unloadDotEnvFile(path, env);
const result = loadAndExplandDotEnvFile(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 {}
}
}
Expand Down Expand Up @@ -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);

Expand Down
89 changes: 57 additions & 32 deletions packages/nx/src/tasks-runner/task-env.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
? [
Expand Down Expand Up @@ -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;
}
Loading

0 comments on commit ba04e2d

Please sign in to comment.