fix(cli): set `npm_config_user_agent` when running npm packages or tasks #26639

nathanwhit · 2024-10-30T04:58:43Z

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)
`sv create`	❌	uses `package-manager-detector`, needs a PR (code)
`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)
`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

birkskyum · 2024-10-30T13:35:28Z

@nathanwhit , I had a look at adding deno to the solid-cli just now, but it's quite tricky to work on with deno specifically. This is because I keep hitting errors with missing "node:" prefixes when testing the package locally. All the deno node-compat that takes effect when running e.g. deno run -A npm:create-solid appear to be missing whenexecuting the package locally deno run index.ts or after building deno run index.mjs, before it's published to npm.

Repro of this:

clone https://github.com/solidjs-community/solid-cli
pnpm i, pnpm run build
deno run ./packages/create-solid/src/index.ts

➜ deno run ./packages/create-solid/src/index.ts
error: Relative import path "fs" not prefixed with / or ./ or ../
  hint: If you want to use a built-in Node module, add a "node:" prefix (ex. "node:fs").
    at file:///Users/admin/repos/solid-cli/packages/commands/dist/handlers/new.mjs:1:485

bartlomieju · 2024-10-30T15:28:24Z

@birkskyum try running with --unstable-sloppy-imports. Then the node: prefixes shouldn't be necessary. I think the hint there is wrong, by not suggesting it.

birkskyum · 2024-10-30T15:32:53Z

@bartlomieju , it's a good idea to have it in the hint - am I using it correctly? (deno 2.0.3)

➜ deno run --unstable-sloppy-imports ./packages/create-solid/dist/index.mjs 
error: Relative import path "zlib" not prefixed with / or ./ or ../
  hint: If you want to use a built-in Node module, add a "node:" prefix (ex. "node:zlib").
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:28:16

➜ deno run --unstable-sloppy-imports ./packages/create-solid/src/index.ts  
error: Relative import path "fs" not prefixed with / or ./ or ../
  hint: If you want to use a built-in Node module, add a "node:" prefix (ex. "node:fs").
    at file:///Users/admin/repos/solid-cli/packages/commands/dist/handlers/new.mjs:1:485

bartlomieju · 2024-10-30T15:35:17Z

@birkskyum I was close :D it's actually --unstable-bare-node-builtins

birkskyum · 2024-10-30T15:38:53Z

That works better, is there a trick for allowing importing .json files (such as package.json) too? This runs

Gives:

➜ deno run --unstable-bare-node-builtins ./packages/create-solid/src/index.ts     
Warning Resolving "fs" as "node:fs" at file:///Users/admin/repos/solid-cli/packages/commands/dist/handlers/new.mjs:1:485. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "util" as "node:util" at file:///Users/admin/repos/solid-cli/packages/utils/dist/index.mjs:5:158. If you want to use a built-in Node module, add a "node:" prefix.
error: Expected a JavaScript or TypeScript module, but identified a Json module. Consider importing Json modules with an import attribute with the type of "json".
  Specifier: file:///Users/admin/repos/solid-cli/packages/create-solid/package.json
    at file:///Users/admin/repos/solid-cli/packages/create-solid/src/index.ts:5:25

Because of

import { version } from "../package.json";

I tried adding the with { type: 'json' } too, but still get:

error: Uncaught SyntaxError: The requested module '../package.json' does not provide an export named 'version'
import { version } from "../package.json" with { type: "json" };

bartlomieju · 2024-10-30T15:41:24Z

No, I don't think we have a trick for that, it's strange that it allows to destructure JSON imports, I don't think it's supported in Node either. Shouldn't this code be somehow handled by a bundler first?

birkskyum · 2024-10-30T15:42:51Z

You're right - node, like deno, don't support the named json export, but bun does which I had tried prior:

➜ bun run  ./packages/create-solid/src/index.ts                                   
┌  
 Create-Solid v0.5.13
│
◆  Project Name
│  solid-project
└

If I instead bundle, and run the dist/index.mjs with deno, I then get this issue with global not being defined - is there a flag for that?

➜ deno run -A --unstable-bare-node-builtins ./packages/create-solid/dist/index.mjs
Warning Resolving "module" as "node:module" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:2:31. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "fs" as "node:fs" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:16:150. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "path" as "node:path" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:17:57. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "fs/promises" as "node:fs/promises" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:18:42. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "stream" as "node:stream" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:19:26. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "stream/promises" as "node:stream/promises" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:20:26. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "os" as "node:os" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:21:24. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "events" as "node:events" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:22:34. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "assert" as "node:assert" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:26:16. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "buffer" as "node:buffer" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:27:36. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "zlib" as "node:zlib" at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:28:16. If you want to use a built-in Node module, add a "node:" prefix.
error: Uncaught (in promise) ReferenceError: global is not defined
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:4663
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:459
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:5290
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:459
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:6869
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:459
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:8070
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:459
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:9542
    at file:///Users/admin/repos/solid-cli/packages/create-solid/dist/index.mjs:33:459

birkskyum · 2024-10-30T15:55:57Z

@bartlomieju , if I add with {type: "json"} to the import, and make it not-named like this:

import packageJson from "../package.json" with { type: "json" };

and just use packageJson.version

then deno can run the index.ts 🎉

➜ deno run -A --unstable-bare-node-builtins ../packages/create-solid/src/index.ts
Warning Resolving "fs" as "node:fs" at file:///Users/admin/repos/solid-cli/packages/commands/dist/handlers/new.mjs:1:485. If you want to use a built-in Node module, add a "node:" prefix.
Warning Resolving "util" as "node:util" at file:///Users/admin/repos/solid-cli/packages/utils/dist/index.mjs:5:158. If you want to use a built-in Node module, add a "node:" prefix.
┌  
 Create-Solid v0.5.13
│
◇  Project Name
│  solid-start-created-with-deno

The bundled dist/index.mjs still gives the error about global mentioned above - but now it's a lot easier to investige adding the deno cli as an option to the solid-cli. Thanks!

bartlomieju · 2024-10-30T16:20:44Z

@birkskyum no flag yet to enable global, but there's a PR that will add it: #26617

birkskyum · 2024-10-31T21:49:17Z

Just realized it's a lot easier to work on adding support for deno in the various cli's after this pr is merged.

bartlomieju

LGTM 👍

bartlomieju · 2024-10-31T23:35:27Z

cli/task_runner.rs

@@ -204,7 +210,7 @@ impl ShellCommand for NpmCommand {
    mut context: ShellCommandContext,
  ) -> LocalBoxFuture<'static, ExecuteResult> {
    if context.args.first().map(|s| s.as_str()) == Some("run")
-      && context.args.len() > 2
+      && context.args.len() >= 2


Why is it now >=?

Turned out this code wasn't triggering. context.args doesn't include the command name (npm), so the first arg is run and the second arg is the name of the script. So npm run foo was missing this branch.

bartlomieju · 2024-10-31T23:50:05Z

Opened a PR to package-manager-detector: antfu-collective/package-manager-detector#25

birkskyum · 2024-11-01T18:25:37Z

Hmm, with canary deno 2.0.4+4774eab, the commit after this, I still see:

console.log('agent:', process.env.npm_config_user_agent) -> agent: undefined

nathanwhit · 2024-11-01T18:33:45Z

Hmm, with canary deno 2.0.4+4774eab, the commit after this, I still see:

console.log('agent:', process.env.npm_config_user_agent) -> agent: undefined

It’s only set when running npm: entrypoints and running scripts currently. We could always set it, but not sure if that’s desirable. WDYT @bartlomieju?

birkskyum · 2024-11-01T18:40:01Z

@nathanwhit , ah that explains it, thanks

So this doesn't work

deno run --unstable-bare-node-builtins -A ./packages/create-solid/src/index.ts -> agent: undefined

And this does work:

configure package.json

{
  "scripts": {
    "runwithagent": "deno run --unstable-bare-node-builtins -A ./packages/create-solid/src/index.ts",
  }
}

deno task runwithagent -> agent: deno/2.0.4 npm/? deno/2.0.4 macos aarch64

bartlomieju · 2024-11-02T13:56:36Z

Hm, I'm not sure. Maybe it should also be set if there's a local package.json file?

…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

nathanwhit added 3 commits October 18, 2024 14:05

npm config user agent

a7e4aa3

always apply

2089475

cleanup + test

7f4d229

nathanwhit mentioned this pull request Oct 30, 2024

fix: set 'npm_config_user_agent' env var when installing or running npm deps #26311

Closed

birkskyum mentioned this pull request Oct 30, 2024

[solid-create] Add deno to list of package managers / task runners solidjs-community/solid-cli#55

Open

birkskyum mentioned this pull request Oct 30, 2024

Add with { type: "json" } to package.json import solidjs-community/solid-cli#56

Merged

set for all tasks, better test

cc43288

nathanwhit force-pushed the npm-config branch from 0a75502 to cc43288 Compare October 30, 2024 17:33

fix test output

fa1e3fb

birkskyum mentioned this pull request Oct 31, 2024

Add deno support for solid cli solidjs-community/solid-cli#57

Merged

bartlomieju approved these changes Oct 31, 2024

View reviewed changes

bartlomieju mentioned this pull request Oct 31, 2024

feat: add deno as package manager antfu-collective/package-manager-detector#25

Merged

nathanwhit merged commit 04ae1a5 into denoland:main Nov 1, 2024
17 checks passed

nathanwhit deleted the npm-config branch November 1, 2024 05:19

birkskyum mentioned this pull request Nov 1, 2024

Add support for Deno as package manager antfu-collective/package-manager-detector#21

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): set `npm_config_user_agent` when running npm packages or tasks #26639

fix(cli): set `npm_config_user_agent` when running npm packages or tasks #26639

nathanwhit commented Oct 30, 2024

birkskyum commented Oct 30, 2024 •

edited

Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 •

edited

Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 •

edited

Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 30, 2024 •

edited

Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 31, 2024 •

edited

Loading

bartlomieju left a comment

bartlomieju Oct 31, 2024

nathanwhit Nov 1, 2024

bartlomieju commented Oct 31, 2024

birkskyum commented Nov 1, 2024

nathanwhit commented Nov 1, 2024

birkskyum commented Nov 1, 2024 •

edited

Loading

bartlomieju commented Nov 2, 2024

fix(cli): set npm_config_user_agent when running npm packages or tasks #26639

fix(cli): set npm_config_user_agent when running npm packages or tasks #26639

Conversation

nathanwhit commented Oct 30, 2024

birkskyum commented Oct 30, 2024 • edited Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 • edited Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 • edited Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 30, 2024 • edited Loading

birkskyum commented Oct 30, 2024 • edited Loading

bartlomieju commented Oct 30, 2024

birkskyum commented Oct 31, 2024 • edited Loading

bartlomieju left a comment

Choose a reason for hiding this comment

bartlomieju Oct 31, 2024

Choose a reason for hiding this comment

nathanwhit Nov 1, 2024

Choose a reason for hiding this comment

bartlomieju commented Oct 31, 2024

birkskyum commented Nov 1, 2024

nathanwhit commented Nov 1, 2024

birkskyum commented Nov 1, 2024 • edited Loading

So this doesn't work

And this does work:

bartlomieju commented Nov 2, 2024

fix(cli): set `npm_config_user_agent` when running npm packages or tasks #26639

fix(cli): set `npm_config_user_agent` when running npm packages or tasks #26639

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 30, 2024 •

edited

Loading

birkskyum commented Oct 31, 2024 •

edited

Loading

birkskyum commented Nov 1, 2024 •

edited

Loading