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.

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

ProgressPlugin: support progress by entry points #8279

Merged
merged 10 commits into from Nov 5, 2018
34 changes: 34 additions & 0 deletions declarations/plugins/ProgressPlugin.d.ts
@@ -0,0 +1,34 @@
/**
* This file was automatically generated.
* DO NOT MODIFY BY HAND.
* Run `yarn special-lint-fix` to update
*/

export type ProgressPluginArgument = ProgressPluginOptions | HandlerFunction;
/**
* Function that executes for every progress step
*/
export type HandlerFunction = ((
percentage: number,
msg: string,
...args
) => void);

export interface ProgressPluginOptions {
/**
* Function that executes for every progress step
*/
handler?: HandlerFunction;
/**
* Counting mode. Default: modules
*/
mode?: "modules" | "entries";
/**
* Minimum modules count to start with. Only for mode=modules. Default: 500
*/
modulesCount?: number;
/**
* Collect profile data for progress steps. Default: false
*/
profile?: true | false | null;
}
14 changes: 14 additions & 0 deletions lib/Compilation.js
Expand Up @@ -224,6 +224,15 @@ class Compilation extends Tapable {
/** @type {SyncHook<Module>} */
succeedModule: new SyncHook(["module"]),

/** @type {SyncHook<Dependency>} */
addEntry: new SyncHook(["entry"]),
/** @type {SyncHook<Module>} */
buildEntry: new SyncHook(["module"]),
/** @type {SyncHook<Module, Error>} */
failedEntry: new SyncHook(["module", "error"]),
/** @type {SyncHook<Module>} */
succeedEntry: new SyncHook(["module"]),

/** @type {SyncWaterfallHook<DependencyReference, Dependency, Module>} */
dependencyReference: new SyncWaterfallHook([
"dependencyReference",
Expand Down Expand Up @@ -1041,6 +1050,8 @@ class Compilation extends Tapable {
* @returns {void} returns
*/
addEntry(context, entry, name, callback) {
this.hooks.addEntry.call(entry);
Copy link
Member

Choose a reason for hiding this comment

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

rename to buildEntry and also pass name as arguments

Copy link
Member Author

@smelukov smelukov Oct 29, 2018

Choose a reason for hiding this comment

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

addEntry is not the same that buildEntry
addEntry will be called when adding an entry. This need to calculate total entries
buildEntry will be called when the module for an entry will be built
Or what do you mean? :)


const slot = {
name: name,
// TODO webpack 5 remove `request`
Expand All @@ -1064,10 +1075,12 @@ class Compilation extends Tapable {
context,
entry,
module => {
this.hooks.buildEntry.call(module);
smelukov marked this conversation as resolved.
Show resolved Hide resolved
this.entries.push(module);
},
(err, module) => {
if (err) {
this.hooks.failedEntry.call(module, err);
smelukov marked this conversation as resolved.
Show resolved Hide resolved
return callback(err);
}

Expand All @@ -1079,6 +1092,7 @@ class Compilation extends Tapable {
this._preparedEntrypoints.splice(idx, 1);
}
}
this.hooks.succeedEntry.call(module);
smelukov marked this conversation as resolved.
Show resolved Hide resolved
return callback(null, module);
}
);
Expand Down
69 changes: 58 additions & 11 deletions lib/ProgressPlugin.js
Expand Up @@ -4,6 +4,12 @@
*/
"use strict";

const validateOptions = require("schema-utils");
const schema = require("../schemas/plugins/ProgressPlugin.json");

/** @typedef {import("../declarations/plugins/ProgressPlugin").ProgressPluginArgument} ProgressPluginArgument */
/** @typedef {import("../declarations/plugins/ProgressPlugin").ProgressPluginOptions} ProgressPluginOptions */

const createDefaultHandler = profile => {
let lineCaretPosition = 0;
let lastState;
Expand Down Expand Up @@ -66,18 +72,28 @@ const createDefaultHandler = profile => {
};

class ProgressPlugin {
/**
* @param {ProgressPluginArgument} options options
*/
constructor(options) {
if (typeof options === "function") {
options = {
handler: options
};
}

options = options || {};
validateOptions(schema, options, "Progress Plugin");
options = Object.assign(ProgressPlugin.defaultOptions, options);
smelukov marked this conversation as resolved.
Show resolved Hide resolved

this.profile = options.profile;
this.handler = options.handler;
this.mode = options.mode;
smelukov marked this conversation as resolved.
Show resolved Hide resolved
this.modulesCount = options.modulesCount;
}

apply(compiler) {
const { mode, modulesCount } = this;
const handler = this.handler || createDefaultHandler(this.profile);
if (compiler.compilers) {
const states = new Array(compiler.compilers.length);
Expand All @@ -94,16 +110,16 @@ class ProgressPlugin {
}).apply(compiler);
});
} else {
let lastModulesCount = 0;
let moduleCount = 500;
let lastCount = 0;
let count = mode === "modules" ? modulesCount : 0;
let doneModules = 0;
const activeModules = [];

const update = module => {
handler(
0.1 + (doneModules / Math.max(lastModulesCount, moduleCount)) * 0.6,
"building modules",
`${doneModules}/${moduleCount} modules`,
0.1 + (doneModules / Math.max(lastCount, count)) * 0.6,
`building ${mode}`,
`${doneModules}/${count} ${mode}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be much more useful if the handler took raw data (as an object or arguments in a particular order), rather than pre-made strings. E.g. in my implementation of a progress bar using the ProgressPlugin I have to resort to regexps/splits to re-parse the data out of the strings in order to be able to position them:

progress-bar

If instead we provided a default handler that makes these strings above, we can get the same behavior, and still give greater flexibility to consumers, alternative handlers.

I would imagine handler being most flexible e.g. when called like this:

handler({compilerIndex, currentStage, doneModules, activeModules, lastModulesCount, moduleCount, count, mode})

And some of those parameters would obviously be undefined in certain stages (which is fine). By stage I mean the hook currently being executed, or compilation.

Copy link
Member Author

Choose a reason for hiding this comment

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

@niieani Seems like this is a breaking change, but in this PR I don't want to make any breaking changes
I think you may create your own PR for this feature to the next-branch

`${activeModules.length} active`,
activeModules[activeModules.length - 1]
);
Expand All @@ -120,20 +136,44 @@ class ProgressPlugin {
};
compiler.hooks.compilation.tap("ProgressPlugin", compilation => {
if (compilation.compiler.isChild()) return;
lastModulesCount = moduleCount;
moduleCount = 0;
lastCount = count;
count = 0;
doneModules = 0;
handler(0, "compiling");
compilation.hooks.buildModule.tap("ProgressPlugin", module => {
moduleCount++;
const addHook =
mode === "modules"
? compilation.hooks.buildModule
: compilation.hooks.addEntry;
const startHook =
mode === "modules"
? compilation.hooks.buildModule
: compilation.hooks.buildEntry;
const failedHook =
mode === "modules"
? compilation.hooks.failedModule
: compilation.hooks.failedEntry;
const succeedHook =
mode === "modules"
? compilation.hooks.succeedModule
: compilation.hooks.succeedEntry;

lastCount = count;
count = 0;
doneModules = 0;

addHook.tap("ProgressPlugin", () => {
count++;
update();
});
startHook.tap("ProgressPlugin", module => {
const ident = module.identifier();
if (ident) {
activeModules.push(ident);
}
update();
});
compilation.hooks.failedModule.tap("ProgressPlugin", moduleDone);
compilation.hooks.succeedModule.tap("ProgressPlugin", moduleDone);
failedHook.tap("ProgressPlugin", moduleDone);
succeedHook.tap("ProgressPlugin", moduleDone);
const hooks = {
finishModules: "finish module graph",
seal: "sealing",
Expand Down Expand Up @@ -243,4 +283,11 @@ class ProgressPlugin {
}
}
}

ProgressPlugin.defaultOptions = {
profile: false,
mode: "modules",
modulesCount: 500
};

module.exports = ProgressPlugin;
42 changes: 42 additions & 0 deletions schemas/plugins/ProgressPlugin.json
@@ -0,0 +1,42 @@
{
"definitions": {
"HandlerFunction": {
"description": "Function that executes for every progress step",
"instanceof": "Function",
"tsType": "((percentage: number, msg: string, ...args) => void)"
smelukov marked this conversation as resolved.
Show resolved Hide resolved
}
},
"title": "ProgressPluginArgument",
"oneOf": [
{
"title": "ProgressPluginOptions",
"type": "object",
"additionalProperties": false,
"properties": {
"handler": {
"description": "Function that executes for every progress step",
"anyOf": [
{
"$ref": "#/definitions/HandlerFunction"
}
]
},
"mode": {
"description": "Counting mode. Default: modules",
"enum": ["modules", "entries"]
},
"modulesCount": {
"description": "Minimum modules count to start with. Only for mode=modules. Default: 500",
"type": "number"
},
"profile": {
"description": "Collect profile data for progress steps. Default: false",
"enum": [true, false, null]
smelukov marked this conversation as resolved.
Show resolved Hide resolved
}
}
},
{
"$ref": "#/definitions/HandlerFunction"
}
]
}