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) <noreply@anthropic.com>
This commit is contained in:
@@ -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: '<p>stranded</p>', 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<typeof usePages> | null = null;
|
||||||
|
const Consumer: React.FC = () => {
|
||||||
|
ctx = usePages();
|
||||||
|
return null;
|
||||||
|
};
|
||||||
|
render(
|
||||||
|
<PageProvider>
|
||||||
|
<Consumer />
|
||||||
|
</PageProvider>,
|
||||||
|
);
|
||||||
|
|
||||||
|
// 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();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -393,6 +393,11 @@ export const PageProvider: React.FC<{ children: ReactNode }> = ({ children }) =>
|
|||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error('Failed to deserialize state:', e);
|
console.error('Failed to deserialize state:', e);
|
||||||
try {
|
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);
|
actions.deserialize(fallback);
|
||||||
} catch (_e2) {
|
} catch (_e2) {
|
||||||
// give up
|
// give up
|
||||||
|
|||||||
Reference in New Issue
Block a user