diff --git a/e2e/test-utils.ts b/e2e/test-utils.ts index 99bf604f..ee10df3e 100644 --- a/e2e/test-utils.ts +++ b/e2e/test-utils.ts @@ -144,7 +144,8 @@ export async function getBgColor(selector: string): Promise { /** * Update a file in the vrowzer virtual filesystem via the browser API. - * Triggers HMR in serve mode. + * Triggers HMR in serve mode. Resolves when the Workers have applied the change, + * without waiting for HMR. * * @param filename - Virtual path (e.g. '/App.vue', '/main.ts') * @param content - Full new file content @@ -152,7 +153,7 @@ export async function getBgColor(selector: string): Promise { export async function updateFile(filename: string, content: string): Promise { await page.evaluate( ([path, code]) => { - ;(window as any).__vrowzer__.updateFile(path, code) + return (window as any).__vrowzer__.updateFile(path, code) }, [filename, content] as const ) @@ -164,7 +165,7 @@ export async function updateFile(filename: string, content: string): Promise { await page.evaluate( ([path, code]) => { - ;(window as any).__vrowzer__.addFile(path, code) + return (window as any).__vrowzer__.addFile(path, code) }, [filename, content] as const ) @@ -175,7 +176,7 @@ export async function addFile(filename: string, content: string): Promise */ export async function removeFile(filename: string): Promise { await page.evaluate(path => { - ;(window as any).__vrowzer__.deleteFile(path) + return (window as any).__vrowzer__.deleteFile(path) }, filename) } diff --git a/packages/fs/README.md b/packages/fs/README.md index ec1f1198..5fc5d28c 100644 --- a/packages/fs/README.md +++ b/packages/fs/README.md @@ -136,12 +136,18 @@ const publisher = createFileSystemPublisher([worker]) // node:fs-like API publisher.writeFile('/main.js', 'export const x = 1') // text -publisher.writeFile('/image.png', arrayBuffer) // binary (zero-copy transfer) +publisher.writeFile('/image.png', arrayBuffer) // binary (each target gets its own copy) publisher.unlink('/old-file.js') publisher.mkdir('/new-dir') publisher.initFiles({ '/main.js': 'code' }) // bulk init ``` +`writeFile()` and `unlink()` accept an operation ID, which is set on the message. A Worker that applies such a message can acknowledge it with a `V_FS_ACK` message (`{ type: 'V_FS_ACK', id, error? }`), so that the main thread knows when the change is applied. `@vrowzer/fs` does not send acknowledgements by itself: the Worker that handles the messages decides when a change counts as applied. + +```ts +publisher.writeFile('/main.js', 'export const x = 2', { id: crypto.randomUUID() }) +``` + #### Subscriber (Worker) ```ts diff --git a/packages/fs/docs/watcher/index.md b/packages/fs/docs/watcher/index.md index 96957984..53ca1072 100644 --- a/packages/fs/docs/watcher/index.md +++ b/packages/fs/docs/watcher/index.md @@ -6,6 +6,7 @@ Virtual filesystem watcher with Pub-Sub sync protocol. | Variable | Description | | ------ | ------ | +| [V_FS_ACK](/packages/fs/docs/watcher/variables/V_FS_ACK.md) | | | [V_FS_INIT](/packages/fs/docs/watcher/variables/V_FS_INIT.md) | | | [V_FS_MKDIR](/packages/fs/docs/watcher/variables/V_FS_MKDIR.md) | | | [V_FS_UNLINK](/packages/fs/docs/watcher/variables/V_FS_UNLINK.md) | | @@ -27,7 +28,9 @@ Virtual filesystem watcher with Pub-Sub sync protocol. | [FileSystemInterfaces](/packages/fs/docs/watcher/interfaces/FileSystemInterfaces.md) | Minimal fs interface required by FileSystemSubscriber. | | [FileSystemPublisher](/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md) | Publisher for broadcasting filesystem operations to Workers. | | [FileSystemPublisherTarget](/packages/fs/docs/watcher/interfaces/FileSystemPublisherTarget.md) | A postMessage target compatible like Service Worker and Web Worker APIs. | +| [FileSystemPublishOptions](/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md) | Options for a single [FileSystemPublisher](/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md) operation. | | [FileSystemSubscriber](/packages/fs/docs/watcher/interfaces/FileSystemSubscriber.md) | Subscriber for processing filesystem sync messages in Workers. | +| [FSAckMessage](/packages/fs/docs/watcher/interfaces/FSAckMessage.md) | Worker -> Main Thread: Acknowledge a message that has an `id`. | | [FSInitMessage](/packages/fs/docs/watcher/interfaces/FSInitMessage.md) | Main Thread -> Worker: Initialize files in bulk. Used during setup to populate the virtual filesystem. | | [FSMkdirMessage](/packages/fs/docs/watcher/interfaces/FSMkdirMessage.md) | Main Thread -> Worker: Create a directory. | | [FSUnlinkMessage](/packages/fs/docs/watcher/interfaces/FSUnlinkMessage.md) | Main Thread -> Worker: Delete a file. | diff --git a/packages/fs/docs/watcher/interfaces/FSAckMessage.md b/packages/fs/docs/watcher/interfaces/FSAckMessage.md new file mode 100644 index 00000000..e76eb05d --- /dev/null +++ b/packages/fs/docs/watcher/interfaces/FSAckMessage.md @@ -0,0 +1,20 @@ +# Interface: FSAckMessage + +Worker -> Main Thread: Acknowledge a message that has an `id`. + +`@vrowzer/fs` does not send it by itself. A Worker sends it after applying the message, +with `error` when applying failed. + +## Signature + +```ts +export interface FSAckMessage +``` + +## Properties + +| Name | Type | Description | +| --- | --- | --- | +| `error` _(optional)_ | `{ name: string; message: string }` | Why applying the message failed. Absent when it succeeded. | +| `id` | `string` | The `id` of the acknowledged message. | +| `type` | `"V_FS_ACK"` | | diff --git a/packages/fs/docs/watcher/interfaces/FSUnlinkMessage.md b/packages/fs/docs/watcher/interfaces/FSUnlinkMessage.md index 7971a11d..bb1cf33e 100644 --- a/packages/fs/docs/watcher/interfaces/FSUnlinkMessage.md +++ b/packages/fs/docs/watcher/interfaces/FSUnlinkMessage.md @@ -12,5 +12,6 @@ export interface FSUnlinkMessage | Name | Type | Description | | --- | --- | --- | +| `id` _(optional)_ | `string` | Operation ID. A Worker that applies the message can acknowledge it with [FSAckMessage](/packages/fs/docs/watcher/interfaces/FSAckMessage.md). | | `path` | `string` | Path of the file to delete. Must not end with '/' (directories use FS_MKDIR with path ending in '/'). | | `type` | `"V_FS_UNLINK"` | | diff --git a/packages/fs/docs/watcher/interfaces/FSWriteMessage.md b/packages/fs/docs/watcher/interfaces/FSWriteMessage.md index 3ea69c1b..28dfb1f8 100644 --- a/packages/fs/docs/watcher/interfaces/FSWriteMessage.md +++ b/packages/fs/docs/watcher/interfaces/FSWriteMessage.md @@ -8,7 +8,7 @@ For text files: For binary files: { type: 'V_FS_WRITE', path: '/image.png', encoding: 'binary', content: ArrayBuffer } - -> postMessage(message, [message.content]) // transfer list + -> postMessage(message, [message.content]) // transfer list, a copy for each target ## Signature @@ -20,7 +20,8 @@ export interface FSWriteMessage | Name | Type | Description | | --- | --- | --- | -| `content` | `string \| ArrayBuffer` | Content of the file. Type depends on encoding: - 'text': UTF-8 string content - 'binary': ArrayBuffer content (transferred via postMessage's transfer list) | +| `content` | `string \| ArrayBuffer` | Content of the file. Type depends on encoding: - 'text': UTF-8 string content - 'binary': ArrayBuffer content (a copy transferred via postMessage's transfer list) | | `encoding` | [`FSContentEncoding`](/packages/fs/docs/watcher/type-aliases/FSContentEncoding.md) | Encoding of the content. Determines how the Worker should interpret the content. | +| `id` _(optional)_ | `string` | Operation ID. A Worker that applies the message can acknowledge it with [FSAckMessage](/packages/fs/docs/watcher/interfaces/FSAckMessage.md). | | `path` | `string` | Path of the file to write. Must not end with '/' (directories use FS_MKDIR with path ending in '/'). | | `type` | `"V_FS_WRITE"` | | diff --git a/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md b/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md new file mode 100644 index 00000000..30d472bb --- /dev/null +++ b/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md @@ -0,0 +1,15 @@ +# Interface: FileSystemPublishOptions + +Options for a single [FileSystemPublisher](/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md) operation. + +## Signature + +```ts +export interface FileSystemPublishOptions +``` + +## Properties + +| Name | Type | Description | +| --- | --- | --- | +| `id` _(optional)_ | `string` | Operation ID set on the message, so that a Worker can acknowledge it with `V_FS_ACK`. | diff --git a/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md b/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md index eb4dfd8d..fc2784f3 100644 --- a/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md +++ b/packages/fs/docs/watcher/interfaces/FileSystemPublisher.md @@ -99,7 +99,7 @@ Remove a postMessage target. ### unlink() ```ts -unlink(path: string): void; +unlink(path: string, options?: FileSystemPublishOptions): void; ``` Delete a file. @@ -109,6 +109,7 @@ Delete a file. | Name | Type | Description | | --- | --- | --- | | `path` | `string` | Path of the file to delete. Must not end with '/' (directories use mkdir with path ending in '/'). | +| `options` | [`FileSystemPublishOptions`](/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md) | Options for this operation, such as its ID _(optional)_ | #### Returns @@ -119,7 +120,7 @@ Delete a file. ### writeFile() ```ts -writeFile(path: string, content: string | ArrayBuffer): void; +writeFile(path: string, content: string | ArrayBuffer, options?: FileSystemPublishOptions): void; ``` Write a file. Encoding is inferred: string → text, ArrayBuffer → binary. @@ -129,7 +130,8 @@ Write a file. Encoding is inferred: string → text, ArrayBuffer → binary. | Name | Type | Description | | --- | --- | --- | | `path` | `string` | Path of the file to write. Must not end with '/' (directories use mkdir with path ending in '/'). | -| `content` | `string \| ArrayBuffer` | Content of the file. Type determines encoding: - string: UTF-8 text content - ArrayBuffer: binary content (transferred via postMessage's transfer list for zero-copy performance) | +| `content` | `string \| ArrayBuffer` | Content of the file. Type determines encoding: - string: UTF-8 text content - ArrayBuffer: binary content. Each target receives its own copy via postMessage's transfer list, so the caller's ArrayBuffer stays usable. | +| `options` | [`FileSystemPublishOptions`](/packages/fs/docs/watcher/interfaces/FileSystemPublishOptions.md) | Options for this operation, such as its ID _(optional)_ | #### Returns diff --git a/packages/fs/docs/watcher/type-aliases/FSContentEncoding.md b/packages/fs/docs/watcher/type-aliases/FSContentEncoding.md index 50dd1efe..989f7f35 100644 --- a/packages/fs/docs/watcher/type-aliases/FSContentEncoding.md +++ b/packages/fs/docs/watcher/type-aliases/FSContentEncoding.md @@ -4,9 +4,9 @@ File content encoding type. - 'text': UTF-8 string content (JS, TS, JSON, CSS, HTML, etc.) - 'binary': ArrayBuffer content (images, WASM, fonts, etc.) -When encoding is 'binary', the content is an ArrayBuffer -and MUST be transferred via postMessage's transfer list -for zero-copy performance. +When encoding is 'binary', the content is an ArrayBuffer. +The publisher transfers a copy of it to each target via postMessage's transfer list, +so the caller's ArrayBuffer stays usable. ## Signature diff --git a/packages/fs/docs/watcher/variables/V_FS_ACK.md b/packages/fs/docs/watcher/variables/V_FS_ACK.md new file mode 100644 index 00000000..835575a8 --- /dev/null +++ b/packages/fs/docs/watcher/variables/V_FS_ACK.md @@ -0,0 +1,7 @@ +# Variable: V_FS_ACK + +## Signature + +```ts +export const V_FS_ACK = 'V_FS_ACK' as const +``` diff --git a/packages/fs/src/watcher/protocol.test.ts b/packages/fs/src/watcher/protocol.test.ts index 8cb776a5..7f41d67e 100644 --- a/packages/fs/src/watcher/protocol.test.ts +++ b/packages/fs/src/watcher/protocol.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vite-plus/test' -import { V_FS_INIT, V_FS_MKDIR, V_FS_UNLINK, V_FS_WRITE } from './protocol.ts' +import { V_FS_ACK, V_FS_INIT, V_FS_MKDIR, V_FS_UNLINK, V_FS_WRITE } from './protocol.ts' describe('@vrowzer/fs/watcher protocol', () => { test('V_FS_WRITE constant has correct value', () => { @@ -17,4 +17,8 @@ describe('@vrowzer/fs/watcher protocol', () => { test('V_FS_INIT constant has correct value', () => { expect(V_FS_INIT).toBe('V_FS_INIT') }) + + test('V_FS_ACK constant has correct value', () => { + expect(V_FS_ACK).toBe('V_FS_ACK') + }) }) diff --git a/packages/fs/src/watcher/protocol.ts b/packages/fs/src/watcher/protocol.ts index 48ec7f5a..c2792c66 100644 --- a/packages/fs/src/watcher/protocol.ts +++ b/packages/fs/src/watcher/protocol.ts @@ -17,9 +17,9 @@ * - 'text': UTF-8 string content (JS, TS, JSON, CSS, HTML, etc.) * - 'binary': ArrayBuffer content (images, WASM, fonts, etc.) * - * When encoding is 'binary', the content is an ArrayBuffer - * and MUST be transferred via postMessage's transfer list - * for zero-copy performance. + * When encoding is 'binary', the content is an ArrayBuffer. + * The publisher transfers a copy of it to each target via postMessage's transfer list, + * so the caller's ArrayBuffer stays usable. */ export type FSContentEncoding = 'text' | 'binary' @@ -32,7 +32,7 @@ export type FSContentEncoding = 'text' | 'binary' * * For binary files: * { type: 'V_FS_WRITE', path: '/image.png', encoding: 'binary', content: ArrayBuffer } - * -> postMessage(message, [message.content]) // transfer list + * -> postMessage(message, [message.content]) // transfer list, a copy for each target */ export interface FSWriteMessage { type: 'V_FS_WRITE' @@ -47,9 +47,13 @@ export interface FSWriteMessage { /** * Content of the file. Type depends on encoding: * - 'text': UTF-8 string content - * - 'binary': ArrayBuffer content (transferred via postMessage's transfer list) + * - 'binary': ArrayBuffer content (a copy transferred via postMessage's transfer list) */ content: string | ArrayBuffer + /** + * Operation ID. A Worker that applies the message can acknowledge it with {@link FSAckMessage}. + */ + id?: string } /** @@ -61,6 +65,10 @@ export interface FSUnlinkMessage { * Path of the file to delete. Must not end with '/' (directories use FS_MKDIR with path ending in '/'). */ path: string + /** + * Operation ID. A Worker that applies the message can acknowledge it with {@link FSAckMessage}. + */ + id?: string } /** @@ -99,8 +107,30 @@ export type FileSystemSyncMessage = | FSMkdirMessage | FSInitMessage +/** + * Worker -> Main Thread: Acknowledge a message that has an `id`. + * + * `@vrowzer/fs` does not send it by itself. A Worker sends it after applying the message, + * with `error` when applying failed. + */ +export interface FSAckMessage { + type: 'V_FS_ACK' + /** + * The `id` of the acknowledged message. + */ + id: string + /** + * Why applying the message failed. Absent when it succeeded. + */ + error?: { + name: string + message: string + } +} + // Constants export const V_FS_WRITE = 'V_FS_WRITE' as const export const V_FS_UNLINK = 'V_FS_UNLINK' as const export const V_FS_MKDIR = 'V_FS_MKDIR' as const export const V_FS_INIT = 'V_FS_INIT' as const +export const V_FS_ACK = 'V_FS_ACK' as const diff --git a/packages/fs/src/watcher/publisher.test.ts b/packages/fs/src/watcher/publisher.test.ts index 47739708..994d4d1f 100644 --- a/packages/fs/src/watcher/publisher.test.ts +++ b/packages/fs/src/watcher/publisher.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, test } from 'vite-plus/test' +import { describe, expect, onTestFinished, test } from 'vite-plus/test' import type { FileSystemPublisherTarget } from './publisher.ts' import { createFileSystemPublisher } from './publisher.ts' @@ -14,6 +14,21 @@ function createMockTarget(): FileSystemPublisherTarget & { } } +/** + * A target that really transfers buffers, so that a transferred ArrayBuffer is detached. + */ +function createPortTarget(): { target: MessagePort; received: Promise } { + const channel = new MessageChannel() + const received = new Promise(resolve => { + channel.port2.onmessage = event => resolve(event.data) + }) + onTestFinished(() => { + channel.port1.close() + channel.port2.close() + }) + return { target: channel.port1, received } +} + describe('FileSystemPublisher', () => { describe('target management', () => { test('addTarget() adds a target', () => { @@ -64,17 +79,21 @@ describe('FileSystemPublisher', () => { }) }) - test('ArrayBuffer content sends V_FS_WRITE with encoding: "binary" and transfer list', () => { + test('ArrayBuffer content sends V_FS_WRITE with encoding: "binary" and transfers a copy', () => { const target = createMockTarget() const publisher = createFileSystemPublisher([target]) - const buffer = new ArrayBuffer(4) + const buffer = new Uint8Array([1, 2, 3, 4]).buffer publisher.writeFile('/image.png', buffer) - expect(target.calls[0]!.message.type).toBe('V_FS_WRITE') - expect(target.calls[0]!.message.encoding).toBe('binary') - expect(target.calls[0]!.message.content).toBeInstanceOf(ArrayBuffer) - expect(target.calls[0]!.transfer).toEqual([buffer]) + const { message, transfer } = target.calls[0]! + expect(message.type).toBe('V_FS_WRITE') + expect(message.encoding).toBe('binary') + expect(message.content).toBeInstanceOf(ArrayBuffer) + expect(message.content).not.toBe(buffer) + expect(transfer).toEqual([message.content]) + expect(transfer[0]).toBe(message.content) + expect([...new Uint8Array(message.content)]).toEqual([1, 2, 3, 4]) }) test('broadcasts the same message to multiple targets', () => { @@ -87,11 +106,11 @@ describe('FileSystemPublisher', () => { expect(target1.calls[0]!.message).toEqual(target2.calls[0]!.message) }) - test('ArrayBuffer with multiple targets: first gets original, rest get copies', () => { + test('ArrayBuffer with multiple targets: every target gets its own copy', () => { const target1 = createMockTarget() const target2 = createMockTarget() const publisher = createFileSystemPublisher([target1, target2]) - const buffer = new ArrayBuffer(4) + const buffer = new Uint8Array([1, 2, 3, 4]).buffer publisher.writeFile('/image.png', buffer) @@ -99,11 +118,71 @@ describe('FileSystemPublisher', () => { expect(target1.calls[0]!.message.encoding).toBe('binary') expect(target2.calls[0]!.message.encoding).toBe('binary') - // First target gets the original buffer - expect(target1.calls[0]!.transfer[0]).toBe(buffer) - // Second target gets a copy (different reference) - expect(target2.calls[0]!.transfer[0]).not.toBe(buffer) - expect(target2.calls[0]!.transfer[0].byteLength).toBe(buffer.byteLength) + // Neither target gets the caller's buffer, and the copies are not shared + const copy1 = target1.calls[0]!.transfer[0] + const copy2 = target2.calls[0]!.transfer[0] + expect(copy1).not.toBe(buffer) + expect(copy2).not.toBe(buffer) + expect(copy1).not.toBe(copy2) + expect([...new Uint8Array(copy1)]).toEqual([1, 2, 3, 4]) + expect([...new Uint8Array(copy2)]).toEqual([1, 2, 3, 4]) + }) + + test('keeps the caller ArrayBuffer usable and delivers the same bytes to every target', async () => { + const first = createPortTarget() + const second = createPortTarget() + const publisher = createFileSystemPublisher([first.target, second.target]) + const buffer = new Uint8Array([1, 2, 3, 4]).buffer + + publisher.writeFile('/image.png', buffer) + + expect(buffer.byteLength).toBe(4) + expect([...new Uint8Array(buffer)]).toEqual([1, 2, 3, 4]) + for (const message of await Promise.all([first.received, second.received])) { + expect(message).toMatchObject({ + type: 'V_FS_WRITE', + path: '/image.png', + encoding: 'binary' + }) + expect([...new Uint8Array(message.content)]).toEqual([1, 2, 3, 4]) + } + }) + + test('sets the operation id on the message', () => { + const target = createMockTarget() + const publisher = createFileSystemPublisher([target]) + + publisher.writeFile('/main.js', 'export const x = 1', { id: 'op-1' }) + + expect(target.calls[0]!.message).toEqual({ + type: 'V_FS_WRITE', + path: '/main.js', + encoding: 'text', + content: 'export const x = 1', + id: 'op-1' + }) + }) + + test('sets the operation id on the binary message for every target', () => { + const target1 = createMockTarget() + const target2 = createMockTarget() + const publisher = createFileSystemPublisher([target1, target2]) + + publisher.writeFile('/image.png', new ArrayBuffer(4), { id: 'op-2' }) + + expect(target1.calls[0]!.message.id).toBe('op-2') + expect(target2.calls[0]!.message.id).toBe('op-2') + }) + + test('omits the operation id without the option', () => { + const target = createMockTarget() + const publisher = createFileSystemPublisher([target]) + + publisher.writeFile('/main.js', 'text') + publisher.writeFile('/image.png', new ArrayBuffer(4)) + + expect(target.calls[0]!.message).not.toHaveProperty('id') + expect(target.calls[1]!.message).not.toHaveProperty('id') }) }) @@ -118,6 +197,20 @@ describe('FileSystemPublisher', () => { type: 'V_FS_UNLINK', path: '/old-file.js' }) + expect(target.calls[0]!.message).not.toHaveProperty('id') + }) + + test('sets the operation id on the message', () => { + const target = createMockTarget() + const publisher = createFileSystemPublisher([target]) + + publisher.unlink('/old-file.js', { id: 'op-3' }) + + expect(target.calls[0]!.message).toEqual({ + type: 'V_FS_UNLINK', + path: '/old-file.js', + id: 'op-3' + }) }) }) diff --git a/packages/fs/src/watcher/publisher.ts b/packages/fs/src/watcher/publisher.ts index 0b5ef569..0e417ad0 100644 --- a/packages/fs/src/watcher/publisher.ts +++ b/packages/fs/src/watcher/publisher.ts @@ -12,7 +12,12 @@ * @license MIT */ -import type { FSInitMessage, FSWriteMessage, FileSystemSyncMessage } from './protocol.ts' +import type { + FSInitMessage, + FSUnlinkMessage, + FSWriteMessage, + FileSystemSyncMessage +} from './protocol.ts' /** * A postMessage target compatible like Service Worker and Web Worker APIs. @@ -22,6 +27,16 @@ export interface FileSystemPublisherTarget { postMessage(message: any, options?: StructuredSerializeOptions): void } +/** + * Options for a single {@link FileSystemPublisher} operation. + */ +export interface FileSystemPublishOptions { + /** + * Operation ID set on the message, so that a Worker can acknowledge it with `V_FS_ACK`. + */ + id?: string +} + /** * Publisher for broadcasting filesystem operations to Workers. * @@ -37,15 +52,18 @@ export interface FileSystemPublisher { * @param path - Path of the file to write. Must not end with '/' (directories use mkdir with path ending in '/'). * @param content - Content of the file. Type determines encoding: * - string: UTF-8 text content - * - ArrayBuffer: binary content (transferred via postMessage's transfer list for zero-copy performance) + * - ArrayBuffer: binary content. Each target receives its own copy via postMessage's transfer list, + * so the caller's ArrayBuffer stays usable. + * @param options - Options for this operation, such as its ID */ - writeFile(path: string, content: string | ArrayBuffer): void + writeFile(path: string, content: string | ArrayBuffer, options?: FileSystemPublishOptions): void /** * Delete a file. * * @param path - Path of the file to delete. Must not end with '/' (directories use mkdir with path ending in '/'). + * @param options - Options for this operation, such as its ID */ - unlink(path: string): void + unlink(path: string, options?: FileSystemPublishOptions): void /** * Create a directory. * @@ -90,25 +108,34 @@ export function createFileSystemPublisher( } } + function idOf(options?: FileSystemPublishOptions): { id?: string } { + return options?.id === undefined ? {} : { id: options.id } + } + const instance: FileSystemPublisher = { - writeFile(path, content) { + writeFile(path, content, options) { if (typeof content === 'string') { - broadcast({ type: 'V_FS_WRITE', path, encoding: 'text', content }) + broadcast({ type: 'V_FS_WRITE', path, encoding: 'text', content, ...idOf(options) }) } else { - // ArrayBuffer: transfer list for zero-copy. - // First target gets the original buffer, subsequent targets get copies. - const targetList = [..._targets] - for (let i = 0; i < targetList.length; i++) { - const buf = i === 0 ? content : content.slice(0) - const msg: FSWriteMessage = { type: 'V_FS_WRITE', path, encoding: 'binary', content: buf } - // @ts-expect-error - postMessage with transfer list is supported by both Worker and ServiceWorker targets, but TypeScript typings may not reflect this accurately. - targetList[i].postMessage(msg, [buf]) + // ArrayBuffer: each target gets its own copy in the transfer list. + // The caller's buffer is never transferred, so it stays usable. + for (const target of _targets) { + const buf = content.slice(0) + const msg: FSWriteMessage = { + type: 'V_FS_WRITE', + path, + encoding: 'binary', + content: buf, + ...idOf(options) + } + target.postMessage(msg, [buf]) } } }, - unlink(path) { - broadcast({ type: 'V_FS_UNLINK', path }) + unlink(path, options) { + const msg: FSUnlinkMessage = { type: 'V_FS_UNLINK', path, ...idOf(options) } + broadcast(msg) }, mkdir(path) { diff --git a/packages/play-vrowzer/src/App.vue b/packages/play-vrowzer/src/App.vue index d8f864bd..3db183ff 100644 --- a/packages/play-vrowzer/src/App.vue +++ b/packages/play-vrowzer/src/App.vue @@ -111,7 +111,9 @@ onMounted(async () => { }) function handleFileChange({ path, content }: { path: string; content: string }) { - vrowzer.updateFile(path, content) + vrowzer.updateFile(path, content).catch((err: Error) => { + console.error('[Vrowzer Playground] updateFile failed:', err) + }) } function handleReload() { diff --git a/packages/vite-dev-server/src/node/transformer.test.ts b/packages/vite-dev-server/src/node/transformer.test.ts index e1b204b3..7ef8900c 100644 --- a/packages/vite-dev-server/src/node/transformer.test.ts +++ b/packages/vite-dev-server/src/node/transformer.test.ts @@ -1,5 +1,11 @@ import { createVirtualFSWatcher } from '@vrowzer/fs/watcher' -import { describe, expect, onTestFinished, test, vi } from 'vite-plus/test' +import { beforeEach, describe, expect, onTestFinished, test, vi } from 'vite-plus/test' + +const hmrMocks = vi.hoisted(() => ({ + handleHMRUpdate: vi.fn< + (type: 'create' | 'delete' | 'update', file: string, server: unknown) => Promise + >(async () => undefined), +})) // transformer.ts is also a runtime barrel. Stub its re-export graph so this // test exercises setupWorker and setupHMR without initializing browser WASM. @@ -76,7 +82,7 @@ vi.mock('./server/transformRequest', () => ({ vi.mock('./server/hmr', () => ({ createServerHotChannel: () => ({}), getShortName: (file: string) => file, - handleHMRUpdate: async () => undefined, + handleHMRUpdate: hmrMocks.handleHMRUpdate, handlePrunedModules: async () => undefined, lexAcceptedHmrDeps: () => undefined, lexAcceptedHmrExports: () => undefined, @@ -248,3 +254,171 @@ describe('setupHMR watcher error handling', () => { }, ) }) + +interface Deferred { + promise: Promise + resolve: () => void +} + +function deferred(): Deferred { + let resolve!: () => void + const promise = new Promise((resolvePromise) => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +async function isPending(promise: Promise): Promise { + let settled = false + promise.then( + () => { settled = true }, + () => { settled = true }, + ) + await new Promise((resolve) => setTimeout(resolve, 0)) + return !settled +} + +function createHMRServer() { + const calls: string[] = [] + const logError = vi.fn<(error: unknown) => void>() + const watchChange = vi.fn< + (file: string, options: { event: 'create' | 'delete' | 'update' }) => Promise + >(async (file, { event }) => { + calls.push(`watchChange:${event}:${file}`) + }) + const watcher = createVirtualFSWatcher() + onTestFinished(() => watcher.close()) + + const server = { + watcher, + environments: { + client: { + pluginContainer: { watchChange }, + moduleGraph: { + onFileChange: vi.fn<(file: string) => void>((file) => { + calls.push(`onFileChange:${file}`) + }), + onFileDelete: vi.fn<(file: string) => void>((file) => { + calls.push(`onFileDelete:${file}`) + }), + }, + }, + }, + config: { + publicDir: false, + server: { + hmr: true, + }, + logger: { + error: logError, + }, + }, + } as unknown as Parameters[0] + + return { calls, logError, server, watchChange, watcher } +} + +describe('setupHMR file change tracking', () => { + beforeEach(() => { + hmrMocks.handleHMRUpdate.mockReset() + }) + + test.each([ + ['change', 'update', ['watchChange:update:/src/main.ts', 'onFileChange:/src/main.ts']], + ['add', 'create', ['watchChange:create:/src/main.ts']], + // A deleted file is invalidated before HMR as well, so that its cached transform + // result is not served after waitForFileChange() resolves + [ + 'unlink', + 'delete', + ['watchChange:delete:/src/main.ts', 'onFileChange:/src/main.ts', 'onFileDelete:/src/main.ts'], + ], + ] as const)( + 'waits for watchChange and invalidation of %s events, but not for HMR', + async (watcherEvent, hmrType, invalidation) => { + const { calls, server, watcher } = createHMRServer() + const hmr = deferred() + onTestFinished(() => hmr.resolve()) + hmrMocks.handleHMRUpdate.mockImplementation(async (type, file) => { + calls.push(`hmr:${type}:${file}`) + await hmr.promise + }) + const { waitForFileChange } = await setupHMR(server) + + watcher.notify(watcherEvent, '/src/main.ts') + + await expect(waitForFileChange('/src/main.ts')).resolves.toBeUndefined() + expect(calls.slice(0, invalidation.length)).toEqual(invalidation) + // HMR still runs after the invalidation, and it has not finished + await vi.waitFor(() => { + expect(calls).toEqual([...invalidation, `hmr:${hmrType}:/src/main.ts`]) + }) + }, + ) + + test.each(['add', 'change', 'unlink'] as const)( + 'rejects with the watchChange error of %s events and skips HMR', + async (watcherEvent) => { + const { logError, server, watchChange, watcher } = createHMRServer() + const error = new Error(`${watcherEvent} failed`) + watchChange.mockRejectedValue(error) + const { waitForFileChange } = await setupHMR(server) + + watcher.notify(watcherEvent, '/src/main.ts') + + await expect(waitForFileChange('/src/main.ts')).rejects.toBe(error) + await vi.waitFor(() => { + expect(logError).toHaveBeenCalledWith(error) + }) + expect(hmrMocks.handleHMRUpdate).not.toHaveBeenCalled() + }, + ) + + test('returns the processing of each event for the same path', async () => { + const { server, watchChange, watcher } = createHMRServer() + const held = deferred() + watchChange.mockImplementationOnce(() => held.promise) + const { waitForFileChange } = await setupHMR(server) + + watcher.notify('change', '/src/main.ts') + const first = waitForFileChange('/src/main.ts') + watcher.notify('change', '/src/main.ts') + const second = waitForFileChange('/src/main.ts') + + await expect(second).resolves.toBeUndefined() + expect(await isPending(first)).toBe(true) + held.resolve() + await expect(first).resolves.toBeUndefined() + }) + + test('resolves at once without an event to wait for', async () => { + const { server, watchChange, watcher } = createHMRServer() + const held = deferred() + watchChange.mockImplementationOnce(() => held.promise) + const { waitForFileChange } = await setupHMR(server) + + await expect(waitForFileChange('/src/none.ts')).resolves.toBeUndefined() + + watcher.notify('change', '/src/main.ts') + const taken = waitForFileChange('/src/main.ts') + // The event was already taken, so nothing is left to wait for + await expect(waitForFileChange('/src/main.ts')).resolves.toBeUndefined() + expect(await isPending(taken)).toBe(true) + held.resolve() + await expect(taken).resolves.toBeUndefined() + }) + + test('forgets an event that settled without being taken', async () => { + const { logError, server, watchChange, watcher } = createHMRServer() + const error = new Error('change failed') + watchChange.mockRejectedValueOnce(error) + const { waitForFileChange } = await setupHMR(server) + + watcher.notify('change', '/src/main.ts') + await vi.waitFor(() => { + expect(logError).toHaveBeenCalledWith(error) + }) + + await expect(waitForFileChange('/src/main.ts')).resolves.toBeUndefined() + }) +}) diff --git a/packages/vite-dev-server/src/node/transformer.ts b/packages/vite-dev-server/src/node/transformer.ts index 4817eab4..e3074430 100644 --- a/packages/vite-dev-server/src/node/transformer.ts +++ b/packages/vite-dev-server/src/node/transformer.ts @@ -66,6 +66,13 @@ export type ViteDevServerForWorker = Pick & { /** Virtual filesystem used by this transformer's DevEnvironment instance. */ readonly fileSystem: typeof fs + /** + * Waits for the latest watcher event for `file` to be processed up to module graph + * invalidation. HMR runs afterwards and is not awaited. + * + * @see {@link FileChangeTracker.waitForFileChange} + */ + waitForFileChange(file: string): Promise } /** @@ -247,6 +254,23 @@ function clientBasePlugin() { } } +/** + * Watcher events processed by {@link setupHMR}. + */ +export interface FileChangeTracker { + /** + * Takes the processing of the latest watcher event for `file`, up to module graph invalidation. + * + * The processing starts synchronously in the watcher listener, so call this right after the + * event is emitted. HMR runs afterwards and is not included. Resolves at once when there is + * no event for `file` to wait for, including when it was already taken. + * + * @param file - The path that the watcher event was emitted with + * @returns A promise that rejects when processing the event failed, e.g. in a plugin's `watchChange` hook + */ + waitForFileChange(file: string): Promise +} + /** * Connect watcher events to HMR pipeline. * @@ -255,8 +279,9 @@ function clientBasePlugin() { * to trigger module graph invalidation and HMR updates. * * @see https://github.com/vitejs/vite/blob/main/packages/vite/src/node/server/index.ts + * @returns A tracker to wait for the module graph invalidation of watcher events */ -export async function setupHMR(server: ViteDevServer): Promise { +export async function setupHMR(server: ViteDevServer): Promise { const { watcher, environments, config } = server const { server: serverConfig } = config @@ -265,6 +290,10 @@ export async function setupHMR(server: ViteDevServer): Promise { const { publicDir } = config debug?.('publicDir:', publicDir, 'publicFiles:', publicFiles) + // Processing of the latest watcher event for each path, up to module graph invalidation. + // Keyed by the path that the watcher emitted, before normalization. + const fileChanges = new Map>() + const onHMRUpdate = async ( type: 'create' | 'delete' | 'update', file: string, @@ -274,7 +303,8 @@ export async function setupHMR(server: ViteDevServer): Promise { } } - const onFileAddUnlink = async (file: string, isUnlink: boolean) => { + // Processes an add or unlink event up to module graph invalidation and returns the normalized file + const invalidateOnFileAddUnlink = async (file: string, isUnlink: boolean) => { file = normalizePath(file) reloadOnTsconfigChange(server, file) @@ -304,15 +334,20 @@ export async function setupHMR(server: ViteDevServer): Promise { } } if (isUnlink) { - // invalidate module graph cache on file change for (const environment of Object.values(server.environments)) { + // onFileDelete() keeps the cached transform result of the deleted file, which Vite + // clears only during the HMR update. Clear it before waitForFileChange() resolves, + // so that later reads do not get the deleted file's transform result. + environment.moduleGraph.onFileChange(file) + // invalidate module graph cache on file change environment.moduleGraph.onFileDelete(file) } } - await onHMRUpdate(isUnlink ? 'delete' : 'create', file) + return file } - const onFileChange = async (file: string) => { + // Processes a change event up to module graph invalidation and returns the normalized file + const invalidateOnFileChange = async (file: string) => { debug?.('watcher change:', file) file = normalizePath(file) @@ -327,21 +362,52 @@ export async function setupHMR(server: ViteDevServer): Promise { for (const environment of Object.values(environments)) { environment.moduleGraph.onFileChange(file) } + return file + } + + // Records the invalidation of a watcher event, then runs HMR after it + const trackFileChange = ( + watchedFile: string, + type: 'create' | 'delete' | 'update', + invalidation: Promise, + ) => { + const invalidated = invalidation.then(() => undefined) + fileChanges.set(watchedFile, invalidated) + const forget = () => { + // A later event for the same path may have replaced this one + if (fileChanges.get(watchedFile) === invalidated) { + fileChanges.delete(watchedFile) + } + } + invalidated.then(forget, forget) - await onHMRUpdate('update', file) + invalidation + .then((file) => onHMRUpdate(type, file)) + .catch((e) => server.config.logger.error(e)) } watcher.on('change', (file) => { - onFileChange(file).catch((e) => server.config.logger.error(e)) + trackFileChange(file, 'update', invalidateOnFileChange(file)) }) watcher.on('add', (file) => { - onFileAddUnlink(file, false).catch((e) => server.config.logger.error(e)) + trackFileChange(file, 'create', invalidateOnFileAddUnlink(file, false)) }) watcher.on('unlink', (file) => { - onFileAddUnlink(file, true).catch((e) => server.config.logger.error(e)) + trackFileChange(file, 'delete', invalidateOnFileAddUnlink(file, true)) }) + + return { + waitForFileChange(file) { + const invalidated = fileChanges.get(file) + if (!invalidated) { + return Promise.resolve() + } + fileChanges.delete(file) + return invalidated + }, + } } function setupVirtualFiles(files?: Record): void { diff --git a/packages/vite-dev-server/src/node/web-worker.test.ts b/packages/vite-dev-server/src/node/web-worker.test.ts index c9b068ff..c1bfaa56 100644 --- a/packages/vite-dev-server/src/node/web-worker.test.ts +++ b/packages/vite-dev-server/src/node/web-worker.test.ts @@ -11,7 +11,9 @@ const transformerMocks = vi.hoisted(() => ({ createDevHtmlTransformFn: vi.fn<() => () => void>(() => vi.fn<() => void>()), fs: {}, isServerAccessDeniedForTransform: vi.fn<() => boolean>(() => false), - setupHMR: vi.fn<() => Promise>(async () => undefined), + setupHMR: vi.fn<() => Promise<{ waitForFileChange: (file: string) => Promise }>>( + async () => ({ waitForFileChange: async () => undefined }), + ), setupWorker: vi.fn<() => Promise>(), })) @@ -276,3 +278,28 @@ describe('Web Worker transform requests', () => { expect(transformerMocks.isServerAccessDeniedForTransform).not.toHaveBeenCalled() }) }) + +describe('Web Worker file changes', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + afterEach(() => { + vi.restoreAllMocks() + }) + + test('waits for file changes with the tracker from setupHMR', async () => { + const waitForFileChange = vi.fn<(file: string) => Promise>(async () => undefined) + transformerMocks.setupHMR.mockResolvedValueOnce({ waitForFileChange }) + transformerMocks.setupWorker.mockResolvedValue(createSetupResult()) + const workerScope = createWorkerScope() + const server = createServer(workerScope) + const listening = server.listen(0) + + await dispatchSetup(workerScope) + const readyServer = await listening + await readyServer.waitForFileChange('/src/main.ts') + + expect(waitForFileChange).toHaveBeenCalledExactlyOnceWith('/src/main.ts') + }) +}) diff --git a/packages/vite-dev-server/src/node/web-worker.ts b/packages/vite-dev-server/src/node/web-worker.ts index 457b7518..03da77d3 100644 --- a/packages/vite-dev-server/src/node/web-worker.ts +++ b/packages/vite-dev-server/src/node/web-worker.ts @@ -28,7 +28,7 @@ import { V_WW_READY, V_WW_SETUP_ACK, V_WW_SETUP_ERROR, V_SW_CONNECT_PORT_ACK } f import type { ConnectServiceWorkerPortMessage, SetupWorkerMessage, WorkerReadyMessage } from '../shared/messages' import type { ViteDevServer } from './server/index' import type { Plugin } from './plugin' -import type { ViteDevServerForWorker } from './transformer' +import type { FileChangeTracker, ViteDevServerForWorker } from './transformer' const debug = createDebugger('vrowzer:web-worker') @@ -122,6 +122,7 @@ export function createServer( // State: set after V_WW_SETUP completes let server: ViteDevServerForWorker | null = null let ws: import('./server/ws').MessageChannelServer | null = null + let fileChanges: FileChangeTracker | null = null // Register onmessage immediately (lightweight, no WASM) workerScope.onmessage = async (event: MessageEvent) => { @@ -182,10 +183,12 @@ export function createServer( debug?.('transformIndexHtml:', url) return devHtmlTransformFn(server as unknown as ViteDevServer, url, html, originalUrl) }, + // Nothing is being processed before setupHMR() connects the watcher + waitForFileChange: (file) => fileChanges?.waitForFileChange(file) ?? Promise.resolve(), } // Setup HMR after server is ready - await transformer.setupHMR(server as unknown as ViteDevServer) + fileChanges = await transformer.setupHMR(server as unknown as ViteDevServer) // Call configureServer hooks on user plugins. // Plugins like @vitejs/plugin-vue store the server reference in configureServer diff --git a/packages/vite-plugin/src/ide/IdeApp.vue b/packages/vite-plugin/src/ide/IdeApp.vue index 94510ea0..84862211 100644 --- a/packages/vite-plugin/src/ide/IdeApp.vue +++ b/packages/vite-plugin/src/ide/IdeApp.vue @@ -51,7 +51,9 @@ function connectRpc() { editorPanel.value.editableFiles.set(path, content) } // Update vrowzer preview - vrowzer.updateFile(path, content) + vrowzer.updateFile(path, content).catch((err: Error) => { + console.error('[Vrowzer IDE] updateFile failed:', err) + }) } }, { @@ -122,7 +124,9 @@ onUnmounted(() => { function handleFileChange({ path, content }: { path: string; content: string }) { // 1. Update preview via HMR (immediate) - vrowzer.updateFile(path, content) + vrowzer.updateFile(path, content).catch((err: Error) => { + console.error('[Vrowzer IDE] updateFile failed:', err) + }) // 2. Write back to local FS via birpc (async, no await) if (rpc) { diff --git a/packages/vrowzer/README.md b/packages/vrowzer/README.md index 21d25bfa..e0c90ab3 100644 --- a/packages/vrowzer/README.md +++ b/packages/vrowzer/README.md @@ -63,8 +63,8 @@ if (ready) { vrowzer.mount(document.getElementById('preview-container'), { id: 'preview' }) } -// Update files (triggers HMR) -vrowzer.updateFile( +// Update files (triggers HMR). The promise resolves when later preview requests see the change. +await vrowzer.updateFile( '/main.js', ` document.getElementById('app').innerHTML = '

Updated!

' @@ -143,6 +143,12 @@ const vrowzer = Vrowzer({ webWorkerSetupTimeout: 120000 }) Set this option to `0` for an immediate timeout. It does not apply to Service Worker readiness and does not need a corresponding Vite plugin option. +`fileSyncTimeout` controls how long `addFile()`, `updateFile()` and `deleteFile()` wait for the Web Worker and the Service Worker to apply a change. It defaults to 10000 milliseconds. The Web Worker applies a change after the plugins' `watchChange` hooks finish, so slow plugins make it take longer. The default is shorter than the 30 seconds that the Service Worker waits for the Web Worker to transform a request, so in such environments an operation can reject although the change is applied later. Increase the timeout there: + +```ts +const vrowzer = Vrowzer({ fileSyncTimeout: 30000 }) +``` + **Options:** | Option | Type | Default | Description | @@ -152,6 +158,7 @@ Set this option to `0` for an immediate timeout. It does not apply to Service Wo | `serviceWorkerScope` | `string` | Plugin value or `'/'` | SW registration scope; must match the plugin value | | `serviceWorkerReadyTimeout` | `number` | `60000` | Milliseconds to wait for the Service Worker page controller | | `webWorkerSetupTimeout` | `number` | `90000` | Milliseconds from Web Worker creation through setup completion | +| `fileSyncTimeout` | `number` | `10000` | Milliseconds to wait for the Workers to apply a file change | ### Instance Methods @@ -220,21 +227,39 @@ desktop.unmount() vrowzer.unmount() ``` -#### `updateFile(path, content): void` +#### `updateFile(path, content): Promise` Updates a file in the virtual filesystem. Triggers HMR if the preview supports it. ```ts -vrowzer.updateFile('/main.js', 'console.log("updated")') +await vrowzer.updateFile('/main.js', 'console.log("updated")') ``` -#### `addFile(path, content): void` +#### `addFile(path, content): Promise` Adds a new file to the virtual filesystem. -#### `deleteFile(path): void` +#### `deleteFile(path): Promise` + +Deletes a file from the virtual filesystem. Deleting a file that does not exist resolves as well. + +#### Waiting for file changes + +The promise of `addFile()`, `updateFile()` and `deleteFile()` resolves when later preview requests see the change: the Web Worker and the Service Worker have written the file to their virtual filesystems, and the Web Worker has invalidated the modules that depend on it. It does not wait for HMR updates of mounted previews. To load the new contents in a fresh document, wait for the promise before reloading: -Deletes a file from the virtual filesystem. +```ts +await vrowzer.updateFile('/main.js', source) +vrowzer.reloadPreview() +``` + +- Operations awaited one after another are applied in that order. Operations started together may resolve in any order, and several files are not applied as one transaction. +- `ArrayBuffer` content is copied for each Worker, so the caller's buffer stays usable. +- The promise rejects without sending the change before `ready()` resolves to `true`, after it fails, and after `dispose()`. It also rejects when a Worker fails to apply the change (for example, a plugin's `watchChange` hook throws), when the Web Worker reports an error, when the Workers do not reply within `fileSyncTimeout`, or when the instance is disposed first. The error message names the operation, the path and the Worker. +- After a rejection, the change may be partly applied. Write the file again, or delete it, to resynchronize. +- Public files (under `/public/`) are currently not served by the Service Worker, whether they are passed to `ready()` or added later: the Service Worker builds its list of public files when it starts, before it receives any file. The promises above therefore do not make them visible. + +> [!NOTE] +> Up to vrowzer 0.4.x, these methods returned `void` without waiting for the Workers, and calls made before `ready()` completed were dropped or reached only the Web Worker. #### `dispose(): Promise` @@ -262,7 +287,7 @@ With `await using`, the instance is disposed at the end of the scope. ``` - If `ready()` is still in progress, it is aborted and resolves to `false`. -- After disposal, `ready()` rejects, `mount()`, `addFile()`, `updateFile()` and `deleteFile()` throw, and `unmount()` and `reloadPreview()` do nothing. Create a new instance to start again. +- File operations still waiting for the Workers reject. After disposal, `ready()`, `addFile()`, `updateFile()` and `deleteFile()` reject, `mount()` throws, and `unmount()` and `reloadPreview()` do nothing. Create a new instance to start again. - Calling `dispose()` again returns the same promise. If some resources cannot be released, the remaining ones are still released, and the promise rejects with an `AggregateError`. > [!NOTE] diff --git a/packages/vrowzer/docs/default/interfaces/Vrowzer.md b/packages/vrowzer/docs/default/interfaces/Vrowzer.md index 25bd4059..7d003333 100644 --- a/packages/vrowzer/docs/default/interfaces/Vrowzer.md +++ b/packages/vrowzer/docs/default/interfaces/Vrowzer.md @@ -17,32 +17,48 @@ export interface Vrowzer extends Emittable ### addFile() ```ts -addFile(filePath: string, content: string | ArrayBuffer): void; +addFile(filePath: string, content: string | ArrayBuffer): Promise; ``` Adds a new file to the preview environment with the specified content. +The promise resolves when later preview requests see the change: the Web Worker and the +Service Worker have written the file to their virtual filesystems, and the Web Worker has +invalidated the modules that depend on it. HMR updates of mounted previews are not awaited. + +It rejects without sending the change before [Vrowzer.ready](#method-ready) resolves to `true`, after it +fails, and after [Vrowzer.dispose](#method-dispose). It also rejects when a Worker fails to apply the change, +when the Web Worker reports an error, when the Workers do not reply within +[VrowzerOptions.fileSyncTimeout](/packages/vrowzer/docs/default/interfaces/VrowzerOptions.md#property-filesynctimeout), or when the instance is disposed first. The change may +be partly applied then; write the file again to resynchronize. + +Public files (under `/public/`) are currently not served by the Service Worker, so they do not +become visible to preview requests. + #### Parameters | Name | Type | Description | | --- | --- | --- | | `filePath` | `string` | The path of the file to be added. | -| `content` | `string \| ArrayBuffer` | The content of the file, which can be a string or an ArrayBuffer. | +| `content` | `string \| ArrayBuffer` | The content of the file, which can be a string or an ArrayBuffer. An ArrayBuffer is copied for the Workers and stays usable. | #### Returns -`void` +`Promise` *** ### deleteFile() ```ts -deleteFile(filePath: string): void; +deleteFile(filePath: string): Promise; ``` Deletes a specific file from the preview environment. +The promise resolves when later preview requests no longer see the file, and rejects as with +[Vrowzer.addFile](#method-addfile). Deleting a file that does not exist resolves as well. + #### Parameters | Name | Type | Description | @@ -51,7 +67,7 @@ Deletes a specific file from the preview environment. #### Returns -`void` +`Promise` *** @@ -68,8 +84,9 @@ is unmounted, the Web Worker is terminated, Service Worker controller events are forwarded, and all event handlers are removed right away. The Service Worker registration and its virtual filesystem are kept for other clients. -After disposal, `ready()` rejects, `mount()` and the file methods throw, and `unmount()` and -`reloadPreview()` do nothing. Create a new instance to start again. +File operations still waiting for the Workers reject. After disposal, `ready()` and the file +methods reject, `mount()` throws, and `unmount()` and `reloadPreview()` do nothing. Create a +new instance to start again. #### Returns @@ -207,18 +224,20 @@ Use [Vrowzer.dispose](#method-dispose) to release the whole instance. ### updateFile() ```ts -updateFile(filePath: string, content: string | ArrayBuffer): void; +updateFile(filePath: string, content: string | ArrayBuffer): Promise; ``` Updates the content of a specific file in the preview environment. +The promise resolves and rejects as with [Vrowzer.addFile](#method-addfile). + #### Parameters | Name | Type | Description | | --- | --- | --- | | `filePath` | `string` | The path of the file to be updated. | -| `content` | `string \| ArrayBuffer` | The new content for the file, which can be a string or an ArrayBuffer. | +| `content` | `string \| ArrayBuffer` | The new content for the file, which can be a string or an ArrayBuffer. An ArrayBuffer is copied for the Workers and stays usable. | #### Returns -`void` +`Promise` diff --git a/packages/vrowzer/docs/default/interfaces/VrowzerOptions.md b/packages/vrowzer/docs/default/interfaces/VrowzerOptions.md index 662b2955..79f56a32 100644 --- a/packages/vrowzer/docs/default/interfaces/VrowzerOptions.md +++ b/packages/vrowzer/docs/default/interfaces/VrowzerOptions.md @@ -13,6 +13,7 @@ export interface VrowzerOptions | Name | Type | Description | | --- | --- | --- | | `basePath` _(optional)_ | `string` | Preview URL pathname. When `@vrowzer/vite-plugin` is used, its `basePath` is injected and this option can be omitted. If both are provided, their canonical values must match. Without the plugin, this option defaults to `'/__preview__/'`. | +| `fileSyncTimeout` _(optional)_ | `number` | Timeout in milliseconds for the Web Worker and the Service Worker to apply a change made with [Vrowzer.addFile](/packages/vrowzer/docs/default/interfaces/Vrowzer.md#method-addfile), [Vrowzer.updateFile](/packages/vrowzer/docs/default/interfaces/Vrowzer.md#method-updatefile) or [Vrowzer.deleteFile](/packages/vrowzer/docs/default/interfaces/Vrowzer.md#method-deletefile). The Web Worker applies a change after the plugins' `watchChange` hooks finish. This timeout is shorter than the 30 seconds that the Service Worker waits for the Web Worker to transform a request, so with slow plugins or heavy transforms an operation can reject although the change is applied later. Increase it in such environments. Writing the file again resynchronizes it. **Default:** `10000` | | `serviceWorkerReadyTimeout` _(optional)_ | `number` | Timeout in milliseconds for the Service Worker to become the page controller. This timeout does not apply to Service Worker listen readiness or Web Worker setup. **Default:** `60000` | | `serviceWorkerScope` _(optional)_ | `string` | Service Worker registration scope, independent of `basePath`. When `@vrowzer/vite-plugin` is used, its `serviceWorkerScope` is injected and this option can be omitted. If both are provided, their values must match. Without the plugin, this option defaults to `'/'`. | | `serviceWorkerVersion` _(optional)_ | `string` | Service Worker version for cache management. When `@vrowzer/vite-plugin` is used, its `serviceWorkerVersion` is injected and this option can be omitted. If both are provided, their values must match. Without the plugin, this option defaults to `'vrowzer-v1'`. | diff --git a/packages/vrowzer/docs/index.md b/packages/vrowzer/docs/index.md index a4c46ff8..8faec195 100644 --- a/packages/vrowzer/docs/index.md +++ b/packages/vrowzer/docs/index.md @@ -24,8 +24,8 @@ if (ready) { vrowzer.mount(document.getElementById('preview-container'), { id: 'preview' }) } -// Update files (triggers HMR) -vrowzer.updateFile( +// Update files (triggers HMR). The promise resolves when later preview requests see the change. +await vrowzer.updateFile( '/main.js', ` document.getElementById('app').innerHTML = '

Updated!

' diff --git a/packages/vrowzer/integration/dispose.integration-test.ts b/packages/vrowzer/integration/dispose.integration-test.ts index e7f1853c..a7292189 100644 --- a/packages/vrowzer/integration/dispose.integration-test.ts +++ b/packages/vrowzer/integration/dispose.integration-test.ts @@ -97,7 +97,7 @@ describe('Vrowzer dispose', () => { errors.mount = (error as Error).message } try { - vrowzer.addFile('/added.js', '') + await vrowzer.addFile('/added.js', '') } catch (error) { errors.addFile = (error as Error).message } diff --git a/packages/vrowzer/integration/playground/vite.config.ts b/packages/vrowzer/integration/playground/vite.config.ts index d456896d..4cbbc157 100644 --- a/packages/vrowzer/integration/playground/vite.config.ts +++ b/packages/vrowzer/integration/playground/vite.config.ts @@ -210,6 +210,66 @@ function hmrClientTrackingWebWorkerPlugin(): Plugin { } } +/** + * Lets the file synchronization tests delay and fail how the Web Worker applies file changes. + * + * - `watchChange` of an update under `/file-sync/held/` waits until `/file-sync/release` is written. + * - `watchChange` under `/file-sync/fail-once/` throws the first time for each path. + * - `hotUpdate` of a deletion under `/file-sync/held-hmr/` waits until `/file-sync/release` is written. + */ +function fileSyncWebWorkerPlugin(): Plugin { + const failedPaths = new Set() + let isWebWorker = false + let hold: { promise: Promise; release: () => void } | null = null + + function waitForRelease(): Promise { + if (!hold) { + let release!: () => void + const promise = new Promise(resolve => { + release = resolve + }) + hold = { promise, release } + } + return hold.promise + } + + return { + name: 'vrowzer-test:file-sync-web-worker', + apply: 'serve', + configureServer(server) { + const middlewares = (server as { middlewares?: unknown }).middlewares + if (server.config.root !== '/' || middlewares) { + return + } + + isWebWorker = true + }, + async watchChange(id, { event }) { + if (!isWebWorker) { + return + } + if (id === '/file-sync/release') { + hold?.release() + hold = null + return + } + if (id.startsWith('/file-sync/held/') && event === 'update') { + await waitForRelease() + return + } + if (id.startsWith('/file-sync/fail-once/') && !failedPaths.has(id)) { + failedPaths.add(id) + throw new Error(`vrowzer-test: watchChange failed for ${id}`) + } + }, + async hotUpdate({ type, file }) { + if (isWebWorker && type === 'delete' && file.startsWith('/file-sync/held-hmr/')) { + await waitForRelease() + } + } + } +} + export default defineConfig({ server: { origin: 'https://assets.vrowzer.test' @@ -219,6 +279,7 @@ export default defineConfig({ fsHtmlProxyWebWorkerPlugin(), postcssOnceExitWebWorkerPlugin(), hmrClientTrackingWebWorkerPlugin(), + fileSyncWebWorkerPlugin(), Vrowzer({ auto: false, basePath: '/__preview__/', diff --git a/packages/vrowzer/integration/vrowzer.integration-test.ts b/packages/vrowzer/integration/vrowzer.integration-test.ts index 3546faf1..73c9b233 100644 --- a/packages/vrowzer/integration/vrowzer.integration-test.ts +++ b/packages/vrowzer/integration/vrowzer.integration-test.ts @@ -125,11 +125,11 @@ interface SourceMapPayload { } async function addPreviewFiles(files: Record): Promise { - await page.evaluate(filesToAdd => { + await page.evaluate(async filesToAdd => { const vrowzer = (window as any).__vrowzer__ - for (const [filePath, content] of Object.entries(filesToAdd)) { - vrowzer.addFile(filePath, content) - } + await Promise.all( + Object.entries(filesToAdd).map(([filePath, content]) => vrowzer.addFile(filePath, content)) + ) }, files) } @@ -224,6 +224,42 @@ async function fetchFromPreview(requestPath: string): Promise { }, requestPath) } +/** + * Reads a preview file through the Service Worker from the host page. Unlike the preview iframe, + * the host page is not reloaded by HMR, so a read right after a file change is not interrupted. + */ +async function fetchFromServiceWorker(requestPath: string): Promise { + return page.evaluate(async path => { + const response = await fetch(`/__preview__${path}`) + return { status: response.status, body: await response.text() } + }, requestPath) +} + +async function fetchBytesFromServiceWorker( + requestPath: string +): Promise<{ status: number; bytes: number[] }> { + return page.evaluate(async path => { + const response = await fetch(`/__preview__${path}`) + return { status: response.status, bytes: [...new Uint8Array(await response.arrayBuffer())] } + }, requestPath) +} + +/** + * Makes a test module accept its own updates, so that changing it does not reload the preview. + */ +function selfAccepting(code: string): string { + return `${code}\nif (import.meta.hot) { import.meta.hot.accept() }\n` +} + +/** + * Releases what the playground's file sync test plugin holds in the Web Worker. + */ +async function releaseHeldFileChanges(): Promise { + await page.evaluate(() => { + return (window as any).__vrowzer__.updateFile('/file-sync/release', '') + }) +} + async function waitForPreviewResponse( requestPath: string, expectedStatus: number @@ -522,7 +558,7 @@ describe('Vrowzer E2E', () => { // Update main.js via vrowzer API await page.evaluate(() => { const vrowzer = (window as any).__vrowzer__ - vrowzer.updateFile( + return vrowzer.updateFile( '/main.js', ` document.getElementById('app').innerHTML = '

Updated!

1 + 1 = 2

' @@ -551,6 +587,131 @@ if (import.meta.hot) { ) .toEqual({ updated: true, result: true }) }, 30000) + + test('stays pending while the Web Worker applies a change, and later reads see it', async () => { + const path = '/file-sync/held/module.js' + await addPreviewFiles({ [path]: selfAccepting(`export const value = 'held-v1'`) }) + // Cache the transform result of the first version + expect((await fetchFromServiceWorker(path)).body).toContain('held-v1') + + try { + await page.evaluate( + ({ path, content }) => { + const state = { settled: false } + ;(window as any).__fileSyncHeld__ = state + ;(window as any).__fileSyncHeldUpdate__ = (window as any).__vrowzer__ + .updateFile(path, content) + .finally(() => { + state.settled = true + }) + }, + { path, content: selfAccepting(`export const value = 'held-v2'`) } + ) + + await page.waitForTimeout(500) + expect(await page.evaluate(() => (window as any).__fileSyncHeld__.settled)).toBe(false) + // The Web Worker has not invalidated the module yet, so reads still get the cached result + expect((await fetchFromServiceWorker(path)).body).toContain('held-v1') + + // Releasing writes another path, so it resolves without waiting for the held change + await releaseHeldFileChanges() + await page.evaluate(() => (window as any).__fileSyncHeldUpdate__) + + const response = await fetchFromServiceWorker(path) + expect(response.body).toContain('held-v2') + expect(response.body).not.toContain('held-v1') + } finally { + await releaseHeldFileChanges() + } + }, 30000) + + test('resolves each kind of operation, and later reads follow them', async () => { + const path = '/file-sync/kinds/module.js' + const run = (operation: string, args: unknown[]) => + page.evaluate(({ operation, args }) => (window as any).__vrowzer__[operation](...args), { + operation, + args + }) + + await run('addFile', [path, selfAccepting(`export const value = 'kinds-v1'`)]) + expect((await fetchFromServiceWorker(path)).body).toContain('kinds-v1') + + await run('updateFile', [path, selfAccepting(`export const value = 'kinds-v2'`)]) + expect((await fetchFromServiceWorker(path)).body).toContain('kinds-v2') + + // Writing the same content, and adding a file that nothing imports, settle as well + await run('updateFile', [path, selfAccepting(`export const value = 'kinds-v2'`)]) + await run('addFile', ['/file-sync/kinds/unused.js', 'export {}']) + + await run('deleteFile', [path]) + expect((await fetchFromServiceWorker(path)).body).not.toContain('kinds-v2') + // Deleting a file that no longer exists settles too + await run('deleteFile', [path]) + + await run('addFile', [path, selfAccepting(`export const value = 'kinds-v3'`)]) + expect((await fetchFromServiceWorker(path)).body).toContain('kinds-v3') + }, 30000) + + test('makes a deletion visible before its HMR update finishes', async () => { + const path = '/file-sync/held-hmr/module.js' + await addPreviewFiles({ [path]: selfAccepting(`export const value = 'held-hmr-v1'`) }) + // Cache the transform result of the file + expect((await fetchFromServiceWorker(path)).body).toContain('held-hmr-v1') + + try { + // The plugin holds the HMR update of this deletion, which does not delay the promise + await page.evaluate(path => (window as any).__vrowzer__.deleteFile(path), path) + expect((await fetchFromServiceWorker(path)).body).not.toContain('held-hmr-v1') + } finally { + await releaseHeldFileChanges() + } + }, 30000) + + test('copies binary content for both Workers and keeps the caller buffer usable', async () => { + const path = '/file-sync/binary/pixel.png' + const bytes = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0xff] + + const caller = await page.evaluate( + async ({ path, bytes }) => { + const buffer = new Uint8Array(bytes).buffer + await (window as any).__vrowzer__.addFile(path, buffer) + return { byteLength: buffer.byteLength, bytes: [...new Uint8Array(buffer)] } + }, + { path, bytes } + ) + expect(caller).toEqual({ byteLength: bytes.length, bytes }) + + // The Service Worker serves the file from its virtual filesystem + expect(await fetchBytesFromServiceWorker(path)).toEqual({ status: 200, bytes }) + // The Web Worker inlines the file from its own virtual filesystem + const inlined = await fetchFromServiceWorker(`${path}?import&inline`) + const base64 = inlined.body.match(/data:image\/png;base64,([A-Za-z0-9+/=]+)/)?.[1] + expect(base64).toBeDefined() + expect([...Buffer.from(base64!, 'base64')]).toEqual(bytes) + }, 30000) + + test('rejects when the Web Worker fails to apply a change, and writing again converges', async () => { + const path = '/file-sync/fail-once/module.js' + + const message = await page.evaluate(async path => { + try { + await (window as any).__vrowzer__.addFile(path, `export const value = 'fail-v1'`) + return null + } catch (error) { + return (error as Error).message + } + }, path) + expect(message).toContain(`addFile("${path}") failed in the Web Worker`) + expect(message).toContain(`vrowzer-test: watchChange failed for ${path}`) + + await page.evaluate( + path => (window as any).__vrowzer__.updateFile(path, `export const value = 'fail-v2'`), + path + ) + const response = await fetchFromServiceWorker(path) + expect(response.body).toContain('fail-v2') + expect(response.body).not.toContain('fail-v1') + }, 30000) }) describe('preview sessions', () => { @@ -558,7 +719,7 @@ if (import.meta.hot) { const dangerousMarker = '\u2028\u2029' await page.evaluate(source => { - ;(window as any).__vrowzer__.updateFile('/main.js', source) + return (window as any).__vrowzer__.updateFile('/main.js', source) }, createMultiSessionSource('multi-1')) await expect @@ -671,7 +832,7 @@ if (import.meta.hot) { expect(new Set(clientIdsBeforeReload).size).toBe(3) await page.evaluate(source => { - ;(window as any).__vrowzer__.updateFile('/main.js', source) + return (window as any).__vrowzer__.updateFile('/main.js', source) }, createMultiSessionSource('multi-2')) await page.waitForFunction( @@ -758,7 +919,7 @@ if (import.meta.hot) { expect(await waitForHmrClientCount(2)).toHaveLength(2) await page.evaluate(source => { - ;(window as any).__vrowzer__.updateFile('/main.js', source) + return (window as any).__vrowzer__.updateFile('/main.js', source) }, createMultiSessionSource('multi-3')) await page.waitForFunction( @@ -943,7 +1104,7 @@ if (import.meta.hot) { afterAll(async () => { await page.evaluate( - ({ ids, paths }) => { + async ({ ids, paths }) => { const vrowzer = (window as any).__vrowzer__ for (const id of ids) { vrowzer.unmount(id) @@ -952,9 +1113,7 @@ if (import.meta.hot) { ;(window as any).__stopPreviewLoadErrors?.() delete (window as any).__previewLoadErrors delete (window as any).__stopPreviewLoadErrors - for (const path of paths) { - vrowzer.deleteFile(path) - } + await Promise.all(paths.map((path: string) => vrowzer.deleteFile(path))) }, { ids: sessionIds, paths: Object.keys(fixtureFiles) } ) @@ -1188,7 +1347,7 @@ if (import.meta.hot) { expect(response.body).toContain(filePath) } finally { await page.evaluate(path => { - ;(window as any).__vrowzer__.deleteFile(path) + return (window as any).__vrowzer__.deleteFile(path) }, filePath) } }) diff --git a/packages/vrowzer/src/index-dispose.test.ts b/packages/vrowzer/src/index-dispose.test.ts index da7ff4bb..77a7980b 100644 --- a/packages/vrowzer/src/index-dispose.test.ts +++ b/packages/vrowzer/src/index-dispose.test.ts @@ -23,6 +23,7 @@ vi.mock('./controller.ts', () => ({ initServiceWorker: (options: { signal?: AbortSignal }) => runtime.initServiceWorker(options) })) vi.mock('@vrowzer/fs/watcher', () => ({ + V_FS_ACK: 'V_FS_ACK', createFileSystemPublisher: () => runtime.publisher })) vi.mock('@vrowzer/vite-dev-server/dist/client/client.mjs?raw', () => ({ default: '' })) @@ -80,8 +81,9 @@ class FakeController { class FakePublisher { readonly targets = new Set() - readonly writeFile = vi.fn<(path: string, content: string | ArrayBuffer) => void>() - readonly unlink = vi.fn<(path: string) => void>() + readonly writeFile = + vi.fn<(path: string, content: string | ArrayBuffer, options?: { id?: string }) => void>() + readonly unlink = vi.fn<(path: string, options?: { id?: string }) => void>() addTarget(target: unknown): void { this.targets.add(target) @@ -254,13 +256,17 @@ describe('Vrowzer dispose', () => { expect(() => vrowzer.mount(createContainer() as unknown as HTMLElement, { id: 'x' })).toThrow( 'mount() cannot be called after dispose()' ) - expect(() => vrowzer.addFile('/a.js', '')).toThrow('addFile() cannot be called after dispose()') - expect(() => vrowzer.updateFile('/a.js', '')).toThrow( + await expect(vrowzer.addFile('/a.js', '')).rejects.toThrow( + 'addFile() cannot be called after dispose()' + ) + await expect(vrowzer.updateFile('/a.js', '')).rejects.toThrow( 'updateFile() cannot be called after dispose()' ) - expect(() => vrowzer.deleteFile('/a.js')).toThrow( + await expect(vrowzer.deleteFile('/a.js')).rejects.toThrow( 'deleteFile() cannot be called after dispose()' ) + expect(publisher.writeFile).not.toHaveBeenCalled() + expect(publisher.unlink).not.toHaveBeenCalled() expect(vrowzer.sessions()).toEqual([]) expect(vrowzer.getSession('x')).toBeUndefined() expect(() => { @@ -361,6 +367,38 @@ describe('Vrowzer dispose', () => { expect(vi.getTimerCount()).toBe(0) }) + test('rejects the file operations that wait for the Workers', async () => { + const vrowzer = Vrowzer() + const worker = await readyFully(vrowzer) + expect(controller.container.listeners.size).toBe(1) + + const updating = vrowzer.updateFile('/a.js', 'updated') + const deleting = vrowzer.deleteFile('/b.js') + const rejections = Promise.allSettled([updating, deleting]) + expect(vi.getTimerCount()).toBe(2) + + await vrowzer.dispose() + + expect(await rejections).toEqual([ + { + status: 'rejected', + reason: expect.objectContaining({ + message: '[Vrowzer] updateFile("/a.js") was cancelled by dispose()' + }) + }, + { + status: 'rejected', + reason: expect.objectContaining({ + message: '[Vrowzer] deleteFile("/b.js") was cancelled by dispose()' + }) + } + ]) + expect(controller.container.listeners.size).toBe(0) + expect(worker.onmessage).toBeNull() + expect(worker.onerror).toBeNull() + expect(vi.getTimerCount()).toBe(0) + }) + test('releases what a failed ready() created', async () => { const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) const vrowzer = Vrowzer() diff --git a/packages/vrowzer/src/index-file-sync.test.ts b/packages/vrowzer/src/index-file-sync.test.ts index 75ff6515..ce67f9ae 100644 --- a/packages/vrowzer/src/index-file-sync.test.ts +++ b/packages/vrowzer/src/index-file-sync.test.ts @@ -9,78 +9,162 @@ import { V_SW_CONNECT_PORT_ACK } from '@vrowzer/vite-dev-server/messages' +interface TestMessage { + type: string + id?: string + path?: string + content?: unknown +} + +interface AckError { + name: string + message: string +} + const controllerMocks = vi.hoisted(() => ({ container: new EventTarget(), - on: vi.fn<(...args: unknown[]) => void>(), - postMessage: vi.fn<(message: { type: string }) => void>() + on: vi.fn<(...args: unknown[]) => () => void>(() => () => {}), + postMessage: vi.fn<(message: { type: string }, transfer?: Transferable[]) => void>(), + initServiceWorker: vi.fn<() => Promise>(async () => undefined) })) vi.mock('./controller.ts', () => ({ getController: () => controllerMocks, getServiceWorker: () => ({ postMessage: controllerMocks.postMessage }), - initServiceWorker: async () => undefined + initServiceWorker: controllerMocks.initServiceWorker })) vi.mock('@vrowzer/vite-dev-server/dist/client/client.mjs?raw', () => ({ default: 'client code' })) vi.mock('@vrowzer/vite-dev-server/dist/client/env.mjs?raw', () => ({ default: 'env code' })) -import { Vrowzer } from './index.ts' +import { Vrowzer, type VrowzerOptions } from './index.ts' let workers: TestWorker[] +// Whether the fake Workers acknowledge file sync messages by themselves +let autoAck: boolean class TestWorker { onmessage: ((event: MessageEvent) => void) | null = null onerror: ((event: ErrorEvent) => void) | null = null - readonly messages: unknown[] = [] + readonly messages: TestMessage[] = [] + readonly transfers: (Transferable[] | undefined)[] = [] constructor() { workers.push(this) - this.reply(V_WW_READY) + this.reply({ type: V_WW_READY }) } - postMessage(message: { type: string }): void { + postMessage(message: TestMessage, transfer?: Transferable[]): void { this.messages.push(message) + this.transfers.push(transfer) if (message.type === V_WW_SETUP) { - this.reply(V_WW_SETUP_ACK) + this.reply({ type: V_WW_SETUP_ACK }) } else if (message.type === V_SW_CONNECT_PORT) { - this.reply(V_SW_CONNECT_PORT_ACK) + this.reply({ type: V_SW_CONNECT_PORT_ACK }) + } else if (autoAck && message.id !== undefined) { + this.reply({ type: 'V_FS_ACK', id: message.id }) } } - private reply(type: string): void { - queueMicrotask(() => this.onmessage?.({ data: { type } } as MessageEvent)) + ack(id: string, error?: AckError): void { + this.onmessage?.({ + data: { type: 'V_FS_ACK', id, ...(error ? { error } : {}) } + } as MessageEvent) + } + + private reply(data: object): void { + queueMicrotask(() => this.onmessage?.({ data } as MessageEvent)) } terminate(): void {} } -describe('Vrowzer file synchronization', () => { - beforeEach(() => { - vi.clearAllMocks() - workers = [] - vi.stubGlobal('Worker', TestWorker) - vi.stubGlobal( - 'MessageChannel', - class { - port1 = {} - port2 = {} - } - ) - controllerMocks.postMessage.mockImplementation(message => { - if (message.type === V_WW_CONNECT_PORT) { - queueMicrotask(() => { - controllerMocks.container.dispatchEvent( - new MessageEvent('message', { data: { type: V_WW_CONNECT_PORT_ACK } }) - ) - }) - } - }) - }) +function ackFromServiceWorker(id: string, error?: AckError): void { + controllerMocks.container.dispatchEvent( + new MessageEvent('message', { data: { type: 'V_FS_ACK', id, ...(error ? { error } : {}) } }) + ) +} + +function isFileChange(message: TestMessage): boolean { + return message.type === 'V_FS_WRITE' || message.type === 'V_FS_UNLINK' +} + +function webWorkerFileMessages(worker: TestWorker): TestMessage[] { + return worker.messages.filter(isFileChange) +} + +function serviceWorkerFileMessages(): TestMessage[] { + return controllerMocks.postMessage.mock.calls + .map(([message]) => message as TestMessage) + .filter(isFileChange) +} + +function lastId(worker: TestWorker): string { + const id = webWorkerFileMessages(worker).at(-1)?.id + if (!id) { + throw new Error('No file sync message with an id was sent to the Web Worker') + } + return id +} + +async function isPending(promise: Promise): Promise { + let settled = false + promise.then( + () => { + settled = true + }, + () => { + settled = true + } + ) + // Replies settle operations synchronously, so a few microtasks are enough, even with fake timers + for (let index = 0; index < 10; index++) { + await Promise.resolve() + } + return !settled +} - afterEach(() => { - vi.restoreAllMocks() - vi.unstubAllGlobals() +async function readyInstance(options?: VrowzerOptions) { + const vrowzer = Vrowzer(options) + await expect(vrowzer.ready({ files: {} })).resolves.toBe(true) + return { vrowzer, worker: workers.at(-1)! } +} + +beforeEach(() => { + vi.clearAllMocks() + workers = [] + autoAck = false + controllerMocks.container = new EventTarget() + vi.stubGlobal('Worker', TestWorker) + vi.stubGlobal( + 'MessageChannel', + class { + port1 = {} + port2 = {} + } + ) + controllerMocks.postMessage.mockImplementation(message => { + if (message.type === V_WW_CONNECT_PORT) { + queueMicrotask(() => { + controllerMocks.container.dispatchEvent( + new MessageEvent('message', { data: { type: V_WW_CONNECT_PORT_ACK } }) + ) + }) + return + } + const id = (message as TestMessage).id + if (autoAck && id !== undefined) { + queueMicrotask(() => ackFromServiceWorker(id)) + } }) +}) +afterEach(() => { + vi.useRealTimers() + vi.restoreAllMocks() + vi.unstubAllGlobals() +}) + +describe('Vrowzer file synchronization', () => { test('initializes each Worker once without broadcasting initial files to the ready Web Worker', async () => { const files = { '/index.html': '', '/main.js': 'export const value = 1' } const initialFiles = { @@ -103,21 +187,267 @@ describe('Vrowzer file synchronization', () => { }) test('continues sending file additions, updates, and deletions to both Workers', async () => { - const vrowzer = Vrowzer() - await expect(vrowzer.ready({ files: {} })).resolves.toBe(true) - workers[0]!.messages.length = 0 + autoAck = true + const { vrowzer, worker } = await readyInstance() + worker.messages.length = 0 controllerMocks.postMessage.mockClear() - vrowzer.addFile('/main.js', 'initial') - vrowzer.updateFile('/main.js', 'updated') - vrowzer.deleteFile('/main.js') + await vrowzer.addFile('/main.js', 'initial') + await vrowzer.updateFile('/main.js', 'updated') + await vrowzer.deleteFile('/main.js') const expected = [ - { type: 'V_FS_WRITE', path: '/main.js', encoding: 'text', content: 'initial' }, - { type: 'V_FS_WRITE', path: '/main.js', encoding: 'text', content: 'updated' }, - { type: 'V_FS_UNLINK', path: '/main.js' } + { + type: 'V_FS_WRITE', + path: '/main.js', + encoding: 'text', + content: 'initial', + id: expect.any(String) + }, + { + type: 'V_FS_WRITE', + path: '/main.js', + encoding: 'text', + content: 'updated', + id: expect.any(String) + }, + { type: 'V_FS_UNLINK', path: '/main.js', id: expect.any(String) } ] - expect(workers[0]!.messages).toEqual(expected) + expect(worker.messages).toEqual(expected) expect(controllerMocks.postMessage.mock.calls.map(([message]) => message)).toEqual(expected) + // Each operation has its own id, shared by both Workers + expect(new Set(worker.messages.map(message => message.id)).size).toBe(3) + expect(serviceWorkerFileMessages().map(message => message.id)).toEqual( + worker.messages.map(message => message.id) + ) + }) +}) + +describe('Vrowzer file operation results', () => { + test.each(['Web Worker', 'Service Worker'] as const)( + 'resolves only after both Workers acknowledge, the %s first', + async first => { + const { vrowzer, worker } = await readyInstance() + + const updating = vrowzer.updateFile('/main.js', 'updated') + const id = lastId(worker) + expect(serviceWorkerFileMessages().at(-1)?.id).toBe(id) + + if (first === 'Web Worker') { + worker.ack(id) + } else { + ackFromServiceWorker(id) + } + expect(await isPending(updating)).toBe(true) + + if (first === 'Web Worker') { + ackFromServiceWorker(id) + } else { + worker.ack(id) + } + await expect(updating).resolves.toBeUndefined() + } + ) + + test.each(['Web Worker', 'Service Worker'] as const)( + 'rejects with the operation, the path and the cause when the %s fails to apply it', + async target => { + const { vrowzer, worker } = await readyInstance() + const error = { name: 'TypeError', message: 'watchChange failed' } + + const updating = vrowzer.updateFile('/main.js', 'updated') + const id = lastId(worker) + if (target === 'Web Worker') { + worker.ack(id, error) + } else { + ackFromServiceWorker(id, error) + } + + await expect(updating).rejects.toThrow( + `[Vrowzer] updateFile("/main.js") failed in the ${target}: watchChange failed` + ) + await expect(updating).rejects.toMatchObject({ cause: error }) + // A late reply from the other Worker does nothing + worker.ack(id) + ackFromServiceWorker(id) + } + ) + + test('rejects after fileSyncTimeout and names the Workers that did not reply', async () => { + vi.useFakeTimers() + const { vrowzer, worker } = await readyInstance({ fileSyncTimeout: 1000 }) + + const deleting = vrowzer.deleteFile('/main.js') + ackFromServiceWorker(lastId(worker)) + const failure = deleting.catch((error: unknown) => error) + await vi.advanceTimersByTimeAsync(1000) + + expect(await failure).toMatchObject({ + message: '[Vrowzer] deleteFile("/main.js") timed out after 1000ms waiting for the Web Worker' + }) + expect(vi.getTimerCount()).toBe(0) + }) + + test('waits 10 seconds for the replies by default', async () => { + vi.useFakeTimers() + const { vrowzer } = await readyInstance() + + const adding = vrowzer.addFile('/main.js', 'added') + let settled = false + adding.then( + () => { + settled = true + }, + () => { + settled = true + } + ) + await vi.advanceTimersByTimeAsync(9_999) + expect(settled).toBe(false) + await vi.advanceTimersByTimeAsync(1) + + await expect(adding).rejects.toThrow( + '[Vrowzer] addFile("/main.js") timed out after 10000ms waiting for the Web Worker and the Service Worker' + ) + }) + + test('rejects the operations waiting for the Web Worker when it reports an error', async () => { + const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + const { vrowzer, worker } = await readyInstance() + + const updating = vrowzer.updateFile('/main.js', 'updated') + const updateId = lastId(worker) + const adding = vrowzer.addFile('/other.js', 'other') + const addId = lastId(worker) + // This one only waits for the Service Worker + worker.ack(addId) + + worker.onerror?.({ message: 'Uncaught ReferenceError: x is not defined' } as ErrorEvent) + + await expect(updating).rejects.toThrow( + '[Vrowzer] updateFile("/main.js") failed because the Web Worker reported an error: Uncaught ReferenceError: x is not defined' + ) + expect(consoleError).toHaveBeenCalled() + expect(await isPending(adding)).toBe(true) + ackFromServiceWorker(addId) + await expect(adding).resolves.toBeUndefined() + // A late reply for the rejected operation does nothing + ackFromServiceWorker(updateId) + }) + + test('ignores unknown, repeated and late replies', async () => { + const { vrowzer, worker } = await readyInstance() + + const updating = vrowzer.updateFile('/main.js', 'updated') + const id = lastId(worker) + worker.ack('unknown') + ackFromServiceWorker('unknown') + worker.onmessage?.({ data: { type: 'V_FS_OTHER', id } } as MessageEvent) + worker.ack(id) + // A second reply from the Web Worker does not stand in for the Service Worker + worker.ack(id) + expect(await isPending(updating)).toBe(true) + + ackFromServiceWorker(id) + await expect(updating).resolves.toBeUndefined() + + // Replies after the operation settled do nothing + worker.ack(id, { name: 'Error', message: 'late' }) + ackFromServiceWorker(id, { name: 'Error', message: 'late' }) + const deleting = vrowzer.deleteFile('/main.js') + const nextId = lastId(worker) + expect(nextId).not.toBe(id) + worker.ack(nextId) + ackFromServiceWorker(nextId) + await expect(deleting).resolves.toBeUndefined() + }) + + test('does not settle an operation of another instance in the same page', async () => { + const first = await readyInstance() + const second = await readyInstance() + + const firstUpdate = first.vrowzer.updateFile('/a.js', 'a') + const secondUpdate = second.vrowzer.updateFile('/b.js', 'b') + const firstId = lastId(first.worker) + const secondId = lastId(second.worker) + + // Both instances receive the Service Worker's replies through the same container + first.worker.ack(firstId) + ackFromServiceWorker(firstId) + + await expect(firstUpdate).resolves.toBeUndefined() + expect(await isPending(secondUpdate)).toBe(true) + second.worker.ack(secondId) + ackFromServiceWorker(secondId) + await expect(secondUpdate).resolves.toBeUndefined() + }) + + test('rejects before ready() without sending anything', async () => { + const vrowzer = Vrowzer() + + await expect(vrowzer.updateFile('/main.js', 'updated')).rejects.toThrow( + '[Vrowzer] updateFile() can only be called after ready() resolves to true (current state: idle)' + ) + expect(controllerMocks.postMessage).not.toHaveBeenCalled() + }) + + test('rejects while ready() is in progress without sending anything', async () => { + const vrowzer = Vrowzer() + const ready = vrowzer.ready({ files: {} }) + + await expect(vrowzer.addFile('/main.js', 'added')).rejects.toThrow( + '[Vrowzer] addFile() can only be called after ready() resolves to true (current state: initializing)' + ) + await expect(ready).resolves.toBe(true) + expect(webWorkerFileMessages(workers[0]!)).toEqual([]) + expect(serviceWorkerFileMessages()).toEqual([]) + }) + + test('rejects after ready() failed without sending anything', async () => { + controllerMocks.initServiceWorker.mockRejectedValueOnce(new Error('Service Worker failed')) + vi.spyOn(console, 'error').mockImplementation(() => undefined) + const vrowzer = Vrowzer() + await expect(vrowzer.ready({ files: {} })).resolves.toBe(false) + + await expect(vrowzer.deleteFile('/main.js')).rejects.toThrow( + '[Vrowzer] deleteFile() can only be called after ready() resolves to true (current state: failed)' + ) + expect(webWorkerFileMessages(workers[0]!)).toEqual([]) + expect(serviceWorkerFileMessages()).toEqual([]) + }) + + test('sends each Worker its own copy of binary content', async () => { + const { vrowzer, worker } = await readyInstance() + const buffer = new Uint8Array([1, 2, 3]).buffer + + const adding = vrowzer.addFile('/data.bin', buffer) + + const webWorkerMessage = webWorkerFileMessages(worker).at(-1)! + const serviceWorkerMessage = serviceWorkerFileMessages().at(-1)! + expect(buffer.byteLength).toBe(3) + expect(webWorkerMessage.content).not.toBe(buffer) + expect(serviceWorkerMessage.content).not.toBe(buffer) + expect(webWorkerMessage.content).not.toBe(serviceWorkerMessage.content) + expect([...new Uint8Array(webWorkerMessage.content as ArrayBuffer)]).toEqual([1, 2, 3]) + expect([...new Uint8Array(serviceWorkerMessage.content as ArrayBuffer)]).toEqual([1, 2, 3]) + expect(worker.transfers.at(-1)?.[0]).toBe(webWorkerMessage.content) + + worker.ack(webWorkerMessage.id!) + ackFromServiceWorker(webWorkerMessage.id!) + await expect(adding).resolves.toBeUndefined() + }) + + test('rejects when the content cannot be sent and keeps nothing pending', async () => { + vi.useFakeTimers() + const { vrowzer } = await readyInstance() + const detached = new ArrayBuffer(4) + structuredClone(detached, { transfer: [detached] }) + + const adding = vrowzer.addFile('/data.bin', detached) + + await expect(adding).rejects.toThrow('[Vrowzer] addFile("/data.bin") could not be sent:') + await expect(adding).rejects.toMatchObject({ cause: expect.any(TypeError) }) + expect(vi.getTimerCount()).toBe(0) + expect(serviceWorkerFileMessages()).toEqual([]) }) }) diff --git a/packages/vrowzer/src/index-web-worker-timeout.test.ts b/packages/vrowzer/src/index-web-worker-timeout.test.ts index fc9251d7..83c6ba24 100644 --- a/packages/vrowzer/src/index-web-worker-timeout.test.ts +++ b/packages/vrowzer/src/index-web-worker-timeout.test.ts @@ -182,7 +182,9 @@ describe('Vrowzer Web Worker setup timeout', () => { await ready const messageCount = worker.messages.length - vrowzer.updateFile('/main.ts', 'export const value = 1') + await expect(vrowzer.updateFile('/main.ts', 'export const value = 1')).rejects.toThrow( + 'updateFile() can only be called after ready() resolves to true (current state: failed)' + ) expect(worker.messages).toHaveLength(messageCount) }) @@ -193,9 +195,13 @@ describe('Vrowzer Web Worker setup timeout', () => { await expect(ready).resolves.toBe(true) const messageCount = worker.messages.length - vrowzer.updateFile('/main.ts', 'export const value = 1') + const updating = vrowzer.updateFile('/main.ts', 'export const value = 1') expect(worker.terminate).not.toHaveBeenCalled() expect(worker.messages.length).toBeGreaterThan(messageCount) + // Without a Service Worker controller, only the Web Worker is waited for + const { id } = worker.messages.at(-1) as { id: string } + worker.onmessage?.({ data: { type: 'V_FS_ACK', id } } as MessageEvent) + await expect(updating).resolves.toBeUndefined() }) }) diff --git a/packages/vrowzer/src/index.ts b/packages/vrowzer/src/index.ts index 80eac4de..cc658ffd 100644 --- a/packages/vrowzer/src/index.ts +++ b/packages/vrowzer/src/index.ts @@ -22,8 +22,8 @@ * vrowzer.mount(document.getElementById('preview-container'), { id: 'preview' }) * } * - * // Update files (triggers HMR) - * vrowzer.updateFile( + * // Update files (triggers HMR). The promise resolves when later preview requests see the change. + * await vrowzer.updateFile( * '/main.js', * ` * document.getElementById('app').innerHTML = '

Updated!

' @@ -41,7 +41,7 @@ */ import { Emitter } from '@kazupon/jts-utils/event' -import { createFileSystemPublisher } from '@vrowzer/fs/watcher' +import { V_FS_ACK, createFileSystemPublisher } from '@vrowzer/fs/watcher' import { V_WW_READY, V_WW_SETUP, @@ -68,6 +68,7 @@ import type { SvcWorkerControllerEventMap } from '@vrowzer/service-worker/contro const DEFAULT_SERVICE_WORKER_READY_TIMEOUT = 60_000 const DEFAULT_WEB_WORKER_SETUP_TIMEOUT = 90_000 +const DEFAULT_FILE_SYNC_TIMEOUT = 10_000 /** * `postMessage()` type used by the preview bootstrap to report load failures to the host. @@ -79,6 +80,10 @@ const PREVIEW_LOAD_ERROR_NAME_MAX_LENGTH = 100 type ReadyState = 'idle' | 'initializing' | 'ready' | 'failed' | 'disposed' +type FileOperation = 'addFile' | 'updateFile' | 'deleteFile' + +type FileSyncTarget = 'Web Worker' | 'Service Worker' + /** * VrowzerOptions defines the configuration options for {@link Vrowzer}. */ @@ -125,6 +130,18 @@ export interface VrowzerOptions { * @default 90000 */ webWorkerSetupTimeout?: number + /** + * Timeout in milliseconds for the Web Worker and the Service Worker to apply a change made with + * {@link Vrowzer.addFile}, {@link Vrowzer.updateFile} or {@link Vrowzer.deleteFile}. + * + * The Web Worker applies a change after the plugins' `watchChange` hooks finish. This timeout is + * shorter than the 30 seconds that the Service Worker waits for the Web Worker to transform a + * request, so with slow plugins or heavy transforms an operation can reject although the change + * is applied later. Increase it in such environments. Writing the file again resynchronizes it. + * + * @default 10000 + */ + fileSyncTimeout?: number } /** @@ -311,21 +328,43 @@ export interface Vrowzer extends Emittable { /** * Adds a new file to the preview environment with the specified content. * + * The promise resolves when later preview requests see the change: the Web Worker and the + * Service Worker have written the file to their virtual filesystems, and the Web Worker has + * invalidated the modules that depend on it. HMR updates of mounted previews are not awaited. + * + * It rejects without sending the change before {@link Vrowzer.ready} resolves to `true`, after it + * fails, and after {@link Vrowzer.dispose}. It also rejects when a Worker fails to apply the change, + * when the Web Worker reports an error, when the Workers do not reply within + * {@link VrowzerOptions.fileSyncTimeout}, or when the instance is disposed first. The change may + * be partly applied then; write the file again to resynchronize. + * + * Public files (under `/public/`) are currently not served by the Service Worker, so they do not + * become visible to preview requests. + * * @param filePath - The path of the file to be added. - * @param content - The content of the file, which can be a string or an ArrayBuffer. + * @param content - The content of the file, which can be a string or an ArrayBuffer. An + * ArrayBuffer is copied for the Workers and stays usable. */ - addFile(filePath: string, content: string | ArrayBuffer): void + addFile(filePath: string, content: string | ArrayBuffer): Promise /** * Updates the content of a specific file in the preview environment. + * + * The promise resolves and rejects as with {@link Vrowzer.addFile}. + * * @param filePath - The path of the file to be updated. - * @param content - The new content for the file, which can be a string or an ArrayBuffer. + * @param content - The new content for the file, which can be a string or an ArrayBuffer. An + * ArrayBuffer is copied for the Workers and stays usable. */ - updateFile(filePath: string, content: string | ArrayBuffer): void + updateFile(filePath: string, content: string | ArrayBuffer): Promise /** * Deletes a specific file from the preview environment. + * + * The promise resolves when later preview requests no longer see the file, and rejects as with + * {@link Vrowzer.addFile}. Deleting a file that does not exist resolves as well. + * * @param filePath - The path of the file to be deleted. */ - deleteFile(filePath: string): void + deleteFile(filePath: string): Promise /** * Disposes this instance. * @@ -334,8 +373,9 @@ export interface Vrowzer extends Emittable { * forwarded, and all event handlers are removed right away. The Service Worker registration and * its virtual filesystem are kept for other clients. * - * After disposal, `ready()` rejects, `mount()` and the file methods throw, and `unmount()` and - * `reloadPreview()` do nothing. Create a new instance to start again. + * File operations still waiting for the Workers reject. After disposal, `ready()` and the file + * methods reject, `mount()` throws, and `unmount()` and `reloadPreview()` do nothing. Create a + * new instance to start again. * * @returns A promise that resolves when every resource is released. Calling this method again * returns the same promise. It rejects with an `AggregateError` when some resources could not @@ -354,6 +394,22 @@ interface ResolvedVrowzerOptions { serviceWorkerScope: string serviceWorkerReadyTimeout: number webWorkerSetupTimeout: number + fileSyncTimeout: number +} + +/** + * A file operation waiting for the Workers to acknowledge it. + */ +interface PendingFileOperation { + operation: FileOperation + path: string + /** + * Workers that have not acknowledged the operation yet. + */ + waitingFor: Set + resolve: () => void + reject: (error: Error) => void + timer: ReturnType } interface PreviewSessionRecord { @@ -372,7 +428,8 @@ function resolveVrowzerOptions(options: VrowzerOptions): ResolvedVrowzerOptions serviceWorkerScope: resolveServiceWorkerScope(options.serviceWorkerScope), serviceWorkerReadyTimeout: options.serviceWorkerReadyTimeout ?? DEFAULT_SERVICE_WORKER_READY_TIMEOUT, - webWorkerSetupTimeout: options.webWorkerSetupTimeout ?? DEFAULT_WEB_WORKER_SETUP_TIMEOUT + webWorkerSetupTimeout: options.webWorkerSetupTimeout ?? DEFAULT_WEB_WORKER_SETUP_TIMEOUT, + fileSyncTimeout: options.fileSyncTimeout ?? DEFAULT_FILE_SYNC_TIMEOUT } } @@ -432,6 +489,21 @@ function truncate(value: string, maxLength: number): string { return value.length > maxLength ? value.slice(0, maxLength) : value } +function describeFileOperation(operation: FileOperation, path: string): string { + return `[Vrowzer] ${operation}(${JSON.stringify(path)})` +} + +/** + * Rebuilds the error that a Worker reported in a `V_FS_ACK` message. + */ +function toWorkerError(error: unknown): Error { + const workerError = new Error( + isRecord(error) && typeof error.message === 'string' ? error.message : 'unknown error' + ) + workerError.name = isRecord(error) && typeof error.name === 'string' ? error.name : 'Error' + return workerError +} + /** * Runs a cleanup step and collects its error, so that the remaining steps still run. */ @@ -510,6 +582,9 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { const readyReleaseErrors: unknown[] = [] const controllerSubscriptions: (() => void)[] = [] let serviceWorkerTarget: FileSystemPublisherTarget | null = null + // File operations waiting for acknowledgements, by operation id + const pendingFileOperations = new Map() + let stopListeningServiceWorkerAcks: (() => void) | null = null function cleanupWebWorker(): void { const worker = webWorker @@ -527,12 +602,18 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { /** * Releases what initialization created: Service Worker controller event forwarding, - * the file sync targets, and the Web Worker. Used by a failed ready() and by dispose(). + * the file sync targets and acknowledgement listener, and the Web Worker. + * Used by a failed ready() and by dispose(). */ function releaseRuntime(errors: unknown[]): void { for (const stop of controllerSubscriptions.splice(0)) { attempt(errors, stop) } + const stopListeningAcks = stopListeningServiceWorkerAcks + stopListeningServiceWorkerAcks = null + if (stopListeningAcks) { + attempt(errors, stopListeningAcks) + } const target = serviceWorkerTarget serviceWorkerTarget = null if (target) { @@ -547,6 +628,144 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { } } + function settleFileOperation(id: string, error?: Error): void { + const pending = pendingFileOperations.get(id) + if (!pending) { + return + } + pendingFileOperations.delete(id) + clearTimeout(pending.timer) + if (error) { + pending.reject(error) + } else { + pending.resolve() + } + } + + /** + * Rejects the pending file operations, or only those still waiting for `target`. + */ + function rejectFileOperations( + toError: (pending: PendingFileOperation) => Error, + target?: FileSyncTarget + ): void { + // Deleting entries while iterating a Map is safe, so no copy is needed + for (const [id, pending] of pendingFileOperations) { + if (target === undefined || pending.waitingFor.has(target)) { + settleFileOperation(id, toError(pending)) + } + } + } + + function handleFileSyncAck(target: FileSyncTarget, data: unknown): void { + if (!isRecord(data) || data.type !== V_FS_ACK || typeof data.id !== 'string') { + return + } + const pending = pendingFileOperations.get(data.id) + // Unknown ids belong to another instance in the same page, or to settled operations + if (!pending || !pending.waitingFor.has(target)) { + return + } + if (data.error !== undefined) { + const cause = toWorkerError(data.error) + settleFileOperation( + data.id, + new Error( + `${describeFileOperation(pending.operation, pending.path)} failed in the ${target}: ${cause.message}`, + { cause } + ) + ) + return + } + pending.waitingFor.delete(target) + if (pending.waitingFor.size === 0) { + settleFileOperation(data.id) + } + } + + /** + * Receives file sync acknowledgements from the Workers, once the instance is ready. + */ + function listenFileSyncAcks(worker: Worker): void { + worker.onmessage = event => handleFileSyncAck('Web Worker', event.data) + worker.onerror = event => { + console.error('[Vrowzer] Web Worker error:', event.message, event.filename, event.lineno) + const message = event.message || 'unknown error' + rejectFileOperations( + pending => + new Error( + `${describeFileOperation(pending.operation, pending.path)} failed because the Web Worker reported an error: ${message}` + ), + 'Web Worker' + ) + } + + const container = getController()?.container + if (container) { + const handler = (event: MessageEvent) => handleFileSyncAck('Service Worker', event.data) + container.addEventListener('message', handler) + stopListeningServiceWorkerAcks = () => container.removeEventListener('message', handler) + } + } + + /** + * Sends a file operation to the Workers and waits until both have applied it. + */ + function syncFile( + operation: FileOperation, + path: string, + send: (options: { id: string }) => void + ): Promise { + if (readyState === 'disposed') { + return Promise.reject(new Error(`[Vrowzer] ${operation}() cannot be called after dispose()`)) + } + if (readyState !== 'ready') { + return Promise.reject( + new Error( + `[Vrowzer] ${operation}() can only be called after ready() resolves to true (current state: ${readyState})` + ) + ) + } + + const waitingFor = new Set() + if (webWorker) { + waitingFor.add('Web Worker') + } + if (serviceWorkerTarget) { + waitingFor.add('Service Worker') + } + const id = crypto.randomUUID() + const timeout = resolved.fileSyncTimeout + + return new Promise((resolve, reject) => { + const timer = setTimeout(() => { + const pending = pendingFileOperations.get(id) + if (pending) { + const targets = [...pending.waitingFor].map(target => `the ${target}`).join(' and ') + settleFileOperation( + id, + new Error( + `${describeFileOperation(operation, path)} timed out after ${timeout}ms waiting for ${targets}` + ) + ) + } + }, timeout) + pendingFileOperations.set(id, { operation, path, waitingFor, resolve, reject, timer }) + + try { + send({ id }) + } catch (error) { + settleFileOperation( + id, + new Error( + `${describeFileOperation(operation, path)} could not be sent: ${error instanceof Error ? error.message : String(error)}`, + { cause: error } + ) + ) + } + }) + } + /** * Establish MessageChannel between Service Worker and Web Worker. * Creates a MessageChannel, sends one port to each side, @@ -976,6 +1195,9 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { // dispose() may have been called while the last ACK was being delivered signal.throwIfAborted() + // 9. File operations are accepted from now on, so listen for their acknowledgements + listenFileSyncAcks(currentWebWorker) + readyState = 'ready' return true } catch (error) { @@ -1005,6 +1227,12 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { // Remove the handlers first, so that nothing reaches the host after dispose() attempt(errors, () => _emitter.dispose()) readyAbortController?.abort(new Error('[Vrowzer] The instance was disposed')) + rejectFileOperations( + pending => + new Error( + `${describeFileOperation(pending.operation, pending.path)} was cancelled by dispose()` + ) + ) disposePromise = (async () => { // An aborted ready() releases what it created and resolves to false @@ -1136,19 +1364,20 @@ export function Vrowzer(options: VrowzerOptions = {}): Readonly { } }, - addFile(filePath: string, content: string | ArrayBuffer): void { - assertNotDisposed('addFile') - publisher.writeFile(filePath, content) + addFile(filePath: string, content: string | ArrayBuffer): Promise { + return syncFile('addFile', filePath, options => + publisher.writeFile(filePath, content, options) + ) }, - updateFile(filePath: string, content: string | ArrayBuffer): void { - assertNotDisposed('updateFile') - publisher.writeFile(filePath, content) + updateFile(filePath: string, content: string | ArrayBuffer): Promise { + return syncFile('updateFile', filePath, options => + publisher.writeFile(filePath, content, options) + ) }, - deleteFile(filePath: string): void { - assertNotDisposed('deleteFile') - publisher.unlink(filePath) + deleteFile(filePath: string): Promise { + return syncFile('deleteFile', filePath, options => publisher.unlink(filePath, options)) }, dispose, diff --git a/packages/vrowzer/src/service-worker-core.test.ts b/packages/vrowzer/src/service-worker-core.test.ts new file mode 100644 index 00000000..50529e4b --- /dev/null +++ b/packages/vrowzer/src/service-worker-core.test.ts @@ -0,0 +1,100 @@ +import { afterEach, beforeEach, describe, expect, test, vi } from 'vite-plus/test' + +type MessageListener = (event: { data: unknown; source: unknown }) => void + +const mocks = vi.hoisted(() => ({ + handleMessage: vi.fn<(message: unknown) => void>(), + listen: vi.fn<() => Promise>() +})) + +vi.mock('@vrowzer/fs', () => ({ + fs: { + mkdirSync: vi.fn<() => void>(), + writeFileSync: vi.fn<() => void>() + }, + vol: { + fromJSON: vi.fn<() => void>() + } +})) +vi.mock('@vrowzer/fs/watcher', () => ({ + V_FS_ACK: 'V_FS_ACK', + createFileSystemSubscriber: () => ({ watcher: {}, handleMessage: mocks.handleMessage }) +})) +vi.mock('@vrowzer/vite-dev-server/dist/client/client.mjs?raw', () => ({ default: 'client code' })) +vi.mock('@vrowzer/vite-dev-server/dist/client/env.mjs?raw', () => ({ default: 'env code' })) +vi.mock('@vrowzer/vite-dev-server/service-worker', () => ({ + createServer: () => mocks.listen +})) + +import { initServiceWorker } from './service-worker-core.ts' + +let listeners: Map + +function createClient() { + return { id: 'client-1', postMessage: vi.fn<(message: unknown) => void>() } +} + +function receive(data: unknown, source: unknown): void { + listeners.get('message')!({ data, source }) +} + +describe('initServiceWorker file synchronization', () => { + beforeEach(async () => { + vi.clearAllMocks() + mocks.handleMessage.mockReset() + // Keep the server starting, so that only message handling runs + mocks.listen.mockReturnValue(new Promise(() => {})) + listeners = new Map() + vi.stubGlobal('self', { + location: { href: 'https://vrowzer.test/service-worker.js' }, + addEventListener: (type: string, listener: MessageListener) => { + listeners.set(type, listener) + } + }) + await initServiceWorker() + }) + + afterEach(() => { + vi.restoreAllMocks() + vi.unstubAllGlobals() + }) + + test.each([ + { type: 'V_FS_WRITE', id: 'op-1', path: '/main.ts', encoding: 'text', content: 'test' }, + { type: 'V_FS_UNLINK', id: 'op-1', path: '/main.ts' } + ])('acknowledges $type to the sending client after applying it', message => { + const client = createClient() + mocks.handleMessage.mockImplementation(() => { + expect(client.postMessage).not.toHaveBeenCalled() + }) + + receive(message, client) + + expect(mocks.handleMessage).toHaveBeenCalledExactlyOnceWith(message) + expect(client.postMessage).toHaveBeenCalledExactlyOnceWith({ type: 'V_FS_ACK', id: 'op-1' }) + }) + + test('acknowledges with the error when writing the file fails', () => { + const client = createClient() + mocks.handleMessage.mockImplementation(() => { + throw new TypeError('invalid content') + }) + + receive({ type: 'V_FS_WRITE', id: 'op-2', path: '/main.ts', content: 'x' }, client) + + expect(client.postMessage).toHaveBeenCalledExactlyOnceWith({ + type: 'V_FS_ACK', + id: 'op-2', + error: { name: 'TypeError', message: 'invalid content' } + }) + }) + + test('does not acknowledge messages without an id', () => { + const client = createClient() + + receive({ type: 'V_FS_WRITE', path: '/main.ts', content: 'x' }, client) + + expect(mocks.handleMessage).toHaveBeenCalledOnce() + expect(client.postMessage).not.toHaveBeenCalled() + }) +}) diff --git a/packages/vrowzer/src/service-worker-core.ts b/packages/vrowzer/src/service-worker-core.ts index cc0984ab..d9872e38 100644 --- a/packages/vrowzer/src/service-worker-core.ts +++ b/packages/vrowzer/src/service-worker-core.ts @@ -20,7 +20,7 @@ /// import { fs, vol } from '@vrowzer/fs' -import { createFileSystemSubscriber } from '@vrowzer/fs/watcher' +import { V_FS_ACK, createFileSystemSubscriber } from '@vrowzer/fs/watcher' import client from '@vrowzer/vite-dev-server/dist/client/client.mjs?raw' // oxlint-disable-line import/default -- ignore for raw import import env from '@vrowzer/vite-dev-server/dist/client/env.mjs?raw' import { createServer } from '@vrowzer/vite-dev-server/service-worker' @@ -28,12 +28,18 @@ import { V_SW_LISTEN_READY, V_SW_LISTEN_READY_PING } from '@vrowzer/vite-dev-ser import { resolvePreviewBasePath } from './preview-base.ts' import { resolveServiceWorkerVersionForWorker } from './service-worker-version.ts' -import type { FileSystemSyncMessage } from '@vrowzer/fs/watcher' +import type { FSAckMessage, FileSystemSyncMessage } from '@vrowzer/fs/watcher' import type { CreateServerOptions } from '@vrowzer/vite-dev-server/service-worker' import type { Plugin } from '@vrowzer/vite-dev-server/vite' declare const self: ServiceWorkerGlobalScope +function toAckError(error: unknown): NonNullable { + return error instanceof Error + ? { name: error.name, message: error.message } + : { name: 'Error', message: String(error) } +} + export async function initServiceWorker(options?: { plugins?: Plugin[] }) { // Initial volume setup: client files + public dir vol.fromJSON({ @@ -106,7 +112,25 @@ export async function initServiceWorker(options?: { plugins?: Plugin[] }) { // V_FS_* messages: update virtual FS via subscriber if (typeof message?.type === 'string' && message.type.startsWith('V_FS_')) { - subscriber.handleMessage(event.data as FileSystemSyncMessage) + const syncMessage = message as FileSystemSyncMessage + const id = + syncMessage.type === 'V_FS_WRITE' || syncMessage.type === 'V_FS_UNLINK' + ? syncMessage.id + : undefined + if (id === undefined) { + subscriber.handleMessage(syncMessage) + return + } + + // The virtual filesystem is written synchronously, so later requests see the change + // as soon as handleMessage() returns. Acknowledge it to the client that sent it. + let ack: FSAckMessage = { type: V_FS_ACK, id } + try { + subscriber.handleMessage(syncMessage) + } catch (error) { + ack = { type: V_FS_ACK, id, error: toAckError(error) } + } + event.source?.postMessage(ack) return } }) diff --git a/packages/vrowzer/src/web-worker-core.test.ts b/packages/vrowzer/src/web-worker-core.test.ts index d7626109..f8dd4e92 100644 --- a/packages/vrowzer/src/web-worker-core.test.ts +++ b/packages/vrowzer/src/web-worker-core.test.ts @@ -8,8 +8,13 @@ type TestServerOptions = { onUnhandledMessage: (event: MessageEvent) => Promise } +type TestServer = { + fileSystem: object + waitForFileChange: (file: string) => Promise +} + const fileSystemMocks = vi.hoisted(() => { - const subscriber: TestSubscriber = { + const subscriber = { handleMessage: vi.fn<(message: unknown) => void>() } return { @@ -22,15 +27,17 @@ const fileSystemMocks = vi.hoisted(() => { }) const serverMocks = vi.hoisted(() => { - const listen = vi.fn<(timeout?: number) => Promise<{ fileSystem: object }>>() + const listen = vi.fn<(timeout?: number) => Promise>() return { createServer: vi.fn<(_scope: unknown, options: TestServerOptions) => { listen: typeof listen }>(), - listen + listen, + waitForFileChange: vi.fn<(file: string) => Promise>() } }) vi.mock('@vrowzer/fs/watcher', () => ({ + V_FS_ACK: 'V_FS_ACK', createFileSystemSubscriber: fileSystemMocks.createFileSystemSubscriber, createVirtualFSWatcher: fileSystemMocks.createVirtualFSWatcher })) @@ -40,13 +47,40 @@ vi.mock('@vrowzer/vite-dev-server/web-worker', () => ({ import { initWebWorker } from './web-worker-core.ts' +let postMessage: ReturnType void>> + +function deferred(): { promise: Promise; resolve: () => void } { + let resolve!: () => void + const promise = new Promise(resolvePromise => { + resolve = resolvePromise + }) + return { promise, resolve } +} + +async function startWorker(): Promise { + serverMocks.listen.mockResolvedValue({ + fileSystem: {}, + waitForFileChange: serverMocks.waitForFileChange + }) + await initWebWorker() + return serverMocks.createServer.mock.calls[0]![1] +} + +function receive(options: TestServerOptions, data: unknown): Promise { + return options.onUnhandledMessage({ data } as MessageEvent) +} + describe('initWebWorker', () => { beforeEach(() => { vi.clearAllMocks() fileSystemMocks.createVirtualFSWatcher.mockReturnValue(fileSystemMocks.watcher) fileSystemMocks.createFileSystemSubscriber.mockReturnValue(fileSystemMocks.subscriber) + fileSystemMocks.subscriber.handleMessage.mockReset() serverMocks.createServer.mockReturnValue({ listen: serverMocks.listen }) - vi.stubGlobal('self', {}) + serverMocks.waitForFileChange.mockReset() + serverMocks.waitForFileChange.mockResolvedValue(undefined) + postMessage = vi.fn<(message: unknown) => void>() + vi.stubGlobal('self', { postMessage }) }) afterEach(() => { @@ -55,8 +89,8 @@ describe('initWebWorker', () => { }) test('uses the server filesystem and flushes queued messages', async () => { - let resolveListen!: (server: { fileSystem: object }) => void - const listening = new Promise<{ fileSystem: object }>(resolve => { + let resolveListen!: (server: TestServer) => void + const listening = new Promise(resolve => { resolveListen = resolve }) const fileSystem = {} @@ -69,7 +103,7 @@ describe('initWebWorker', () => { const queuedMessage = { type: 'V_FS_WRITE', path: '/main.ts', content: 'test' } await options.onUnhandledMessage({ data: queuedMessage } as MessageEvent) - resolveListen({ fileSystem }) + resolveListen({ fileSystem, waitForFileChange: serverMocks.waitForFileChange }) await initializing expect(serverMocks.listen).toHaveBeenCalledWith(0) @@ -78,4 +112,100 @@ describe('initWebWorker', () => { }) expect(fileSystemMocks.subscriber.handleMessage).toHaveBeenCalledWith(queuedMessage) }) + + test.each([ + { type: 'V_FS_WRITE', id: 'op-1', path: '/main.ts', encoding: 'text', content: 'test' }, + { type: 'V_FS_UNLINK', id: 'op-1', path: '/main.ts' } + ])('acknowledges $type after the file change is processed', async message => { + const calls: string[] = [] + const processing = deferred() + fileSystemMocks.subscriber.handleMessage.mockImplementation(() => { + calls.push('handleMessage') + }) + serverMocks.waitForFileChange.mockImplementation(file => { + calls.push(`waitForFileChange:${file}`) + return processing.promise + }) + const options = await startWorker() + + await receive(options, message) + + // The change is taken right after the message is applied + expect(calls).toEqual(['handleMessage', 'waitForFileChange:/main.ts']) + expect(fileSystemMocks.subscriber.handleMessage).toHaveBeenCalledWith(message) + await new Promise(resolve => setTimeout(resolve, 0)) + expect(postMessage).not.toHaveBeenCalled() + + processing.resolve() + await vi.waitFor(() => { + expect(postMessage).toHaveBeenCalledExactlyOnceWith({ type: 'V_FS_ACK', id: 'op-1' }) + }) + }) + + test('acknowledges with the error when processing the file change fails', async () => { + serverMocks.waitForFileChange.mockRejectedValue(new TypeError('watchChange failed')) + const options = await startWorker() + + await receive(options, { type: 'V_FS_WRITE', id: 'op-2', path: '/main.ts', content: 'x' }) + + await vi.waitFor(() => { + expect(postMessage).toHaveBeenCalledExactlyOnceWith({ + type: 'V_FS_ACK', + id: 'op-2', + error: { name: 'TypeError', message: 'watchChange failed' } + }) + }) + }) + + test('acknowledges with the error when writing the file fails', async () => { + fileSystemMocks.subscriber.handleMessage.mockImplementation(() => { + throw new Error('EISDIR: illegal operation on a directory') + }) + const options = await startWorker() + + await receive(options, { type: 'V_FS_WRITE', id: 'op-3', path: '/src', content: 'x' }) + + await vi.waitFor(() => { + expect(postMessage).toHaveBeenCalledExactlyOnceWith({ + type: 'V_FS_ACK', + id: 'op-3', + error: { name: 'Error', message: 'EISDIR: illegal operation on a directory' } + }) + }) + expect(serverMocks.waitForFileChange).not.toHaveBeenCalled() + }) + + test('acknowledges queued messages after they are processed', async () => { + let resolveListen!: (server: TestServer) => void + serverMocks.listen.mockReturnValue( + new Promise(resolve => { + resolveListen = resolve + }) + ) + const initializing = initWebWorker() + await vi.waitFor(() => expect(serverMocks.createServer).toHaveBeenCalledOnce()) + const options = serverMocks.createServer.mock.calls[0]![1] + + await receive(options, { type: 'V_FS_UNLINK', id: 'op-4', path: '/old.ts' }) + expect(postMessage).not.toHaveBeenCalled() + + resolveListen({ fileSystem: {}, waitForFileChange: serverMocks.waitForFileChange }) + await initializing + + expect(serverMocks.waitForFileChange).toHaveBeenCalledWith('/old.ts') + await vi.waitFor(() => { + expect(postMessage).toHaveBeenCalledExactlyOnceWith({ type: 'V_FS_ACK', id: 'op-4' }) + }) + }) + + test('does not acknowledge messages without an id', async () => { + const options = await startWorker() + + await receive(options, { type: 'V_FS_WRITE', path: '/main.ts', content: 'x' }) + await new Promise(resolve => setTimeout(resolve, 0)) + + // The change is still taken, so that it is not left for a later message + expect(serverMocks.waitForFileChange).toHaveBeenCalledWith('/main.ts') + expect(postMessage).not.toHaveBeenCalled() + }) }) diff --git a/packages/vrowzer/src/web-worker-core.ts b/packages/vrowzer/src/web-worker-core.ts index 169e4be7..02db387d 100644 --- a/packages/vrowzer/src/web-worker-core.ts +++ b/packages/vrowzer/src/web-worker-core.ts @@ -18,11 +18,14 @@ /// -import { createFileSystemSubscriber, createVirtualFSWatcher } from '@vrowzer/fs/watcher' +import { V_FS_ACK, createFileSystemSubscriber, createVirtualFSWatcher } from '@vrowzer/fs/watcher' import { createServer } from '@vrowzer/vite-dev-server/web-worker' -import type { FileSystemSyncMessage } from '@vrowzer/fs/watcher' -import type { CreateServerOptions } from '@vrowzer/vite-dev-server/web-worker' +import type { FSAckMessage, FileSystemSubscriber, FileSystemSyncMessage } from '@vrowzer/fs/watcher' +import type { + CreateServerOptions, + ViteDevServerForWorker +} from '@vrowzer/vite-dev-server/web-worker' import type { Plugin, UserConfig } from '@vrowzer/vite-dev-server/vite' declare const self: DedicatedWorkerGlobalScope @@ -35,11 +38,55 @@ declare const self: DedicatedWorkerGlobalScope */ type InitWebWorkerOptions = UserConfig & { plugins?: Plugin[] } +function toAckError(error: unknown): NonNullable { + return error instanceof Error + ? { name: error.name, message: error.message } + : { name: 'Error', message: String(error) } +} + +/** + * Applies a V_FS_* message. A write or deletion with an `id` is acknowledged after the dev server + * has processed the file change up to module graph invalidation, so that later transform + * requests see it. HMR is not awaited. + */ +function applyMessage( + subscriber: FileSystemSubscriber, + server: ViteDevServerForWorker, + message: FileSystemSyncMessage +): void { + const fileChange = + message.type === 'V_FS_WRITE' || message.type === 'V_FS_UNLINK' ? message : undefined + const id = fileChange?.id + let applied: Promise + try { + subscriber.handleMessage(message) + // handleMessage() emits the watcher event synchronously, so its processing is taken right + // away, even without an id, so that it is not left behind for a later message + applied = fileChange ? server.waitForFileChange(fileChange.path) : Promise.resolve() + } catch (error) { + if (id === undefined) { + throw error + } + applied = Promise.reject(error) + } + + if (id !== undefined) { + void applied.then( + () => { + self.postMessage({ type: V_FS_ACK, id } satisfies FSAckMessage) + }, + (error: unknown) => { + self.postMessage({ type: V_FS_ACK, id, error: toAckError(error) } satisfies FSAckMessage) + } + ) + } +} + export async function initWebWorker(options?: InitWebWorkerOptions) { // Create watcher early so it can be passed to DevEnvironment via createServer. // Subscriber is created later with transformer's fs to share the same vol. const watcher = createVirtualFSWatcher() - let subscriber: ReturnType | null = null + let apply: ((message: FileSystemSyncMessage) => void) | null = null const pendingMessages: FileSystemSyncMessage[] = [] // Separate plugins from other config fields (resolve, define, etc.) @@ -53,8 +100,8 @@ export async function initWebWorker(options?: InitWebWorkerOptions) { onUnhandledMessage: async (event: MessageEvent) => { // V_FS_* messages: update virtual FS via subscriber if (typeof event.data?.type === 'string' && event.data.type.startsWith('V_FS_')) { - if (subscriber) { - subscriber.handleMessage(event.data as FileSystemSyncMessage) + if (apply) { + apply(event.data as FileSystemSyncMessage) } else { pendingMessages.push(event.data as FileSystemSyncMessage) } @@ -68,11 +115,12 @@ export async function initWebWorker(options?: InitWebWorkerOptions) { // Create subscriber with the exact fs used by the DevEnvironment. // and the watcher that was already passed to DevEnvironment. - subscriber = createFileSystemSubscriber(readyServer.fileSystem, { watcher }) + const subscriber = createFileSystemSubscriber(readyServer.fileSystem, { watcher }) + apply = message => applyMessage(subscriber, readyServer, message) // Process queued V_FS_* messages for (const msg of pendingMessages) { - subscriber.handleMessage(msg) + apply(msg) } pendingMessages.length = 0 }