Skip to content

Commit 572ef61

Browse files
authored
Harden and centralize the cache invalidation (#5493)
1 parent 70db235 commit 572ef61

12 files changed

Lines changed: 385 additions & 75 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
---
2+
"apostrophe": patch
3+
"@apostrophecms/vite": patch
4+
---
5+
6+
Fixed the admin UI sometimes serving a stale build after dependencies changed (for example after `npm install` or `npm update`). Apostrophe now detects dependency changes from the content of the lock file rather than its modified time, which could be misleading after a fresh checkout or a restored CI/Docker build cache.
7+
8+
For external build module authors: lock file change detection now happens in the core and is passed to the build module via the `lockChanged` build option. The `apos.asset.getSystemLastChangeMs()` helper is deprecated and the build manifest no longer includes a `ts` timestamp.

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,4 @@ coverage/
99
claude-tools/logs/
1010
.claude
1111
specs/
12+
scratch/

‎packages/apostrophe/modules/@apostrophecms/asset/index.js‎

Lines changed: 130 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -254,20 +254,34 @@ module.exports = {
254254
afterModuleInit: true,
255255
async task(argv = {}) {
256256
self.inBuildTask = true;
257+
// If the lock file changed since the last build, force a full
258+
// rebuild (and clear the build module cache) so a stale admin UI is
259+
// never served after a dependency change (npm install/update).
260+
const lockFileHash = await self.getLockFileHash();
261+
const lockChanged = await self.checkLockFileChanged(lockFileHash);
262+
argv = {
263+
...argv,
264+
lockChanged
265+
};
266+
let result;
257267
if (self.hasBuildModule()) {
258-
return self.build(argv);
268+
result = await self.build(argv);
269+
} else {
270+
// Debugging but only if we don't have an external build module.
271+
// If we do, the debug output is handled by the respective setter.
272+
self.printDebug('setWebpackExtensions', {
273+
builds: self.builds,
274+
extraBundles: self.extraBundles,
275+
webpackExtensions: self.webpackExtensions,
276+
webpackExtensionOptions: self.webpackExtensionOptions,
277+
verifiedBundles: self.verifiedBundles,
278+
rebundleModules: self.rebundleModules
279+
});
280+
result = await webpackBuild.task(argv);
259281
}
260-
// Debugging but only if we don't have an external build module.
261-
// If we do, the debug output is handled by the respective setter.
262-
self.printDebug('setWebpackExtensions', {
263-
builds: self.builds,
264-
extraBundles: self.extraBundles,
265-
webpackExtensions: self.webpackExtensions,
266-
webpackExtensionOptions: self.webpackExtensionOptions,
267-
verifiedBundles: self.verifiedBundles,
268-
rebundleModules: self.rebundleModules
269-
});
270-
return webpackBuild.task(argv);
282+
// Record the lock file hash now that the build has succeeded.
283+
await self.saveLockFileHash(lockFileHash);
284+
return result;
271285
}
272286
},
273287

@@ -402,12 +416,13 @@ module.exports = {
402416
// development related properties: * `devServerUrl` (optional, string or
403417
// null) the base server URL for the dev server when available. *
404418
// `hmrTypes` (optional, array of strings) the entrypoint types that are
405-
// currently served with HMR. * `ts` (optional, number) the timestamp ms
406-
// of the last `apos` build. This number will be written to the
407-
// `.manifest.json` file to allow the external build module to optimize
408-
// the build time based on the last build change. The module can retrieve
409-
// both `getSystemLastChangeMs()` and `loadSavedBuildManifest()` methods
410-
// to perform a cache check.
419+
// currently served with HMR.
420+
//
421+
// Lock file (dependency) changes are detected by the core (by hashing the
422+
// lock file content) and signalled to the build module via the
423+
// `lockChanged` build option, which should force a rebuild. The
424+
// `getSystemLastChangeMs()` modified-time helper is deprecated and no
425+
// longer needed for that purpose.
411426
//
412427
// ** `async watch(watcher, options)`: the method to attach the watcher
413428
// to the external build module.
@@ -500,11 +515,16 @@ module.exports = {
500515
// - `changes` is an array of changed files. This is reserved for the
501516
// legacy build system and should never appear in the external build
502517
// module.
518+
// - `lockChanged` is a boolean flag set by the core when the lock file
519+
// content changed since the last build (or no lock file was found). The
520+
// build module should force a rebuild when it is `true`.
503521
//
504522
// Returns an object:
505523
// - `isTask`: if `true`, the build is executed as a task. If false
506524
// optimization can be applied (e.g. build apostrophe admin UI only
507-
// once). - `hmr`: if `true`, the hot module replacement is enabled. -
525+
// once). - `lockChanged`: if `true`, dependencies changed since the last
526+
// build (or there is no lock file); the build module should force a
527+
// rebuild. - `hmr`: if `true`, the hot module replacement is enabled. -
508528
// `hmrPort`: the port for the HMR WS server. If not set, the default port
509529
// is used. - `devServer`: if `false`, the dev server is disabled.
510530
// Otherwise, it's a string (enum) `public` or `apos`. Note that if `hmr`
@@ -527,6 +547,7 @@ module.exports = {
527547
}
528548
const options = {
529549
isTask: !argv['check-apos-build'],
550+
lockChanged: !!argv.lockChanged,
530551
hmr: self.hasHMR(),
531552
hmrPort: self.options.hmrPort,
532553
modulePreloadPolyfill: self.options.modulePreloadPolyfill,
@@ -862,6 +883,96 @@ module.exports = {
862883
path.join(self.apos.rootDir, 'data/temp/webpack-cache');
863884
},
864885

886+
// Absolute path to the project package manager lock file, or `false`
887+
// when none is found. Shared by the webpack and external build paths.
888+
async findPackageLockPath() {
889+
const candidates = [ 'package-lock.json', 'yarn.lock', 'pnpm-lock.yaml' ];
890+
for (const name of candidates) {
891+
const candidate = path.join(self.apos.npmRootDir, name);
892+
if (await fs.pathExists(candidate)) {
893+
return candidate;
894+
}
895+
}
896+
return false;
897+
},
898+
899+
// md5 of the current lock file content, or `null` when there is no lock
900+
// file (e.g. in tests). Used to detect dependency changes reliably,
901+
// where modified times cannot be trusted (fresh checkouts, restored
902+
// build artifacts, clock skew).
903+
async getLockFileHash() {
904+
const lockPath = await self.findPackageLockPath();
905+
if (!lockPath) {
906+
return null;
907+
}
908+
return self.apos.util.md5(await fs.readFile(lockPath, 'utf8'));
909+
},
910+
911+
// Where the lock file hash from the last build is persisted.
912+
getLockFileHashPath() {
913+
return path.join(
914+
self.apos.rootDir, 'data/temp', self.getNamespace(), 'lock-file-hash'
915+
);
916+
},
917+
918+
async readSavedLockFileHash() {
919+
try {
920+
const value = (await fs.readFile(self.getLockFileHashPath(), 'utf8')).trim();
921+
return value || null;
922+
} catch (e) {
923+
return null;
924+
}
925+
},
926+
927+
async saveLockFileHash(hash) {
928+
const file = self.getLockFileHashPath();
929+
if (!hash) {
930+
await fs.remove(file);
931+
return;
932+
}
933+
await fs.mkdirp(path.dirname(file));
934+
await fs.writeFile(file, hash, 'utf8');
935+
},
936+
937+
// Clear the external build module's cache (e.g. @apostrophecms/vite),
938+
// whose cache directory is not keyed on the lock file content. The
939+
// webpack file system cache is keyed on the lock file content and
940+
// self-invalidates, so it is left in place; forcing a rebuild is enough
941+
// for it to produce a fresh bundle.
942+
async clearBuildModuleCache() {
943+
if (self.hasBuildModule()) {
944+
await self.getBuildModule().clearCache();
945+
}
946+
},
947+
948+
// Decide whether the build should be forced because of a dependency
949+
// change, returning `true` so callers can force a full rebuild.
950+
// `currentHash` is accepted to avoid reading the lock file twice; it is
951+
// `null` when no lock file was found.
952+
//
953+
// - No lock file (e.g. some monorepo setups): force a rebuild so a stale
954+
// admin UI is never served, but do NOT clear the build module cache -
955+
// there is no evidence dependencies changed and clearing it on every
956+
// run would be needlessly expensive.
957+
// - Lock content changed, or there is no record of a previous build:
958+
// clear the build module cache so the rebuild cannot reuse stale
959+
// modules, and force a rebuild.
960+
// - Lock content unchanged: no forced rebuild.
961+
//
962+
// The new hash is persisted by the caller via `saveLockFileHash()` only
963+
// after the build succeeds, so a failed build is retried.
964+
async checkLockFileChanged(currentHash) {
965+
if (currentHash === null) {
966+
return true;
967+
}
968+
const saved = await self.readSavedLockFileHash();
969+
if (saved === currentHash) {
970+
return false;
971+
}
972+
await self.clearBuildModuleCache();
973+
return true;
974+
},
975+
865976
// Override to set externally a build watcher (a `chokidar` instance).
866977
// This method will be invoked only if/when needed.
867978
// Example:

‎packages/apostrophe/modules/@apostrophecms/asset/lib/build/external-module-api.js‎

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -212,28 +212,19 @@ function invoke() {
212212
// Helper function for external build modules to find the last package
213213
// change timestamp in milliseconds. Works with Node.js and npm, yarn, and
214214
// pnpm package managers. Might be extended if a need arises.
215+
//
216+
// @deprecated Lock file modified times are unreliable (fresh checkouts,
217+
// restored build artifacts, clock skew). The core now detects dependency
218+
// changes by hashing the lock file content and forces a rebuild via the
219+
// `lockChanged` build option, so build modules no longer need this. Kept
220+
// for backwards compatibility and will be removed in the next major version.
215221
async getSystemLastChangeMs() {
216-
const packageLock = await findPackageLock();
222+
const packageLock = await self.findPackageLockPath();
217223
if (!packageLock) {
218224
return false;
219225
}
220226

221227
return (await fs.stat(packageLock)).mtimeMs;
222-
223-
async function findPackageLock() {
224-
const packageLockPath = path.join(self.apos.npmRootDir, 'package-lock.json');
225-
const yarnPath = path.join(self.apos.npmRootDir, 'yarn.lock');
226-
const pnpmPath = path.join(self.apos.npmRootDir, 'pnpm-lock.yaml');
227-
if (await fs.pathExists(packageLockPath)) {
228-
return packageLockPath;
229-
} else if (await fs.pathExists(yarnPath)) {
230-
return yarnPath;
231-
} else if (await fs.pathExists(pnpmPath)) {
232-
return pnpmPath;
233-
} else {
234-
return false;
235-
}
236-
}
237228
},
238229

239230
// Retrieve saved during build core metadata. The metadata is saved in the

‎packages/apostrophe/modules/@apostrophecms/asset/lib/build/internals.js‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ module.exports = (self) => {
168168
// more information.
169169
async saveBuildManifest(manifest) {
170170
const {
171-
entrypoints, ts, devServerUrl, hmrTypes
171+
entrypoints, devServerUrl, hmrTypes
172172
} = manifest;
173173
const content = [];
174174

@@ -186,11 +186,9 @@ module.exports = (self) => {
186186
bundles: Array.from(bundles ?? [])
187187
});
188188
}
189-
const current = await self.loadSavedBuildManifest(true);
190189
await fs.outputJson(
191190
path.join(self.getBundleRootDir(), '.manifest.json'),
192191
{
193-
ts: ts || current.ts,
194192
devServerUrl,
195193
hmrTypes,
196194
manifest: content

‎packages/apostrophe/modules/@apostrophecms/asset/lib/build/task.js‎

Lines changed: 7 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,11 @@ module.exports = (self) => ({
106106
}
107107
}
108108

109+
// A lock file change forces a full rebuild of every build.
110+
if (argv && argv.lockChanged) {
111+
rebuild = true;
112+
}
113+
109114
if (rebuild) {
110115
await fs.mkdirp(bundleDir);
111116
await build({
@@ -754,19 +759,8 @@ module.exports = (self) => ({
754759
return pkgTimestamp > parseInt(timestamp);
755760
}
756761

757-
async function findPackageLock() {
758-
const packageLockPath = path.join(self.apos.npmRootDir, 'package-lock.json');
759-
const yarnPath = path.join(self.apos.npmRootDir, 'yarn.lock');
760-
const pnpmPath = path.join(self.apos.npmRootDir, 'pnpm-lock.yaml');
761-
if (await fs.pathExists(packageLockPath)) {
762-
return packageLockPath;
763-
} else if (await fs.pathExists(yarnPath)) {
764-
return yarnPath;
765-
} else if (await fs.pathExists(pnpmPath)) {
766-
return pnpmPath;
767-
} else {
768-
return false;
769-
}
762+
function findPackageLock() {
763+
return self.findPackageLockPath();
770764
}
771765

772766
function getComponentName(component, { enumerateImports } = {}, i) {

‎packages/apostrophe/test/asset-external.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ describe('Asset - External Build', function () {
5555
};
5656
},
5757
async watch() { },
58+
async clearCache() { },
5859
async startDevServer() {
5960
actualDevServer = true;
6061
return {
@@ -124,6 +125,7 @@ describe('Asset - External Build', function () {
124125
return {
125126
async build() { },
126127
async watch() { },
128+
async clearCache() { },
127129
async startDevServer() {},
128130
async entrypoints() {
129131
return [];
@@ -291,6 +293,7 @@ describe('Asset - External Build', function () {
291293
};
292294
},
293295
async watch() { },
296+
async clearCache() { },
294297
async startDevServer() {
295298
return {
296299
entrypoints: []

0 commit comments

Comments
 (0)