From 38c8d22e5dbb7dd27898f6a353d7e94e80bcdba1 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 9 Aug 2026 06:53:55 -0700 Subject: [PATCH] test(site-builder): cover loadState's orphan-repair wiring via PageProvider Review found Task 8's prescribed test never mounts PageProvider, so it never exercises loadState -- deleting the repairOrphanNodes call would still leave that suite green. Adds a PageProvider-mounted test driving switchPage into a page with a stored orphaned node, asserting on what loadState hands to actions.deserialize and that console.warn fires. Verified load-bearing: temporarily neutering the repair call fails the new test (ROOT.nodes missing 'stray'), then restored. Also documents the fallback-branch invariant that `fallback` must always be a known-safe constant, per review. Co-Authored-By: Claude Opus 5 (1M context) --- .../PageContext.orphan-repair-wiring.test.tsx | 147 ++++++++++++++++++ craft/src/state/PageContext.tsx | 5 + 2 files changed, 152 insertions(+) create mode 100644 craft/src/state/PageContext.orphan-repair-wiring.test.tsx diff --git a/craft/src/state/PageContext.orphan-repair-wiring.test.tsx b/craft/src/state/PageContext.orphan-repair-wiring.test.tsx new file mode 100644 index 0000000..56be720 --- /dev/null +++ b/craft/src/state/PageContext.orphan-repair-wiring.test.tsx @@ -0,0 +1,147 @@ +import { describe, test, expect, vi, beforeEach } from 'vitest'; +import React from 'react'; +import { createRoot, Root } from 'react-dom/client'; +import { act } from 'react-dom/test-utils'; +import { PageProvider, usePages } from './PageContext'; + +/** + * Task 8 review finding: `PageContext.orphan-repair.test.ts` (the brief's + * prescribed test) drives `renderEditorHarness()` + `repairOrphanNodes()` + * directly -- it never mounts `PageProvider`, so nothing in it actually + * exercises `loadState`. If the `repairOrphanNodes` call were deleted from + * `loadState` outright, that file would still pass in full. + * + * This file closes that gap: it mounts a real `PageProvider` (same + * `vi.mock('@craftjs/core', ...)` + `deserializeMock` pattern as + * `PageContext.pages-productivity.test.tsx`) and drives `switchPage` -- + * `loadState`'s only reachable-from-the-UI caller for an already-stored + * page -- against a page whose stored `craftState` contains a node with no + * path back to ROOT. It asserts on what `loadState` actually handed to + * `actions.deserialize` (mocked here, same as the sibling suite) rather than + * on Craft.js's own reconciliation, which `PageContext.orphan-repair.test.ts` + * already covers via the real editor. + */ + +let serializeReturn = '{}'; +const deserializeMock = vi.fn(); + +vi.mock('@craftjs/core', () => ({ + useEditor: () => ({ + query: { serialize: () => serializeReturn }, + actions: { deserialize: deserializeMock }, + }), +})); + +let container: HTMLDivElement; +let root: Root; + +function render(ui: React.ReactElement) { + container = document.createElement('div'); + document.body.appendChild(container); + act(() => { + root = createRoot(container); + root.render(ui); + }); +} + +function unmount() { + act(() => { + root.unmount(); + }); + container.remove(); +} + +async function flushTimers() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 10)); + }); +} + +beforeEach(() => { + serializeReturn = '{}'; + deserializeMock.mockClear(); +}); + +/** A ROOT with no children plus an orphan ('stray') whose `parent` points at + * an id that doesn't exist in the tree, and which no node's `nodes`/ + * `linkedNodes` lists -- unreachable by BFS from ROOT. */ +const ORPHAN_STATE = JSON.stringify({ + ROOT: { + type: { resolvedName: 'Container' }, + isCanvas: true, + props: { style: {}, tag: 'div' }, + displayName: 'Container', + custom: {}, hidden: false, nodes: [], linkedNodes: {}, parent: null, + }, + stray: { + type: { resolvedName: 'HtmlBlock' }, + isCanvas: false, + props: { code: '

stranded

', style: {} }, + displayName: 'HTML', + custom: {}, hidden: false, nodes: [], linkedNodes: {}, parent: 'ghost', + }, +}); + +describe('loadState (via switchPage) repairs an orphaned node before handing it to Craft', () => { + test('switching to a page whose stored state has an orphan reattaches it and warns', async () => { + let ctx: ReturnType | null = null; + const Consumer: React.FC = () => { + ctx = usePages(); + return null; + }; + render( + + + , + ); + + // pages: [Home]. Add "About" -- addPage switches the live canvas to it. + act(() => ctx!.addPage('About', 'about')); + await flushTimers(); + const aboutId = ctx!.pages[1].id; + + // Switch back to Home so About is no longer the active page -- switching + // TO an already-active page is a documented no-op in `switchPage`, and + // this test needs a real switch-INTO event to fire `loadState`. + act(() => ctx!.switchPage('home')); + await flushTimers(); + + // Seed About's STORED craftState directly with the orphaned tree, the + // same way a loaded project's saved state reaches PageContext (e.g. via + // `setPagesCraftState` from `useWhpApi`'s `load()`), bypassing the live + // canvas entirely so nothing but `loadState` itself can repair it. + act(() => + ctx!.setPagesCraftState( + ctx!.pages.map((p) => ({ + id: p.id, + name: p.name, + slug: p.slug, + craftState: p.id === aboutId ? ORPHAN_STATE : p.craftState, + seo: p.seo, + })), + ), + ); + + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + deserializeMock.mockClear(); + + // The real switch-into-a-stored-page path. + act(() => ctx!.switchPage(aboutId)); + await flushTimers(); + + expect(deserializeMock).toHaveBeenCalled(); + const passedState = deserializeMock.mock.calls[deserializeMock.mock.calls.length - 1][0]; + const parsed = JSON.parse(passedState); + // The orphan is now an ordinary, reachable child of ROOT. + expect(parsed.ROOT.nodes).toContain('stray'); + expect(parsed.stray.parent).toBe('ROOT'); + + // The observable signal that repair actually ran, not just that the + // orphan happened to be absent for some unrelated reason. + expect(warnSpy).toHaveBeenCalled(); + expect(warnSpy.mock.calls[0][0]).toContain('reattached'); + + warnSpy.mockRestore(); + unmount(); + }); +}); diff --git a/craft/src/state/PageContext.tsx b/craft/src/state/PageContext.tsx index 41fac54..f19f40c 100644 --- a/craft/src/state/PageContext.tsx +++ b/craft/src/state/PageContext.tsx @@ -393,6 +393,11 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) => } catch (e) { console.error('Failed to deserialize state:', e); try { + // NOT run through repairOrphanNodes: `fallback` must always be + // one of the module's own known-safe constants (EMPTY_CANVAS / + // EMPTY_HEADER / EMPTY_FOOTER -- true at all current call sites), + // never untrusted/stored data, since this is the last line of + // defense before giving up silently below. actions.deserialize(fallback); } catch (_e2) { // give up