From 15bbbf8192d07b2cfba98fdfe97ce4fca2b0f4f0 Mon Sep 17 00:00:00 2001 From: Vladimir Date: Thu, 4 Jan 2024 17:40:15 +0100 Subject: [PATCH] fix(vite-node): correctly return cached result (#4870) --- packages/vite-node/src/server.ts | 12 +- test/vite-node/test/server.test.ts | 202 +++++++++++++++++++++++++++-- test/vite-node/vitest.config.ts | 4 + 3 files changed, 205 insertions(+), 13 deletions(-) diff --git a/packages/vite-node/src/server.ts b/packages/vite-node/src/server.ts index 3b57d1f2b..fdfb4915a 100644 --- a/packages/vite-node/src/server.ts +++ b/packages/vite-node/src/server.ts @@ -211,10 +211,16 @@ export class ViteNodeServer { const { path: filePath } = toFilePath(id, this.server.config.root) - const module = this.server.moduleGraph.getModuleById(id) - const timestamp = module ? module.lastHMRTimestamp : null + const moduleNode = this.server.moduleGraph.getModuleById(id) || this.server.moduleGraph.getModuleById(filePath) const cache = this.fetchCaches[transformMode].get(filePath) - if (timestamp && cache && cache.timestamp >= timestamp) + + // lastUpdateTimestamp is the timestamp that marks the last time the module was changed + // if lastUpdateTimestamp is 0, then the module was not changed since the server started + // we test "timestamp === 0" for expressiveness, but it's not necessary + const timestamp = moduleNode + ? Math.max(moduleNode.lastHMRTimestamp, moduleNode.lastInvalidationTimestamp) + : 0 + if (cache && (timestamp === 0 || cache.timestamp >= timestamp)) return cache.result const time = Date.now() diff --git a/test/vite-node/test/server.test.ts b/test/vite-node/test/server.test.ts index 7020d91f7..e6f0e42c6 100644 --- a/test/vite-node/test/server.test.ts +++ b/test/vite-node/test/server.test.ts @@ -1,7 +1,7 @@ -import { resolve } from 'pathe' +import { join, resolve } from 'pathe' import { ViteNodeServer } from 'vite-node/server' import { describe, expect, test, vi } from 'vitest' -import { createServer } from 'vite' +import { type Plugin, type ViteDevServer, createServer } from 'vite' import { extractSourceMap } from '../../../packages/vite-node/src/source-map' describe('server works correctly', async () => { @@ -29,19 +29,201 @@ describe('server works correctly', async () => { await vnServer.resolveId('/ssr', '/ssr path') expect(resolveId).toHaveBeenCalledWith('/ssr', '/ssr path', { ssr: true }) }) - test('fetchModule with id, and got sourcemap source in absolute path', async () => { - const server = await createServer({ - logLevel: 'error', - root: resolve(__dirname, '../'), - }) - const vnServer = new ViteNodeServer(server) +}) - // fetchModule in not a valid filesystem path - const fetchResult = await vnServer.fetchModule('/src/foo.js') +const wait = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)) + +describe('server correctly caches data', () => { + const it = test.extend<{ + root: string + plugin: Plugin + ssrFiles: string[] + webFiles: string[] + server: ViteDevServer + viteNode: ViteNodeServer + }>({ + ssrFiles: async ({}, use) => { + await use([]) + }, + webFiles: async ({}, use) => { + await use([]) + }, + root: resolve(__dirname, '../'), + plugin: async ({ ssrFiles, webFiles }, use) => { + const plugin: Plugin = { + name: 'test', + transform(code, id, options) { + // this should be called only once if cached is configured correctly + if (options?.ssr) + ssrFiles.push(id) + else + webFiles.push(id) + }, + } + await use(plugin) + }, + server: async ({ root, plugin }, use) => { + const server = await createServer({ + configFile: false, + root, + server: { + middlewareMode: true, + watch: null, + }, + optimizeDeps: { + disabled: true, + }, + plugins: [plugin], + }) + await use(server) + await server.close() + }, + viteNode: async ({ server }, use) => { + const vnServer = new ViteNodeServer(server) + await use(vnServer) + }, + }) + + it('fetchModule with id, and got sourcemap source in absolute path', async ({ viteNode }) => { + const fetchResult = await viteNode.fetchModule('/src/foo.js') const sourceMap = extractSourceMap(fetchResult.code!) // expect got sourcemap source in a valid filesystem path expect(sourceMap?.sources[0]).toBe('foo.js') }) + + it('correctly returns cached and invalidated ssr modules', async ({ root, viteNode, ssrFiles, webFiles, server }) => { + await viteNode.fetchModule('/src/foo.js', 'ssr') + + const fsPath = join(root, './src/foo.js') + + expect(viteNode.fetchCaches.web.has(fsPath)).toBe(false) + expect(viteNode.fetchCache.has(fsPath)).toBe(true) + expect(viteNode.fetchCaches.ssr.has(fsPath)).toBe(true) + + expect(webFiles).toHaveLength(0) + expect(ssrFiles).toHaveLength(1) + expect(ssrFiles).toContain(fsPath) + + await viteNode.fetchModule('/src/foo.js', 'ssr') + + expect(ssrFiles).toHaveLength(1) + + server.moduleGraph.invalidateModule( + server.moduleGraph.getModuleById(fsPath)!, + new Set(), + Date.now(), + false, + ) + + // wait so TS are different + await wait(10) + + await viteNode.fetchModule('/src/foo.js', 'ssr') + + expect(ssrFiles).toHaveLength(2) + + // another fetch after invalidation returns cached result + await viteNode.fetchModule('/src/foo.js', 'ssr') + + expect(ssrFiles).toHaveLength(2) + + server.moduleGraph.invalidateModule( + server.moduleGraph.getModuleById(fsPath)!, + new Set(), + Date.now(), + true, + ) + + // wait so TS are different + await wait(10) + + await viteNode.fetchModule('/src/foo.js', 'ssr') + + expect(ssrFiles).toHaveLength(3) + + // another fetch after invalidation returns cached result + await viteNode.fetchModule('/src/foo.js', 'ssr') + + expect(ssrFiles).toHaveLength(3) + expect(webFiles).toHaveLength(0) + }) + + it('correctly returns cached and invalidated web modules', async ({ root, viteNode, webFiles, ssrFiles, server }) => { + await viteNode.fetchModule('/src/foo.js', 'web') + + const fsPath = join(root, './src/foo.js') + + expect(viteNode.fetchCaches.ssr.has(fsPath)).toBe(false) + expect(viteNode.fetchCache.has(fsPath)).toBe(true) + expect(viteNode.fetchCaches.web.has(fsPath)).toBe(true) + + expect(ssrFiles).toHaveLength(0) + expect(webFiles).toHaveLength(1) + expect(webFiles).toContain(fsPath) + + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(webFiles).toHaveLength(1) + + server.moduleGraph.invalidateModule( + server.moduleGraph.getModuleById(fsPath)!, + new Set(), + Date.now(), + false, + ) + + // wait so TS are different + await wait(10) + + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(webFiles).toHaveLength(2) + + // another fetch after invalidation returns cached result + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(webFiles).toHaveLength(2) + + server.moduleGraph.invalidateModule( + server.moduleGraph.getModuleById(fsPath)!, + new Set(), + Date.now(), + true, + ) + + // wait so TS are different + await wait(10) + + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(webFiles).toHaveLength(3) + + // another fetch after invalidation returns cached result + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(webFiles).toHaveLength(3) + expect(ssrFiles).toHaveLength(0) + }) + + it('correctly processes the same file with both transform modes', async ({ viteNode, ssrFiles, webFiles, root }) => { + await viteNode.fetchModule('/src/foo.js', 'ssr') + await viteNode.fetchModule('/src/foo.js', 'web') + + const fsPath = join(root, './src/foo.js') + + expect(viteNode.fetchCaches.ssr.has(fsPath)).toBe(true) + expect(viteNode.fetchCache.has(fsPath)).toBe(true) + expect(viteNode.fetchCaches.web.has(fsPath)).toBe(true) + + expect(ssrFiles).toHaveLength(1) + expect(webFiles).toHaveLength(1) + + await viteNode.fetchModule('/src/foo.js', 'ssr') + await viteNode.fetchModule('/src/foo.js', 'web') + + expect(ssrFiles).toHaveLength(1) + expect(webFiles).toHaveLength(1) + }) }) diff --git a/test/vite-node/vitest.config.ts b/test/vite-node/vitest.config.ts index 3212242e1..49725ad25 100644 --- a/test/vite-node/vitest.config.ts +++ b/test/vite-node/vitest.config.ts @@ -4,5 +4,9 @@ export default defineConfig({ test: { clearMocks: true, testTimeout: process.env.CI ? 120_000 : 5_000, + onConsoleLog(log) { + if (log.includes('Port is already')) + return false + }, }, })