bun icon indicating copy to clipboard operation
bun copied to clipboard

Compatibility with `node` and `yarn`: `process.version`

Open coderaiser opened this issue 3 years ago • 12 comments

1008 |     ? Number(process.env.YARGS_MIN_NODE_VERSION)
1009 |     : 12;
1010 | if (process1008 |     ? Number(process.env.YARGS_MIN_NODE_VERSION)
1009 |     : 12;
1010 | if (process && process.version) {
1011 |     const major = Number(process.version.match(/v([^.]+)/)[1]);
1012 |     if (major < minNodeVersion) {
1013 |         throw Error(`yargs parser supports a minimum Node.js version of ${minNodeVersion}. Read our version support policy: https://github.com/yargs/yargs-parser#supported-nodejs-versions`);
                  ^
 error: yargs parser supports a minimum Node.js version of 12. Read our version support policy: https://github.com/yargs/yargs-parser#supported-nodejs-versions
      at /Users/coderaiser/putout/packages/putout/node_modules/yargs-parser/build/index.cjs:1013:14
      at bun:wrap:1:16430
      at /Users/coderaiser/putout/packages/putout/lib/cli/index.js:5:28
      at bun:wrap:1:16430
      at /Users/coderaiser/putout/packages/putout/bin/putout.mjs:10:0&& process.version) {

Setting YARGS_MIN_NODE_VERSION=0.1.2 solves the issue, but then I see:

error: Could not resolve: "readline". Maybe you need to "bun install"?

const readline = require('readline');

Without a trailing newline.

Also a little question: is it possible to pass command line arguments to script I run with bun?

bun bin/putout.mjs -f dump

-f dump is handled by bun, but I need it to use other formatter that don't uses readline :).

coderaiser avatar Jul 08 '22 08:07 coderaiser

  1. The readline issue is documented in #311
  2. You can pass CLI arguments by writing bun bin/putout.mjs -- -f dump

FinnRG avatar Jul 08 '22 08:07 FinnRG

The version issue was also resolved by #392. Please close this issue.

FinnRG avatar Jul 08 '22 09:07 FinnRG

Thanks for a quick response!

coderaiser avatar Jul 08 '22 10:07 coderaiser

The version issue was also resolved by #392. Please close this issue.

this only modifies the napi function, and not process.version. broken on bun 0.1.3 still.

paperclover avatar Jul 12 '22 03:07 paperclover

@davecaruso You're right, I'm sorry about that. Since we probably don't want to change our current process.version output, we could expose a flag like --set-version which replaces every occurrence of process.version with the set version (similar to how we replace env variable).

FinnRG avatar Jul 12 '22 08:07 FinnRG

we could expose a flag like --set-version which replaces every occurrence of process.version with the set version (similar to how we replace env variable).

I dislike this because it now requires anyone using yargs or depending on a package that uses it, all devs and users need that flag. It feels the same as YARGS_MIN_NODE_VERSION, and env variable you have to specify everywhere. that would still be better than nothing, but a lot of people will try this (and any other package that may do this type of check) and complain it doesn't work.

but i do think process.version should contain the bun one since it's bun that we're running and not node. maybe they can convince yargs to switch to reading process.versions.node, but i kind of feel like they won't implement that change.

paperclover avatar Jul 12 '22 08:07 paperclover

@davecaruso We should definitely try to get this into yargs. As a temporary workaround, you can use this flag: --define process.version:"\"13.0.0\"", which replaces every occurrence of process.version with the specified version.

FinnRG avatar Jul 12 '22 08:07 FinnRG

I think the best possible process.version would be Nodejs LTS that Bun going to be compatible with and for Bun version it can be process.bunVersion or something like this.

coderaiser avatar Jul 12 '22 08:07 coderaiser

Good news: The yargs-parser team seems to be open about adding feature detection for bun: https://github.com/yargs/yargs-parser/issues/447#issuecomment-1183550650

FinnRG avatar Jul 13 '22 21:07 FinnRG

and better news, i just opened PR https://github.com/yargs/yargs-parser/pull/450 which should fix it. this obviously wont fix any other libraries doing this, but they should all change to this way of checking node.

paperclover avatar Jul 13 '22 23:07 paperclover

~If someone really needs to edit process.version, you'll need two files at best. One will do:~

// @ts-expect-error
process.version = 300;

await import("./other-file");

~And the other one then imports the modules like yargs, etc.~ define flag is better

xhyrom avatar Jul 17 '22 06:07 xhyrom

btw, the process version check has been fixed, but i believe theres some other stuff we need before yargs fully works on bun; however those steps are our responsibility here and not theirs.

paperclover avatar Aug 04 '22 20:08 paperclover

process.version is supported as of 0.2.2. For other compatibility issues, please file another issue and we'll track it down.

Electroid avatar Nov 01 '22 23:11 Electroid