diff --git a/src/store/labelStore.test.ts b/src/store/labelStore.test.ts index b86d11b2..b3a507c1 100644 --- a/src/store/labelStore.test.ts +++ b/src/store/labelStore.test.ts @@ -1,8 +1,34 @@ -import { describe, it, expect, beforeEach } from 'vitest'; -import { useLabelStore, currentObjects } from './labelStore'; +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import type * as Labelary from '../lib/labelary'; +import { + useLabelStore, + currentObjects, + __resetPreviewCacheForTests, +} from './labelStore'; import { isGroup, type LabelObject } from '../types/Group'; import { defined, props } from '../test/helpers'; +// Mock the Labelary client so cache tests run without network I/O. +// `fetchPreview` returns a unique blob URL on each call so the test can +// see when a re-fetch happened vs. a cache hit (same URL = cached). +vi.mock('../lib/labelary', async (importOriginal) => { + const actual = await importOriginal(); + let counter = 0; + return { + ...actual, + fetchPreview: vi.fn(async () => `blob:mock-${++counter}`), + }; +}); + +// URL.revokeObjectURL is a no-op stub in jsdom by default but emits a +// warning in some setups; provide an explicit spy so we can also assert +// the cache revokes the previous URL on miss without flagging warnings. +// Also drop any cached preview entry so the cache tests stay isolated. +beforeEach(() => { + globalThis.URL.revokeObjectURL = vi.fn(); + __resetPreviewCacheForTests(); +}); + /** Reset store to clean state before each test. */ function reset() { useLabelStore.setState({ @@ -943,4 +969,95 @@ describe('ungroup', () => { expect(state().previewMode.status).toBe('idle'); }); }); + + describe('preview cache', () => { + function activeUrl(): string | null { + const mode = state().previewMode; + return mode.status === 'active' ? mode.url : null; + } + + it('skips the fetch when the ZPL is unchanged across toggles', async () => { + const labelary = await import('../lib/labelary'); + const fetchSpy = vi.mocked(labelary.fetchPreview); + fetchSpy.mockClear(); + + await state().enterPreviewMode(); + expect(state().previewMode.status).toBe('active'); + expect(fetchSpy).toHaveBeenCalledTimes(1); + const firstUrl = activeUrl(); + + state().exitPreviewMode(); + expect(state().previewMode.status).toBe('idle'); + + // Re-open with no changes: cache hit, no extra fetch, same URL. + await state().enterPreviewMode(); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(activeUrl()).toBe(firstUrl); + }); + + it('discards a stale in-flight fetch when the user exits, edits, and re-enters', async () => { + // Scenario: user opens preview (fetch 1 in flight), exits, mutates + // design, opens preview again (fetch 2 in flight). Without the + // stale-fetch guard, fetch 1 resolves while status is `loading` + // (from fetch 2) and would overwrite state with the previous + // design's URL. + const labelary = await import('../lib/labelary'); + const fetchSpy = vi.mocked(labelary.fetchPreview); + fetchSpy.mockClear(); + + // Hand-controlled promises so we can interleave the two fetches. + type Resolver = (url: string) => void; + const noop: Resolver = () => undefined; + let resolveFirst: Resolver = noop; + let resolveSecond: Resolver = noop; + fetchSpy.mockImplementationOnce( + () => new Promise((r) => (resolveFirst = r)), + ); + fetchSpy.mockImplementationOnce( + () => new Promise((r) => (resolveSecond = r)), + ); + + const first = state().enterPreviewMode(); + state().exitPreviewMode(); + state().addObject('text'); // changes the ZPL + const second = state().enterPreviewMode(); + + resolveFirst('blob:stale-1'); + await first; + + // Status must still be `loading` for fetch 2 — fetch 1's URL was + // for the previous design and must have been discarded + revoked. + expect(state().previewMode.status).toBe('loading'); + expect(vi.mocked(globalThis.URL.revokeObjectURL)).toHaveBeenCalledWith( + 'blob:stale-1', + ); + + resolveSecond('blob:fresh-2'); + await second; + + expect(state().previewMode.status).toBe('active'); + expect(activeUrl()).toBe('blob:fresh-2'); + }); + + it('re-fetches and revokes the stale URL when the ZPL changes', async () => { + const labelary = await import('../lib/labelary'); + const fetchSpy = vi.mocked(labelary.fetchPreview); + fetchSpy.mockClear(); + const revokeSpy = vi.mocked(globalThis.URL.revokeObjectURL); + + await state().enterPreviewMode(); + const firstUrl = activeUrl(); + expect(firstUrl).toBeTruthy(); + state().exitPreviewMode(); + + // Mutate the design — the next enterPreviewMode generates different + // ZPL and must hit the API again, revoking the prior blob. + state().addObject('text'); + await state().enterPreviewMode(); + + expect(fetchSpy).toHaveBeenCalledTimes(2); + expect(revokeSpy).toHaveBeenCalledWith(firstUrl); + expect(activeUrl()).not.toBe(firstUrl); + }); + }); }); diff --git a/src/store/labelStore.ts b/src/store/labelStore.ts index 2c239b7e..d2ad6a60 100644 --- a/src/store/labelStore.ts +++ b/src/store/labelStore.ts @@ -250,6 +250,42 @@ function updateCurrentObjects( * multiplies it by pasteCount because the clipboard source stays put. */ const DUPLICATE_OFFSET_DOTS = 20; +/** Single-entry cache for the Labelary preview blob URL, keyed by the + * exact ZPL string that produced it. Module-level rather than store- + * state because the blob URL is a non-serialisable side-effect handle: + * persisting it through `partialize` would resurrect a stale identifier + * across reloads, and including it in Zustand state would churn every + * selector that observes the store. + * + * The closure owns the URL: `set` revokes the previous blob before + * replacing it, so callers can't leak by forgetting to clean up. */ +const previewCache = (() => { + let entry: { zpl: string; url: string } | null = null; + return { + /** Returns the cached URL if `zpl` matches the cached key, else null. */ + get(zpl: string): string | null { + return entry && entry.zpl === zpl ? entry.url : null; + }, + /** Stores a fresh (zpl, url) pair. Revokes the previously held URL + * if any so the browser can reclaim the blob memory. */ + set(zpl: string, url: string): void { + if (entry) URL.revokeObjectURL(entry.url); + entry = { zpl, url }; + }, + /** Test-only: drop the cached entry without revoking, so a fresh + * test starts from a clean slate. Production callers should never + * need this — `set` handles eviction on its own. */ + _resetForTests(): void { + entry = null; + }, + }; +})(); + +/** Test-only handle to clear the preview cache between test cases. + * Marked underscored to discourage production use; the cache otherwise + * manages its own lifecycle via the `set` revoke path. */ +export const __resetPreviewCacheForTests = (): void => previewCache._resetForTests(); + /** Build offset copies of objects identified by `ids`. Missing ids are * silently dropped. Props are shallow-cloned to match the pattern in * copySelectedObjects — even though no current code path mutates props, @@ -840,29 +876,47 @@ export const useLabelStore = create()( if (state.previewMode.status === 'loading' || state.previewMode.status === 'active') { return; } - set({ previewMode: { status: 'loading' } }); const objs = currentObjects(state); const zpl = generateZPL(state.label, objs); + // Toggling preview off then on for a side-by-side pixel compare + // shouldn't burn an API call when nothing changed. + const cachedUrl = previewCache.get(zpl); + if (cachedUrl !== null) { + set({ previewMode: { status: 'active', url: cachedUrl } }); + return; + } + set({ previewMode: { status: 'loading' } }); + // Two checks guard against settling a stale request: the status + // check catches an exit that happened during the fetch; the + // reference-equality check catches the harder case where the + // user exited AND re-entered with a different design (so status + // is `loading` again — but for a different request whose result + // we mustn't overwrite). The store mutates label and objects + // immutably, so a reference change is the cheapest, most + // precise way to detect a divergent state — no string rebuild + // needed, and a page switch is caught too (different array). + const isStale = (): boolean => + get().previewMode.status !== 'loading' || + get().label !== state.label || + currentObjects(get()) !== objs; try { const url = await fetchPreview(zpl, state.label); - // Avoid clobbering an exit that happened while the request was in - // flight — if the user toggled off, the loading state is gone. - if (get().previewMode.status !== 'loading') { + if (isStale()) { URL.revokeObjectURL(url); return; } + previewCache.set(zpl, url); set({ previewMode: { status: 'active', url } }); } catch (e) { - if (get().previewMode.status !== 'loading') return; + if (isStale()) return; set({ previewMode: { status: 'error', error: labelaryErrorMessage(e) } }); } }, exitPreviewMode: () => set((state) => { - if (state.previewMode.status === 'active') { - URL.revokeObjectURL(state.previewMode.url); - } + // The blob URL is owned by `previewCache` and intentionally + // kept alive across exits so a re-toggle skips the fetch. if (state.previewMode.status === 'idle') return {}; return { previewMode: { status: 'idle' } }; }),