refactor(typography): lazy getTypographySettingsStore + fix effect leak
Convert the eager typographySettingsStore singleton to getTypographySettingsStore() (+ __resetTypographySettingsStore for tests) and update barrels and consumers (TypographyMenu, FontSampler, SampleList, ComparisonView Line/SliderArea, comparisonStore + its mock). Fix a latent leak while here: the store ran $effect.root and discarded the returned cleanup, so its storage-sync effects outlived every instance. Capture the disposer and expose destroy(), which __reset now calls.
This commit is contained in:
@@ -1,9 +1,9 @@
|
|||||||
export {
|
export {
|
||||||
createTypographySettingsStore,
|
createTypographySettingsStore,
|
||||||
|
getTypographySettingsStore,
|
||||||
MULTIPLIER_L,
|
MULTIPLIER_L,
|
||||||
MULTIPLIER_M,
|
MULTIPLIER_M,
|
||||||
MULTIPLIER_S,
|
MULTIPLIER_S,
|
||||||
type TypographySettingsStore,
|
type TypographySettingsStore,
|
||||||
typographySettingsStore,
|
|
||||||
} from './model';
|
} from './model';
|
||||||
export { TypographyMenu } from './ui';
|
export { TypographyMenu } from './ui';
|
||||||
|
|||||||
@@ -5,6 +5,6 @@ export {
|
|||||||
} from './const/const';
|
} from './const/const';
|
||||||
export {
|
export {
|
||||||
createTypographySettingsStore,
|
createTypographySettingsStore,
|
||||||
|
getTypographySettingsStore,
|
||||||
type TypographySettingsStore,
|
type TypographySettingsStore,
|
||||||
typographySettingsStore,
|
|
||||||
} from './store/typographySettingsStore/typographySettingsStore.svelte';
|
} from './store/typographySettingsStore/typographySettingsStore.svelte';
|
||||||
|
|||||||
+31
-4
@@ -94,6 +94,12 @@ export class TypographySettingsStore {
|
|||||||
* The underlying font size before responsive scaling is applied
|
* The underlying font size before responsive scaling is applied
|
||||||
*/
|
*/
|
||||||
#baseSize = $state(DEFAULT_FONT_SIZE);
|
#baseSize = $state(DEFAULT_FONT_SIZE);
|
||||||
|
/**
|
||||||
|
* Disposes the $effect.root that backs the storage-sync effects.
|
||||||
|
* $effect.root lives outside component lifecycle, so callers must invoke
|
||||||
|
* destroy() to avoid leaking the subscriptions.
|
||||||
|
*/
|
||||||
|
#disposeEffects: () => void;
|
||||||
|
|
||||||
constructor(configs: ControlModel<ControlId>[], storage: PersistentStore<TypographySettings>) {
|
constructor(configs: ControlModel<ControlId>[], storage: PersistentStore<TypographySettings>) {
|
||||||
this.#storage = storage;
|
this.#storage = storage;
|
||||||
@@ -117,7 +123,7 @@ export class TypographySettingsStore {
|
|||||||
|
|
||||||
// The Sync Effect (UI -> Storage)
|
// The Sync Effect (UI -> Storage)
|
||||||
// We access .value explicitly to ensure Svelte 5 tracks the dependency
|
// We access .value explicitly to ensure Svelte 5 tracks the dependency
|
||||||
$effect.root(() => {
|
this.#disposeEffects = $effect.root(() => {
|
||||||
$effect(() => {
|
$effect(() => {
|
||||||
// EXPLICIT DEPENDENCIES: Accessing these triggers the effect
|
// EXPLICIT DEPENDENCIES: Accessing these triggers the effect
|
||||||
const fontSize = this.#baseSize;
|
const fontSize = this.#baseSize;
|
||||||
@@ -155,6 +161,13 @@ export class TypographySettingsStore {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tears down the storage-sync effects. Call on unmount / store disposal.
|
||||||
|
*/
|
||||||
|
destroy(): void {
|
||||||
|
this.#disposeEffects();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Gets initial value for a control from storage or defaults
|
* Gets initial value for a control from storage or defaults
|
||||||
*/
|
*/
|
||||||
@@ -336,10 +349,24 @@ export function createTypographySettingsStore(
|
|||||||
return new TypographySettingsStore(configs, storage);
|
return new TypographySettingsStore(configs, storage);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export type TypographySettingsStoreInstance = ReturnType<typeof createTypographySettingsStore>;
|
||||||
|
|
||||||
|
let _typographySettingsStore: TypographySettingsStoreInstance | undefined;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* App-wide typography settings singleton, keyed for the comparison view.
|
* App-wide typography settings store, keyed for the comparison view.
|
||||||
|
* Created on first access so its persistent-store sync effects aren't set up
|
||||||
|
* at module load.
|
||||||
*/
|
*/
|
||||||
export const typographySettingsStore = createTypographySettingsStore(
|
export function getTypographySettingsStore(): TypographySettingsStoreInstance {
|
||||||
|
return (_typographySettingsStore ??= createTypographySettingsStore(
|
||||||
DEFAULT_TYPOGRAPHY_CONTROLS_DATA,
|
DEFAULT_TYPOGRAPHY_CONTROLS_DATA,
|
||||||
COMPARISON_STORAGE_KEY,
|
COMPARISON_STORAGE_KEY,
|
||||||
);
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
// test-only reset, so specs don't share persisted typography state or leak effects
|
||||||
|
export function __resetTypographySettingsStore() {
|
||||||
|
_typographySettingsStore?.destroy();
|
||||||
|
_typographySettingsStore = undefined;
|
||||||
|
}
|
||||||
|
|||||||
@@ -23,7 +23,7 @@ import {
|
|||||||
MULTIPLIER_L,
|
MULTIPLIER_L,
|
||||||
MULTIPLIER_M,
|
MULTIPLIER_M,
|
||||||
MULTIPLIER_S,
|
MULTIPLIER_S,
|
||||||
typographySettingsStore,
|
getTypographySettingsStore,
|
||||||
} from '../../model';
|
} from '../../model';
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
@@ -46,6 +46,7 @@ interface Props {
|
|||||||
let { class: className, hidden = false, open = $bindable(false) }: Props = $props();
|
let { class: className, hidden = false, open = $bindable(false) }: Props = $props();
|
||||||
|
|
||||||
const responsive = getContext<ResponsiveManager>('responsive');
|
const responsive = getContext<ResponsiveManager>('responsive');
|
||||||
|
const typographySettingsStore = getTypographySettingsStore();
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Sets the common font size multiplier based on the current responsive state.
|
* Sets the common font size multiplier based on the current responsive state.
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import {
|
|||||||
type FontLoadStatus,
|
type FontLoadStatus,
|
||||||
type UnifiedFont,
|
type UnifiedFont,
|
||||||
} from '$entities/Font';
|
} from '$entities/Font';
|
||||||
import { typographySettingsStore } from '$features/AdjustTypography/model';
|
import { getTypographySettingsStore } from '$features/AdjustTypography/model';
|
||||||
import {
|
import {
|
||||||
Badge,
|
Badge,
|
||||||
ContentEditable,
|
ContentEditable,
|
||||||
@@ -43,6 +43,8 @@ interface Props {
|
|||||||
|
|
||||||
let { font, status, text = $bindable(), index = 0 }: Props = $props();
|
let { font, status, text = $bindable(), index = 0 }: Props = $props();
|
||||||
|
|
||||||
|
const typographySettingsStore = getTypographySettingsStore();
|
||||||
|
|
||||||
// Extract provider badge with fallback
|
// Extract provider badge with fallback
|
||||||
const providerBadge = $derived(
|
const providerBadge = $derived(
|
||||||
font.providerBadge
|
font.providerBadge
|
||||||
|
|||||||
@@ -24,7 +24,10 @@ import {
|
|||||||
fontLifecycleManager,
|
fontLifecycleManager,
|
||||||
getFontCatalog,
|
getFontCatalog,
|
||||||
} from '$entities/Font/model';
|
} from '$entities/Font/model';
|
||||||
import { typographySettingsStore } from '$features/AdjustTypography/model';
|
import {
|
||||||
|
type TypographySettingsStore,
|
||||||
|
getTypographySettingsStore,
|
||||||
|
} from '$features/AdjustTypography/model';
|
||||||
import { createPersistentStore } from '$shared/lib';
|
import { createPersistentStore } from '$shared/lib';
|
||||||
import { untrack } from 'svelte';
|
import { untrack } from 'svelte';
|
||||||
import { getPretextFontString } from '../../lib';
|
import { getPretextFontString } from '../../lib';
|
||||||
@@ -101,11 +104,14 @@ export class ComparisonStore {
|
|||||||
|
|
||||||
#fontCatalog: FontCatalogStore;
|
#fontCatalog: FontCatalogStore;
|
||||||
|
|
||||||
|
#typography: TypographySettingsStore;
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
// Synchronously seed the batch store with any IDs already in storage
|
// Synchronously seed the batch store with any IDs already in storage
|
||||||
const { fontAId, fontBId } = storage.value;
|
const { fontAId, fontBId } = storage.value;
|
||||||
this.#fontsByIdsStore = new FontsByIdsStore(fontAId && fontBId ? [fontAId, fontBId] : []);
|
this.#fontsByIdsStore = new FontsByIdsStore(fontAId && fontBId ? [fontAId, fontBId] : []);
|
||||||
this.#fontCatalog = getFontCatalog();
|
this.#fontCatalog = getFontCatalog();
|
||||||
|
this.#typography = getTypographySettingsStore();
|
||||||
|
|
||||||
$effect.root(() => {
|
$effect.root(() => {
|
||||||
// Sync batch results → fontA / fontB
|
// Sync batch results → fontA / fontB
|
||||||
@@ -134,7 +140,7 @@ export class ComparisonStore {
|
|||||||
$effect(() => {
|
$effect(() => {
|
||||||
const fa = this.#fontA;
|
const fa = this.#fontA;
|
||||||
const fb = this.#fontB;
|
const fb = this.#fontB;
|
||||||
const weight = typographySettingsStore.weight;
|
const weight = this.#typography.weight;
|
||||||
|
|
||||||
if (!fa || !fb) {
|
if (!fa || !fb) {
|
||||||
return;
|
return;
|
||||||
@@ -195,7 +201,7 @@ export class ComparisonStore {
|
|||||||
$effect(() => {
|
$effect(() => {
|
||||||
const fa = this.#fontA;
|
const fa = this.#fontA;
|
||||||
const fb = this.#fontB;
|
const fb = this.#fontB;
|
||||||
const w = typographySettingsStore.weight;
|
const w = this.#typography.weight;
|
||||||
if (fa) {
|
if (fa) {
|
||||||
fontLifecycleManager.pin(fa.id, w, fa.features?.isVariable);
|
fontLifecycleManager.pin(fa.id, w, fa.features?.isVariable);
|
||||||
}
|
}
|
||||||
@@ -226,8 +232,8 @@ export class ComparisonStore {
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const weight = typographySettingsStore.weight;
|
const weight = this.#typography.weight;
|
||||||
const size = typographySettingsStore.renderedSize;
|
const size = this.#typography.renderedSize;
|
||||||
const fontAName = this.#fontA?.name;
|
const fontAName = this.#fontA?.name;
|
||||||
const fontBName = this.#fontB?.name;
|
const fontBName = this.#fontB?.name;
|
||||||
|
|
||||||
@@ -356,7 +362,7 @@ export class ComparisonStore {
|
|||||||
this.#fontB = undefined;
|
this.#fontB = undefined;
|
||||||
this.#fontsByIdsStore.setIds([]);
|
this.#fontsByIdsStore.setIds([]);
|
||||||
storage.clear();
|
storage.clear();
|
||||||
typographySettingsStore.reset();
|
this.#typography.reset();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -89,12 +89,14 @@ vi.mock('$features/AdjustTypography', () => ({
|
|||||||
})),
|
})),
|
||||||
}));
|
}));
|
||||||
|
|
||||||
vi.mock('$features/AdjustTypography/model', () => ({
|
const mockTypography = vi.hoisted(() => ({
|
||||||
typographySettingsStore: {
|
|
||||||
weight: 400,
|
weight: 400,
|
||||||
renderedSize: 48,
|
renderedSize: 48,
|
||||||
reset: vi.fn(),
|
reset: vi.fn(),
|
||||||
},
|
}));
|
||||||
|
|
||||||
|
vi.mock('$features/AdjustTypography/model', () => ({
|
||||||
|
getTypographySettingsStore: () => mockTypography,
|
||||||
}));
|
}));
|
||||||
|
|
||||||
import * as proxyFonts from '$entities/Font/api/proxy/proxyFonts';
|
import * as proxyFonts from '$entities/Font/api/proxy/proxyFonts';
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ import {
|
|||||||
type ComparisonLine,
|
type ComparisonLine,
|
||||||
computeLineRenderModel,
|
computeLineRenderModel,
|
||||||
} from '$entities/Font';
|
} from '$entities/Font';
|
||||||
import { typographySettingsStore } from '$features/AdjustTypography';
|
import { getTypographySettingsStore } from '$features/AdjustTypography';
|
||||||
import { getComparisonStore } from '../../model';
|
import { getComparisonStore } from '../../model';
|
||||||
import Character from '../Character/Character.svelte';
|
import Character from '../Character/Character.svelte';
|
||||||
|
|
||||||
@@ -36,7 +36,7 @@ const comparisonStore = getComparisonStore();
|
|||||||
|
|
||||||
const model = $derived(computeLineRenderModel(line, split, windowSize));
|
const model = $derived(computeLineRenderModel(line, split, windowSize));
|
||||||
|
|
||||||
const typography = $derived(typographySettingsStore);
|
const typography = getTypographySettingsStore();
|
||||||
const fontA = $derived(comparisonStore.fontA);
|
const fontA = $derived(comparisonStore.fontA);
|
||||||
const fontB = $derived(comparisonStore.fontB);
|
const fontB = $derived(comparisonStore.fontB);
|
||||||
|
|
||||||
|
|||||||
@@ -18,7 +18,7 @@ import {
|
|||||||
MULTIPLIER_M,
|
MULTIPLIER_M,
|
||||||
MULTIPLIER_S,
|
MULTIPLIER_S,
|
||||||
TypographyMenu,
|
TypographyMenu,
|
||||||
typographySettingsStore,
|
getTypographySettingsStore,
|
||||||
} from '$features/AdjustTypography';
|
} from '$features/AdjustTypography';
|
||||||
import {
|
import {
|
||||||
type ResponsiveManager,
|
type ResponsiveManager,
|
||||||
@@ -81,7 +81,7 @@ const SLIDER_STEP_COARSE = 10;
|
|||||||
const fontA = $derived(comparisonStore.fontA);
|
const fontA = $derived(comparisonStore.fontA);
|
||||||
const fontB = $derived(comparisonStore.fontB);
|
const fontB = $derived(comparisonStore.fontB);
|
||||||
const isLoading = $derived(comparisonStore.isLoading || !comparisonStore.isReady);
|
const isLoading = $derived(comparisonStore.isLoading || !comparisonStore.isReady);
|
||||||
const typography = $derived(typographySettingsStore);
|
const typography = getTypographySettingsStore();
|
||||||
|
|
||||||
let container = $state<HTMLElement>();
|
let container = $state<HTMLElement>();
|
||||||
|
|
||||||
|
|||||||
@@ -15,7 +15,7 @@ import {
|
|||||||
} from '$entities/Font/model';
|
} from '$entities/Font/model';
|
||||||
import {
|
import {
|
||||||
TypographyMenu,
|
TypographyMenu,
|
||||||
typographySettingsStore,
|
getTypographySettingsStore,
|
||||||
} from '$features/AdjustTypography';
|
} from '$features/AdjustTypography';
|
||||||
import { FontSampler } from '$features/DisplayFont';
|
import { FontSampler } from '$features/DisplayFont';
|
||||||
import { throttle } from '$shared/lib/utils';
|
import { throttle } from '$shared/lib/utils';
|
||||||
@@ -23,6 +23,7 @@ import { Skeleton } from '$shared/ui';
|
|||||||
import { layoutManager } from '../../model';
|
import { layoutManager } from '../../model';
|
||||||
|
|
||||||
const fontCatalog = getFontCatalog();
|
const fontCatalog = getFontCatalog();
|
||||||
|
const typographySettingsStore = getTypographySettingsStore();
|
||||||
|
|
||||||
// FontSampler chrome heights — derived from Tailwind classes in FontSampler.svelte.
|
// FontSampler chrome heights — derived from Tailwind classes in FontSampler.svelte.
|
||||||
// Header: py-3 (12+12px padding) + ~32px content row ≈ 56px.
|
// Header: py-3 (12+12px padding) + ~32px content row ≈ 56px.
|
||||||
|
|||||||
Reference in New Issue
Block a user