diff --git a/craft/src/editor/LoadGate.save-guard.test.tsx b/craft/src/editor/LoadGate.save-guard.test.tsx index 7d0d127..4c6d67a 100644 --- a/craft/src/editor/LoadGate.save-guard.test.tsx +++ b/craft/src/editor/LoadGate.save-guard.test.tsx @@ -30,7 +30,7 @@ import { LoadGate } from './LoadGate'; import { TopBar } from '../panels/topbar/TopBar'; import { SiteDesignPanel } from '../panels/right/SiteDesignPanel'; import { EditorConfigProvider } from '../state/EditorConfigContext'; -import { SaveGuardProvider } from '../state/SaveGuardContext'; +import { SaveGuardProvider, useSaveGuard } from '../state/SaveGuardContext'; import { PageProvider } from '../state/PageContext'; import { SiteDesignProvider } from '../state/SiteDesignContext'; import { WhpConfig } from '../types'; @@ -55,6 +55,9 @@ const saveBody = (i: number) => JSON.parse(saveCalls()[i][1].body); const q = (sel: string) => container.querySelector(sel) as HTMLElement | null; const flush = () => act(async () => { await Promise.resolve(); await Promise.resolve(); }); +let guard: ReturnType; +const GuardProbe: React.FC = () => { guard = useSaveGuard(); return null; }; + function mount() { container = document.createElement('div'); document.body.appendChild(container); @@ -68,6 +71,7 @@ function mount() { {}} showGuides onToggleGuides={() => {}} /> + @@ -179,6 +183,8 @@ describe('load gate', () => { mount(); await flush(); expect(q('[data-testid="load-gate"]')).toBeNull(); + // Autosave only fires for unsaved changes. + act(() => { guard.markDirty(); }); await act(async () => { vi.advanceTimersByTime(30000); }); expect(saveCalls()).toHaveLength(1); }); diff --git a/craft/src/editor/SaveState.test.tsx b/craft/src/editor/SaveState.test.tsx index 2d62ee1..2707418 100644 --- a/craft/src/editor/SaveState.test.tsx +++ b/craft/src/editor/SaveState.test.tsx @@ -203,13 +203,39 @@ describe('dirty lifecycle and status pill', () => { }); }); -describe('Draft badge', () => { - test('a no-op autosave does not flip the badge to Draft; a save after real changes does', async () => { +describe('autosave only when dirty', () => { + test('an idle 30s tick makes no request; an edit makes exactly one on the next tick', async () => { respondWith({}); await mount(); + await act(async () => { vi.advanceTimersByTime(90_000); }); + await flush(); + expect(calls('save')).toHaveLength(0); + + edit(); await act(async () => { vi.advanceTimersByTime(30_000); }); await flush(); expect(calls('save')).toHaveLength(1); + + // Saved and clean again: later ticks stay quiet. + await act(async () => { vi.advanceTimersByTime(60_000); }); + await flush(); + expect(calls('save')).toHaveLength(1); + }); + + test('the Save button still saves a clean editor', async () => { + respondWith({}); + await mount(); + await clickSave(); + expect(calls('save')).toHaveLength(1); + }); +}); + +describe('Draft badge', () => { + test('a manual save with nothing changed does not flip the badge to Draft; a save after real changes does', async () => { + respondWith({}); + await mount(); + await clickSave(); + expect(calls('save')).toHaveLength(1); expect(q('.publish-badge.draft')).toBeNull(); edit(); diff --git a/craft/src/hooks/useWhpApi.export-cache.test.ts b/craft/src/hooks/useWhpApi.export-cache.test.ts new file mode 100644 index 0000000..4c2db7d --- /dev/null +++ b/craft/src/hooks/useWhpApi.export-cache.test.ts @@ -0,0 +1,83 @@ +import { describe, test, expect, vi, beforeEach } from 'vitest'; + +vi.mock('../utils/html-export', () => ({ + exportBodyHtml: vi.fn((state: string) => { + if (state.startsWith('BAD')) throw new Error('export boom'); + return { html: `

${state}

`, css: `/*${state}*/` }; + }), +})); + +import { exportBodyHtml } from '../utils/html-export'; +import { buildSavePayload, clearExportCache } from './useWhpApi'; +import { PageData } from '../types'; +import { DEFAULT_SITE_DESIGN } from '../state/SiteDesignContext'; + +const spy = vi.mocked(exportBodyHtml); +const page = (id: string, craftState: string | null): PageData => ({ id, name: id, slug: id, craftState }); +const header = page('__header__', 'H1'); +const footer = page('__footer__', 'F1'); + +function build(pages: PageData[], live: string, activePageId = pages[0].id, h = header, f = footer) { + return buildSavePayload({ + siteId: 1, siteName: 's', liveCraftState: live, pages, headerPage: h, footerPage: f, + activePageId, isEditingHeader: false, isEditingFooter: false, + headCode: DEFAULT_SITE_DESIGN.headCode, design: DEFAULT_SITE_DESIGN, + }); +} +const exported = () => spy.mock.calls.map(([s]) => s); + +beforeEach(() => { + clearExportCache(); + spy.mockClear(); + vi.spyOn(console, 'error').mockImplementation(() => {}); +}); + +describe('buildSavePayload export memoisation', () => { + test('an identical second build only re-exports the live canvas page', () => { + const pages = [page('home', 'HOME'), page('a', 'A'), page('b', 'B')]; + build(pages, 'LIVE1'); + expect(exported().sort()).toEqual(['A', 'B', 'F1', 'H1', 'LIVE1']); + spy.mockClear(); + const p = build(pages, 'LIVE2'); + expect(exported()).toEqual(['LIVE2']); + expect(p.pages.map((x) => x.html)).toEqual(['

LIVE2

', '

A

', '

B

']); + }); + + test('only the page whose state changed is re-exported', () => { + const pages = [page('home', 'HOME'), page('a', 'A'), page('b', 'B')]; + build(pages, 'LIVE'); + spy.mockClear(); + const p = build([pages[0], page('a', 'A2'), pages[2]], 'LIVE'); + expect(exported().sort()).toEqual(['A2', 'LIVE']); + expect(p.pages[1].html).toBe('

A2

'); + expect(p.pages[2].html).toBe('

B

'); + }); + + test('header and footer are memoised the same way', () => { + const pages = [page('home', 'HOME')]; + build(pages, 'LIVE'); + spy.mockClear(); + const p = build(pages, 'LIVE', 'home', page('__header__', 'H2')); + expect(exported().sort()).toEqual(['H2', 'LIVE']); + expect(p.header_html).toBe('

H2

'); + expect(p.footer_html).toBe('

F1

'); + }); + + test('a failed export is not cached and is retried on the next build', () => { + const pages = [page('home', 'HOME'), page('x', 'BAD1')]; + const first = build(pages, 'LIVE'); + expect(first.pages[1].html).toBe(''); + spy.mockClear(); + build(pages, 'LIVE'); + expect(exported()).toContain('BAD1'); + }); + + test('a deleted page is evicted, so re-adding its state exports again', () => { + const pages = [page('home', 'HOME'), page('a', 'A')]; + build(pages, 'LIVE'); + build([pages[0]], 'LIVE'); + spy.mockClear(); + build(pages, 'LIVE'); + expect(exported()).toContain('A'); + }); +}); diff --git a/craft/src/hooks/useWhpApi.ts b/craft/src/hooks/useWhpApi.ts index b830408..873c65d 100644 --- a/craft/src/hooks/useWhpApi.ts +++ b/craft/src/hooks/useWhpApi.ts @@ -8,6 +8,20 @@ import { repairOrphanNodes } from '../utils/orphan-repair'; import { PageData } from '../types'; import { useSaveGuard } from '../state/SaveGuardContext'; +/** + * Per-state export memo. A craft-state string fully determines its exported + * HTML/CSS, so unchanged pages skip the export on every autosave. Only + * stored (non-live) states are cached, failures never are, and each build + * evicts every entry it did not use, so deleted/edited pages cannot pile up. + */ +type ExportResult = ReturnType; +const exportCache = new Map(); + +/** Test hook: forget every memoised export. */ +export function clearExportCache() { + exportCache.clear(); +} + export interface BuildSavePayloadInput { siteId: number; siteName: string; @@ -58,6 +72,18 @@ export function buildSavePayload(input: BuildSavePayloadInput) { const isPageActive = !isEditingHeader && !isEditingFooter; + // Stored states touched by this build; the cache is pruned to this set. + const usedStates = new Set(); + /** Export a stored (not live) state, reusing the memo. Throws like exportBodyHtml. */ + const exportStored = (state: string): ExportResult => { + usedStates.add(state); + const hit = exportCache.get(state); + if (hit) return hit; + const result = exportBodyHtml(state); + exportCache.set(state, result); + return result; + }; + // I-1 (data-loss): `activePageId` can go dangling -- e.g. the original // Home page (id 'home') is deleted, its replacement gets a fresh id // (`page_`), and a reload re-initializes `activePageId` back to the @@ -79,7 +105,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) { let headerHtml = ''; try { if (headerCraftState) { - headerHtml = exportBodyHtml(headerCraftState).html; + headerHtml = (isEditingHeader ? exportBodyHtml(headerCraftState) : exportStored(headerCraftState)).html; } } catch (e) { console.error('Header HTML export failed:', e); @@ -88,7 +114,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) { let footerHtml = ''; try { if (footerCraftState) { - footerHtml = exportBodyHtml(footerCraftState).html; + footerHtml = (isEditingFooter ? exportBodyHtml(footerCraftState) : exportStored(footerCraftState)).html; } } catch (e) { console.error('Footer HTML export failed:', e); @@ -117,7 +143,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) { currentCraftState = landingPage?.craftState ?? null; try { if (currentCraftState) { - const result = exportBodyHtml(currentCraftState); + const result = exportStored(currentCraftState); currentHtml = result.html; css = result.css; } @@ -141,7 +167,7 @@ export function buildSavePayload(input: BuildSavePayloadInput) { pageHtml = currentHtml; } else if (page.craftState) { try { - pageHtml = exportBodyHtml(page.craftState).html; + pageHtml = exportStored(page.craftState).html; } catch (e) { console.error(`HTML export failed for page ${page.name}:`, e); } @@ -179,6 +205,10 @@ export function buildSavePayload(input: BuildSavePayloadInput) { seo: page.seo, })); + for (const key of exportCache.keys()) { + if (!usedStates.has(key)) exportCache.delete(key); + } + return { site_id: siteId, name: siteName, diff --git a/craft/src/panels/topbar/TopBar.tsx b/craft/src/panels/topbar/TopBar.tsx index 377e270..404021b 100644 --- a/craft/src/panels/topbar/TopBar.tsx +++ b/craft/src/panels/topbar/TopBar.tsx @@ -106,9 +106,11 @@ export const TopBar: React.FC = ({ device, onDeviceChange, showGuid // Paused until the site has loaded, and while the empty-save banner is up. if (!isWHP || !canSave || emptyRefusal) return; // A running publish does its own save; a second one would race it. - const interval = setInterval(() => { if (!publishingRef.current) void runSave(); }, 30000); + // Read through refs/isDirty() at tick time, never a stale closure. + const autosaveDue = () => isDirty() && !publishingRef.current; + const interval = setInterval(() => { if (autosaveDue()) void runSave(); }, 30000); return () => clearInterval(interval); - }, [isWHP, canSave, emptyRefusal, runSave]); + }, [isWHP, canSave, emptyRefusal, runSave, isDirty]); const handleSave = runSave;