From 1485eb51b9789b98659b483ef5593206a42824fb Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Wed, 16 Feb 2022 18:12:59 +0300 Subject: [PATCH 1/2] keep hmr update with output.clean=true based on timestamp --- lib/CleanPlugin.js | 82 +++++++++++++++++++------ test/HotModuleReplacementPlugin.test.js | 75 ++++++++++++++++++++++ 2 files changed, 139 insertions(+), 18 deletions(-) diff --git a/lib/CleanPlugin.js b/lib/CleanPlugin.js index 074b90bf450..3b50a1c2008 100644 --- a/lib/CleanPlugin.js +++ b/lib/CleanPlugin.js @@ -19,6 +19,7 @@ const processAsyncTree = require("./util/processAsyncTree"); /** @typedef {import("./util/fs").StatsCallback} StatsCallback */ /** @typedef {(function(string):boolean)|RegExp} IgnoreItem */ +/** @typedef {Map} Assets */ /** @typedef {function(IgnoreItem): void} AddToIgnoreCallback */ /** @@ -40,18 +41,32 @@ const validate = createSchemaValidation( baseDataPath: "options" } ); +const _10sec = 10 * 1000; + +/** + * marge assets map 2 into map 1 + * @param {Assets} as1 assets + * @param {Assets} as2 assets + * @returns {void} + */ +const mergeAssets = (as1, as2) => { + for (const [key, value1] of as2) { + const value2 = as1.get(key); + if (!value2 || value1 > value2) as1.set(key, value1); + } +}; /** * @param {OutputFileSystem} fs filesystem * @param {string} outputPath output path - * @param {Set} currentAssets filename of the current assets (must not start with .. or ., must only use / as path separator) + * @param {Map} currentAssets filename of the current assets (must not start with .. or ., must only use / as path separator) * @param {function((Error | null)=, Set=): void} callback returns the filenames of the assets that shouldn't be there * @returns {void} */ const getDiffToFs = (fs, outputPath, currentAssets, callback) => { const directories = new Set(); // get directories of assets - for (const asset of currentAssets) { + for (const [asset] of currentAssets) { directories.add(asset.replace(/(^|\/)[^/]*$/, "")); } // and all parent directories @@ -91,13 +106,15 @@ const getDiffToFs = (fs, outputPath, currentAssets, callback) => { }; /** - * @param {Set} currentAssets assets list - * @param {Set} oldAssets old assets list + * @param {Assets} currentAssets assets list + * @param {Assets} oldAssets old assets list * @returns {Set} diff */ const getDiffToOldAssets = (currentAssets, oldAssets) => { const diff = new Set(); - for (const asset of oldAssets) { + const now = Date.now(); + for (const [asset, ts] of oldAssets) { + if (ts >= now) continue; if (!currentAssets.has(asset)) diff.add(asset); } return diff; @@ -124,7 +141,7 @@ const doStat = (fs, filename, callback) => { * @param {Logger} logger logger * @param {Set} diff filenames of the assets that shouldn't be there * @param {function(string): boolean} isKept check if the entry is ignored - * @param {function(Error=): void} callback callback + * @param {function(Error=, Assets=): void} callback callback * @returns {void} */ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => { @@ -137,11 +154,13 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => { }; /** @typedef {{ type: "check" | "unlink" | "rmdir", filename: string, parent: { remaining: number, job: Job } | undefined }} Job */ /** @type {Job[]} */ - const jobs = Array.from(diff, filename => ({ + const jobs = Array.from(diff.keys(), filename => ({ type: "check", filename, parent: undefined })); + /** @type {Assets} */ + const keptAssets = new Map(); processAsyncTree( jobs, 10, @@ -161,6 +180,7 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => { switch (type) { case "check": if (isKept(filename)) { + keptAssets.set(filename, Date.now()); // do not decrement parent entry as we don't want to delete the parent log(`${filename} will be kept`); return process.nextTick(callback); @@ -247,7 +267,10 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => { break; } }, - callback + err => { + if (err) return callback(err); + callback(undefined, keptAssets); + } ); }; @@ -302,6 +325,7 @@ class CleanPlugin { // We assume that no external modification happens while the compiler is active // So we can store the old assets and only diff to them to avoid fs access on // incremental builds + /** @type {undefined|Assets} */ let oldAssets; compiler.hooks.emit.tapAsync( @@ -322,7 +346,9 @@ class CleanPlugin { ); } - const currentAssets = new Set(); + /** @type {Assets} */ + const currentAssets = new Map(); + const now = Date.now(); for (const asset of Object.keys(compilation.assets)) { if (/^[A-Za-z]:\\|^\/|^\\\\/.test(asset)) continue; let normalizedAsset; @@ -335,7 +361,12 @@ class CleanPlugin { ); } while (newNormalizedAsset !== normalizedAsset); if (normalizedAsset.startsWith("../")) continue; - currentAssets.add(normalizedAsset); + const assetInfo = compilation.assetsInfo.get(asset); + if (assetInfo && assetInfo.hotModuleReplacement) { + currentAssets.set(normalizedAsset, now + _10sec); + } else { + currentAssets.set(normalizedAsset, now); + } } const outputPath = compilation.getPath(compiler.outputPath, {}); @@ -346,19 +377,34 @@ class CleanPlugin { return keepFn(path); }; + /** + * @param {Error=} err err + * @param {Set=} diff diff + */ const diffCallback = (err, diff) => { if (err) { oldAssets = undefined; - return callback(err); + callback(err); + return; } - applyDiff(fs, outputPath, dry, logger, diff, isKept, err => { - if (err) { - oldAssets = undefined; - } else { - oldAssets = currentAssets; + applyDiff( + fs, + outputPath, + dry, + logger, + diff, + isKept, + (err, notDeletedAssets) => { + if (err) { + oldAssets = undefined; + } else { + if (oldAssets) mergeAssets(currentAssets, oldAssets); + oldAssets = currentAssets; + if (notDeletedAssets) mergeAssets(oldAssets, notDeletedAssets); + } + callback(err); } - callback(err); - }); + ); }; if (oldAssets) { diff --git a/test/HotModuleReplacementPlugin.test.js b/test/HotModuleReplacementPlugin.test.js index dd3fb6c4907..94409085138 100644 --- a/test/HotModuleReplacementPlugin.test.js +++ b/test/HotModuleReplacementPlugin.test.js @@ -99,6 +99,81 @@ describe("HotModuleReplacementPlugin", () => { }); }, 120000); + it("output.clean=true should keep 1 last update", done => { + const outputPath = path.join(__dirname, "js", "HotModuleReplacementPlugin"); + const entryFile = path.join(outputPath, "entry.js"); + const recordsFile = path.join(outputPath, "records.json"); + let step = 0; + let firstUpdate; + try { + fs.mkdirSync(outputPath, { recursive: true }); + } catch (e) { + // empty + } + fs.writeFileSync(entryFile, `${++step}`, "utf-8"); + const updates = new Set(); + const hasFile = file => { + try { + fs.statSync(path.join(outputPath, file)); + return true; + } catch (err) { + return false; + } + }; + const compiler = webpack({ + mode: "development", + cache: false, + entry: { + 0: entryFile + }, + recordsPath: recordsFile, + output: { + path: outputPath, + clean: true + }, + plugins: [new webpack.HotModuleReplacementPlugin()] + }); + const callback = (err, stats) => { + if (err) return done(err); + const jsonStats = stats.toJson(); + const hash = jsonStats.hash; + const hmrUpdateMainFileName = `0.${hash}.hot-update.json`; + + switch (step) { + case 1: + expect(updates.size).toBe(0); + firstUpdate = hmrUpdateMainFileName; + break; + case 2: + expect(updates.size).toBe(1); + expect(updates.has(firstUpdate)).toBe(true); + expect(hasFile(firstUpdate)).toBe(true); + break; + case 3: + expect(updates.size).toBe(2); + for (const file of updates) { + expect(hasFile(file)).toBe(true); + } + return setTimeout(() => { + fs.writeFileSync(entryFile, `${++step}`, "utf-8"); + compiler.run(err => { + if (err) return done(err); + for (const file of updates) { + expect(hasFile(file)).toBe(false); + } + done(); + }); + }, 10100); + } + + updates.add(hmrUpdateMainFileName); + fs.writeFileSync(entryFile, `${++step}`, "utf-8"); + compiler.run(callback); + }; + + compiler.run(callback); + }, 20000); + it("should correct working when entry is Object and key is a number", done => { const outputPath = path.join(__dirname, "js", "HotModuleReplacementPlugin"); const entryFile = path.join(outputPath, "entry.js"); From 8f4807dcbfa8d21e904b82482c042f32fa1c642e Mon Sep 17 00:00:00 2001 From: Ivan Kopeykin Date: Mon, 28 Feb 2022 15:20:44 +0300 Subject: [PATCH 2/2] fix timestamps --- lib/CleanPlugin.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/CleanPlugin.js b/lib/CleanPlugin.js index 3b50a1c2008..ee4a9a8b7a9 100644 --- a/lib/CleanPlugin.js +++ b/lib/CleanPlugin.js @@ -180,7 +180,7 @@ const applyDiff = (fs, outputPath, dry, logger, diff, isKept, callback) => { switch (type) { case "check": if (isKept(filename)) { - keptAssets.set(filename, Date.now()); + keptAssets.set(filename, 0); // do not decrement parent entry as we don't want to delete the parent log(`${filename} will be kept`); return process.nextTick(callback); @@ -365,7 +365,7 @@ class CleanPlugin { if (assetInfo && assetInfo.hotModuleReplacement) { currentAssets.set(normalizedAsset, now + _10sec); } else { - currentAssets.set(normalizedAsset, now); + currentAssets.set(normalizedAsset, 0); } } @@ -394,13 +394,13 @@ class CleanPlugin { logger, diff, isKept, - (err, notDeletedAssets) => { + (err, keptAssets) => { if (err) { oldAssets = undefined; } else { if (oldAssets) mergeAssets(currentAssets, oldAssets); oldAssets = currentAssets; - if (notDeletedAssets) mergeAssets(oldAssets, notDeletedAssets); + if (keptAssets) mergeAssets(oldAssets, keptAssets); } callback(err); }