-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Neutral JavaScript runtime support (Deno, Bun, etc) #2169
Comments
Fixes and closes tj#2169 by using Node-prefixed imports. Signed-off-by: Sam Gammon <[email protected]>
Using the node prefix is probably fine, but I haven't reproduced a requirement to do so in a simple program. What failure do you see? (I haven't tried reproducing using your full package yet.) I can run a simple program with Deno without code changes: import { Command } from "npm:commander";
const program = new Command();
program.parse(); % deno run --allow-read prefixed.ts --help
Usage: prefixed [options] <target>
Options:
-d, --debug
-h, --help display help for command It also works if I import without the
|
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
- feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]>
* feat: support all runtimes - feat: support all popular js runtimes - patches for `glob`, `minipass`, and `path-scurry` - upstream prs (listed below) - test entrypoint commands - test: add test entrypoints for each major runtime - test: add scripts to test entrypoint with each major runtime - chore: sync lockfiles Related Issues - isaacs/node-glob#580 - isaacs/path-scurry#16 - isaacs/minipass#54 - tj/commander.js#2169 Upstream PRs - isaacs/node-glob#581 - isaacs/minipass#55 - isaacs/path-scurry#17 - tj/commander.js#2170 Signed-off-by: Sam Gammon <[email protected]> * chore: version bump → `1.0.3` Signed-off-by: Sam Gammon <[email protected]> --------- Signed-off-by: Sam Gammon <[email protected]> Signed-off-by: Sam Gammon <[email protected]>
@shadowspawn In my Commander app, when I run:
I get:
Deno latest is I can't address these with my build system - I can try importing commander with Applying the enclosed patch fixes the issue for me and it runs smoothly as expected. |
I would like to see this for myself to make sure what problem is being resolving. If I understand correctly, after bundling your application with esbuild you get runtime errors when using Deno to run the application. The example you give shows
|
❤️ |
No worries! That is reasonable.
Yes, there are some other libs I am using which have this same issue (I have posted PRs for those too). For Commander, it is the builtin modules
👆 Ignoring scripts will skip the
Each |
I get multiple errors when I try the build. (On Building on Mac (Apple Silicon), in case that makes a difference. % bun --version
1.0.36
% pnpm --version
8.15.5
% node --version
v20.11.1
% pnpm run build
> [email protected] build /Users/john/Documents/Sandpits/commander/issues/2169/hashlock
> bun ./scripts/build.mjs && bun run build:standalone && bun run build:types && cp -fv ./dist/index.d.ts ./dist/index.d.mts
Building 'verify-hashes'...
- Building 'hashlock' (lib, cjs)...
- Building 'hashlock' (CLI, esm)...
- Building 'hashlock' (action)...
✘ [ERROR] Could not resolve "glob"
src/main.ts:19:21:
19 │ import { glob } from 'glob'
╵ ~~~~~~
The Yarn Plug'n'Play manifest forbids importing "glob" here because it's not listed as a
dependency of this package:
... |
@shadowspawn That is odd--it should not be using Yarn PNP. I use 'pnpm install' |
I'm away from my machine but I will look into this shortly. From the top of my head
All told you can probably run "entry:*" whether the build passes or not, because the entry points there are pure TypeScript and thus don't need to be built. |
The I tried downloading a build package to examine the build files, but got an error for that too and didn't pursue further: % npm install hashlock
npm ERR! code 127
npm ERR! path /Users/john/Documents/Sandpits/commander/issues/2169/bin/node_modules/hashlock
npm ERR! command failed
npm ERR! command sh -c patch-package
npm ERR! sh: patch-package: command not found Edit: installed |
I assume the transpile/bundle is effectively removing the implicit context of Commander code being in an |
@shadowspawn Yes, I think that's right. When it runs in CLI form it is fully bundled; the standalone executable also bundles this code. I really can't say why you're getting the PNP issue... I can prepare a smaller repro, or perhaps a devcontainer? I don't build with Deno, I just want the final target to be able to run under it. Thanks again for helping me diagnose this. I'd be happy to prepare a smaller repro in a project or devcontainer as well, maybe that can get around any env issues. |
Hey there @tj,
First of all, big fan of
commander
; I've used it for a long time, so, longtime user first time contributor 😁. I have a patch forcommander
which makes it work neutrally on all JS runtimes; it was already working fine on Bun and Node, but with the following two small changes:node:*
process
into an explicit symbol... it will now run smoothly on Deno as well.
For context: I'm trying to ship a tool downstream,
hashlock
, which uses commander, but I want it to be able to run on any JS runtime.Distribution diff:
PR incoming shortly.
This issue body was partially generated by patch-package.
The text was updated successfully, but these errors were encountered: