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

fix(robo): reload environment variables in dev mode #355

Merged
merged 5 commits into from
Nov 10, 2024

Conversation

icmedev
Copy link
Contributor

@icmedev icmedev commented Nov 3, 2024

This PR fixes environment variable reloading in dev mode by:

  • Stop env vars from being inherited in worker threads
  • Only pass needed vars (PATH, FORCE_COLOR, NODE_ENV)
  • Makes .env changes work right away in dev mode

Fixes #302, #352

Copy link

vercel bot commented Nov 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 3, 2024 4:51pm

@@ -308,7 +308,12 @@ export async function buildAsync(command: string | null, config: Config, verbose

logger.debug(`> ${pkgManager} ${args.join(' ')}`)
const childProcess = spawn(pkgManager, args, {
env: { ...process.env, FORCE_COLOR: '1' },
env: {
// Only pass through essential vars, not the entire env
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm, I'm not sure if this is a good idea. There are several reasons why one would want environment variables to be inherited.

Example 1: Inline variables

Sometimes people pass environment variables inline as part of a script, either as a replacement or as an addition to .env loading.

  "dev": "NODE_ENV=test robo dev",
  "start": "NODE_ENV=prod robo start"

Example 2: Doppler

This is a service I personally use a lot in organizations. Doppler replaces the need for .env files. Similar to example 1, it injects variables to the process expecting it to be inherited.

  "dev": "doppler run -- robo dev"

Passing only essential variables completely breaks the above two scenarios which are commonly used.

A major type-safe upgrade to our internal `env` now powered by a new `Env` class. This new class can be used to load variables as well as use them safely in a nested fashion.
For consistency with external APIs. We also now accept limited options.
Load functions now also return the newly loaded object. We'll be using this shortly.
@Pkmmte Pkmmte merged commit f6ac2da into Wave-Play:main Nov 10, 2024
1 of 2 checks passed
@Wave-Play Wave-Play locked and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

patch(robo): reload environment variables in dev mode
2 participants