Skip to content

Commit 80b1e77

Browse files
sokrasokra
committed
fix duplication due to unsafe cache
1 parent 2306d13 commit 80b1e77

File tree

11 files changed

+224
-99
lines changed

11 files changed

+224
-99
lines changed

‎lib/Compilation.js‎

Lines changed: 163 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -416,10 +416,10 @@ const byLocation = compareSelect(err => err.loc, compareLocations);
416416

417417
const compareErrors = concatComparators(byModule, byLocation, byMessage);
418418

419-
/** @type {WeakMap<Dependency, Module & { restoreFromUnsafeCache: Function }>} */
419+
/** @type {WeakMap<Dependency, Module & { restoreFromUnsafeCache: Function } | null>} */
420420
const unsafeCacheDependencies = new WeakMap();
421421

422-
/** @type {WeakMap<Module, object>} */
422+
/** @type {WeakMap<Module & { restoreFromUnsafeCache: Function }, object>} */
423423
const unsafeCacheData = new WeakMap();
424424

425425
class Compilation {
@@ -1023,8 +1023,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
10231023
this.usedModuleIds = null;
10241024
/** @type {boolean} */
10251025
this.needAdditionalPass = false;
1026-
/** @type {Set<Module>} */
1027-
this._restoredUnsafeCacheEntries = new Set();
1026+
/** @type {Set<Module & { restoreFromUnsafeCache: Function }>} */
1027+
this._restoredUnsafeCacheModuleEntries = new Set();
1028+
/** @type {Map<string, Module & { restoreFromUnsafeCache: Function }>} */
1029+
this._restoredUnsafeCacheEntries = new Map();
10281030
/** @type {WeakSet<Module>} */
10291031
this.builtModules = new WeakSet();
10301032
/** @type {WeakSet<Module>} */
@@ -1424,9 +1426,7 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
14241426
* @returns {void}
14251427
*/
14261428
_processModuleDependencies(module, callback) {
1427-
/**
1428-
* @type {Array<{factory: ModuleFactory, dependencies: Dependency[], originModule: Module|null}>}
1429-
*/
1429+
/** @type {Array<{factory: ModuleFactory, dependencies: Dependency[], originModule: Module|null}>} */
14301430
const sortedDependencies = [];
14311431

14321432
/** @type {DependenciesBlock} */
@@ -1447,7 +1447,46 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
14471447
/** @type {Dependency[]} */
14481448
let listCacheValue;
14491449

1450-
const unsafeRestoredModules = new Set();
1450+
let inProgressSorting = 1;
1451+
let inProgressTransitive = 1;
1452+
1453+
const onDependenciesSorted = err => {
1454+
if (err) return callback(err);
1455+
1456+
// early exit without changing parallelism back and forth
1457+
if (sortedDependencies.length === 0 && inProgressTransitive === 1) {
1458+
return callback();
1459+
}
1460+
1461+
// This is nested so we need to allow one additional task
1462+
this.processDependenciesQueue.increaseParallelism();
1463+
1464+
for (const item of sortedDependencies) {
1465+
inProgressTransitive++;
1466+
this.handleModuleCreation(item, err => {
1467+
// In V8, the Error objects keep a reference to the functions on the stack. These warnings &
1468+
// errors are created inside closures that keep a reference to the Compilation, so errors are
1469+
// leaking the Compilation object.
1470+
if (err && this.bail) {
1471+
if (inProgressTransitive <= 0) return;
1472+
inProgressTransitive = -1;
1473+
// eslint-disable-next-line no-self-assign
1474+
err.stack = err.stack;
1475+
onTransitiveTasksFinished(err);
1476+
return;
1477+
}
1478+
if (--inProgressTransitive === 0) onTransitiveTasksFinished();
1479+
});
1480+
}
1481+
if (--inProgressTransitive === 0) onTransitiveTasksFinished();
1482+
};
1483+
1484+
const onTransitiveTasksFinished = err => {
1485+
if (err) return callback(err);
1486+
this.processDependenciesQueue.decreaseParallelism();
1487+
1488+
return callback();
1489+
};
14511490

14521491
/**
14531492
* @param {Dependency} dep dependency
@@ -1458,34 +1497,111 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
14581497
this.moduleGraph.setParents(dep, currentBlock, module, index);
14591498
if (this._unsafeCache) {
14601499
try {
1461-
const cachedModule = unsafeCacheDependencies.get(dep);
1462-
if (cachedModule === null) return;
1463-
if (cachedModule !== undefined) {
1464-
if (!this._restoredUnsafeCacheEntries.has(cachedModule)) {
1465-
const data = unsafeCacheData.get(cachedModule);
1466-
cachedModule.restoreFromUnsafeCache(
1467-
data,
1468-
this.params.normalModuleFactory,
1469-
this.params
1500+
const unsafeCachedModule = unsafeCacheDependencies.get(dep);
1501+
if (unsafeCachedModule === null) return;
1502+
if (unsafeCachedModule !== undefined) {
1503+
if (
1504+
this._restoredUnsafeCacheModuleEntries.has(unsafeCachedModule)
1505+
) {
1506+
this._handleExistingModuleFromUnsafeCache(
1507+
module,
1508+
dep,
1509+
unsafeCachedModule
14701510
);
1471-
this._restoredUnsafeCacheEntries.add(cachedModule);
1472-
if (!this.modules.has(cachedModule)) {
1473-
this._handleNewModuleFromUnsafeCache(module, dep, cachedModule);
1474-
unsafeRestoredModules.add(cachedModule);
1511+
return;
1512+
}
1513+
const identifier = unsafeCachedModule.identifier();
1514+
const cachedModule =
1515+
this._restoredUnsafeCacheEntries.get(identifier);
1516+
if (cachedModule !== undefined) {
1517+
// update unsafe cache to new module
1518+
unsafeCacheDependencies.set(dep, cachedModule);
1519+
this._handleExistingModuleFromUnsafeCache(
1520+
module,
1521+
dep,
1522+
cachedModule
1523+
);
1524+
return;
1525+
}
1526+
inProgressSorting++;
1527+
this._modulesCache.get(identifier, null, (err, cachedModule) => {
1528+
if (err) {
1529+
if (inProgressSorting <= 0) return;
1530+
inProgressSorting = -1;
1531+
onDependenciesSorted(err);
14751532
return;
14761533
}
1477-
}
1478-
this._handleExistingModuleFromUnsafeCache(
1479-
module,
1480-
dep,
1481-
cachedModule
1482-
);
1534+
try {
1535+
if (!this._restoredUnsafeCacheEntries.has(identifier)) {
1536+
const data = unsafeCacheData.get(cachedModule);
1537+
if (data === undefined) {
1538+
processDependencyForResolving(dep);
1539+
if (--inProgressSorting === 0) onDependenciesSorted();
1540+
return;
1541+
}
1542+
if (cachedModule !== unsafeCachedModule) {
1543+
unsafeCacheDependencies.set(dep, cachedModule);
1544+
}
1545+
cachedModule.restoreFromUnsafeCache(
1546+
data,
1547+
this.params.normalModuleFactory,
1548+
this.params
1549+
);
1550+
this._restoredUnsafeCacheEntries.set(
1551+
identifier,
1552+
cachedModule
1553+
);
1554+
this._restoredUnsafeCacheModuleEntries.add(cachedModule);
1555+
if (!this.modules.has(cachedModule)) {
1556+
inProgressTransitive++;
1557+
this._handleNewModuleFromUnsafeCache(
1558+
module,
1559+
dep,
1560+
cachedModule,
1561+
err => {
1562+
if (err) {
1563+
if (inProgressTransitive <= 0) return;
1564+
inProgressTransitive = -1;
1565+
onTransitiveTasksFinished(err);
1566+
}
1567+
if (--inProgressTransitive === 0)
1568+
return onTransitiveTasksFinished();
1569+
}
1570+
);
1571+
if (--inProgressSorting === 0) onDependenciesSorted();
1572+
return;
1573+
}
1574+
}
1575+
if (unsafeCachedModule !== cachedModule) {
1576+
unsafeCacheDependencies.set(dep, cachedModule);
1577+
}
1578+
this._handleExistingModuleFromUnsafeCache(
1579+
module,
1580+
dep,
1581+
cachedModule
1582+
); // a3
1583+
} catch (err) {
1584+
if (inProgressSorting <= 0) return;
1585+
inProgressSorting = -1;
1586+
onDependenciesSorted(err);
1587+
return;
1588+
}
1589+
if (--inProgressSorting === 0) onDependenciesSorted();
1590+
});
14831591
return;
14841592
}
14851593
} catch (e) {
14861594
console.error(e);
14871595
}
14881596
}
1597+
processDependencyForResolving(dep);
1598+
};
1599+
1600+
/**
1601+
* @param {Dependency} dep dependency
1602+
* @returns {void}
1603+
*/
1604+
const processDependencyForResolving = dep => {
14891605
const resourceIdent = dep.getResourceIdentifier();
14901606
if (resourceIdent !== undefined && resourceIdent !== null) {
14911607
const category = dep.category;
@@ -1570,68 +1686,10 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
15701686
return callback(e);
15711687
}
15721688

1573-
if (sortedDependencies.length === 0 && unsafeRestoredModules.size === 0) {
1574-
callback();
1575-
return;
1576-
}
1577-
1578-
// This is nested so we need to allow one additional task
1579-
this.processDependenciesQueue.increaseParallelism();
1580-
1581-
const processSortedDependency = (item, callback) => {
1582-
this.handleModuleCreation(item, err => {
1583-
// In V8, the Error objects keep a reference to the functions on the stack. These warnings &
1584-
// errors are created inside closures that keep a reference to the Compilation, so errors are
1585-
// leaking the Compilation object.
1586-
if (err && this.bail) {
1587-
// eslint-disable-next-line no-self-assign
1588-
err.stack = err.stack;
1589-
return callback(err);
1590-
}
1591-
callback();
1592-
});
1593-
};
1594-
1595-
const processUnsafeRestoredModule = (item, callback) => {
1596-
this._handleModuleBuildAndDependencies(module, item, true, callback);
1597-
};
1598-
1599-
const finalCallback = err => {
1600-
this.processDependenciesQueue.decreaseParallelism();
1601-
1602-
return callback(err);
1603-
};
1604-
1605-
if (sortedDependencies.length === 0) {
1606-
asyncLib.forEach(
1607-
unsafeRestoredModules,
1608-
processUnsafeRestoredModule,
1609-
finalCallback
1610-
);
1611-
} else if (unsafeRestoredModules.size === 0) {
1612-
asyncLib.forEach(
1613-
sortedDependencies,
1614-
processSortedDependency,
1615-
finalCallback
1616-
);
1617-
} else {
1618-
asyncLib.parallel(
1619-
[
1620-
cb =>
1621-
asyncLib.forEach(
1622-
unsafeRestoredModules,
1623-
processUnsafeRestoredModule,
1624-
cb
1625-
),
1626-
cb =>
1627-
asyncLib.forEach(sortedDependencies, processSortedDependency, cb)
1628-
],
1629-
finalCallback
1630-
);
1631-
}
1689+
if (--inProgressSorting === 0) onDependenciesSorted();
16321690
}
16331691

1634-
_handleNewModuleFromUnsafeCache(originModule, dependency, module) {
1692+
_handleNewModuleFromUnsafeCache(originModule, dependency, module, callback) {
16351693
const moduleGraph = this.moduleGraph;
16361694

16371695
moduleGraph.setResolvedModule(originModule, dependency, module);
@@ -1644,6 +1702,13 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
16441702
this._modules.set(module.identifier(), module);
16451703
this.modules.add(module);
16461704
ModuleGraph.setModuleGraphForModule(module, this.moduleGraph);
1705+
1706+
this._handleModuleBuildAndDependencies(
1707+
originModule,
1708+
module,
1709+
true,
1710+
callback
1711+
);
16471712
}
16481713

16491714
_handleExistingModuleFromUnsafeCache(originModule, dependency, module) {
@@ -1747,20 +1812,24 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
17471812
/** @type {any} */ (module).restoreFromUnsafeCache &&
17481813
this._unsafeCachePredicate(module)
17491814
) {
1815+
const unsafeCacheableModule =
1816+
/** @type {Module & { restoreFromUnsafeCache: Function }} */ (
1817+
module
1818+
);
17501819
for (let i = 0; i < dependencies.length; i++) {
17511820
const dependency = dependencies[i];
17521821
moduleGraph.setResolvedModule(
17531822
connectOrigin ? originModule : null,
17541823
dependency,
1755-
module
1756-
);
1757-
unsafeCacheDependencies.set(
1758-
dependency,
1759-
/** @type {any} */ (module)
1824+
unsafeCacheableModule
17601825
);
1826+
unsafeCacheDependencies.set(dependency, unsafeCacheableModule);
17611827
}
1762-
if (!unsafeCacheData.has(module)) {
1763-
unsafeCacheData.set(module, module.getUnsafeCacheData());
1828+
if (!unsafeCacheData.has(unsafeCacheableModule)) {
1829+
unsafeCacheData.set(
1830+
unsafeCacheableModule,
1831+
unsafeCacheableModule.getUnsafeCacheData()
1832+
);
17641833
}
17651834
} else {
17661835
applyFactoryResultDependencies();

‎test/WatchTestCases.template.js‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ const describeCases = config => {
136136
srcPath: tempDirectory
137137
});
138138
}
139-
const applyConfig = options => {
139+
const applyConfig = (options, idx) => {
140140
if (!options.mode) options.mode = "development";
141141
if (!options.context) options.context = tempDirectory;
142142
if (!options.entry) options.entry = "./index.js";
@@ -148,6 +148,11 @@ const describeCases = config => {
148148
options.output.pathinfo = true;
149149
if (!options.output.filename)
150150
options.output.filename = "bundle.js";
151+
if (options.cache && options.cache.type === "filesystem") {
152+
const cacheDirectory = path.join(tempDirectory, ".cache");
153+
options.cache.cacheDirectory = cacheDirectory;
154+
options.cache.name = `config-${idx}`;
155+
}
151156
if (config.experiments) {
152157
if (!options.experiments) options.experiments = {};
153158
for (const key of Object.keys(config.experiments)) {
@@ -166,7 +171,7 @@ const describeCases = config => {
166171
if (Array.isArray(options)) {
167172
options.forEach(applyConfig);
168173
} else {
169-
applyConfig(options);
174+
applyConfig(options, 0);
170175
}
171176

172177
const state = {};
@@ -194,7 +199,7 @@ const describeCases = config => {
194199
triggeringFilename = filename;
195200
}
196201
);
197-
const watching = compiler.watch(
202+
compiler.watch(
198203
{
199204
aggregateTimeout: 1000
200205
},
@@ -391,10 +396,10 @@ const describeCases = config => {
391396
done
392397
)
393398
) {
394-
watching.close();
399+
compiler.close();
395400
return;
396401
}
397-
watching.close(done);
402+
compiler.close(done);
398403
}
399404
},
400405
45000

‎test/watchCases/cache/unsafe-cache-duplicates/0/after.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export default 0;

‎test/watchCases/cache/unsafe-cache-duplicates/0/index.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import "./unsafe-cache-root";
2+
3+
it("should compile fine", () => {});

‎test/watchCases/cache/unsafe-cache-duplicates/0/module.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export { default } from "./after";

‎test/watchCases/cache/unsafe-cache-duplicates/0/unsafe-cache-root.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export default require.resolve("./module");
2+
export { default as module } from "./module";

‎test/watchCases/cache/unsafe-cache-duplicates/1/alternative-path.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
export default require.resolve("./module");
2+
export { default as module } from "./module";

0 commit comments

Comments
 (0)