yargs / yargs-parser

:muscle: the mighty option parser used by yargs
http://yargs.js.org/
ISC License
491 stars 120 forks source link

Version number check fails on Bun #447

Closed robogeek closed 2 years ago

robogeek commented 2 years ago

I decided to give Bun a try by running the Mocha test suite of one of my packages. The very first failure was in yargs-parser like so:

$ bun run ./node_modules/mocha/bin/mocha.js test-partials.js test-parse.js test-process.js 
986 |     ? Number(process.env.YARGS_MIN_NODE_VERSION)
987 |     : 10;
988 | if (process && process.version) {
989 |     const major = Number(process.version.match(/v([^.]+)/)[1]);
990 |     if (major < minNodeVersion) {
991 |         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 10. Read our version support policy: https://github.com/yargs/yargs-parser#supported-nodejs-versions
      at /home/david/Projects/akasharender/mahabhuta/test/node_modules/yargs-parser/build/index.cjs:991:14
      at bun:wrap:1:16354
      at /home/david/Projects/akasharender/mahabhuta/test/node_modules/mocha/lib/cli/options.js:12:28
      at bun:wrap:1:16354
      at /home/david/Projects/akasharender/mahabhuta/test/node_modules/mocha/bin/mocha.js:13:30

Clearly, Bun isn't going to give a value for process.version that is compatible with Node.js versions. With this script

console.log(process.version);

console.log(process);

I get this output:

$ bun run pv.js 
v0.1.2
{ ... }

For reference: https://github.com/Jarred-Sumner/bun/issues/462

bcoe commented 2 years ago

👍 sounds like we should add some feature detection for Bun.

paperdave commented 2 years ago

you should use process.versions.node (bun sets this to 16.14.0 right now). if that isn't supported on old versions of node, then fallback to process.version. is that possible?

paperdave commented 2 years ago

it seems process.versions (with an s) was added in v0.2.0, so i think its safe to just change all instances of it.