From 38f98855fa9cd7fd376afb84094eba0fda256a74 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Tue, 29 Sep 2026 09:52:13 +0200 Subject: [PATCH] fix(cache): revalidate imports of cached modules (#11381) --- .../vitest/src/node/cache/fsModuleCache.ts | 16 +- .../src/node/environments/fetchModule.ts | 130 +++++++--- packages/vitest/src/node/vite.ts | 2 +- .../test/fs-module-cache-invalidation.test.ts | 227 ++++++++++++++++++ 4 files changed, 342 insertions(+), 33 deletions(-) create mode 100644 test/e2e/test/fs-module-cache-invalidation.test.ts diff --git a/packages/vitest/src/node/cache/fsModuleCache.ts b/packages/vitest/src/node/cache/fsModuleCache.ts index 65d5f0207..2349f27c9 100644 --- a/packages/vitest/src/node/cache/fsModuleCache.ts +++ b/packages/vitest/src/node/cache/fsModuleCache.ts @@ -42,7 +42,7 @@ export class FileSystemModuleCache { private rootCache: string private metadataFilePath: string - private version = '1.0.0-beta.7' + private version = '1.0.0-beta.8' private fsCacheRoots = new WeakMap() private fsEnvironmentHashMap = new WeakMap() private fsCacheKeyGenerators = new Set() @@ -142,7 +142,7 @@ export class FileSystemModuleCache { url: meta.url, file: meta.file, code, - importedUrls: meta.importedUrls, + imports: meta.imports, mappings: meta.mappings, moduleType: meta.moduleType, deps: meta.deps, @@ -155,7 +155,7 @@ export class FileSystemModuleCache { cachedFilePath: string, fetchResult: VitestFetchResult, transformResult: TransformResult | null, - importedUrls: string[] = [], + imports: CachedModuleImports = { urls: [], ids: {} }, mappings: boolean = false, ): Promise { if ('code' in fetchResult) { @@ -163,7 +163,7 @@ export class FileSystemModuleCache { file: fetchResult.file, id: fetchResult.id, url: fetchResult.url, - importedUrls, + imports, mappings, moduleType: fetchResult.moduleType, deps: transformResult?.deps, @@ -407,13 +407,19 @@ export interface CachedInlineModuleMeta { file: string | null code: string mappings: boolean - importedUrls: string[] + imports: CachedModuleImports moduleType?: ModuleType deps?: string[] dynamicDeps?: string[] staticMocks?: StaticMockCall[] | null } +export interface CachedModuleImports { + urls: string[] + // resolved ids that differ from the id derived from the url + ids: Record +} + /** * Generate a unique cache identifier. * diff --git a/packages/vitest/src/node/environments/fetchModule.ts b/packages/vitest/src/node/environments/fetchModule.ts index 5eeaad3e4..f58fe5f82 100644 --- a/packages/vitest/src/node/environments/fetchModule.ts +++ b/packages/vitest/src/node/environments/fetchModule.ts @@ -1,6 +1,12 @@ import type { Span } from '@opentelemetry/api' import type { StaticMockCall } from '@vitest/mocker/node' -import type { DevEnvironment, EnvironmentModuleNode, Rollup, TransformResult } from 'vite' +import type { + DevEnvironment, + EnvironmentModuleNode, + ResolvedConfig as ViteResolvedConfig, + Rollup, + TransformResult, +} from 'vite' import type { FetchFunctionOptions, FetchResult } from 'vite/module-runner' import type { FetchCachedFileSystemResult, @@ -9,17 +15,23 @@ import type { VitestFetchResult, } from '../../types/general' import type { OTELCarrier, Traces } from '../../utils/traces' -import type { FileSystemModuleCache } from '../cache/fsModuleCache' +import type { + CachedInlineModuleMeta, + CachedModuleImports, + FileSystemModuleCache, +} from '../cache/fsModuleCache' import type { VitestResolver } from '../resolver' import type { ResolvedConfig } from '../types/config' import { existsSync, mkdirSync } from 'node:fs' import { readFile } from 'node:fs/promises' import { isExternalUrl, unwrapId } from '@vitest/utils/helpers' import { join } from 'pathe' +import c from 'tinyrainbow' import { fetchModule } from 'vite' import { createDebugger } from '../../utils/debugger' import { hash } from '../hash' import { detectModuleType } from '../resolver' +import { fsPathFromId } from '../vite' import { normalizeResolvedIdToUrl } from './normalizeUrl' const debugFs = createDebugger('vitest:cache:fs') @@ -164,7 +176,7 @@ class ModuleFetcher { moduleGraphModule, options, ) - const importedUrls = this.getSerializedImports(moduleGraphModule) + const imports = this.getCachedImports(environment, moduleGraphModule) const map = moduleGraphModule.transformResult?.map const mappings = map && !('version' in map) && map.mappings === '' @@ -172,7 +184,7 @@ class ModuleFetcher { result, cachePath, moduleGraphModule.transformResult, - importedUrls, + imports, !!mappings, ) // remember where the code is stored on disk so that repeat fetches and the @@ -184,11 +196,19 @@ class ModuleFetcher { return cachedResult } - // we need this for UI to be able to show a module graph - private getSerializedImports(node: EnvironmentModuleNode): string[] { - const imports: string[] = [] - node.importedModules.forEach((importer) => { - imports.push(importer.url) + private getCachedImports( + environment: DevEnvironment, + node: EnvironmentModuleNode, + ): CachedModuleImports { + const imports: CachedModuleImports = { urls: [], ids: {} } + node.importedModules.forEach(({ url, id }) => { + if (id == null) { + return + } + imports.urls.push(url) + if (id !== urlToId(environment, url)) { + imports.ids[url] = id + } }) return imports } @@ -245,12 +265,7 @@ class ModuleFetcher { environment: DevEnvironment, moduleGraphModule: EnvironmentModuleNode, ): Promise { - if ( - moduleGraphModule.file && - // \x00 is a virtual file convention - !moduleGraphModule.file.startsWith('\x00') && - !moduleGraphModule.file.startsWith('virtual:') - ) { + if (moduleGraphModule.file && !isVirtualFile(moduleGraphModule.file)) { const result = await this.readFileConcurrently(moduleGraphModule.file) if (result != null) { return result @@ -302,6 +317,11 @@ class ModuleFetcher { return } + const importedModules = await this.resolveCachedImports(environment, cachedModule) + if (!importedModules) { + return + } + // keep the module graph in sync let map: Rollup.SourceMap | null | { mappings: '' } = extractSourceMap(cachedModule.code) if (map && cachedModule.file) { @@ -331,15 +351,10 @@ class ModuleFetcher { } } - await Promise.all( - cachedModule.importedUrls.map(async (url) => { - const moduleNode = await environment.moduleGraph.ensureEntryFromUrl(url).catch(() => null) - if (moduleNode) { - moduleNode.importers.add(moduleGraphModule) - moduleGraphModule.importedModules.add(moduleNode) - } - }), - ) + for (const moduleNode of importedModules) { + moduleNode.importers.add(moduleGraphModule) + moduleGraphModule.importedModules.add(moduleNode) + } return { cached: true as const, @@ -352,6 +367,43 @@ class ModuleFetcher { } } + // the cached code imports the urls that Vite resolved when the module was + // transformed, so the entry is stale once any of them points elsewhere + private async resolveCachedImports( + environment: DevEnvironment, + cachedModule: CachedInlineModuleMeta, + ): Promise { + const safeModulePaths = getSafeModulePaths(environment) + const { urls, ids } = cachedModule.imports + const importedModules = await Promise.all( + urls.map(async (url) => { + const id = Object.hasOwn(ids, url) ? ids[url] : urlToId(environment, url) + const moduleNode = await environment.moduleGraph.ensureEntryFromUrl(url).catch(() => null) + const stale = + !moduleNode || + moduleNode.id !== id || + // the resolver trusts /@fs/ urls without checking that the file exists + (url.startsWith('/@fs/') && moduleNode.file != null && !existsSync(moduleNode.file)) + if (stale) { + debugFs?.( + `${c.red('[stale]')} ${cachedModule.id} imports ${url}, which no longer resolves to ${id}`, + ) + return null + } + // import analysis marks out-of-root imports as safe to load from + // outside `server.fs.allow`; a cached importer never goes through it + if (url.startsWith('/@fs/') && moduleNode.file) { + safeModulePaths?.add(moduleNode.file) + } + return moduleNode + }), + ) + const resolved = importedModules.filter((moduleNode) => moduleNode != null) + if (resolved.length === importedModules.length) { + return resolved + } + } + private async fetchAndProcess( environment: DevEnvironment, url: string, @@ -382,7 +434,7 @@ class ModuleFetcher { } private sourceLoader(file: string | null): (() => Promise) | undefined { - if (!file || file.startsWith('\x00') || file.startsWith('virtual:')) { + if (!file || isVirtualFile(file)) { return undefined } return () => this.readFileConcurrently(file) @@ -414,7 +466,7 @@ class ModuleFetcher { result: FetchResult, cachePath: string, transformResult: TransformResult | null, - importedUrls: string[] = [], + imports?: CachedModuleImports, mappings = false, ): Promise { const returnResult = 'code' in result ? getCachedResult(result, cachePath) : result @@ -424,7 +476,7 @@ class ModuleFetcher { } const savePromise = this.fsCache - .saveCachedModule(cachePath, result, transformResult, importedUrls, mappings) + .saveCachedModule(cachePath, result, transformResult, imports, mappings) .then(() => returnResult) .catch((error) => { debugFs?.(`failed to cache ${cachePath}, serving it inline: ${error}`) @@ -455,6 +507,30 @@ class ModuleFetcher { } } +// \x00 is a virtual file convention +function isVirtualFile(file: string): boolean { + return file.startsWith('\x00') || file.startsWith('virtual:') +} + +// inverts the url that import analysis writes for a resolved id +function urlToId(environment: DevEnvironment, url: string): string { + if (url.startsWith('/@fs/')) { + return fsPathFromId(url) + } + if (url[0] === '/') { + return environment.config.root + url + } + return url +} + +// Vite keeps the set out of its public types +function getSafeModulePaths(environment: DevEnvironment): Set | undefined { + const config = environment.getTopLevelConfig() as ViteResolvedConfig & { + safeModulePaths?: Set + } + return config.safeModulePaths +} + export interface VitestFetchFunction { ( url: string, diff --git a/packages/vitest/src/node/vite.ts b/packages/vitest/src/node/vite.ts index c529e5f3b..88479ff50 100644 --- a/packages/vitest/src/node/vite.ts +++ b/packages/vitest/src/node/vite.ts @@ -47,7 +47,7 @@ export function isFileServingAllowed( const FS_PREFIX = '/@fs/' const VOLUME_RE = /^[A-Z]:/i -function fsPathFromId(id: string): string { +export function fsPathFromId(id: string): string { const fsPath = normalizePath(id.startsWith(FS_PREFIX) ? id.slice(FS_PREFIX.length) : id) return fsPath[0] === '/' || VOLUME_RE.test(fsPath) ? fsPath : `/${fsPath}` } diff --git a/test/e2e/test/fs-module-cache-invalidation.test.ts b/test/e2e/test/fs-module-cache-invalidation.test.ts new file mode 100644 index 000000000..d6696ad9d --- /dev/null +++ b/test/e2e/test/fs-module-cache-invalidation.test.ts @@ -0,0 +1,227 @@ +import { readdirSync, statSync } from 'node:fs' +import { join } from 'pathe' +import { expect, test } from 'vitest' +import { runInlineTests, runVitest, useTmpFS } from '#test-utils' + +function cacheTimestamps(cachePath: string) { + return readdirSync(cachePath).map((file) => [file, statSync(join(cachePath, file)).mtimeMs]) +} + +// The import check must not report a false miss, or the cache is rewritten on +// every run: virtual modules resolve to ids that are not files, and an id may +// look like an absolute path without being one. +test('an unchanged module graph is served from the cache', async () => { + const cold = await runInlineTests({ + 'vitest.config.js': /* js */ ` + export default { + plugins: [ + { + name: 'answer', + resolveId(id) { + if (id === 'virtual:answer') return '\\0virtual:answer' + if (id === 'virtual:base' || id === '/virtual/base') return '/virtual/base' + }, + load(id) { + if (id === '\\0virtual:answer') return 'export const answer = 42' + if (id === '/virtual/base') return 'export const base = 0' + }, + }, + ], + test: { + fsModuleCache: true, + fsModuleCachePath: './node_modules/.vitest-fs-cache', + }, + } + `, + 'src/a.ts': /* ts */ ` + import { answer } from 'virtual:answer' + import { base } from 'virtual:base' + import { value } from './b' + export const sum = answer + base + value + `, + 'src/b.ts': `export const value = 1`, + 'src/a.test.ts': /* ts */ ` + import { expect, it } from 'vitest' + import { sum } from './a' + it('reads it', () => { + expect(sum).toBe(43) + }) + `, + }) + expect(cold.stderr).toBe('') + expect(cold.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) + await cold.ctx?.close() + + const cachePath = join(cold.root, 'node_modules/.vitest-fs-cache') + const timestamps = cacheTimestamps(cachePath) + expect(timestamps.length).toBeGreaterThan(1) + + const warm = await runVitest({ root: cold.root }) + expect(warm.stderr).toBe('') + expect(warm.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) + expect(cacheTimestamps(cachePath)).toEqual(timestamps) +}) + +// A cached transform embeds the resolved URLs of its imports, so it has to be +// dropped when those imports no longer resolve to the same modules, even if +// the importer's own source did not change (a branch switch renames a file). +test('a cached importer is re-transformed after its dependency is renamed', async () => { + const structure = { + 'src/a.ts': `export { value } from './b'`, + 'src/b.ts': `export const value = 1`, + 'src/a.test.ts': /* ts */ ` + import { expect, it } from 'vitest' + import { value } from './a' + it('reads it', () => { + expect(value).toBe(1) + }) + `, + } + const config = { + fsModuleCache: true, + fsModuleCachePath: './node_modules/.vitest-fs-cache', + } + + const cold = await runInlineTests(structure, config) + expect(cold.stderr).toBe('') + expect(cold.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) + await cold.ctx?.close() + + cold.fs.renameFile('src/b.ts', 'src/b.tsx') + + const warm = await runVitest({ root: cold.root, ...config }) + expect(warm.stderr).toBe('') + expect(warm.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) +}) + +// Vite only lets a client environment read files outside `server.fs.allow` +// when import analysis saw them being imported. A cached importer skips import +// analysis, so its out-of-root imports have to be registered by the cache. +test('a changed dependency outside the root loads after its importer is served from the cache', async () => { + const fs = useTmpFS( + { + 'app/package.json': JSON.stringify({ name: 'app', type: 'module' }), + // keeps the workspace root at app/, so shared/ is outside `server.fs.allow` + 'app/pnpm-workspace.yaml': '', + 'app/vitest.config.mjs': /* js */ ` + export default { + test: { + environment: 'jsdom', + fsModuleCache: true, + fsModuleCachePath: './node_modules/.vitest-fs-cache', + }, + } + `, + 'app/src/a.test.ts': /* ts */ ` + import { expect, it } from 'vitest' + import { value } from '../../shared/index' + it('reads it', () => { + expect(value).toBe(1) + }) + `, + 'shared/index.ts': `export * from './b'`, + 'shared/b.ts': `export const value = 1`, + }, + false, + ) + const root = join(fs.root, 'app') + + const cold = await runVitest({ root }) + expect(cold.stderr).toBe('') + expect(cold.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) + await cold.ctx?.close() + + fs.editFile('shared/b.ts', (content) => `${content}\nexport const probe = 2\n`) + + const warm = await runVitest({ root }) + expect(warm.stderr).toBe('') + expect(warm.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) +}) + +// Imports from outside the root are rewritten to /@fs/ urls, which the +// resolver accepts without checking that the file still exists. +test('a cached importer is re-transformed after a dependency outside the root is renamed', async () => { + const fs = useTmpFS( + { + 'app/package.json': JSON.stringify({ name: 'app', type: 'module' }), + 'app/vitest.config.mjs': /* js */ ` + export default { + test: { + fsModuleCache: true, + fsModuleCachePath: './node_modules/.vitest-fs-cache', + }, + } + `, + 'app/src/a.test.ts': /* ts */ ` + import { expect, it } from 'vitest' + import { value } from '../../shared/index' + it('reads it', () => { + expect(value).toBe(1) + }) + `, + 'shared/index.ts': `export * from './b'`, + 'shared/b.ts': `export const value = 1`, + }, + false, + ) + const root = join(fs.root, 'app') + + const cold = await runVitest({ root }) + expect(cold.stderr).toBe('') + expect(cold.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) + await cold.ctx?.close() + + fs.renameFile('shared/b.ts', 'shared/b.tsx') + + const warm = await runVitest({ root }) + expect(warm.stderr).toBe('') + expect(warm.errorTree()).toMatchInlineSnapshot(` + { + "src/a.test.ts": { + "reads it": "passed", + }, + } + `) +}) -- 2.51.2