From 6c3e64dbeb2ee6f302a71b4e9fef906b00019db5 Mon Sep 17 00:00:00 2001 From: David Murdoch <187813+davidmurdoch@users.noreply.github.com> Date: Tue, 26 May 2026 11:26:38 -0400 Subject: [PATCH] Fix webpack cache script recovery --- src/Plugin/AssetCompiler.js | 39 +++++----- src/Plugin/Collection.js | 145 +++++++++++++++++++++++++++++++++++- src/Plugin/Resolver.js | 9 +++ test/integration.test.js | 5 +- test/unit.test.js | 99 ++++++++++++++++++++++++ test/utils/helpers.js | 13 +++- 6 files changed, 280 insertions(+), 30 deletions(-) diff --git a/src/Plugin/AssetCompiler.js b/src/Plugin/AssetCompiler.js index f0d92e3c..0fc1015a 100644 --- a/src/Plugin/AssetCompiler.js +++ b/src/Plugin/AssetCompiler.js @@ -312,33 +312,29 @@ class AssetCompiler { // TODO: init by PluginIndex, for each instance create own PluginService instance for pluginOption PluginService.init(compiler, this.pluginContext, AssetCompiler); + this.cacheDataComplete = false; + if (this.pluginOption.isCacheable()) { const collectionCache = createPersistentCache(this.collection); const cache = compiler.getCache(pluginName).getItemCache('PersistentCache', null); - let isCached = false; - compiler.hooks.beforeCompile.tap(pluginName, () => { - cache.get((error, data) => { - if (error) { - throw new Error(error); - } - isCached = !!data; + compiler.hooks.beforeCompile.tapPromise(pluginName, () => { + return cache.getPromise().catch(() => { + this.collection.clear(); }); }); - // note: if used `tapAsync` then no webpack statistics or errors will be displayed - // then use in the `done` hook the output of `stats.compilation.options.stats` in Promise.finally - //compiler.cache.hooks.shutdown.tapAsync({ name: pluginName, stage: Cache.STAGE_DISK }, () => { - compiler.cache.hooks.shutdown.tap({ name: pluginName, stage: Cache.STAGE_DISK }, () => { - if (!isCached) { - const cacheData = collectionCache.getData(); + // Store before Webpack's disk cache shutdown task collects pending cache writes. + compiler.cache.hooks.shutdown.tap({ name: pluginName, stage: Cache.STAGE_DISK - 1 }, () => { + if (!this.cacheDataComplete) return; - cache.store(cacheData, (error) => { - if (error) { - throw new Error(error); - } - }); - } + const cacheData = collectionCache.getData(); + + cache.store(cacheData, (error) => { + if (error) { + throw new Error(error); + } + }); }); } @@ -405,6 +401,7 @@ class AssetCompiler { const normalModuleHooks = NormalModule.getCompilationHooks(compilation); const renderStage = this.pluginOption.getRenderStage(); + this.cacheDataComplete = false; this.IS_WEBPACK_VERSION_LOWER_5_96_0 = compareVersions(compilation.compiler.webpack.version, '<', '5.96.0'); this.compilation = compilation; @@ -1776,6 +1773,8 @@ class AssetCompiler { // } if (this.exceptions.size > 0) { + this.cacheDataComplete = false; + const messages = Array.from(this.exceptions) .map((error) => (error.stack ? error.stack : error.toString())) .reduce((previousValue, currentValue) => previousValue + currentValue, ''); @@ -1786,6 +1785,8 @@ class AssetCompiler { if (this.pluginOption.isVerbose()) verbose(pluginCompiler); + this.cacheDataComplete = !hasError; + this.asset.reset(); this.assetEntry.reset(); this.assetTrash.reset(); diff --git a/src/Plugin/Collection.js b/src/Plugin/Collection.js index 83653e35..ffbe0dcd 100644 --- a/src/Plugin/Collection.js +++ b/src/Plugin/Collection.js @@ -484,6 +484,99 @@ class Collection { return this.assets.get(resource)?.type === Collection.type.script; } + /** + * Recover a script entry from Webpack's cached chunk graph. + * + * This is a fallback for the case where Webpack restores a cached template + * module, but the plugin's persistent collection cache is missing the script + * resource that the restored module still requires. + * + * @param {{resource: string, issuer: FileInfo, entry: AssetEntryOptions}} options + * @return {boolean} + */ + recoverMissingScript({ resource, issuer, entry }) { + const issuerResource = issuer?.resource; + const entryFilename = entry?.filename; + + if (!issuerResource || !entryFilename) return false; + + let item = this.assets.get(resource); + if (item && item.type !== Collection.type.script) return false; + + const name = item?.name || this.#findScriptEntryName(resource); + if (!name) return false; + + if (!item) { + item = { + type: Collection.type.script, + inline: undefined, + name, + entries: new Map(), + assets: [], + }; + this.assets.set(resource, item); + } else if (!item.name) { + item.name = name; + } + + let entryFilenames = item.entries.get(issuerResource); + if (!entryFilenames) { + entryFilenames = new Set(); + item.entries.set(issuerResource, entryFilenames); + } + + entryFilenames.add(entryFilename); + + if (entry.id != null) { + let orderedResources = this.orderedResources.get(entry.id); + if (!orderedResources) { + orderedResources = new Set(); + this.orderedResources.set(entry.id, orderedResources); + } + orderedResources.add(resource); + } + + return true; + } + + /** + * Find an entrypoint containing the script resource. + * + * @param {string} resource The script resource. + * @return {string|null} + */ + #findScriptEntryName(resource) { + const compilation = this.compilation; + const chunkGraph = compilation?.chunkGraph; + const namedChunkGroups = + compilation?.entrypoints?.size > 0 ? compilation.entrypoints : compilation?.namedChunkGroups; + const [sourceFile] = resource.split('?', 1); + + if (!chunkGraph || !namedChunkGroups) return null; + + for (const [name, entrypoint] of namedChunkGroups) { + for (const chunk of entrypoint.chunks) { + const modules = chunkGraph.getChunkModulesIterable(chunk); + + if (!modules) continue; + + for (const module of modules) { + const moduleResource = module.resource; + if (!moduleResource) continue; + + const [moduleFile] = moduleResource.split('?', 1); + if (moduleResource === resource || moduleFile === sourceFile) { + const entry = this.assetEntry?.entriesByName.get(name); + + if (!entry?.isTemplate) return name; + } + } + } + } + + return null; + } + /** * Whether the collection contains the style file. * @@ -1308,6 +1401,7 @@ class Collection { this.importStyleRootIssuers.clear(); this.importStyleSources.clear(); this.importStyleIdx = 1000; + this.deserialized = false; } /** @@ -1341,6 +1435,7 @@ class Collection { // the original functions will be recovered by deserialization from the cached object `AssetEntry` entry.filenameFn = null; entry.filenameTemplate = null; + entry.options = null; } write(this.assets); @@ -1352,20 +1447,64 @@ class Collection { * @param {Function} read The deserialize function. */ deserialize({ read }) { - this.assets = read(); - this.data = read(); + const assets = read(); + const data = read(); + + if (!this.#isDeserializedDataValid(assets, data)) { + this.clear(); + return; + } + + this.assets = assets; + this.data = data; + + const assetEntry = this.assetEntry || this.pluginContext.assetEntry; + + if (!assetEntry) { + this.clear(); + return; + } for (let [, { entry }] of this.data) { - const cachedEntry = this.assetEntry.entriesById.get(entry.id); + if (!entry.id) continue; + + const cachedEntry = assetEntry.entriesById.get(entry.id); + + if (!cachedEntry) { + this.clear(); + return; + } // recovery original not serializable functions from the object cached in the memory entry.filenameFn = cachedEntry.filenameFn; entry.filenameTemplate = cachedEntry.filenameTemplate; + entry.options = cachedEntry.options; } this.deserialized = true; } + /** + * @param {Map} assets + * @param {Map} data + * @return {boolean} + */ + #isDeserializedDataValid(assets, data) { + if (!(assets instanceof Map) || !(data instanceof Map)) return false; + + for (const [, item] of assets) { + if (item == null || typeof item !== 'object') return false; + if (!item.type || !(item.entries instanceof Map)) return false; + } + + for (const [, item] of data) { + if (item == null || typeof item !== 'object') return false; + if (item.entry == null || !Array.isArray(item.assets)) return false; + } + + return true; + } + isDeserialized() { return this.deserialized; } diff --git a/src/Plugin/Resolver.js b/src/Plugin/Resolver.js index 7ef14e65..ee9aab9e 100644 --- a/src/Plugin/Resolver.js +++ b/src/Plugin/Resolver.js @@ -277,6 +277,15 @@ class Resolver { const file = resource || rawRequest; if (this.pluginOption.js.test.test(file) && this.assetEntry.isEntryResource(issuer.resource)) { + const [sourceFile] = file.split('?', 1); + + if ( + this.fs.existsSync(sourceFile) && + this.collection.recoverMissingScript({ resource: file, issuer, entry: this.entryPoint }) + ) { + return file; + } + // occur after rename/delete of a js file when the entry module was already rebuilt Snapshot.addMissingFile(issuer.resource, file); resolveException(file, issuer.resource, this.rootContext, this.pluginOption); diff --git a/test/integration.test.js b/test/integration.test.js index 339370b8..a06fb581 100644 --- a/test/integration.test.js +++ b/test/integration.test.js @@ -30,10 +30,7 @@ describe('cache tests', () => { test('filesystem, display stats', () => stdoutContain('cache-filesystem-display-stats', 'compiled successfully')); test('filesystem, multiple config', () => compareFiles('cache-filesystem-multi-config')); - test('filesystem-js-runs_n1', () => compareFilesRuns('cache-filesystem-js', false, 1)); - - // TODO: fix DEP_WEBPACK_COMPILATION_ASSETS warning - //test('filesystem-js-runs_n2', () => compareFilesRuns('cache-filesystem-js', false, 2)); + test('filesystem-js-runs_n2', () => compareFilesRuns('cache-filesystem-js', false, 2)); }); describe('resolve files', () => { diff --git a/test/unit.test.js b/test/unit.test.js index 33f47760..1d63ece9 100644 --- a/test/unit.test.js +++ b/test/unit.test.js @@ -16,6 +16,7 @@ import AssetEntry from '../src/Plugin/AssetEntry'; import Snapshot from '../src/Plugin/Snapshot'; import Option from '../src/Plugin/Option'; import Collection from '../src/Plugin/Collection'; +import PluginResolver from '../src/Plugin/Resolver'; const asset = new Asset(); const assetEntry = new AssetEntry({}); @@ -1943,4 +1944,102 @@ describe('misc tests', () => { const expected = -1; return expect(received).toEqual(expected); }); + + test('Collection.recoverMissingScript restores a script from the chunk graph', () => { + const resource = '/project/src/main.js'; + const issuer = { resource: '/project/src/index.html' }; + const entry = { id: 1, filename: 'index.html' }; + const chunk = {}; + const templateChunk = {}; + const recoveredCollection = new Collection({}); + + recoveredCollection.assetEntry = { + entriesByName: new Map([['__bundler-plugin-entry__index', { isTemplate: true }]]), + }; + recoveredCollection.compilation = { + namedChunkGroups: new Map([ + [ + '__bundler-plugin-entry__index', + { + chunks: [templateChunk], + }, + ], + [ + 'main', + { + chunks: [chunk], + }, + ], + ]), + chunkGraph: { + getChunkModulesIterable: (receivedChunk) => + receivedChunk === chunk || receivedChunk === templateChunk ? [{ resource }] : [], + }, + }; + + const received = recoveredCollection.recoverMissingScript({ resource, issuer, entry }); + const item = recoveredCollection.assets.get(resource); + + expect(received).toBe(true); + expect(recoveredCollection.hasScript(resource)).toBe(true); + expect(item.name).toBe('main'); + expect(Array.from(item.entries.get(issuer.resource))).toEqual([entry.filename]); + expect(Array.from(recoveredCollection.orderedResources.get(entry.id))).toEqual([resource]); + }); + + test('Collection.deserialize clears structurally invalid cache data', () => { + const recoveredCollection = new Collection({}); + const values = [{}, new Map()]; + + recoveredCollection.assets.set('/project/src/main.js', { + type: Collection.type.script, + entries: new Map(), + }); + recoveredCollection.deserialized = true; + recoveredCollection.deserialize({ read: () => values.shift() }); + + expect(recoveredCollection.isDeserialized()).toBe(false); + expect(recoveredCollection.assets.size).toBe(0); + }); + + test('Plugin Resolver recovers an existing script missing from collection cache', () => { + const script = '/project/src/main.js'; + const issuer = '/project/src/index.html'; + const recoverMissingScript = jest.fn(() => true); + const resolver = new PluginResolver({ + pluginOption: { + context: '/project', + js: { test: /\.js$/ }, + isEntry: () => false, + }, + assetEntry: { + isEntryResource: () => true, + }, + assetInline: { + getDataUrl: () => null, + isDataUrl: () => false, + isSvgFile: () => false, + }, + collection: { + hasScript: () => false, + hasStyle: () => false, + isInlineStyle: () => false, + recoverMissingScript, + }, + }); + + resolver.init({ + fs: { + existsSync: (file) => file === script, + }, + }); + resolver.setContext({ filename: 'index.html' }, { resource: issuer, filename: 'index.html' }); + + expect(resolver.require(script)).toBe(script); + expect(recoverMissingScript).toHaveBeenCalledWith({ + resource: script, + issuer: { resource: issuer, filename: 'index.html' }, + entry: { filename: 'index.html' }, + }); + }); }); diff --git a/test/utils/helpers.js b/test/utils/helpers.js index 8f9b1372..f5a87673 100644 --- a/test/utils/helpers.js +++ b/test/utils/helpers.js @@ -1,4 +1,5 @@ import path from 'path'; +import fs from 'fs'; import ansis from 'ansis'; import { readDirRecursiveSync, readTextFileSync } from './file'; import { compile, watch } from './webpack'; @@ -133,9 +134,14 @@ export const compareFilesRuns = (relTestCasePath, compareContent = true, num = 1 const results = []; const expected = Array(num).fill(true); const filter = /.(html|css|css.map|js|js.map|json)$/; + let promise = Promise.resolve(); + + fs.rmSync(webRootPath, { recursive: true, force: true }); + fs.rmSync(path.join(absTestPath, '.cache'), { recursive: true, force: true }); for (let i = 0; i < num; i++) { - const res = compile(PATHS, relTestCasePath, {}) + promise = promise + .then(() => compile(PATHS, relTestCasePath, {})) .then(() => { const { received: receivedFiles, expected: expectedFiles } = getCompareFileList(webRootPath, expectedPath); expect(receivedFiles).toEqual(expectedFiles); @@ -146,15 +152,14 @@ export const compareFilesRuns = (relTestCasePath, compareContent = true, num = 1 }); } - return Promise.resolve(true); + results.push(true); }) .catch((error) => { return Promise.reject(new Error(error.stack)); }); - results.push(res); } - return expect(Promise.all(results)).resolves.toEqual(expected); + return expect(promise.then(() => results)).resolves.toEqual(expected); }; /**