-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(cli): load env vars to memory via vc env
#13440
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
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 1e90243 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this would be better suited as a new subcommand for Did not realize this had already been deliberated, ignore this suggestion.env
.
it('inherits variables', async () => { | ||
const args = ['not-a-command']; | ||
|
||
client.setArgv('env', ...args); | ||
const exitCode = await env(client); | ||
expect(exitCode).toEqual(2); | ||
expect(exitCode).toEqual(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you update this test to verify that the variables are passed to the child process?
await expect(exitCodePromise).resolves.toBe(1); | ||
expect(client.stderr).toOutput( | ||
'Invalid number of arguments. Usage: `vercel env <process>`' | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a regression in the helpfulness of the text output of vercel env
. It goes from
Error: Please specify a valid subcommand: ls | add | rm | pull
▲ vercel env command
Interact with Environment Variables for a Project
Commands:
add name [environment] Add an Environment Variable (see examples below)
list [environment] [git-branch] List all Environment Variables for a Project
pull [filename] Pull all Development Environment Variables from the cloud and write to
a file [.env.local]
remove name [environment] Remove an Environment Variable (see examples below)
to
Error: Invalid number of arguments. Usage: `vercel env <process>`
See also: `vercel env --help`
ELIFECYCLE Command failed with exit code 1.
which requires an additional vercel env --help
to discover what commands are available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do feel we should have a check for if there's no args provided to vercel env
and show vercel env --help
instead of an "Invalid number of arguments"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree! Will fix!
const child = spawn(command, rest, { | ||
env: { ...process.env, ...environmentVariables }, | ||
stdio: 'inherit', | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't properly setup signal forwarding from the vercel
process to the child process. Can you make sure that sending SIGINT
to vercel
results in the child process shutting down? I think you'll need to setup a process group where vercel
is the leader.
# Get process tree of the vercel cli
[0 olszewski@macbookpro] /Users/olszewski/code/vercel/vercel $ pstree -p 88056
-+= 00001 root /sbin/launchd
\-+= 16827 olszewski tmux
\-+= 98314 olszewski -zsh
\-+= 88056 olszewski node /Users/olszewski/.nvm/versions/node/v20.19.2/bin/pnpm vercel env pnpm dev --cwd /Users/olszewski/code/vercel/turborepo/docs/si
\-+- 88077 olszewski node /Users/olszewski/code/vercel/vercel/packages/cli/node_modules/.bin/../../../../node_modules/.pnpm/ts-node@10.9.1_@swc+core@1
\-+- 88123 olszewski node /Users/olszewski/.nvm/versions/node/v20.19.2/bin/pnpm dev
\-+- 88145 olszewski node /Users/olszewski/code/vercel/turborepo/docs/site/node_modules/.bin/../next/dist/bin/next dev --turbopack
|--- 88210 olszewski next-server (v15.4.0-canary.23)
\--- 88233 olszewski /Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/@esbuild+darwin-arm64@0.25.2/node_modules/@esbuild/darwin-arm64/b
# Send an interrupt to vercel cli
[0 olszewski@macbookpro] /Users/olszewski/code/vercel/vercel $ kill -s INT 88056
# Check if next server is running, note that the `pnpm dev` is no longer a child of vercel, but the root process
[0 olszewski@macbookpro] /Users/olszewski/code/vercel/vercel $ pstree -p 88123
-+= 00001 root /sbin/launchd
\-+- 88123 olszewski node /Users/olszewski/.nvm/versions/node/v20.19.2/bin/pnpm dev
\-+- 88145 olszewski node /Users/olszewski/code/vercel/turborepo/docs/site/node_modules/.bin/../next/dist/bin/next dev --turbopack
|-+- 88210 olszewski next-server (v15.4.0-canary.23)
| |--- 89346 olszewski node /Users/olszewski/code/vercel/turborepo/docs/site/.next/transform.js 52496
| \--- 89347 olszewski node /Users/olszewski/code/vercel/turborepo/docs/site/.next/transform.js 52498
\--- 88233 olszewski /Users/olszewski/code/vercel/turborepo/node_modules/.pnpm/@esbuild+darwin-arm64@0.25.2/node_modules/@esbuild/darwin-arm64/bin/esbui
output.error(getInvalidSubcommand(COMMAND_CONFIG)); | ||
output.print(help(envCommand, { columns: client.stderr.columns })); | ||
return 2; | ||
return pull(client, args.concat('--memory')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think creating a new exec
subcommand is the safer option here. Having inferred subcommands can really end up restricting changes we can make to vercel env
in the future without being a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good use case for no subcommands, but we can do this as a follow-up after further discussion, thanks!
Co-authored-by: Chris Olszewski <chrisdolszewski@gmail.com>
Right now, when you want to copy environment variables to your machine, your only option is to pull them into a file via
vc env pull .env.local
.Also,
vc env
is just printing whatvc env --help
would.We could utilize this command to load the environment variables for a subprocess, instead of having to write them to a file.
Example:
This has a security benefit since we do not preserve the environment variables after the process exits.
The implementation reuses
vc env pull
and introduces a new--memory
flag, which is added if thevc env
command is called without any known subcommand.Usage:
Screencast.from.2025-06-11.15-47-39.webm
You can test it via