perf(preview): cache the Labelary blob URL by ZPL string#68
Conversation
Toggling preview off then on for a side-by-side pixel compare with the editor used to fire a fresh fetch every time, even when nothing changed. Keep the (zpl, url) pair in a module-level closure with get/set semantics; the closure owns the URL so callers can't forget to revoke the stale blob on a cache miss. The slot lives outside Zustand state because the blob URL is a non- serialisable side-effect handle: persisting it through `partialize` would resurrect a stale identifier across reloads, and putting it in the store would churn every selector that observes previewMode. Two test cases exercise the cache hit + cache miss + revoke paths; a test-only `__resetPreviewCacheForTests` keeps them isolated since the closure outlives `useLabelStore.setState`.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a single-entry cache for Labelary preview blob URLs to optimize performance by avoiding redundant API calls when the ZPL remains unchanged. The cache is managed outside the store state to handle non-serializable blob URLs and ensure proper memory cleanup via URL.revokeObjectURL. Feedback highlights a race condition where a stale fetch might update the store with an incorrect URL if the design is modified while a request is pending; a validation check was suggested to ensure the returned preview matches the current design.
Gemini's PR review flagged a race the audit missed: if the user opens preview, exits while the fetch is still in flight, edits the design, and re-opens preview, two fetches end up overlapping. The first one resolves while `status === 'loading'` (which it is again, for the new request) and the old check let it through — clobbering the new loading state with the previous design's URL and caching a stale pair. Add a `zpl !== currentZpl` check next to the status check, gated through one `isStale()` helper used by both the success and the error paths. Captured `zpl` in the closure is the ground truth for which request this continuation belongs to. Regression test stages the exact scenario with hand-controlled fetchPreview promises: enter → exit → mutate → enter → resolve the first (stale) → assert state stays `loading` and the stale blob got revoked → resolve the second → assert the fresh URL is the one that lands in state.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements a single-entry cache for Labelary preview blob URLs to prevent unnecessary API calls when toggling the preview mode on an unchanged design. It also introduces a mechanism to discard stale in-flight preview requests if the user exits or modifies the design before the fetch completes. A review comment suggests optimizing the staleness check by comparing object and label references instead of re-generating the ZPL string, which would enhance performance for complex labels.
Gemini PR feedback: regenerating the ZPL string each time a fetch resolves is wasted work when the store already mutates `label` and the objects array immutably — a reference compare is O(1) versus O(generator pass + coordinate conversions) and detects the exact same "state diverged while we were fetching" condition. As a bonus, the reference compare catches a page switch (which yields a different `objects` array) even when both pages happen to generate identical ZPL by coincidence; the previous string-based check would let that through.
1 participant
No description provided.