nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.5k stars 2.34k forks source link

expanded task env variables are immediately overwritten by not expanded vars #19437

Open BorisTB opened 1 year ago

BorisTB commented 1 year ago

Current Behavior

Task env variables that are expanded are immediately overwritten by original values. So expand doesn't work for tasks.

  1. env file is loaded with dotenv: https://github.com/nrwl/nx/blob/601b74d50e8b78fc46aae8e8c862667b8b51b0b2/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L447-L452
  2. dotenv populates processEnv (environmentVariables): https://github.com/motdotla/dotenv/blob/cf4c56957974efb7238ecaba6f16e0afa895c194/lib/main.js#L205
  3. env values are expanded: https://github.com/nrwl/nx/blob/601b74d50e8b78fc46aae8e8c862667b8b51b0b2/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L453-L459
  4. expanded values are overwritten by original values (populated by dotenv): https://github.com/nrwl/nx/blob/601b74d50e8b78fc46aae8e8c862667b8b51b0b2/packages/nx/src/tasks-runner/forked-process-task-runner.ts#L458

Expected Behavior

task runner will expand env variables

GitHub Repo

No response

Steps to Reproduce

  1. create app, e.g. apps/my-app (i used @nx/nest)
  2. create dotenv for app, e.g. apps/my-app/.env
  3. edit .env to look like this
    NX_FOO=foo
    NX_BAR=$NX_FOO
  4. somewhere in app use process.env['NX_BAR']
  5. run app, nx serve my-app
  6. value of process.env.NX_BAR is "$NX_FOO"

Nx Report

Node   : 20.1.0
   OS     : darwin-x64
   yarn   : 3.6.3

   nx (global)        : 16.9.1
   nx                 : 16.9.1
   @nx/js             : 16.9.1
   @nx/jest           : 16.9.1
   @nx/linter         : 16.9.1
   @nx/workspace      : 16.9.1
   @nx/cypress        : 16.9.1
   @nx/devkit         : 16.9.1
   @nx/eslint-plugin  : 16.9.1
   @nx/nest           : 16.9.1
   @nx/node           : 16.9.1
   @nx/plugin         : 16.9.1
   @nx/react          : 16.9.1
   @nrwl/tao          : 16.9.1
   @nx/vite           : 16.9.1
   @nx/web            : 16.9.1
   @nx/webpack        : 16.9.1
   nx-cloud           : 16.4.0
   typescript         : 5.1.6
   ---------------------------------------
   Community plugins:
   @nx-tools/nx-container : 5.0.2

Failure Logs

No response

Package Manager Version

No response

Operating System

Additional Information

No response

Cammisuli commented 12 months ago

Hey @BorisTB we recently changed the way we handled env's with tasks. Can you try it again with the most recent version of Nx (v17.0.1)?

BorisTB commented 12 months ago

Hey, tried it with v17.0.2 and issue's still there. It's here in the source code

https://github.com/nrwl/nx/blob/3e0f5c4993fda9be7a7ab5c4e60eecac8de7eaba/packages/nx/src/tasks-runner/task-env.ts#L169-L180

nocnokneo commented 12 months ago

This is affecting my usage of nested .env variables with vite. However I seem to be able to workaround the problem for the serve target if I don't run nx from the repo root. e.g.

cd apps/cloud-ui
nx serve cloud-ui # works: the value of NX_BAR is 'foo'
cd ../..
nx serve cloud-ui # broken: the value of NX_BAR is '$NX_FOO'

However this does not work with the build target:

cd apps/cloud-ui
nx build cloud-ui --verbose

> nx run cloud-ui:build:production

 >  NX   The "path" argument must be of type string. Received undefined

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)
    at join (node:path:1172:7)
    at validateTypes (/home/tbj/git/cyto/node_modules/.pnpm/@nx+vite@17.0.2_@swc+core@1.3.91_@types+node@18.14.2_nx@17.0.2_typescript@5.1.6_vite@4.3.9_vitest@0.32.4/node_modules/@nx/vite/src/utils/executor-utils.js:22:38)
    at viteBuildExecutor (/home/tbj/git/cyto/node_modules/.pnpm/@nx+vite@17.0.2_@swc+core@1.3.91_@types+node@18.14.2_nx@17.0.2_typescript@5.1.6_vite@4.3.9_vitest@0.32.4/node_modules/@nx/vite/src/executors/build/build.impl.js:34:48)
    at async getLastValueFromAsyncIterableIterator (/home/tbj/git/cyto/node_modules/.pnpm/nx@17.0.2_@swc+core@1.3.91/node_modules/nx/src/utils/async-iterator.js:13:19)
    at async iteratorToProcessStatusCode (/home/tbj/git/cyto/node_modules/.pnpm/nx@17.0.2_@swc+core@1.3.91/node_modules/nx/src/command-line/run/run.js:41:29)
    at async handleErrors (/home/tbj/git/cyto/node_modules/.pnpm/nx@17.0.2_@swc+core@1.3.91/node_modules/nx/src/utils/params.js:9:16)
    at async process.<anonymous> (/home/tbj/git/cyto/node_modules/.pnpm/nx@17.0.2_@swc+core@1.3.91/node_modules/nx/bin/run-executor.js:59:28)

This is with Nx 17.0.2

BorisTB commented 12 months ago

The possible fix is processEnv: { …environmentVariables }, but idk how to write tests for it

github-actions[bot] commented 11 months ago

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

nocnokneo commented 11 months ago

Still an issue.

nx serve foo # broken: ${vars} in vite .env files do not get expanded cd apps/foo && vite serve # works

On Thu, Nov 9, 2023, 7:20 PM github-actions[bot] @.***> wrote:

This issue has been automatically marked as stale because it hasn't had any recent activity. It will be closed in 14 days if no further activity occurs. If we missed this issue please reply to keep it active. Thanks for being a part of the Nx community! 🙏

— Reply to this email directly, view it on GitHub https://github.com/nrwl/nx/issues/19437#issuecomment-1804879865, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAD5XEHM4TT7NP3ATYAY3JDYDVXL5AVCNFSM6AAAAAA5SQIWX2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUHA3TSOBWGU . You are receiving this because you are subscribed to this thread.Message ID: @.***>

PierreNicoletti commented 10 months ago

I confirm this is still an issue. Expanded env variables can't be used.

mattfysh commented 9 months ago

yikes, there should probably be a test for this. is everything okay at nx, this is beyond low hanging fruit. this is fruit that's already dropped to the ground and slowly rotting away

BioPaulK commented 2 days ago

Are there any updates on this issue? We're running into the same thing right now, we can't use env var expansion in any of our tasks.