diff --git a/packages/vite/src/client/bundledDevClient.ts b/packages/vite/src/client/bundledDevClient.ts index 416fe1a816848a..58de0bc2afe4db 100644 --- a/packages/vite/src/client/bundledDevClient.ts +++ b/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 diff --git a/packages/vite/src/node/server/bundledDev.ts b/packages/vite/src/node/server/bundledDev.ts index ceaa0f9111797e..e195cb0c6cf545 100644 --- a/packages/vite/src/node/server/bundledDev.ts +++ b/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)});` +} diff --git a/packages/vite/src/node/server/middlewares/memoryFiles.ts b/packages/vite/src/node/server/middlewares/memoryFiles.ts index d0372ab3e378e4..5972788a70792e 100644 --- a/packages/vite/src/node/server/middlewares/memoryFiles.ts +++ b/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() diff --git a/packages/vite/src/node/server/middlewares/triggerLazyBundling.ts b/packages/vite/src/node/server/middlewares/triggerLazyBundling.ts index 1c6956fd879c38..7c56aa65e1c92a 100644 --- a/packages/vite/src/node/server/middlewares/triggerLazyBundling.ts +++ b/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) } } diff --git a/playground/lazy-compilation/__tests__/lazy-compilation.spec.ts b/playground/lazy-compilation/__tests__/lazy-compilation.spec.ts new file mode 100644 index 00000000000000..3b0dd27caeff7c --- /dev/null +++ b/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 }[] = [] +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() + const releaseA = promiseWithResolvers() + // 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() + + 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')) + }) +}) diff --git a/playground/lazy-compilation/index.html b/playground/lazy-compilation/index.html new file mode 100644 index 00000000000000..cad97363c07c28 --- /dev/null +++ b/playground/lazy-compilation/index.html @@ -0,0 +1,18 @@ +

Lazy compilation

+ +
+ +
pending
+
+ +
+ +
pending
+
+ +
+ +
pending
+
+ + diff --git a/playground/lazy-compilation/main.js b/playground/lazy-compilation/main.js new file mode 100644 index 00000000000000..2f8f1cf84c2a11 --- /dev/null +++ b/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')) diff --git a/playground/lazy-compilation/package.json b/playground/lazy-compilation/package.json new file mode 100644 index 00000000000000..f8e02c0986ccd5 --- /dev/null +++ b/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" + } +} diff --git a/playground/lazy-compilation/page-a.js b/playground/lazy-compilation/page-a.js new file mode 100644 index 00000000000000..1d3e353c7a93a9 --- /dev/null +++ b/playground/lazy-compilation/page-a.js @@ -0,0 +1,3 @@ +import { shared } from './shared.js' + +export const value = `A:${shared}` diff --git a/playground/lazy-compilation/page-b.js b/playground/lazy-compilation/page-b.js new file mode 100644 index 00000000000000..31d1df52ae2de1 --- /dev/null +++ b/playground/lazy-compilation/page-b.js @@ -0,0 +1,3 @@ +import { shared } from './shared.js' + +export const value = `B:${shared}` diff --git a/playground/lazy-compilation/page-c.js b/playground/lazy-compilation/page-c.js new file mode 100644 index 00000000000000..4fe738f1752e4d --- /dev/null +++ b/playground/lazy-compilation/page-c.js @@ -0,0 +1,3 @@ +import { shared } from './shared.js' + +export const value = `C:${shared}` diff --git a/playground/lazy-compilation/shared.js b/playground/lazy-compilation/shared.js new file mode 100644 index 00000000000000..811ddbf86b183f --- /dev/null +++ b/playground/lazy-compilation/shared.js @@ -0,0 +1 @@ +export const shared = 'shared-value' diff --git a/playground/lazy-compilation/vite.config.ts b/playground/lazy-compilation/vite.config.ts new file mode 100644 index 00000000000000..194c91676dd316 --- /dev/null +++ b/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, + }, +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5f876604b497ff..0d26a1a2ff9329 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1051,6 +1051,8 @@ importers: playground/json/json-module: {} + playground/lazy-compilation: {} + playground/legacy: devDependencies: '@vitejs/plugin-legacy':