diff --git a/src/widgets/ComparisonView/model/stores/comparisonStore.svelte.ts b/src/widgets/ComparisonView/model/stores/comparisonStore.svelte.ts index 6136a63..f43cce4 100644 --- a/src/widgets/ComparisonView/model/stores/comparisonStore.svelte.ts +++ b/src/widgets/ComparisonView/model/stores/comparisonStore.svelte.ts @@ -102,7 +102,7 @@ export class ComparisonStore { this.#fontsByIdsStore = new FontsByIdsStore(fontAId && fontBId ? [fontAId, fontBId] : []); $effect.root(() => { - // Effect 1: Sync batch results → fontA / fontB + // Sync batch results → fontA / fontB $effect(() => { const fonts = this.#fontsByIdsStore.fonts; if (fonts.length === 0) { @@ -124,7 +124,7 @@ export class ComparisonStore { } }); - // Effect 2: Trigger font loading whenever selection or weight changes + // Trigger font loading whenever selection or weight changes $effect(() => { const fa = this.#fontA; const fb = this.#fontB; @@ -154,24 +154,38 @@ export class ComparisonStore { } }); - // Effect 3: Set default fonts when storage is empty + // Set default fonts when storage is empty $effect(() => { if (this.#fontA && this.#fontB) { return; } - const fonts = fontCatalogStore.fonts; - if (fonts.length >= 2) { - untrack(() => { - const id1 = fonts[0].id; - const id2 = fonts[fonts.length - 1].id; - storage.value = { fontAId: id1, fontBId: id2 }; - this.#fontsByIdsStore.setIds([id1, id2]); - }); + // Don't clobber a pending rehydration - only seed when storage is empty. + // Untracked: only the catalog load should drive this effect, not the + // user's storage writes that happen as a result of normal selection. + const hasStoredSelection = untrack(() => { + return storage.value.fontAId !== null || storage.value.fontBId !== null; + }); + + if (hasStoredSelection) { + return; } + + const fonts = fontCatalogStore.fonts; + + if (fonts.length < 2) { + return; + } + + untrack(() => { + const id1 = fonts[0].id; + const id2 = fonts[fonts.length - 1].id; + storage.value = { fontAId: id1, fontBId: id2 }; + this.#fontsByIdsStore.setIds([id1, id2]); + }); }); - // Effect 4: Pin fontA/fontB so eviction never removes on-screen fonts + // Pin fontA/fontB so eviction never removes on-screen fonts $effect(() => { const fa = this.#fontA; const fb = this.#fontB; diff --git a/src/widgets/ComparisonView/model/stores/comparisonStore.test.ts b/src/widgets/ComparisonView/model/stores/comparisonStore.test.ts index 21fb026..9227469 100644 --- a/src/widgets/ComparisonView/model/stores/comparisonStore.test.ts +++ b/src/widgets/ComparisonView/model/stores/comparisonStore.test.ts @@ -165,6 +165,46 @@ describe('ComparisonStore', () => { expect(mockStorage._value.fontBId).toBe(mockFontB.id); }); }); + + /** + * Regression: when storage already holds the user's selection, the + * seed-defaults effect must bail out — even if it fires before the + * per-id batch returns (catalog wins the race on slow networks or + * cold reloads). Pre-fix the effect only checked fontA/fontB, both + * still undefined at this point, and clobbered storage with whatever + * the catalog had as fonts[0] / fonts[N-1]. + */ + it('should not overwrite stored IDs when batch is still in flight', async () => { + const seededA = UNIFIED_FONTS.lato; + const seededB = UNIFIED_FONTS.montserrat; + + mockStorage._value.fontAId = seededA.id; + mockStorage._value.fontBId = seededB.id; + + // Catalog defaults differ from the stored selection — if the + // effect mis-seeds, storage will flip to roboto / open-sans. + (fontCatalogStore as any).fonts = [mockFontA, mockFontB]; + + // Delay the batch so the catalog-driven effect runs first. + vi.spyOn(proxyFonts, 'fetchFontsByIds').mockImplementation( + () => new Promise(r => setTimeout(() => r([seededA, seededB]), 50)), + ); + + const store = new ComparisonStore(); + + // Let the catalog effect run; storage must be untouched. + await new Promise(r => setTimeout(r, 10)); + expect(mockStorage._value.fontAId).toBe(seededA.id); + expect(mockStorage._value.fontBId).toBe(seededB.id); + + // Batch resolves with the seeded selection — fontA/B must match. + await vi.waitFor(() => { + expect(store.fontA?.id).toBe(seededA.id); + expect(store.fontB?.id).toBe(seededB.id); + }, { timeout: 2000 }); + expect(mockStorage._value.fontAId).toBe(seededA.id); + expect(mockStorage._value.fontBId).toBe(seededB.id); + }); }); describe('Aggregate Loading State', () => {