diff --git a/.changeset/gentle-lions-hide.md b/.changeset/gentle-lions-hide.md new file mode 100644 index 0000000..5d93fcf --- /dev/null +++ b/.changeset/gentle-lions-hide.md @@ -0,0 +1,5 @@ +--- +"@playfast/echoform": patch +--- + +Withhold `update_view` / `delete_view` broadcasts from clients that have not yet requested the view tree. Previously, in `singleInstance` mode, a client connecting while views were updating could receive an incremental prop diff before its `update_views_tree` snapshot; the client materialized the unknown view from the diff alone, yielding a view with only the changed props (missing callback and data props) and crashing the client renderer on the first missing-prop access (e.g. `props.onSelectCategory.mutate` in the wmux TUI during dev-server startup). diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c2c424..ef5d5d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,8 +42,13 @@ jobs: working-directory: packages/echoform run: bun run test + # Use the workspace-pinned playwright so the browser revision matches + # @playwright/test from the lockfile. `bunx playwright` resolves the + # registry's latest (no root bin exists with isolated installs) and + # installs a mismatched chromium_headless_shell revision. - name: Install Playwright browsers - run: bunx playwright install --with-deps chromium + working-directory: demo/todo-app + run: ./node_modules/.bin/playwright install --with-deps chromium - name: Run Playwright tests (todo-app) working-directory: demo/todo-app diff --git a/packages/echoform/src/server/App.tsx b/packages/echoform/src/server/App.tsx index 0885b1d..e305b92 100644 --- a/packages/echoform/src/server/App.tsx +++ b/packages/echoform/src/server/App.tsx @@ -326,6 +326,13 @@ function removeAt(items: ReadonlyArray, index: number): T[] { return [...items.slice(0, index), ...items.slice(index + 1)]; } +// View-state events are meaningless to a client that has no base state yet: the +// client applies `update_view` as a partial upsert, so a prop diff arriving before +// the `update_views_tree` snapshot materializes a view with only the changed props +// (missing callbacks/data) and crashes the client renderer. These events are +// therefore withheld per client until that client has requested the view tree. +const VIEW_SYNC_EVENTS: ReadonlySet = new Set(["update_view", "delete_view"]); + const App = forwardRef(function App({ children, transport, paused, transportIsClient, skipCallbackValidation }, ref) { const serverRef = useRef(decompileTransport(transport)); const clientsRef = useRef>([]); @@ -335,6 +342,7 @@ const App = forwardRef(function App({ children, transport, const cleanUpFunctionsRef = useRef void>>([]); const clientCleanupMapRef = useRef void>>(new Map()); const clientEventAuthRef = useRef>>(new Map()); + const syncedClientsRef = useRef>(new Set()); const eventChainRef = useRef(Promise.resolve()); const streamBufferRegistryRef = useRef>(new Map()); const skipValidationRef = useRef(skipCallbackValidation ?? false); @@ -350,7 +358,9 @@ const App = forwardRef(function App({ children, transport, const server = serverRef.current; if (!server) return; server.emit(event, data); + const requiresSyncedClient = VIEW_SYNC_EVENTS.has(event); for (const client of clientsRef.current) { + if (requiresSyncedClient && !syncedClientsRef.current.has(client)) continue; client.emit(event, data); } }, []); @@ -379,6 +389,7 @@ const App = forwardRef(function App({ children, transport, const registerSocketListener = useCallback((client: DecompileTransport) => { const cleanReqTree = client.on("request_views_tree", () => { handleViewsTreeRequest(client, existingSharedViewsRef, clientEventAuthRef, streamBufferRegistryRef); + syncedClientsRef.current = new Set([...syncedClientsRef.current, client]); }); const cleanReqEvent = client.on("request_event", (eventData: AppEvents['request_event']) => { @@ -415,6 +426,9 @@ const App = forwardRef(function App({ children, transport, clientEventAuthRef.current = new Map( [...clientEventAuthRef.current].filter(([k]) => k !== clientTransport), ); + syncedClientsRef.current = new Set( + [...syncedClientsRef.current].filter((synced) => synced !== clientTransport), + ); } clientCleanupMapRef.current.get(transportKey)?.(); @@ -481,6 +495,7 @@ const App = forwardRef(function App({ children, transport, clientCleanupMapRef.current = new Map(); viewEventsRef.current = new Map(); clientEventAuthRef.current = new Map(); + syncedClientsRef.current = new Set(); existingSharedViewsRef.current = []; }; }, []); diff --git a/packages/echoform/tests/view-sync-handshake.test.ts b/packages/echoform/tests/view-sync-handshake.test.ts new file mode 100644 index 0000000..b08c4a0 --- /dev/null +++ b/packages/echoform/tests/view-sync-handshake.test.ts @@ -0,0 +1,262 @@ +/** + * Protocol-level integration test for the view-sync handshake. + * + * A client that has connected but not yet requested the view tree has no base + * state. If the server broadcasts an incremental `update_view` prop diff to such + * a client, the client materializes the unknown view from the diff alone — + * producing a view with only the changed props (no callbacks, no other data) + * and crashing the client renderer on the first missing-prop access. + * + * These tests pin the invariant: view-state events (`update_view`, + * `delete_view`) are only sent to clients that have completed the + * `request_views_tree` handshake, while the snapshot itself carries full props + * (data and event props alike). + */ +import { describe, test, expect, afterAll } from "bun:test"; +import type { ServerWebSocket } from "bun"; +import React from "react"; +import { view, callback, passthrough } from "../src/shared/view-builder"; +import { Server, type ServerProps } from "../src/server/Server"; +import { Render } from "@playfast/echoform-render"; +import { decodeMessage, encodeMessage } from "../src/shared/binary-protocol"; +import { createWebSocketTransport } from "../src/shared/transport-handlers"; +import type { Transport } from "../src/shared/types"; + +const TEST_TOKEN = "test-handshake-token"; + +const TestView = view("HandshakeTestView", { + input: { label: passthrough() }, + callbacks: { onSelect: callback({ input: passthrough() }) }, +}); + +interface ViewProp { + readonly name: string; + readonly type: "data" | "event" | "stream"; + readonly data?: unknown; +} + +interface UpdateViewPayload { + readonly view: { + readonly uid: string; + readonly name: string; + readonly props: { + readonly create: ReadonlyArray; + readonly delete: ReadonlyArray; + }; + }; +} + +interface TreePayload { + readonly views: ReadonlyArray<{ + readonly uid: string; + readonly name: string; + readonly props: ReadonlyArray; + }>; +} + +interface ServerHandle { + readonly port: number; + readonly setLabel: (label: string) => void; + readonly stop: () => void; +} + +async function waitFor(predicate: () => boolean, description: string, timeoutMs = 5000): Promise { + const deadline = Date.now() + timeoutMs; + while (!predicate()) { + if (Date.now() > deadline) throw new Error(`Timed out waiting for ${description}`); + await new Promise((resolve) => setTimeout(resolve, 25)); + } +} + +interface SocketData { + readonly id: string; +} + +type IncomingConnection = Transport> & { readonly id: string }; + +function createTestServer(): ServerHandle { + interface ClientEntry { + readonly dispatch: (msg: string | ArrayBuffer | Uint8Array) => void; + readonly disconnect: () => void; + } + + const clients = new Map(); + const connectionRef: { current: ((client: IncomingConnection) => void) | null } = { current: null }; + + const serverTransport = { + on: (_event: "connection", handler: (client: IncomingConnection) => void) => { + connectionRef.current = handler; + }, + emit: () => {}, + off: () => { + connectionRef.current = null; + }, + } as unknown as ServerProps["transport"]; + + const labelControl: { current: ((label: string) => void) | null } = { current: null }; + + function StatefulProducer(): React.ReactElement { + const [label, setLabel] = React.useState("initial"); + labelControl.current = setLabel; + return React.createElement(TestView, { label, onSelect: () => {} }); + } + + const server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + fetch(req, srv) { + const url = new URL(req.url); + if (url.pathname === "/ws") { + const token = url.searchParams.get("token"); + if (token !== TEST_TOKEN) return new Response("Unauthorized", { status: 401 }); + const data: SocketData = { id: globalThis.crypto.randomUUID() }; + const upgraded = srv.upgrade(req, { data }); + if (upgraded) return undefined; + return new Response("Upgrade failed", { status: 500 }); + } + return new Response("Not found", { status: 404 }); + }, + websocket: { + open(ws: ServerWebSocket) { + const { transport: clientTransport, dispatch, disconnect } = createWebSocketTransport(ws); + clients.set(ws.data.id, { dispatch, disconnect }); + connectionRef.current?.({ ...clientTransport, id: ws.data.id }); + }, + message(ws: ServerWebSocket, message: string | Buffer) { + clients.get(ws.data.id)?.dispatch(message as string | Uint8Array); + }, + close(ws: ServerWebSocket) { + const client = clients.get(ws.data.id); + if (client) { + client.disconnect(); + clients.delete(ws.data.id); + } + }, + }, + }); + + Render(React.createElement(Server, { transport: serverTransport, singleInstance: true }, () => React.createElement(StatefulProducer))); + + return { + port: server.port, + setLabel: (label: string) => { + if (!labelControl.current) throw new Error("Server component not mounted yet — cannot set label"); + labelControl.current(label); + }, + stop: () => { + server.stop(); + clients.clear(); + }, + }; +} + +interface TestClient { + readonly updateViews: ReadonlyArray; + readonly trees: ReadonlyArray; + readonly requestTree: () => void; + readonly close: () => void; +} + +function connectClient(port: number): Promise { + return new Promise((resolve, reject) => { + const ws = new WebSocket(`ws://127.0.0.1:${port}/ws?token=${TEST_TOKEN}`); + ws.binaryType = "arraybuffer"; + + const updateViews: Array = []; + const trees: Array = []; + + ws.addEventListener("message", (event: MessageEvent) => { + const bytes = new Uint8Array(event.data as ArrayBuffer); + const { event: eventName, data } = decodeMessage(bytes); + if (eventName === "update_view") updateViews.push(data as UpdateViewPayload); + if (eventName === "update_views_tree") trees.push(data as TreePayload); + }); + + ws.addEventListener("open", () => { + resolve({ + updateViews, + trees, + requestTree: () => ws.send(encodeMessage("request_views_tree", undefined)), + close: () => ws.close(), + }); + }); + + ws.addEventListener("error", reject); + }); +} + +function findTestView(tree: TreePayload) { + return tree.views.find((candidate) => candidate.name === "HandshakeTestView"); +} + +function hasLabelDiff(client: TestClient, label: string): boolean { + return client.updateViews.some((update) => + update.view.props.create.some((prop) => prop.type === "data" && prop.name === "label" && prop.data === label), + ); +} + +async function connectAndSync(port: number): Promise { + const client = await connectClient(port); + await waitFor(() => { + if (client.trees.some((tree) => findTestView(tree))) return true; + client.requestTree(); + return false; + }, "client to receive a snapshot containing the test view"); + return client; +} + +describe("View Sync Handshake", () => { + const servers: Array = []; + + afterAll(() => { + for (const server of servers) server.stop(); + }); + + test("view diffs reach synced clients but never clients that have not requested the tree", async () => { + const server = createTestServer(); + servers.push(server); + + const syncedClient = await connectAndSync(server.port); + + // Unsynced client: connected, but never requests the view tree. + const unsyncedClient = await connectClient(server.port); + + server.setLabel("updated-1"); + + // Positive control: the synced client receives the prop diff, proving the + // broadcast fired — so the unsynced client's silence is the gate, not timing. + await waitFor(() => hasLabelDiff(syncedClient, "updated-1"), "synced client to receive the label diff"); + expect(unsyncedClient.updateViews).toEqual([]); + + // After the handshake, the same client receives subsequent diffs. + unsyncedClient.requestTree(); + await waitFor(() => unsyncedClient.trees.length > 0, "late client to receive the snapshot"); + + server.setLabel("updated-2"); + await waitFor(() => hasLabelDiff(unsyncedClient, "updated-2"), "late client to receive diffs after handshake"); + + syncedClient.close(); + unsyncedClient.close(); + }); + + test("snapshot delivers full props, including callback event props", async () => { + const server = createTestServer(); + servers.push(server); + + const client = await connectAndSync(server.port); + + const snapshotTree = client.trees.find((tree) => findTestView(tree)); + if (!snapshotTree) throw new Error("No snapshot containing the test view was received"); + const snapshotView = findTestView(snapshotTree); + if (!snapshotView) throw new Error("Test view missing from the snapshot"); + + const propTypesByName = new Map(snapshotView.props.map((prop) => [prop.name, prop.type])); + + // The crash mode this guards against: a view reaching the client without its + // callback props, so `props.onSelect.mutate` dereferences undefined. + expect(propTypesByName.get("onSelect")).toBe("event"); + expect(propTypesByName.get("label")).toBe("data"); + + client.close(); + }); +});