Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.env overrides already-set environment variables when nx:run-command executor function is called from another executor #27821

Closed
1 of 4 tasks
ethantkoenig opened this issue Sep 9, 2024 · 1 comment · Fixed by #28156
Assignees
Labels

Comments

@ethantkoenig
Copy link

ethantkoenig commented Sep 9, 2024

Current Behavior

When the "nx:run-command" executor function is called from a custom executor, environment variables already set in the executor environment are overridden by environment variables in .env in the command's environment.

Expected Behavior

The following doc indicates that .env files do not override existing environment variables

{% callout type="warning" title="Order is important" %}
Nx will move through the above list, ignoring files it can't find, and loading environment variables
into the current process for the ones it can find. If it finds a variable that has already been loaded into the process,
it will ignore it. It does this for two reasons:
1. Developers can't accidentally overwrite important system level variables (like `NODE_ENV`)
2. Allows developers to create `.env.local` or `.local.env` files for their local environment and override any project
defaults set in `.env`
3. Allows developers to create target specific `.env.[target-name]` or `.[target-name].env` to overwrite environment variables for specific targets. For instance, you could increase the memory use for node processes only for build targets by setting `NODE_OPTIONS=--max-old-space-size=4096` in `.build.env`
For example:
1. `apps/my-app/.env.local` contains `NX_PUBLIC_API_URL=http://localhost:3333`
2. `apps/my-app/.env` contains `NX_PUBLIC_API_URL=https://api.example.com`
3. Nx will first load the variables from `apps/my-app/.env.local` into the process. When it tries to load the variables
from `apps/my-app/.env`, it will notice that `NX_PUBLIC_API_URL` already exists, so it will ignore it.
We recommend nesting your **app** specific `env` files in `apps/your-app`, and creating workspace/root level `env` files
for workspace-specific settings (like the [Nx Cloud token](/ci/recipes/security/access-tokens)).
{% /callout %}

Also, existing environment variables are not overridden by .env if the "nx:run-command" executor is used directly by a target.

GitHub Repo

https://github.com/ethantkoenig/nx-env-var-bug

Steps to Reproduce

Run ENV_VAR=already-set nx run foo:echo-env-var, and note that the output is from-file (set in .env) rather than already-set.

Nx Report

NX   Report complete - copy this into the issue template

Node           : 21.7.1
OS             : darwin-arm64
Native Target  : aarch64-macos
npm            : 10.5.0

nx (global)        : 19.6.5
nx                 : 19.6.5
@nx/js             : 19.6.5
@nx/linter         : 19.6.5
@nx/eslint         : 19.6.5
@nx/workspace      : 19.6.5
@nx/devkit         : 19.6.5
@nx/eslint-plugin  : 19.6.5
@nrwl/tao          : 19.6.5
typescript         : 5.5.4
---------------------------------------
Registered Plugins:
@nx/eslint/plugin
---------------------------------------
Community plugins:
@nxlv/python : 19.0.1

Failure Logs

No response

Package Manager Version

No response

Operating System

  • macOS
  • Linux
  • Windows
  • Other (Please specify)

Additional Information

Note that the foo:echo-env-var target uses the @nxlv/python:run-commands executor, which is a very thin wrapper around nx:run-command.

I believe this is happening because the options.env property is unset when the @nxlv/python:run-commands executor function is called, and therefore is unset in the subsequent call of the nx:run-command executor function. In contrast, when the nx:run-command executor is used directly by a target, Nx appears to prepopulate options.env with the process's current environment variables when calling the executor function.

options.env is eventually passed as the env argument in processEnv (shown below). Immediately after line 493, res will contain both process environment variables and variables from .env, with the latter taking precedence. If env contains the process environment variables (as is the case when nx:run-command is used directly by a target), lines 495-498 will counter-override the environment variables from .env with the process environment variables. But this does not happen if env is empty.

function processEnv(
color: boolean,
cwd: string,
env: Record<string, string>,
envFile?: string
) {
const localEnv = appendLocalEnv({ cwd: cwd ?? process.cwd() });
let res = {
...process.env,
...localEnv,
};
// env file from envFile option takes priority over process env
if (process.env.NX_LOAD_DOT_ENV_FILES !== 'false') {
loadEnvVars(envFile, res);
}
// env variables from env option takes priority over everything else
res = {
...res,
...env,
};
// 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
if (color) {
res.FORCE_COLOR = `${color}`;
}
return res;
}

I think there are potentially two scenarios in which this behavior is WAI:

  1. The nx:run-command executor function is not intended to be called from other executors in the first place, though https://nx.dev/extending-nx/recipes/compose-executors#compose-executors seems to indicate that executor functions are in general intended to be called from other executors.
  2. I suppose one could stipulate that it is the responsibility of callers of the nx:run-command executor function to set the env option to include the currently-set environment variables, but that would seem to violate the principle of least astonishment, and I would argue should at the very least be clearly documented.
@jaysoo jaysoo added the scope: core core nx functionality label Sep 25, 2024
FrozenPandaz pushed a commit that referenced this issue Sep 27, 2024
<!-- Please make sure you have read the submission guidelines before
posting an PR -->
<!--
https://github.com/nrwl/nx/blob/master/CONTRIBUTING.md#-submitting-a-pr
-->

<!-- Please make sure that your commit message follows our format -->
<!-- Example: `fix(nx): must begin with lowercase` -->

<!-- If this is a particularly complex change or feature addition, you
can request a dedicated Nx release for this pull request branch. Mention
someone from the Nx team or the `@nrwl/nx-pipelines-reviewers` and they
will confirm if the PR warrants its own release for testing purposes,
and generate it for you if appropriate. -->

## Current Behavior
<!-- This is the behavior we have today -->

## Expected Behavior
<!-- This is the behavior we should expect with the changes in this PR
-->

## Related Issue(s)
<!-- Please link the issue being fixed so it gets closed when this is
merged. -->

Fixes #27821

(cherry picked from commit caae4cf)
Copy link

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants