From 7cfb5f31dc0f0db55005475bd183b6fdf41c483c Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Fri, 25 Sep 2020 22:28:06 +0530 Subject: [PATCH 1/4] refactor: migrate basic group --- packages/webpack-cli/lib/groups/BasicGroup.js | 57 ------------------- .../webpack-cli/lib/groups/basicResolver.js | 52 +++++++++++++++++ packages/webpack-cli/lib/utils/GroupHelper.js | 32 ----------- packages/webpack-cli/lib/utils/arg-utils.js | 34 +++++++++++ packages/webpack-cli/lib/webpack-cli.js | 18 ++---- 5 files changed, 91 insertions(+), 102 deletions(-) delete mode 100644 packages/webpack-cli/lib/groups/BasicGroup.js create mode 100644 packages/webpack-cli/lib/groups/basicResolver.js diff --git a/packages/webpack-cli/lib/groups/BasicGroup.js b/packages/webpack-cli/lib/groups/BasicGroup.js deleted file mode 100644 index cbf8a41806d..00000000000 --- a/packages/webpack-cli/lib/groups/BasicGroup.js +++ /dev/null @@ -1,57 +0,0 @@ -const GroupHelper = require('../utils/GroupHelper'); -const logger = require('../utils/logger'); -const { core, groups } = require('../utils/cli-flags'); - -class BasicGroup extends GroupHelper { - constructor(options) { - super(options); - this.WEBPACK_OPTION_FLAGS = core - .filter((coreFlag) => { - return coreFlag.group === groups.BASIC_GROUP; - }) - .reduce((result, flagObject) => { - result.push(flagObject.name); - if (flagObject.alias) { - result.push(flagObject.alias); - } - return result; - }, []); - } - resolveFlags() { - const { args } = this; - if (!args) return; - const { outputOptions, options } = this.opts; - Object.keys(args).forEach((arg) => { - if (this.WEBPACK_OPTION_FLAGS.includes(arg)) { - outputOptions[arg] = args[arg]; - } - // TODO: to add once webpack bundle analyzer is installed, which is not at the moment - // if (arg == 'analyze') { - // const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); - // this.opts.options.plugins = [new BundleAnalyzerPlugin()]; - // } - if (arg === 'devtool') { - options.devtool = args[arg]; - } - if (arg === 'name') { - options.name = args[arg]; - } - if (arg === 'watch') { - options.watch = true; - } - if (arg === 'entry') { - options[arg] = this.resolveFilePath(args[arg], 'index.js'); - if (options[arg].length === 0) { - logger.error('\nError: you provided an invalid entry point.'); - } - } - }); - } - - run() { - this.resolveFlags(); - return this.opts; - } -} - -module.exports = BasicGroup; diff --git a/packages/webpack-cli/lib/groups/basicResolver.js b/packages/webpack-cli/lib/groups/basicResolver.js new file mode 100644 index 00000000000..2db913b2791 --- /dev/null +++ b/packages/webpack-cli/lib/groups/basicResolver.js @@ -0,0 +1,52 @@ +const logger = require('../utils/logger'); +const { core, groups } = require('../utils/cli-flags'); +const { resolveFilePath } = require('../utils/arg-utils'); + +const WEBPACK_OPTION_FLAGS = core + .filter((coreFlag) => { + return coreFlag.group === groups.BASIC_GROUP; + }) + .reduce((result, flagObject) => { + result.push(flagObject.name); + if (flagObject.alias) { + result.push(flagObject.alias); + } + return result; + }, []); + +console.log(WEBPACK_OPTION_FLAGS); + +function resolveArgs(args) { + const finalOptions = { + options: {}, + outputOptions: {}, + }; + Object.keys(args).forEach((arg) => { + if (WEBPACK_OPTION_FLAGS.includes(arg)) { + finalOptions.outputOptions[arg] = args[arg]; + } + // TODO: to add once webpack bundle analyzer is installed, which is not at the moment + // if (arg == 'analyze') { + // const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); + // this.opts.options.plugins = [new BundleAnalyzerPlugin()]; + // } + if (arg === 'devtool') { + finalOptions.options.devtool = args[arg]; + } + if (arg === 'name') { + finalOptions.options.name = args[arg]; + } + if (arg === 'watch') { + finalOptions.options.watch = true; + } + if (arg === 'entry') { + finalOptions.options[arg] = resolveFilePath(args[arg], 'index.js'); + if (finalOptions.options[arg].length === 0) { + logger.error('\nError: you provided an invalid entry point.'); + } + } + }); + return finalOptions; +} + +module.exports = resolveArgs; diff --git a/packages/webpack-cli/lib/utils/GroupHelper.js b/packages/webpack-cli/lib/utils/GroupHelper.js index 18d1acb533d..e5e802f28ae 100644 --- a/packages/webpack-cli/lib/utils/GroupHelper.js +++ b/packages/webpack-cli/lib/utils/GroupHelper.js @@ -1,5 +1,3 @@ -const { resolve, join } = require('path'); -const { existsSync } = require('fs'); const { arrayToObject } = require('../utils/arg-utils'); class GroupHelper { @@ -12,36 +10,6 @@ class GroupHelper { this.strategy = undefined; } - resolveFilePath(filename = '', defaultValue) { - if (filename && Array.isArray(filename)) { - return filename.map((fp) => this.resolveFilePath(fp, defaultValue)).filter((e) => e); - } - if (filename && !filename.includes('.js')) { - filename = filename + '.js'; - } - if (defaultValue && !defaultValue.includes('.js')) { - defaultValue = defaultValue + '.js'; - } - let configPath; - const predefinedConfigPath = filename ? resolve(process.cwd(), filename) : undefined; - const configPathExists = predefinedConfigPath ? existsSync(predefinedConfigPath) : undefined; - - if (!configPathExists) { - const LOOKUP_PATHS = [`${filename}`, `src/${filename}`, ...(defaultValue ? [defaultValue, `src/${defaultValue}`] : [])]; - - if (filename) { - LOOKUP_PATHS.unshift(filename); - } - LOOKUP_PATHS.forEach((p) => { - const lookUpPath = join(process.cwd(), ...p.split('/')); - if (existsSync(lookUpPath) && !configPath) { - configPath = lookUpPath; - } - }); - } - return configPathExists ? predefinedConfigPath : configPath; - } - run() { throw new Error('You must implement the "run" function'); } diff --git a/packages/webpack-cli/lib/utils/arg-utils.js b/packages/webpack-cli/lib/utils/arg-utils.js index d403b9087cd..5eabd36164c 100644 --- a/packages/webpack-cli/lib/utils/arg-utils.js +++ b/packages/webpack-cli/lib/utils/arg-utils.js @@ -1,3 +1,6 @@ +const { resolve, join } = require('path'); +const { existsSync } = require('fs'); + function hyphenToUpperCase(name) { if (!name) { return name; @@ -18,6 +21,37 @@ function arrayToObject(arr) { }, {}); } +function resolveFilePath(filename = '', defaultValue) { + if (filename && Array.isArray(filename)) { + return filename.map((fp) => resolveFilePath(fp, defaultValue)).filter((e) => e); + } + if (filename && !filename.includes('.js')) { + filename = filename + '.js'; + } + if (defaultValue && !defaultValue.includes('.js')) { + defaultValue = defaultValue + '.js'; + } + let configPath; + const predefinedConfigPath = filename ? resolve(process.cwd(), filename) : undefined; + const configPathExists = predefinedConfigPath ? existsSync(predefinedConfigPath) : undefined; + + if (!configPathExists) { + const LOOKUP_PATHS = [`${filename}`, `src/${filename}`, ...(defaultValue ? [defaultValue, `src/${defaultValue}`] : [])]; + + if (filename) { + LOOKUP_PATHS.unshift(filename); + } + LOOKUP_PATHS.forEach((p) => { + const lookUpPath = join(process.cwd(), ...p.split('/')); + if (existsSync(lookUpPath) && !configPath) { + configPath = lookUpPath; + } + }); + } + return configPathExists ? predefinedConfigPath : configPath; +} + module.exports = { arrayToObject, + resolveFilePath, }; diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index 8f08ae88a9d..728a32dea53 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -1,4 +1,6 @@ +const path = require('path'); const { options } = require('colorette'); +const webpackMerge = require('webpack-merge'); const GroupHelper = require('./utils/GroupHelper'); const handleConfigResolution = require('./groups/ConfigGroup'); const resolveMode = require('./groups/resolveMode'); @@ -6,7 +8,7 @@ const resolveStats = require('./groups/resolveStats'); const resolveOutput = require('./groups/resolveOutput'); const { Compiler } = require('./utils/Compiler'); const { groups, core } = require('./utils/cli-flags'); -const webpackMerge = require('webpack-merge'); +const basicResolver = require('./groups/basicResolver'); const { toKebabCase } = require('./utils/helpers'); const argParser = require('./utils/arg-parser'); const { outputStrategy } = require('./utils/merge-strategies'); @@ -106,11 +108,6 @@ class WebpackCLI extends GroupHelper { resolveGroups() { for (const [key, value] of this.groupMap.entries()) { switch (key) { - case groups.BASIC_GROUP: { - const BasicGroup = require('./groups/BasicGroup'); - this.basicGroup = new BasicGroup(value); - break; - } case groups.ADVANCED_GROUP: { const AdvancedGroup = require('./groups/AdvancedGroup'); this.advancedGroup = new AdvancedGroup(value); @@ -217,12 +214,7 @@ class WebpackCLI extends GroupHelper { * @returns {void} */ _handleDefaultEntry() { - if (!this.basicGroup) { - const BasicGroup = require('./groups/BasicGroup'); - this.basicGroup = new BasicGroup(); - } - const defaultEntry = this.basicGroup.resolveFilePath(null, 'index.js'); - const options = { entry: defaultEntry }; + const options = { entry: path.resolve('index.js') }; this._mergeOptionsToConfiguration(options); } @@ -240,7 +232,7 @@ class WebpackCLI extends GroupHelper { .then(() => this._baseResolver(resolveMode, parsedArgs, this.compilerConfiguration)) .then(() => this._baseResolver(resolveOutput, parsedArgs, {}, outputStrategy)) .then(() => this._handleCoreFlags()) - .then(() => this._handleGroupHelper(this.basicGroup)) + .then(() => this._baseResolver(basicResolver, parsedArgs)) .then(() => this._handleGroupHelper(this.advancedGroup)) .then(() => this._baseResolver(resolveStats, parsedArgs)) .then(() => this._handleGroupHelper(this.helpGroup)); From 7d40af232bdefbac58b690d80960bdaa283dcfc5 Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Tue, 29 Sep 2020 19:56:13 +0530 Subject: [PATCH 2/4] chore: rm stale utils, pass entry directly --- .../webpack-cli/lib/groups/basicResolver.js | 14 +------- packages/webpack-cli/lib/utils/arg-utils.js | 34 ------------------- packages/webpack-cli/lib/webpack-cli.js | 20 +++++------ test/entry/flag-entry/entry-with-flag.test.js | 6 ++-- .../entry-absent/zero-config.test.js | 2 +- 5 files changed, 15 insertions(+), 61 deletions(-) diff --git a/packages/webpack-cli/lib/groups/basicResolver.js b/packages/webpack-cli/lib/groups/basicResolver.js index 2db913b2791..f4e548f7ae6 100644 --- a/packages/webpack-cli/lib/groups/basicResolver.js +++ b/packages/webpack-cli/lib/groups/basicResolver.js @@ -1,6 +1,4 @@ -const logger = require('../utils/logger'); const { core, groups } = require('../utils/cli-flags'); -const { resolveFilePath } = require('../utils/arg-utils'); const WEBPACK_OPTION_FLAGS = core .filter((coreFlag) => { @@ -14,8 +12,6 @@ const WEBPACK_OPTION_FLAGS = core return result; }, []); -console.log(WEBPACK_OPTION_FLAGS); - function resolveArgs(args) { const finalOptions = { options: {}, @@ -25,11 +21,6 @@ function resolveArgs(args) { if (WEBPACK_OPTION_FLAGS.includes(arg)) { finalOptions.outputOptions[arg] = args[arg]; } - // TODO: to add once webpack bundle analyzer is installed, which is not at the moment - // if (arg == 'analyze') { - // const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer'); - // this.opts.options.plugins = [new BundleAnalyzerPlugin()]; - // } if (arg === 'devtool') { finalOptions.options.devtool = args[arg]; } @@ -40,10 +31,7 @@ function resolveArgs(args) { finalOptions.options.watch = true; } if (arg === 'entry') { - finalOptions.options[arg] = resolveFilePath(args[arg], 'index.js'); - if (finalOptions.options[arg].length === 0) { - logger.error('\nError: you provided an invalid entry point.'); - } + finalOptions.options[arg] = args[arg]; } }); return finalOptions; diff --git a/packages/webpack-cli/lib/utils/arg-utils.js b/packages/webpack-cli/lib/utils/arg-utils.js index 5eabd36164c..d403b9087cd 100644 --- a/packages/webpack-cli/lib/utils/arg-utils.js +++ b/packages/webpack-cli/lib/utils/arg-utils.js @@ -1,6 +1,3 @@ -const { resolve, join } = require('path'); -const { existsSync } = require('fs'); - function hyphenToUpperCase(name) { if (!name) { return name; @@ -21,37 +18,6 @@ function arrayToObject(arr) { }, {}); } -function resolveFilePath(filename = '', defaultValue) { - if (filename && Array.isArray(filename)) { - return filename.map((fp) => resolveFilePath(fp, defaultValue)).filter((e) => e); - } - if (filename && !filename.includes('.js')) { - filename = filename + '.js'; - } - if (defaultValue && !defaultValue.includes('.js')) { - defaultValue = defaultValue + '.js'; - } - let configPath; - const predefinedConfigPath = filename ? resolve(process.cwd(), filename) : undefined; - const configPathExists = predefinedConfigPath ? existsSync(predefinedConfigPath) : undefined; - - if (!configPathExists) { - const LOOKUP_PATHS = [`${filename}`, `src/${filename}`, ...(defaultValue ? [defaultValue, `src/${defaultValue}`] : [])]; - - if (filename) { - LOOKUP_PATHS.unshift(filename); - } - LOOKUP_PATHS.forEach((p) => { - const lookUpPath = join(process.cwd(), ...p.split('/')); - if (existsSync(lookUpPath) && !configPath) { - configPath = lookUpPath; - } - }); - } - return configPathExists ? predefinedConfigPath : configPath; -} - module.exports = { arrayToObject, - resolveFilePath, }; diff --git a/packages/webpack-cli/lib/webpack-cli.js b/packages/webpack-cli/lib/webpack-cli.js index 728a32dea53..17a7d0646e9 100644 --- a/packages/webpack-cli/lib/webpack-cli.js +++ b/packages/webpack-cli/lib/webpack-cli.js @@ -1,4 +1,5 @@ const path = require('path'); +const { existsSync } = require('fs'); const { options } = require('colorette'); const webpackMerge = require('webpack-merge'); const GroupHelper = require('./utils/GroupHelper'); @@ -19,16 +20,6 @@ class WebpackCLI extends GroupHelper { this.groupMap = new Map(); this.args = {}; this.compilation = new Compiler(); - this.defaultEntry = 'index'; - this.possibleFileNames = [ - `./${this.defaultEntry}`, - `./${this.defaultEntry}.js`, - `${this.defaultEntry}.js`, - this.defaultEntry, - `./src/${this.defaultEntry}`, - `./src/${this.defaultEntry}.js`, - `src/${this.defaultEntry}.js`, - ]; this.compilerConfiguration = {}; this.outputConfiguration = {}; } @@ -214,7 +205,14 @@ class WebpackCLI extends GroupHelper { * @returns {void} */ _handleDefaultEntry() { - const options = { entry: path.resolve('index.js') }; + let defaultEntry; + const rootIndexPath = path.resolve('index.js'); + if (existsSync(rootIndexPath)) { + defaultEntry = './index.js'; + } else { + defaultEntry = './src/index.js'; + } + const options = { entry: defaultEntry }; this._mergeOptionsToConfiguration(options); } diff --git a/test/entry/flag-entry/entry-with-flag.test.js b/test/entry/flag-entry/entry-with-flag.test.js index 0fb810f469e..b6695a75ba5 100644 --- a/test/entry/flag-entry/entry-with-flag.test.js +++ b/test/entry/flag-entry/entry-with-flag.test.js @@ -40,7 +40,9 @@ describe('entry flag', () => { }); it('should throw error for invalid entry file', () => { - const { stderr } = run(__dirname, ['--entry', './src/test.js']); - expect(stderr).toContain('Error: you provided an invalid entry point.'); + const { stdout, stderr, exitCode } = run(__dirname, ['--entry', './src/test.js']); + expect(stdout).toContain("Module not found: Error: Can't resolve"); + expect(exitCode).toEqual(1); + expect(stderr).toBeFalsy(); }); }); diff --git a/test/zero-config/entry-absent/zero-config.test.js b/test/zero-config/entry-absent/zero-config.test.js index 9acc630d895..34f65e83de9 100644 --- a/test/zero-config/entry-absent/zero-config.test.js +++ b/test/zero-config/entry-absent/zero-config.test.js @@ -4,7 +4,7 @@ describe('Zero Config tests', () => { it('runs when config and entry are both absent', () => { const { stdout, stderr } = run(__dirname, [], false); // Entry file is absent, should log the Error from the compiler - expect(stdout).toContain("Error: Can't resolve './src'"); + expect(stdout).toContain("Error: Can't resolve './src/index.js'"); expect(stderr).toBeFalsy(); }); }); From 04fa6d8474b5180e827b9f9bf8d15aac06352b73 Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Wed, 30 Sep 2020 13:35:39 +0530 Subject: [PATCH 3/4] tests: add test for cjs entry --- test/entry/flag-entry/entry-with-flag.test.js | 17 +++++++++++++++++ test/entry/flag-entry/src/index.cjs | 1 + 2 files changed, 18 insertions(+) create mode 100644 test/entry/flag-entry/src/index.cjs diff --git a/test/entry/flag-entry/entry-with-flag.test.js b/test/entry/flag-entry/entry-with-flag.test.js index b6695a75ba5..d964bb53ef2 100644 --- a/test/entry/flag-entry/entry-with-flag.test.js +++ b/test/entry/flag-entry/entry-with-flag.test.js @@ -39,6 +39,23 @@ describe('entry flag', () => { }); }); + it('should resolve the path to src/index.cjs', (done) => { + const { stderr, stdout } = run(__dirname, ['--entry', './src/index.cjs']); + expect(stderr).toBeFalsy(); + expect(stdout).toBeTruthy(); + + stat(resolve(__dirname, './bin/main.js'), (err, stats) => { + expect(err).toBe(null); + expect(stats.isFile()).toBe(true); + done(); + }); + readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => { + expect(err).toBe(null); + expect(data).toContain('Kazuya Miyuki'); + done(); + }); + }); + it('should throw error for invalid entry file', () => { const { stdout, stderr, exitCode } = run(__dirname, ['--entry', './src/test.js']); expect(stdout).toContain("Module not found: Error: Can't resolve"); diff --git a/test/entry/flag-entry/src/index.cjs b/test/entry/flag-entry/src/index.cjs new file mode 100644 index 00000000000..f96f188d697 --- /dev/null +++ b/test/entry/flag-entry/src/index.cjs @@ -0,0 +1 @@ +console.log('Kazuya Miyuki'); From 50399f187179d41abdbd5ab11690947b1fbbf847 Mon Sep 17 00:00:00 2001 From: Anshuman Verma Date: Wed, 30 Sep 2020 21:48:40 +0530 Subject: [PATCH 4/4] tests: fix tests --- test/entry/flag-entry/entry-with-flag.test.js | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/entry/flag-entry/entry-with-flag.test.js b/test/entry/flag-entry/entry-with-flag.test.js index d964bb53ef2..1f335734f3f 100644 --- a/test/entry/flag-entry/entry-with-flag.test.js +++ b/test/entry/flag-entry/entry-with-flag.test.js @@ -5,25 +5,25 @@ const { stat, readFile } = require('fs'); const { resolve } = require('path'); describe('entry flag', () => { - it('should load ./src/a.js as entry', (done) => { - const { stderr, stdout } = run(__dirname, ['--entry', './src/a.js']); + it('should resolve the path to src/index.cjs', (done) => { + const { stderr, stdout } = run(__dirname, ['--entry', './src/index.cjs', '-o', './dist/'], false); expect(stderr).toBeFalsy(); expect(stdout).toBeTruthy(); - stat(resolve(__dirname, './bin/main.js'), (err, stats) => { + stat(resolve(__dirname, './dist/main.js'), (err, stats) => { expect(err).toBe(null); expect(stats.isFile()).toBe(true); done(); }); - readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => { + readFile(resolve(__dirname, './dist/main.js'), 'utf-8', (err, data) => { expect(err).toBe(null); - expect(data).toContain('Hello from a.js'); + expect(data).toContain('Kazuya Miyuki'); done(); }); }); - it('should resolve the path to src/a.js as ./src/a.js', (done) => { - const { stderr, stdout } = run(__dirname, ['--entry', 'src/a.js']); + it('should load ./src/a.js as entry', (done) => { + const { stderr, stdout } = run(__dirname, ['--entry', './src/a.js']); expect(stderr).toBeFalsy(); expect(stdout).toBeTruthy(); @@ -39,8 +39,8 @@ describe('entry flag', () => { }); }); - it('should resolve the path to src/index.cjs', (done) => { - const { stderr, stdout } = run(__dirname, ['--entry', './src/index.cjs']); + it('should resolve the path to src/a.js as ./src/a.js', (done) => { + const { stderr, stdout } = run(__dirname, ['--entry', 'src/a.js']); expect(stderr).toBeFalsy(); expect(stdout).toBeTruthy(); @@ -51,7 +51,7 @@ describe('entry flag', () => { }); readFile(resolve(__dirname, './bin/main.js'), 'utf-8', (err, data) => { expect(err).toBe(null); - expect(data).toContain('Kazuya Miyuki'); + expect(data).toContain('Hello from a.js'); done(); }); });