Improve command line argument parsing

lib/cli.js

@@ -1,84 +1,78 @@
+const assert = require('assert');
 const minimist = require('minimist');
 const { resolve } = require('path');
 
-const { defaultConfig, getConfig } = require('./cfg');
+const { getConfig } = require('./cfg');
 
-const configKeys = Object.keys(defaultConfig);
+const arrayify = v => (Array.isArray(v) ? [...v] : [v]);
+const argify = key => ({ arg: `--${key}`, key });
 
-function resolvePath(unresolvedPath) {
-  return resolve(process.cwd(), unresolvedPath);
-}
+const resolvePath = p => resolve(process.cwd(), p);
 
-const doubleDash = s => /^--/.test(s);
-const dash = s => /^-[^-]*$/.test(s);
+const nodeAlias = { require: 'r' };
+const nodeBoolean = ['expose_gc'];
+const nodeOptional = ['inspect', 'inspect-brk'];
+const nodeString = ['require'];
 
-function getFirstNonOptionArgIndex(args) {
-  for (let i = 2; i < args.length; i += 1) {
-    if (!doubleDash(args[i]) && !dash(args[i]) && !dash(args[i - 1] || '')) return i;
-  }
+const nodeDevBoolean = ['clear', 'dedupe', 'fork', 'notify', 'poll', 'respawn', 'vm'];
+const nodeDevNumber = ['debounce', 'deps', 'interval'];
+const nodeDevString = ['graceful_ipc', 'ignore', 'timestamp'];
 
-  return args.length - 1;
-}
+const alias = Object.assign({}, nodeAlias);
+const boolean = [...nodeBoolean, ...nodeDevBoolean];
+const string = [...nodeString, ...nodeDevString];
 
-function unique(k) {
-  const seen = [];
-  return o => {
-    if (!seen.includes(o[k])) {
-      seen.push(o[k]);
-      return true;
-    }
-    return false;
-  };
-}
+const nodeArgsReducer = opts => (out, { arg, key }) => {
+  const value = opts[key];
 
-module.exports = argv => {
-  const unknownArgs = [];
+  if (typeof value === 'boolean') {
+    value && out.push(arg);
+  } else if (typeof value !== 'undefined') {
+    arrayify(value).forEach(v => {
+      if (arg.includes('=')) {
+        out.push(`${arg.split('=')[0]}=${v}`);
+      } else {
+        out.push(`${arg}=${v}`);
+      }
+    });
+  }
+
+  delete opts[key];
+
+  return out;
+};
 
-  const scriptIndex = getFirstNonOptionArgIndex(argv);
+const nodeOptionalFactory = args => arg => {
+  const isNodeOptional = nodeOptional.includes(arg.substring(2));
+  if (isNodeOptional) args.push(arg);
+  return !isNodeOptional;
+};
 
-  const script = argv[scriptIndex];
-  const scriptArgs = argv.slice(scriptIndex + 1);
-  const devArgs = argv.slice(2, scriptIndex);
+const unknownFactory = args => arg => {
+  const [, key] = Object.keys(minimist([arg]));
+  key && !nodeDevNumber.includes(key) && args.push({ arg, key });
+};
 
-  const opts = minimist(devArgs, {
-    boolean: ['clear', 'dedupe', 'fork', 'notify', 'poll', 'respawn', 'vm'],
-    string: ['graceful_ipc', 'ignore', 'timestamp'],
-    default: getConfig(script),
-    unknown: arg => {
-      const key = Object.keys(minimist([arg]))[1];
+module.exports = argv => {
+  const nodeOptionalArgs = [];
+  const args = argv.slice(2).filter(nodeOptionalFactory(nodeOptionalArgs));
 
-      if (!configKeys.includes(key)) {
-        unknownArgs.push({ arg, key });
-      }
-    }
-  });
+  const unknownArgs = [];
+  const unknown = unknownFactory(unknownArgs);
 
-  const nodeArgs = unknownArgs.filter(unique('key')).reduce((out, { arg, key }) => {
-    const value = opts[key];
+  const {
+    _: [script, ...scriptArgs]
+  } = minimist(args, { alias, boolean, string, unknown });
 
-    if (typeof value !== 'boolean' && !arg.includes('=')) {
-      if (Array.isArray(value)) {
-        value.forEach(v => out.push(arg, v));
-      } else {
-        out.push(arg, value);
-      }
-    } else {
-      out.push(arg);
-    }
+  assert(script, 'Could not parse command line arguments');
 
-    return out;
-  }, []);
+  const opts = minimist(args, { alias, boolean, default: getConfig(script) });
 
-  unknownArgs.forEach(({ key }) => {
-    delete opts[key];
-  });
+  const nodeArgs = [...nodeBoolean.map(argify), ...nodeString.map(argify), ...unknownArgs]
+    .sort((a, b) => a.key - b.key)
+    .reduce(nodeArgsReducer(opts), [...nodeOptionalArgs]);
 
-  opts.ignore = [...(Array.isArray(opts.ignore) ? opts.ignore : [opts.ignore])].map(resolvePath);
+  opts.ignore = arrayify(opts.ignore).map(resolvePath);
 
-  return {
-    script,
-    scriptArgs,
-    nodeArgs,
-    opts
-  };
+  return { nodeArgs, opts, script, scriptArgs };
 };

package.json

@@ -35,14 +35,14 @@
     "dateformat": "^3.0.3",
     "dynamic-dedupe": "^0.3.0",
     "filewatcher": "~3.0.0",
-    "minimist": "^1.1.3",
+    "minimist": "^1.2.5",
     "node-notifier": "^8.0.1",
     "resolve": "^1.0.0",
     "semver": "^7.3.5"
   },
   "devDependencies": {
     "@types/node": "^14.14.37",
-    "eslint": "^7.23.0",
+    "eslint": "^7.25.0",
     "eslint-plugin-import": "^2.22.1",
     "husky": "^6.0.0",
     "lint-staged": "^10.5.4",

test/cli.js

@@ -68,19 +68,19 @@ tap.test('cli overrides .node-dev.json from false to true', t => {
 tap.test('-r ts-node/register --inspect test/fixture/server.js', t => {
   const argv = 'node bin/node-dev -r ts-node/register --inspect test/fixture/server.js'.split(' ');
   const { nodeArgs } = cli(argv);
-  t.same(nodeArgs, ['-r', 'ts-node/register', '--inspect']);
+  t.same(nodeArgs, ['--inspect', '--require=ts-node/register']);
   t.end();
 });
 
 tap.test('--inspect -r ts-node/register test/fixture/server.js', t => {
   const argv = 'node bin/node-dev --inspect -r ts-node/register test/fixture/server.js'.split(' ');
   const { nodeArgs } = cli(argv);
-  t.same(nodeArgs, ['--inspect', '-r', 'ts-node/register']);
+  t.same(nodeArgs, ['--inspect', '--require=ts-node/register']);
   t.end();
 });
 
 tap.test('--expose_gc gc.js foo', t => {
-  const argv = 'node bin/node-dev --expose_gc test/fixture/gc.js test/fixture/foo'.split(' ');
+  const argv = 'node bin/node-dev --expose_gc test/fixture/gc.js foo'.split(' ');
   const { nodeArgs } = cli(argv);
   t.same(nodeArgs, ['--expose_gc']);
   t.end();
@@ -139,3 +139,147 @@ tap.test('--debounce=2000', t => {
   t.equal(debounce, 2000);
   t.end();
 });
+
+tap.test('--require source-map-support/register', t => {
+  const { nodeArgs } = cli([
+    'node',
+    'bin/node-dev',
+    '--require',
+    'source-map-support/register',
+    'test'
+  ]);
+
+  t.same(nodeArgs, ['--require=source-map-support/register']);
+  t.end();
+});
+
+tap.test('--require=source-map-support/register', t => {
+  const { nodeArgs } = cli([
+    'node',
+    'bin/node-dev',
+    '--require=source-map-support/register',
+    'test'
+  ]);
+
+  t.same(nodeArgs, ['--require=source-map-support/register']);
+  t.end();
+});
+
+tap.test('-r source-map-support/register', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-r', 'source-map-support/register', 'test']);
+
+  t.same(nodeArgs, ['--require=source-map-support/register']);
+  t.end();
+});
+
+tap.test('-r=source-map-support/register', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-r=source-map-support/register', 'test']);
+
+  t.same(nodeArgs, ['--require=source-map-support/register']);
+  t.end();
+});
+
+tap.test('--inspect=127.0.0.1:12345', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '--inspect=127.0.0.1:12345', 'test']);
+
+  t.same(nodeArgs, ['--inspect=127.0.0.1:12345']);
+  t.end();
+});
+
+tap.test('--inspect', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '--inspect', 'test']);
+
+  t.same(nodeArgs, ['--inspect']);
+  t.end();
+});
+
+tap.test('--require source-map-support/register --require ts-node/register', t => {
+  const { nodeArgs } = cli([
+    'node',
+    'bin/node-dev',
+    '--require',
+    'source-map-support/register',
+    '--require',
+    'ts-node/register',
+    'test'
+  ]);
+
+  t.same(nodeArgs, ['--require=source-map-support/register', '--require=ts-node/register']);
+  t.end();
+});
+
+// This should display usage information at some point
+tap.test('No script or option should fail', t => {
+  t.throws(() => cli(['node', 'bin/node-dev']));
+  t.end();
+});
+
+tap.test('Just an option should fail', t => {
+  t.throws(() => cli(['node', 'bin/node-dev', '--option']));
+  t.end();
+});
+
+tap.test('Just an option with a value should fail', t => {
+  t.throws(() => cli(['node', 'bin/node-dev', '--option=value']));
+  t.end();
+});
+
+tap.test('An unknown argument with a value instead of a script should fail.', t => {
+  t.throws(() => cli(['node', 'bin/node-dev', '--unknown-arg', 'value']));
+  t.end();
+});
+
+tap.test('An unknown argument with a value', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '--unknown-arg=value', 'test']);
+
+  t.same(nodeArgs, ['--unknown-arg=value']);
+  t.end();
+});
+
+tap.test('An unknown argument without a value can use -- to delimit', t => {
+  // use -- to delimit the end of options
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '--unknown-arg', '--', 'test']);
+
+  t.same(nodeArgs, ['--unknown-arg']);
+  t.end();
+});
+
+tap.test('Single dash with value', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', 'value', 'test']);
+
+  t.same(nodeArgs, ['-u=value']);
+  t.end();
+});
+
+tap.test('Single dash with = and value', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-u=value', 'test']);
+
+  t.same(nodeArgs, ['-u=value']);
+  t.end();
+});
+
+tap.test('Single dash without value should fail', t => {
+  t.throws(() => cli(['node', 'bin/node-dev', '-u', 'test']));
+  t.end();
+});
+
+tap.test('Single dash without value can use -- to delimit', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', '--', 'test']);
+
+  t.same(nodeArgs, ['-u']);
+  t.end();
+});
+
+tap.test('Repeated single dash', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-u=value1', '-u=value2', 'test']);
+
+  t.same(nodeArgs, ['-u=value1', '-u=value2']);
+  t.end();
+});
+
+tap.test('Repeated single dash without =', t => {
+  const { nodeArgs } = cli(['node', 'bin/node-dev', '-u', 'value1', '-u', 'value2', 'test']);
+
+  t.same(nodeArgs, ['-u=value1', '-u=value2']);
+  t.end();
+});