[pull] main from vitejs:main

packages/vite/src/client/bundledDevClient.ts

@@ -24,6 +24,14 @@ export {
 
 if (typeof DevRuntime !== 'undefined') {
   class ViteDevRuntime extends DevRuntime {
+    payloadDelivered(filename: string): void {
+      transport.send({
+        type: 'custom',
+        event: 'vite:bundled-dev:payload-delivered',
+        data: { filename },
+      })
+    }
+
     override createModuleHotContext(moduleId: string) {
       const ctx = new BundledDevHMRContext(bundledDevHmrClient, moduleId)
       // @ts-expect-error TODO: support CSS properly

packages/vite/src/node/server/bundledDev.ts

@@ -138,6 +138,9 @@ export class BundledDev {
         this.devEngine.registerClient(payload.clientId)
       },
     )
+    this.environment.hot.on('vite:bundled-dev:payload-delivered', (payload) => {
+      this.markPayloadDelivered(payload.filename)
+    })
     this.environment.hot.on('vite:client:connect', (_payload, client) => {
       // Replay the cached build error to freshly connected clients.
       if (this.lastBuildError) {
@@ -343,13 +346,17 @@ export class BundledDev {
     )
     const result = await this.devEngine.compileEntry(moduleId, clientId)
     this.pendingPayloadFilenames.add(result.filename)
-    return result
+    return {
+      filename: result.filename,
+      code: result.code + payloadDeliveredAck(result.filename),
+    }
   }
 
   /**
-   * Called by the serving middlewares when the response for a payload completed.
-   * Only delivered payloads are recorded on the server's per-client ship map, so
-   * later chunks may omit a module only if the payload carrying it was delivered.
+   * Called when the client reports that it evaluated a payload (the line appended
+   * by `payloadDeliveredAck`). Only then is the payload recorded on the server's
+   * per-client ship map, so later chunks may omit a module only if the client
+   * already registered it.
    *
    * Note: the payload filename is unique across all clients.
    */
@@ -467,7 +474,10 @@ export class BundledDev {
       // https://green.sapphi.red/blog/local-server-security-best-practices#properly-check-the-request-origin
       // we can also use `Cross-Origin Resource Policy` header instead of this
       // but we cannot use `Sec-Fetch-*` headers as they are only sent to potentially-trustworthy origins
-      source: hmrOutput.code + '\n; export {}',
+      source:
+        hmrOutput.code +
+        payloadDeliveredAck(hmrOutput.filename) +
+        '\n; export {}',
     })
     if (hmrOutput.sourcemapFilename && hmrOutput.sourcemap) {
       this.memoryFiles.set(hmrOutput.sourcemapFilename, {
@@ -541,3 +551,13 @@ function debounce(time: number, cb: () => void) {
     timer = globalThis.setTimeout(cb, time)
   }
 }
+
+/**
+ * The line appended to a lazy chunk or HMR patch so the client reports the
+ * payload as delivered once its factories are registered. Placed after the
+ * chunk's tail: if the tail throws, no report is sent and the next payload
+ * simply re-ships those factories, which is safe (registration overwrites).
+ */
+function payloadDeliveredAck(filename: string): string {
+  return `\n;__rolldown_runtime__.payloadDelivered(${JSON.stringify(filename)});`
+}

packages/vite/src/node/server/middlewares/memoryFiles.ts

@@ -48,7 +48,6 @@ export function memoryFilesMiddleware(
         res.setHeader(name, headers[name]!)
       }
 
-      res.on('finish', () => bundledDev.markPayloadDelivered(filePath))
       return res.end(file.source)
     }
     next()

packages/vite/src/node/server/middlewares/triggerLazyBundling.ts

@@ -42,7 +42,6 @@ export function triggerLazyBundlingMiddleware(
     }
 
     res!.setHeader('Content-Type', 'application/javascript')
-    res!.on('finish', () => bundledDev.markPayloadDelivered(result.filename))
     return res!.end(result.code)
   }
 }

playground/lazy-compilation/__tests__/lazy-compilation.spec.ts

@@ -0,0 +1,114 @@
+import type { Response, Route } from 'playwright-chromium'
+import { beforeAll, describe, expect, test } from 'vitest'
+import { isServe, page, promiseWithResolvers } from '~utils'
+
+// Regression test for rolldown/rolldown#10774.
+//
+// A lazy chunk omits every factory the server believes this client already
+// holds. The server must learn that from the client (the chunk reports back
+// after registering its factories), not from its own HTTP response finishing:
+// bytes that left the server may still be on the wire while a later chunk is
+// compiled, delivered and evaluated. If the server recorded delivery too
+// early, the later chunk would omit a shared factory and `initModule` would
+// throw `MissingFactoryError` in the browser.
+//
+// The spec makes that window deterministic with Playwright routing: A's
+// response is fetched from the server (so the server finished writing it) but
+// held back from the page until B has been requested and evaluated.
+//
+// Both tests share one page session on purpose. A lazy entry is served through
+// `/@vite/lazy` only until it is first fetched on this server; a later page
+// load gets it as an ordinary chunk. So the second test cannot reload — it
+// continues from the state the first test leaves behind.
+
+const factoryFor = (file: string) =>
+  new RegExp(`registerFactory\\("[^"]*/${file}"`)
+
+const lazyIdOf = (url: string) => new URL(url).searchParams.get('id') ?? ''
+
+const lazyBodies: { id: string; body: Promise<string> }[] = []
+const lazyBody = (route: string) =>
+  lazyBodies.find((e) => e.id.includes(route))!.body
+
+const onResponse = (res: Response) => {
+  if (res.url().includes('/@vite/lazy?')) {
+    lazyBodies.push({ id: lazyIdOf(res.url()), body: res.text() })
+  }
+}
+
+describe.runIf(isServe)('lazy compilation', () => {
+  beforeAll(() => {
+    page.on('response', onResponse)
+    return () => {
+      page.off('response', onResponse)
+    }
+  })
+
+  test('a chunk compiled while an earlier chunk is still in flight carries the shared factory', async () => {
+    // Resolved once A's response has fully arrived from the server; the route
+    // then waits on `releaseA` before handing it to the page.
+    const aFetched = promiseWithResolvers<void>()
+    const releaseA = promiseWithResolvers<void>()
+    // Resolved once the held response was handed to the page, so teardown can
+    // dispose the route safely (disposing first makes the pending `fulfill`
+    // throw).
+    const aFulfilled = promiseWithResolvers<void>()
+
+    const lazyRoute = async (route: Route) => {
+      if (!lazyIdOf(route.request().url()).includes('page-a')) {
+        return route.continue()
+      }
+      const response = await route.fetch()
+      aFetched.resolve()
+      await releaseA.promise
+      await route.fulfill({ response })
+      aFulfilled.resolve()
+    }
+    const router = await page.route('**/@vite/lazy?*', lazyRoute)
+
+    try {
+      await page.click('#route-a-btn')
+      await aFetched.promise
+
+      // B is compiled while A's bytes are held back from the page, so the
+      // client has not reported A yet and B must still carry shared.js.
+      await page.click('#route-b-btn')
+      await expect
+        .poll(() => lazyBodies.some((e) => e.id.includes('page-b')))
+        .toBe(true)
+      const bodyB = await lazyBody('page-b')
+      expect(bodyB).toMatch(factoryFor('page-b.js'))
+      expect(bodyB).toMatch(factoryFor('shared.js'))
+
+      await expect
+        .poll(() => page.textContent('#route-b-content'))
+        .toBe('B:shared-value')
+
+      releaseA.resolve()
+      await expect
+        .poll(() => page.textContent('#route-a-content'))
+        .toBe('A:shared-value')
+      expect(await lazyBody('page-a')).toMatch(factoryFor('shared.js'))
+    } finally {
+      releaseA.resolve()
+      await aFulfilled.promise
+      await router.dispose()
+    }
+  })
+
+  test('a chunk requested after the earlier chunks were evaluated omits the shared factory', async () => {
+    // The client reports A and B over the websocket after evaluating them;
+    // the next lazy request is a separate HTTP request, so give the reports
+    // a moment to land before C is compiled.
+    await new Promise((resolve) => setTimeout(resolve, 300))
+
+    await page.click('#route-c-btn')
+    await expect
+      .poll(() => page.textContent('#route-c-content'))
+      .toBe('C:shared-value')
+
+    const bodyC = await lazyBody('page-c')
+    expect(bodyC).toMatch(factoryFor('page-c.js'))
+    expect(bodyC).not.toMatch(factoryFor('shared.js'))
+  })
+})

playground/lazy-compilation/index.html

@@ -0,0 +1,18 @@
+<h1>Lazy compilation</h1>
+
+<section>
+  <button id="route-a-btn">load A</button>
+  <div id="route-a-content">pending</div>
+</section>
+
+<section>
+  <button id="route-b-btn">load B</button>
+  <div id="route-b-content">pending</div>
+</section>
+
+<section>
+  <button id="route-c-btn">load C</button>
+  <div id="route-c-content">pending</div>
+</section>
+
+<script type="module" src="/main.js"></script>

playground/lazy-compilation/main.js

@@ -0,0 +1,21 @@
+// Three lazy "routes" that all statically import shared.js. Each button starts
+// its import right away and never waits for the others, so a spec can hold
+// A's response in the browser while B is requested and evaluated.
+// A failed import is written to the DOM so the spec can read the error.
+function load(name, importer) {
+  document.getElementById(`${name}-btn`).addEventListener('click', () => {
+    importer().then(
+      (mod) => {
+        document.getElementById(`${name}-content`).textContent = mod.value
+      },
+      (err) => {
+        document.getElementById(`${name}-content`).textContent =
+          `error:${err.message}`
+      },
+    )
+  })
+}
+
+load('route-a', () => import('./page-a.js'))
+load('route-b', () => import('./page-b.js'))
+load('route-c', () => import('./page-c.js'))

playground/lazy-compilation/package.json

@@ -0,0 +1,12 @@
+{
+  "name": "@vitejs/test-lazy-compilation",
+  "private": true,
+  "version": "0.0.0",
+  "type": "module",
+  "scripts": {
+    "dev": "vite",
+    "build": "vite build",
+    "debug": "node --inspect-brk ../../packages/vite/bin/vite",
+    "preview": "vite preview"
+  }
+}

playground/lazy-compilation/page-a.js

@@ -0,0 +1,3 @@
+import { shared } from './shared.js'
+
+export const value = `A:${shared}`

playground/lazy-compilation/page-b.js

@@ -0,0 +1,3 @@
+import { shared } from './shared.js'
+
+export const value = `B:${shared}`

playground/lazy-compilation/page-c.js

@@ -0,0 +1,3 @@
+import { shared } from './shared.js'
+
+export const value = `C:${shared}`

playground/lazy-compilation/shared.js

@@ -0,0 +1 @@
+export const shared = 'shared-value'

playground/lazy-compilation/vite.config.ts

@@ -0,0 +1,9 @@
+import { defineConfig } from 'vite'
+
+// Lazy compilation only exists in bundled dev, so this playground always runs
+// with it on, also under plain `pnpm test-serve`.
+export default defineConfig({
+  experimental: {
+    bundledDev: true,
+  },
+})