Skip to content

Commit 3e0aa2e

Browse files
bjohansebasbjohansebas
committed
fix: purge pre-rebuild connections of deferred re-factorizations
A deferred module is rebuilt by its importer's walk, which replaces the dependencies without the direct re-factorize cleanup, so removed imports kept their stale graph edges and stayed in the bundle.
1 parent c360118 commit 3e0aa2e

8 files changed

Lines changed: 90 additions & 28 deletions

File tree

‎lib/Compilation.js‎

Lines changed: 45 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1383,6 +1383,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
13831383
this._incrementalPrevSealModules = incrementalPrev
13841384
? incrementalPrev.modules
13851385
: undefined;
1386+
// old deps/blocks of deferred re-factorizations; the importer-driven
1387+
// rebuild replaces the deps without the direct re-factorize cleanup
1388+
/** @type {Map<Module, { dependencies: Dependency[], blocks: AsyncDependenciesBlock[] }>} */
1389+
this._incrementalDeferredStaleRefs = new Map();
13861390
// restores released parsers of persisted modules built during make seeding
13871391
/** @type {import("./NormalModuleFactory") | undefined} */
13881392
this._incrementalNormalModuleFactory = undefined;
@@ -1847,6 +1851,15 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
18471851
this.hooks.failedModule.call(module, err);
18481852
return callback(err);
18491853
}
1854+
if (this._incrementalDeferredStaleRefs.size > 0) {
1855+
const staleRefs = this._incrementalDeferredStaleRefs.get(module);
1856+
if (staleRefs !== undefined) {
1857+
// deferred re-factorize: purge the pre-rebuild connections the
1858+
// direct path removes via its own cleanup
1859+
this._incrementalDeferredStaleRefs.delete(module);
1860+
this._removePreRebuildConnections(module, staleRefs);
1861+
}
1862+
}
18501863
if (
18511864
this._incrementalRebuild &&
18521865
module.factoryMeta &&
@@ -3452,6 +3465,12 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
34523465
// force the build so a still-valid build snapshot (resolve-snapshot
34533466
// invalidations) doesn't short-circuit the re-walk of its own deps
34543467
module.invalidateBuild();
3468+
// that walk replaces the deps without this path's cleanup — record
3469+
// the old ones so the build callback can purge their connections
3470+
this._incrementalDeferredStaleRefs.set(module, {
3471+
dependencies: [...module.dependencies],
3472+
blocks: [...module.blocks]
3473+
});
34553474
return callback(null, module);
34563475
}
34573476
// no factorizable incoming dependency (unusual) — plain rebuild
@@ -3493,21 +3512,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
34933512
);
34943513
}
34953514
// drop the pre-rebuild deps of the old module
3496-
this.removeReasonsOfDependencyBlock(module, {
3515+
this._removePreRebuildConnections(module, {
34973516
dependencies: oldDependencies,
34983517
blocks: oldBlocks
34993518
});
3500-
/** @type {Set<Dependency>} */
3501-
const staleDeps = new Set(oldDependencies);
3502-
/** @type {(blocks: import("./AsyncDependenciesBlock")[]) => void} */
3503-
const collectBlockDeps = (blocks) => {
3504-
for (const b of blocks) {
3505-
for (const d of b.dependencies) staleDeps.add(d);
3506-
if (b.blocks) collectBlockDeps(b.blocks);
3507-
}
3508-
};
3509-
collectBlockDeps(oldBlocks);
3510-
this.moduleGraph._removeStaleOutgoingConnections(module, staleDeps);
35113519
if (built !== module) {
35123520
this._incrementalRefactorReplaced = true;
35133521
// move the other importers too, else they render against the
@@ -3531,6 +3539,30 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
35313539
);
35323540
}
35333541

3542+
/**
3543+
* Removes a rebuilt module's pre-rebuild reasons and, on incremental
3544+
* rebuilds, the stale outgoing connections (incl. inactive ones) the
3545+
* dependency-keyed removal misses.
3546+
* @param {Module} module the rebuilt module
3547+
* @param {{ dependencies: Dependency[], blocks: AsyncDependenciesBlock[] }} staleRefs pre-rebuild deps and blocks
3548+
* @returns {void}
3549+
*/
3550+
_removePreRebuildConnections(module, staleRefs) {
3551+
this.removeReasonsOfDependencyBlock(module, staleRefs);
3552+
if (!this._incrementalRebuild) return;
3553+
/** @type {Set<Dependency>} */
3554+
const staleDeps = new Set(staleRefs.dependencies);
3555+
/** @type {(blocks: AsyncDependenciesBlock[]) => void} */
3556+
const collectBlockDeps = (blocks) => {
3557+
for (const b of blocks) {
3558+
for (const d of b.dependencies) staleDeps.add(d);
3559+
if (b.blocks) collectBlockDeps(b.blocks);
3560+
}
3561+
};
3562+
collectBlockDeps(staleRefs.blocks);
3563+
this.moduleGraph._removeStaleOutgoingConnections(module, staleDeps);
3564+
}
3565+
35343566
/**
35353567
* Processes the provided module.
35363568
* @param {Module} module module to be rebuilt
@@ -3560,25 +3592,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
35603592
this.moduleGraph.unfreeze();
35613593
this.processModuleDependencies(module, (err) => {
35623594
if (err) return callback(err);
3563-
this.removeReasonsOfDependencyBlock(module, {
3595+
this._removePreRebuildConnections(module, {
35643596
dependencies: oldDependencies,
35653597
blocks: oldBlocks
35663598
});
3567-
// purge stale outgoing connections (incl. inactive ones) that the
3568-
// dependency-keyed removeReasonsOfDependencyBlock misses
3569-
if (this._incrementalRebuild) {
3570-
/** @type {Set<Dependency>} */
3571-
const staleDeps = new Set(oldDependencies);
3572-
/** @type {(blocks: import("./AsyncDependenciesBlock")[]) => void} */
3573-
const collectBlockDeps = (blocks) => {
3574-
for (const b of blocks) {
3575-
for (const d of b.dependencies) staleDeps.add(d);
3576-
if (b.blocks) collectBlockDeps(b.blocks);
3577-
}
3578-
};
3579-
collectBlockDeps(oldBlocks);
3580-
this.moduleGraph._removeStaleOutgoingConnections(module, staleDeps);
3581-
}
35823599
this.hooks.finishRebuildingModule.callAsync(module, (err2) => {
35833600
if (err2) {
35843601
callback(

‎test/watchCases/incremental/deferred-removed-import/0/index.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { value } from "./m";
2+
3+
it("should drop modules a deferred rebuild stopped importing (step 0)", () => {
4+
const fs = __non_webpack_require__("fs");
5+
const path = __non_webpack_require__("path");
6+
const content = fs.readFileSync(path.resolve(__dirname, "bundle.js"), "utf8");
7+
expect(value).toBe(1);
8+
expect(content).toContain(`__X_${"MARKER"}__`);
9+
});

‎test/watchCases/incremental/deferred-removed-import/0/m.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import "./x";
2+
3+
export const value = 1;

‎test/watchCases/incremental/deferred-removed-import/0/x.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
global.__X_MARKER__ = true;

‎test/watchCases/incremental/deferred-removed-import/1/index.js‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
import { value } from "./m";
2+
3+
it("should drop modules a deferred rebuild stopped importing (step 1)", () => {
4+
const fs = __non_webpack_require__("fs");
5+
const path = __non_webpack_require__("path");
6+
const content = fs.readFileSync(path.resolve(__dirname, "bundle.js"), "utf8");
7+
expect(value).toBe(2);
8+
expect(content).not.toContain(`__X_${"MARKER"}__`);
9+
});

‎test/watchCases/incremental/deferred-removed-import/1/m.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export const value = 2;

‎test/watchCases/incremental/deferred-removed-import/loader.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
"use strict";
2+
3+
/**
4+
* @param {string} source source
5+
* @returns {string} source
6+
*/
7+
module.exports = (source) => source;

‎test/watchCases/incremental/deferred-removed-import/webpack.config.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
"use strict";
2+
3+
const path = require("path");
4+
5+
/** @type {import("../../../../").Configuration} */
6+
module.exports = {
7+
module: {
8+
rules: [
9+
{
10+
test: /m\.js$/,
11+
use: path.resolve(__dirname, "loader.js")
12+
}
13+
]
14+
}
15+
};

0 commit comments

Comments
 (0)