Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve performance #2626

Closed
wants to merge 6 commits into from
Closed

feat: improve performance #2626

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a7eebb
feat: improve performance
anshumanv Apr 14, 2021
80fedb1
fix: type
anshumanv Apr 14, 2021
262e86d
fix: code
anshumanv Apr 14, 2021
13489d8
fix: code
anshumanv Apr 14, 2021
cb9a148
fix: code
anshumanv Apr 14, 2021
b24a8a5
fix: tests
anshumanv Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/serve/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ class ServeCommand {
process.exit(2);
}

const builtInOptions = cli.getBuiltInOptions().filter((option) => option.name !== 'watch');
const builtInOptions = Object.values(cli.getBuiltInOptions().options).filter((option: any) => option.name !== 'watch');

return [...builtInOptions, ...devServerFlags];
},
async (entries, options) => {
const builtInOptions = cli.getBuiltInOptions();
const { options: builtInOptions } = cli.getBuiltInOptions();
let devServerFlags = [];

try {
Expand All @@ -51,8 +51,7 @@ class ServeCommand {
for (const optionName in options) {
const kebabedOption = cli.utils.toKebabCase(optionName);
// `webpack-dev-server` has own logic for the `--hot` option
const isBuiltInOption =
kebabedOption !== 'hot' && builtInOptions.find((builtInOption) => builtInOption.name === kebabedOption);
const isBuiltInOption = kebabedOption !== 'hot' && builtInOptions[kebabedOption];

if (isBuiltInOption) {
webpackOptions[optionName] = options[optionName];
Expand Down
119 changes: 63 additions & 56 deletions packages/webpack-cli/lib/webpack-cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class WebpackCLI {
}
}

options.forEach((optionForCommand) => {
Object.values(options).forEach((optionForCommand) => {
this.makeOption(command, optionForCommand);
});
}
Expand Down Expand Up @@ -316,7 +316,7 @@ class WebpackCLI {
return this.builtInOptionsCache;
}

const minimumHelpFlags = [
const minimumHelpFlags = new Set([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perf here would have such little impact that there's no point. If there is 10000 objects I understand, but this is Max 30, so O(30ns) vs O(30^2) wouldnt make much difference

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we did a lookup for every flag, there could be ~300 flags

'config',
'config-name',
'merge',
Expand All @@ -333,11 +333,11 @@ class WebpackCLI {
'name',
'output-path',
'node-env',
];
]);

const builtInFlags = [
const builtInFlags = {
// For configs
{
config: {
name: 'config',
alias: 'c',
configs: [
Expand All @@ -348,7 +348,7 @@ class WebpackCLI {
multiple: true,
description: 'Provide path to a webpack configuration file e.g. ./webpack.config.js.',
},
{
'config-name': {
name: 'config-name',
configs: [
{
Expand All @@ -358,7 +358,7 @@ class WebpackCLI {
multiple: true,
description: 'Name of the configuration to use.',
},
{
merge: {
name: 'merge',
alias: 'm',
configs: [
Expand All @@ -370,7 +370,7 @@ class WebpackCLI {
description: "Merge two or more configurations using 'webpack-merge'.",
},
// Complex configs
{
env: {
name: 'env',
type: (value, previous = {}) => {
// This ensures we're only splitting by the first `=`
Expand Down Expand Up @@ -400,7 +400,7 @@ class WebpackCLI {
multiple: true,
description: 'Environment passed to the configuration when it is a function.',
},
{
'node-env': {
name: 'node-env',
configs: [
{
Expand All @@ -412,7 +412,7 @@ class WebpackCLI {
},

// Adding more plugins
{
hot: {
name: 'hot',
alias: 'h',
configs: [
Expand All @@ -427,7 +427,7 @@ class WebpackCLI {
description: 'Enables Hot Module Replacement',
negatedDescription: 'Disables Hot Module Replacement.',
},
{
analyze: {
name: 'analyze',
configs: [
{
Expand All @@ -438,7 +438,7 @@ class WebpackCLI {
multiple: false,
description: 'It invokes webpack-bundle-analyzer plugin to get bundle information.',
},
{
progress: {
name: 'progress',
configs: [
{
Expand All @@ -451,7 +451,7 @@ class WebpackCLI {
],
description: 'Print compilation progress during build.',
},
{
prefetch: {
name: 'prefetch',
configs: [
{
Expand All @@ -462,7 +462,7 @@ class WebpackCLI {
},

// Output options
{
json: {
name: 'json',
configs: [
{
Expand All @@ -478,7 +478,7 @@ class WebpackCLI {
},

// For webpack@4
{
entry: {
name: 'entry',
configs: [
{
Expand All @@ -488,7 +488,7 @@ class WebpackCLI {
multiple: true,
description: 'The entry point(s) of your application e.g. ./src/main.js.',
},
{
'output-path': {
name: 'output-path',
alias: 'o',
configs: [
Expand All @@ -498,7 +498,7 @@ class WebpackCLI {
],
description: 'Output location of the file generated by webpack e.g. ./dist/.',
},
{
target: {
name: 'target',
alias: 't',
configs: [
Expand All @@ -509,7 +509,7 @@ class WebpackCLI {
multiple: this.webpack.cli !== undefined,
description: 'Sets the build target e.g. node.',
},
{
devtool: {
name: 'devtool',
configs: [
{
Expand All @@ -525,7 +525,7 @@ class WebpackCLI {
description: 'Determine source maps to use.',
negatedDescription: 'Do not generate source maps.',
},
{
mode: {
name: 'mode',
configs: [
{
Expand All @@ -534,7 +534,7 @@ class WebpackCLI {
],
description: 'Defines the mode to pass to webpack.',
},
{
name: {
name: 'name',
configs: [
{
Expand All @@ -543,7 +543,7 @@ class WebpackCLI {
],
description: 'Name of the configuration. Used when loading multiple configurations.',
},
{
stats: {
name: 'stats',
configs: [
{
Expand All @@ -557,7 +557,7 @@ class WebpackCLI {
description: 'It instructs webpack on how to treat the stats e.g. verbose.',
negatedDescription: 'Disable stats output.',
},
{
watch: {
name: 'watch',
configs: [
{
Expand All @@ -569,7 +569,7 @@ class WebpackCLI {
description: 'Watch for files changes.',
negatedDescription: 'Do not watch for file changes.',
},
{
'watch-options-stdin': {
name: 'watch-options-stdin',
configs: [
{
Expand All @@ -580,34 +580,40 @@ class WebpackCLI {
description: 'Stop watching when stdin stream has ended.',
negatedDescription: 'Do not stop watching when stdin stream has ended.',
},
];
};

// Assign help level to all flags
Object.entries(builtInFlags).forEach(([flag, meta]) => {
meta.helpLevel = minimumHelpFlags.has(flag) ? 'minimum' : 'verbose';
});

// Extract all the flags being exported from core.
// A list of cli flags generated by core can be found here https://github.com/webpack/webpack/blob/master/test/__snapshots__/Cli.test.js.snap
const coreFlags = this.webpack.cli
? Object.entries(this.webpack.cli.getArguments()).map(([flag, meta]) => {
const inBuiltIn = builtInFlags.find((builtInFlag) => builtInFlag.name === flag);

if (inBuiltIn) {
return { ...meta, name: flag, group: 'core', ...inBuiltIn, configs: meta.configs || [] };
}

return { ...meta, name: flag, group: 'core' };
})
: [];

const options = []
.concat(builtInFlags.filter((builtInFlag) => !coreFlags.find((coreFlag) => builtInFlag.name === coreFlag.name)))
.concat(coreFlags)
.map((option) => {
option.helpLevel = minimumHelpFlags.includes(option.name) ? 'minimum' : 'verbose';

return option;
const coreFlags = (() => {
if (!this.webpack.cli) return [];
const defaultCoreFlags = this.webpack.cli.getArguments();
Object.entries(defaultCoreFlags).forEach(([flag, meta]) => {
meta.name = flag;
meta.helpLevel = minimumHelpFlags.has(flag) ? 'minimum' : 'verbose';
});
// This list is small so won't cause perf problems
Object.entries(builtInFlags).forEach(([flag, meta]) => {
if (defaultCoreFlags[flag]) {
defaultCoreFlags[flag] = {
...defaultCoreFlags[flag],
...meta,
configs: defaultCoreFlags[flag].configs || [],
};
}
});
return defaultCoreFlags;
})();

this.builtInOptionsCache = options;
const options = { ...builtInFlags, ...coreFlags };

return options;
this.builtInOptionsCache = { options, coreFlags };

return { options, coreFlags };
}

applyNodeEnv(options) {
Expand Down Expand Up @@ -726,11 +732,18 @@ class WebpackCLI {
const isWatchCommandUsed = isCommand(commandName, watchCommandOptions);

if (isBuildCommandUsed || isWatchCommandUsed) {
const options = this.getBuiltInOptions();
const { options } = this.getBuiltInOptions();

const commandArgs = (() => {
if (!isWatchCommandUsed) return options;
// eslint-disable-next-line
const { watch, ...withoutWatchOptions } = options;
return withoutWatchOptions;
})();

await this.makeCommand(
isBuildCommandUsed ? buildCommandOptions : watchCommandOptions,
isWatchCommandUsed ? options.filter((option) => option.name !== 'watch') : options,
commandArgs,
async (entries, options) => {
if (entries.length > 0) {
options.entry = [...entries, ...(options.entry || [])];
Expand Down Expand Up @@ -1582,13 +1595,7 @@ class WebpackCLI {

if (this.webpack.cli) {
const processArguments = (configOptions) => {
const args = this.getBuiltInOptions()
.filter((flag) => flag.group === 'core')
.reduce((accumulator, flag) => {
accumulator[flag.name] = flag;

return accumulator;
}, {});
const { coreFlags } = this.getBuiltInOptions();

const values = Object.keys(options).reduce((accumulator, name) => {
if (name === 'argv') {
Expand All @@ -1597,14 +1604,14 @@ class WebpackCLI {

const kebabName = this.utils.toKebabCase(name);

if (args[kebabName]) {
if (coreFlags[kebabName]) {
accumulator[kebabName] = options[name];
}

return accumulator;
}, {});

const problems = this.webpack.cli.processArguments(args, configOptions, values);
const problems = this.webpack.cli.processArguments(coreFlags, configOptions, values);

if (problems) {
const groupBy = (xs, key) => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/experiments-flag.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const experimentsFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('experiments-'));
const experimentsFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('experiments-'));

describe('experiments option related flag', () => {
experimentsFlags.forEach((flag) => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/externals-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const externalsPresetsFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('externals-presets-'));
const externalsPresetsFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('externals-presets-'));

describe('externals related flag', () => {
it('should set externals properly', async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/module-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase, normalizeStdout } = require('../../utils/test-ut
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const moduleFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('module-'));
const moduleFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('module-'));

describe('module config related flag', () => {
moduleFlags.forEach((flag) => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/optimization-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const optimizationFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('optimization-'));
const optimizationFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('optimization-'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Util func?


describe('optimization config related flag', () => {
optimizationFlags.forEach((flag) => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/output-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const outputFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('output-'));
const outputFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('output-'));

describe('output config related flag', () => {
outputFlags.forEach((flag) => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/performance-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const performanceFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('performance-'));
const performanceFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('performance-'));

describe('module config related flag', () => {
it(`should config --performance option correctly`, async () => {
Expand Down
2 changes: 1 addition & 1 deletion test/build/core-flags/resolve-flags.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { run, hyphenToUpperCase } = require('../../utils/test-utils');
const CLI = require('../../../packages/webpack-cli/lib/index');

const cli = new CLI();
const resolveFlags = cli.getBuiltInOptions().filter(({ name }) => name.startsWith('resolve'));
const resolveFlags = Object.values(cli.getBuiltInOptions().coreFlags).filter(({ name }) => name.startsWith('resolve'));

describe('resolve config related flags', () => {
resolveFlags.forEach((flag) => {
Expand Down
Loading