perf: do not spawn new process for running webpack (#1741)

packages/webpack-cli/README.md

@@ -54,7 +54,6 @@ yarn add webpack-cli --dev
   -j, --json               Prints result as JSON
   --mode string            Defines the mode to pass to webpack
   -v, --version            Get current version
-  --node-args string[]     NodeJS flags
   --stats string           It instructs webpack on how to treat the stats
   --no-stats               Disables stats output
 ```

packages/webpack-cli/__tests__/cli-executer.test.js

@@ -47,7 +47,7 @@ describe('CLI Executer', () => {
     it('runs enquirer options then runs webpack', async () => {
         await cliExecuter();
 
-        // ensure that the webpack runner is called
+        // ensure that the webpack runCLI is called
         expect(runner.mock.calls.length).toEqual(1);
         expect(runner.mock.calls[0]).toEqual([[], ['--config', 'test1', '--entry', 'test2', '--progress']]);
 

packages/webpack-cli/bin/cli.js

@@ -3,8 +3,7 @@
 'use strict';
 require('v8-compile-cache');
 const importLocal = require('import-local');
-const parseNodeArgs = require('../lib/utils/parse-node-args');
-const runner = require('../lib/runner');
+const runCLI = require('../lib/bootstrap');
 
 // Prefer the local installation of webpack-cli
 if (importLocal(__filename)) {
@@ -13,6 +12,5 @@ if (importLocal(__filename)) {
 process.title = 'webpack';
 
 const [, , ...rawArgs] = process.argv;
-const { cliArgs, nodeArgs } = parseNodeArgs(rawArgs);
 
-runner(nodeArgs, cliArgs);
+runCLI(rawArgs);

packages/webpack-cli/lib/bootstrap.js

@@ -7,20 +7,25 @@ const argParser = require('./utils/arg-parser');
 require('./utils/process-log');
 process.title = 'webpack-cli';
 
-const isCommandUsed = (commands) =>
+// Create a new instance of the CLI object
+const cli = new WebpackCLI();
+
+const isCommandUsed = (args) =>
     commands.find((cmd) => {
-        return process.argv.includes(cmd.name) || process.argv.includes(cmd.alias);
+        return args.includes(cmd.name) || args.includes(cmd.alias);
     });
 
-async function runCLI(cli, commandIsUsed) {
+async function runCLI(cliArgs) {
     let args;
+
+    const commandIsUsed = isCommandUsed(cliArgs);
     const runVersion = () => {
-        cli.runVersion(process.argv, commandIsUsed);
+        cli.runVersion(cliArgs, commandIsUsed);
     };
-    const parsedArgs = argParser(core, process.argv, false, process.title, cli.runHelp, runVersion, commands);
+    const parsedArgs = argParser(core, cliArgs, true, process.title, cli.runHelp, runVersion, commands);
 
     if (parsedArgs.unknownArgs.includes('help')) {
-        cli.runHelp(process.argv);
+        cli.runHelp(cliArgs);
         process.exit(0);
     }
 
@@ -55,7 +60,7 @@ async function runCLI(cli, commandIsUsed) {
             parsedArgs.unknownArgs.forEach((unknown) => {
                 logger.warn(`Unknown argument: ${unknown}`);
             });
-            cliExecuter();
+            await cliExecuter();
             return;
         }
         const parsedArgsOpts = parsedArgs.opts;
@@ -77,10 +82,10 @@ async function runCLI(cli, commandIsUsed) {
         } else if (err.name === 'ALREADY_SET') {
             const argsMap = {};
             const keysToDelete = [];
-            process.argv.forEach((arg, idx) => {
+            cliArgs.forEach((arg, idx) => {
                 const oldMapValue = argsMap[arg];
                 argsMap[arg] = {
-                    value: process.argv[idx],
+                    value: cliArgs[idx],
                     pos: idx,
                 };
                 // Swap idx of overridden value
@@ -92,8 +97,8 @@ async function runCLI(cli, commandIsUsed) {
             // Filter out the value for the overridden key
             const newArgKeys = Object.keys(argsMap).filter((arg) => !keysToDelete.includes(argsMap[arg].pos));
             // eslint-disable-next-line require-atomic-updates
-            process.argv = newArgKeys;
-            args = argParser('', core, process.argv);
+            cliArgs = newArgKeys;
+            args = argParser('', core, cliArgs);
             await cli.run(args.opts, core);
             process.stdout.write('\n');
             logger.warn('Duplicate flags found, defaulting to last set value');
@@ -104,6 +109,4 @@ async function runCLI(cli, commandIsUsed) {
     }
 }
 
-const commandIsUsed = isCommandUsed(commands);
-const cli = new WebpackCLI();
-runCLI(cli, commandIsUsed);
+module.exports = runCLI;

packages/webpack-cli/lib/utils/cli-executer.js

@@ -1,8 +1,8 @@
 const { MultiSelect, Input } = require('enquirer');
 const { cyan } = require('colorette');
-const runner = require('../runner');
 const logger = require('./logger');
 const cliArgs = require('./cli-flags').core;
+const runner = require('../runner');
 
 async function prompter() {
     const args = [];
@@ -56,7 +56,7 @@ async function run() {
         const args = await prompter();
         process.stdout.write('\n');
         logger.info('Executing CLI\n');
-        runner([], args);
+        await runner([], args);
     } catch (err) {
         logger.error(`Action Interrupted, use ${cyan('webpack-cli help')} to see possible options.`);
     }

packages/webpack-cli/lib/utils/cli-flags.js

@@ -236,14 +236,6 @@ module.exports = {
             group: HELP_GROUP,
             description: 'Get current version',
         },
-        {
-            name: 'node-args',
-            usage: '--node-args "--max-old-space-size=1024"',
-            type: String,
-            multiple: true,
-            group: BASIC_GROUP,
-            description: 'NodeJS flags',
-        },
         {
             name: 'stats',
             usage: '--stats <value>',

packages/webpack-cli/lib/webpack-cli.js

@@ -281,7 +281,7 @@ class WebpackCLI extends GroupHelper {
         const subject = allNames.filter((name) => {
             return args.includes(name);
         })[0];
-        const invalidArgs = hasUnknownArgs(args.slice(2), ...allNames);
+        const invalidArgs = hasUnknownArgs(args, ...allNames);
         const isCommand = commands.includes(subject);
         options.enabled = !args.includes('--no-color');
         return new HelpGroup().outputHelp(isCommand, subject, invalidArgs);
@@ -291,7 +291,7 @@ class WebpackCLI extends GroupHelper {
         const HelpGroup = require('./groups/HelpGroup');
         const { commands, allNames, hasUnknownArgs } = require('./utils/unknown-args');
         const commandsUsed = args.filter((val) => commands.includes(val));
-        const invalidArgs = hasUnknownArgs(args.slice(2), ...allNames);
+        const invalidArgs = hasUnknownArgs(args, ...allNames);
         options.enabled = !args.includes('--no-color');
         return new HelpGroup().outputVersion(externalPkg, commandsUsed, invalidArgs);
     }

test/node/node.test.js

@@ -2,78 +2,40 @@
 const { stat } = require('fs');
 const { resolve } = require('path');
 const { run } = require('../utils/test-utils');
-const parseNodeArgs = require('../../packages/webpack-cli/lib/utils/parse-node-args');
 
+// TODO - We pass node args to `nodeOptions` in execa,
+// passing via NODE_OPTIONS=<args> in env in execa
+// throws different error from what we manually see
 describe('node flags', () => {
-    it('parseNodeArgs helper must work correctly', () => {
-        [
-            {
-                rawArgs: ['--foo', '--bar', '--baz=quux'],
-                expectedCliArgs: ['--foo', '--bar', '--baz=quux'],
-                expectedNodeArgs: [],
-            },
-            {
-                rawArgs: ['--foo', '--bar', '--baz=quux', '--node-args', '--name1=value1', '--node-args', '--name2 value2'],
-                expectedCliArgs: ['--foo', '--bar', '--baz=quux'],
-                expectedNodeArgs: ['--name1=value1', '--name2', 'value2'],
-            },
-            {
-                rawArgs: [
-                    '--node-args',
-                    '--name1=value1',
-                    '--node-args',
-                    '--name2="value2"',
-                    '--node-args',
-                    '--name3 value3',
-                    '--node-args',
-                    '-k v',
-                ],
-                expectedCliArgs: [],
-                expectedNodeArgs: ['--name1=value1', '--name2="value2"', '--name3', 'value3', '-k', 'v'],
-            },
-        ].map(({ rawArgs, expectedNodeArgs, expectedCliArgs }) => {
-            const { nodeArgs, cliArgs } = parseNodeArgs(rawArgs);
-            expect(nodeArgs).toEqual(expectedNodeArgs);
-            expect(cliArgs).toEqual(expectedCliArgs);
-        });
-    });
-
-    it('is able to pass the options flags to node js', (done) => {
-        const { stdout } = run(
-            __dirname,
-            [
-                '--node-args',
-                `--require=${resolve(__dirname, 'bootstrap.js')}`,
-                '--node-args',
-                `-r ${resolve(__dirname, 'bootstrap2.js')}`,
-                '--output',
-                './bin/[name].bundle.js',
-            ],
-            false,
-        );
+    it('is able to pass the options flags to node js', async (done) => {
+        const { stdout, stderr } = await run(__dirname, ['--output', './bin/[name].bundle.js'], false, [
+            `--require=${resolve(__dirname, 'bootstrap.js')}`,
+            `--require=${resolve(__dirname, 'bootstrap2.js')}`,
+        ]);
         expect(stdout).toContain('---from bootstrap.js---');
         expect(stdout).toContain('---from bootstrap2.js---');
+        expect(stderr).toBeFalsy();
         stat(resolve(__dirname, './bin/main.bundle.js'), (err, stats) => {
             expect(err).toBe(null);
             expect(stats.isFile()).toBe(true);
             done();
         });
     });
 
-    it('throws an error on supplying unknown flags', () => {
-        const { stderr } = run(__dirname, ['--node-args', '--unknown']);
+    it('throws an error on supplying unknown flags', async () => {
+        const { stderr } = await run(__dirname, [], false, ['--unknown']);
         expect(stderr).toContain('bad option');
     });
 
-    it('throws an error if no values were supplied with --max-old-space-size', () => {
-        const { stderr, stdout } = run(__dirname, ['--node-args', '--max-old-space-size']);
+    it('throws an error if no values were supplied with --max-old-space-size', async () => {
+        const { stderr, stdout } = await run(__dirname, [], false, ['--max-old-space-size']);
         expect(stderr).toContain('missing value for flag --max-old-space-size');
         expect(stdout).toBeFalsy();
     });
 
-    it('throws an error if an illegal value was supplied with --max-old-space-size', () => {
-        const { stderr, stdout } = run(__dirname, ['--node-args', '--max-old-space-size=1024a']);
-        expect(stderr).toContain('illegal value for flag --max-old-space-size');
+    it('throws an error if an illegal value was supplied with --max-old-space-size', async () => {
+        const { stderr, stdout } = await run(__dirname, [], true, ['--max_old_space_size=1024a']);
+        expect(stderr).toContain('Error: illegal value for flag --max_old_space_size=1024a of type size_t');
         expect(stdout).toBeFalsy();
     });
 });

test/utils/test-utils.js

@@ -2,7 +2,7 @@
 const path = require('path');
 const fs = require('fs');
 const execa = require('execa');
-const { sync: spawnSync } = execa;
+const { sync: spawnSync, node: execaNode } = execa;
 const { Writable } = require('readable-stream');
 const concat = require('concat-stream');
 
@@ -15,16 +15,18 @@ const ENABLE_LOG_COMPILATION = process.env.ENABLE_PIPE || false;
  * @param {String} testCase The path to folder that contains the webpack.config.js
  * @param {Array} args Array of arguments to pass to webpack
  * @param {Boolean} setOutput Boolean that decides if a default output path will be set or not
- * @returns {Object} The webpack output
+ * @returns {Object} The webpack output or Promise when nodeOptions are present
  */
-function run(testCase, args = [], setOutput = true) {
+function run(testCase, args = [], setOutput = true, nodeArgs = []) {
     const cwd = path.resolve(testCase);
 
     const outputPath = path.resolve(testCase, 'bin');
+    const processExecutor = nodeArgs.length ? execaNode : spawnSync;
     const argsWithOutput = setOutput ? args.concat('--output', outputPath) : args;
-    const result = spawnSync(WEBPACK_PATH, argsWithOutput, {
+    const result = processExecutor(WEBPACK_PATH, argsWithOutput, {
         cwd,
         reject: false,
+        nodeOptions: nodeArgs,
         stdio: ENABLE_LOG_COMPILATION ? 'inherit' : 'pipe',
     });