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

Set npm_config_user_agent when running scripts #25342

Closed
redabacha opened this issue Sep 1, 2024 · 8 comments · Fixed by #26639
Closed

Set npm_config_user_agent when running scripts #25342

redabacha opened this issue Sep 1, 2024 · 8 comments · Fixed by #26639
Assignees
Labels
feat new feature (which has been agreed to/accepted) task runner related to deno task

Comments

@redabacha
Copy link

this is an env var that many scripts use to detect the package manager that ran the script. all major package managers including bun set this env var and it would be great if deno did too.

@lucacasonato lucacasonato added feat new feature (which has been agreed to/accepted) task runner related to deno task labels Sep 2, 2024
@lucacasonato
Copy link
Member

All of these:

npm_config_global_prefix=/opt/homebrew
npm_config_noproxy=
npm_config_local_prefix=/Users/lucacasonato/projects/local/test
npm_config_globalconfig=/opt/homebrew/etc/npmrc
npm_config_userconfig=/Users/lucacasonato/.npmrc
npm_config_init_module=/Users/lucacasonato/.npm-init.js
npm_config_npm_version=10.7.0
npm_config_node_gyp=/opt/homebrew/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js
npm_config_cache=/Users/lucacasonato/.npm
npm_lifecycle_script=env | grep 'npm_config_'
npm_config_user_agent=npm/10.7.0 node/v22.1.0 darwin arm64 workspaces/false
npm_config_prefix=/opt/homebrew

@benmccann
Copy link

benmccann commented Oct 15, 2024

This environment variable is used by numerous CLIs to detect which package manager to run to install a package. E.g. Svelte uses it when creating a new project or adding integrations to an existing project

https://www.npmjs.com/package/which-pm-runs and https://www.npmjs.com/package/package-manager-detector are two highly used packages, which make use of this

@bartlomieju
Copy link
Member

@benmccann thanks for pointing this out. Would it be okay if we only supported npm_config_user_agent env var to be able to figure out that you're currently running in Deno? If so would value like Deno/v2.x.y be enough?

@benmccann
Copy link

That would be enough for package-manager-detector used by Svelte, but would break the which-pm-runs library used elsewhere, which expects there to be at least one space: https://github.com/zkochan/packages/blob/221d0b81da4ecaa3489edbc158376fec52ca75eb/which-pm-runs/index.js#L11

to be safe I'd probably print something like:

deno/2.x.y npm/? deno/2.x.y

you can see examples of what other package managers do here: https://github.com/antfu-collective/package-manager-detector/blob/d8655170eae1c786de4eda1854af16480575fbb6/test/user-agent.spec.ts#L9

@redabacha
Copy link
Author

yep it should be in the same style of format as how npm defines it, https://github.com/npm/cli/blob/f75da94f3bed6c0b637044e88098ec354cf302b0/workspaces/config/lib/definitions/definitions.js#L2107-L2126. e.g. with bun i get bun/1.1.30 npm/? node/v22.6.0 darwin arm64. deno already provides the node compat version in process.versions.node so could just use that here as well.

@bartlomieju
Copy link
Member

Thanks for replies, @dsherret suggests that this should only be set for subprocesses spawned with node:child_process APIs. If that's the case it should be an easy fix and we can try to get it out in 2.0.1 tomorrow/Thursday.

@lucacasonato
Copy link
Member

@bartlomieju i think this should only be set for deno task script execution

@redabacha
Copy link
Author

redabacha commented Oct 15, 2024

well it should also be set for any kind of script you're running that's not a local one, i.e. deno -A npm:create-next-app should be setting the env var but running deno --allow-env ./main.ts shouldn't (well that's what bun is currently doing at least as the only other runtime+pm - but with that said i don't see any harm if that was to be included there too though 🤷‍♂️).

bartlomieju pushed a commit that referenced this issue Nov 5, 2024
…sks (#26639)

Fixes #25342.

Still not sure on the exact user agent to set (should it include
`node`?).

After this PR, here's the state of running some `create-*` packages
(just ones I could think of off the top of my head):

| package                  | prints/runs/suggests deno install | notes |
| ---------------- | ------------- | ------ |
| `create-next-app` | ❌ | falls back to npm, needs a PR
([code](https://github.com/vercel/next.js/blob/c32e2802097c03fd9f95b1dae228d6e0257569c0/packages/create-next-app/helpers/get-pkg-manager.ts#L3))
| `sv create` | ❌ | uses `package-manager-detector`, needs a PR
([code](https://github.com/antfu-collective/package-manager-detector/tree/main))
| `create-qwik` | ✅ | runs `deno install` but suggests `deno start`
which doesn't work (should be `deno task start` or `deno run start`)
| `create-astro` | ✅ | runs `deno install` but suggests `npm run dev`
later in output, probably needs a PR
| `nuxi init` | ❌ | deno not an option in dialog, needs a PR
([code](https://github.com/nuxt/cli/blob/f04e2e894446f597da9d971b7eb03191d5a0da7e/src/commands/init.ts#L96-L102))
| `create-react-app` | ❌ | uses npm
| `ng new` (`@angular/cli`) | ❌ | uses npm
| `create-vite` | ✅ | suggests working deno commands 🎉 
| `create-solid` |  ❌ | suggests npm commands, needs PR

It's possible that fixing `package-manager-detector` or other packages
might make some of these just work, but haven't looked too carefully at
each
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat new feature (which has been agreed to/accepted) task runner related to deno task
Projects
None yet
5 participants