fix(vite-node): correctly return cached result (#4870)

Author: sheremet-va

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()

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)
+  })
 })

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
+    },
   },
 })