refactor(filters): mount-scope store bindings and fix effect-update loop
Replace the side-effect-on-import $effect.root in bindings with an explicit startFilterBindings() started from an AppBindings provider in onMount, so the filters/sort -> font-catalog bridge has a lifecycle tied to the app tree and a returned cleanup. bindings now consumes getFontCatalog(). Fix the effect-update loop this surfaced: setGroups populated the reactive groups array in place via `groups.length = 0; groups.push(...)`. push reads the array's length signal, so the populating effect both read and wrote groups.length each run and re-triggered itself forever (effect_update_depth_exceeded). setGroups now reassigns the array (groups is `let`), which does not read length. Extract mapFilterMetadataToGroups to own the metadata -> group-config mapping, including sorting a copy of options (the source is TanStack-cached data; an in-place sort corrupts the cache and writes into the effect's read dependency).
This commit is contained in:
+9
-4
@@ -16,12 +16,17 @@
|
||||
*/
|
||||
import '$routes/router';
|
||||
import { Router } from 'sv-router';
|
||||
import { QueryProvider } from './providers';
|
||||
import {
|
||||
AppBindingsProvider,
|
||||
QueryProvider,
|
||||
} from './providers';
|
||||
import Layout from './ui/Layout.svelte';
|
||||
</script>
|
||||
|
||||
<QueryProvider>
|
||||
<Layout>
|
||||
<Router />
|
||||
</Layout>
|
||||
<AppBindingsProvider>
|
||||
<Layout>
|
||||
<Router />
|
||||
</Layout>
|
||||
</AppBindingsProvider>
|
||||
</QueryProvider>
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
<!--
|
||||
Component: AppBindings
|
||||
Provider that starts app-wide store bindings (filters → sort → font catalog)
|
||||
for its subtree. Mount-scoped so the bindings' lifetime tracks the app tree.
|
||||
-->
|
||||
<script lang="ts">
|
||||
import { startFilterBindings } from '$features/FilterAndSortFonts';
|
||||
import { onMount } from 'svelte';
|
||||
import type { Snippet } from 'svelte';
|
||||
|
||||
interface Props {
|
||||
/**
|
||||
* Content snippet
|
||||
*/
|
||||
children?: Snippet;
|
||||
}
|
||||
|
||||
let { children }: Props = $props();
|
||||
|
||||
// startFilterBindings returns its $effect.root cleanup; onMount runs it on unmount.
|
||||
onMount(() => startFilterBindings());
|
||||
</script>
|
||||
|
||||
{@render children?.()}
|
||||
@@ -1 +1,2 @@
|
||||
export { default as AppBindingsProvider } from './AppBindings.svelte';
|
||||
export { default as QueryProvider } from './QueryProvider.svelte';
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
export { mapAppliedFiltersToParams } from './lib';
|
||||
|
||||
export {
|
||||
type AppliedFilterStore,
|
||||
appliedFilterStore,
|
||||
/**
|
||||
* Filter Store
|
||||
@@ -16,9 +15,14 @@ export {
|
||||
*/
|
||||
SORT_MAP,
|
||||
SORT_OPTIONS,
|
||||
type SortApiValue,
|
||||
type SortOption,
|
||||
sortStore,
|
||||
startFilterBindings,
|
||||
} from './model';
|
||||
|
||||
export type {
|
||||
AppliedFilterStore,
|
||||
SortApiValue,
|
||||
SortOption,
|
||||
} from './model';
|
||||
|
||||
export {
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import {
|
||||
describe,
|
||||
expect,
|
||||
it,
|
||||
} from 'vitest';
|
||||
import type {
|
||||
FilterMetadata,
|
||||
FilterOption,
|
||||
} from '../../api/filters/filters';
|
||||
import { mapFilterMetadataToGroups } from './mapFilterMetadataToGroups';
|
||||
|
||||
/**
|
||||
* Build a FilterOption with a known value and count.
|
||||
*/
|
||||
function option(value: string, count: number): FilterOption {
|
||||
return { id: value, name: value, value, count };
|
||||
}
|
||||
|
||||
/**
|
||||
* Build filter metadata for one group from (value, count) entries.
|
||||
*/
|
||||
function metadata(id: string, options: Array<[string, number]>): FilterMetadata {
|
||||
return {
|
||||
id,
|
||||
name: id,
|
||||
description: '',
|
||||
type: 'array',
|
||||
options: options.map(([value, count]) => option(value, count)),
|
||||
};
|
||||
}
|
||||
|
||||
describe('mapFilterMetadataToGroups', () => {
|
||||
it('maps id and name onto group id and label', () => {
|
||||
const [group] = mapFilterMetadataToGroups([metadata('categories', [['serif', 1]])]);
|
||||
|
||||
expect(group.id).toBe('categories');
|
||||
expect(group.label).toBe('categories');
|
||||
});
|
||||
|
||||
it('projects each option to a property with selected: false', () => {
|
||||
const [group] = mapFilterMetadataToGroups([metadata('providers', [['google', 5]])]);
|
||||
|
||||
expect(group.properties).toEqual([
|
||||
{ id: 'google', name: 'google', value: 'google', selected: false },
|
||||
]);
|
||||
});
|
||||
|
||||
it('orders properties by descending count', () => {
|
||||
const [group] = mapFilterMetadataToGroups([
|
||||
metadata('subsets', [['latin', 2], ['cyrillic', 9], ['greek', 5]]),
|
||||
]);
|
||||
|
||||
expect(group.properties.map(p => p.value)).toEqual(['cyrillic', 'greek', 'latin']);
|
||||
});
|
||||
|
||||
it('does not mutate the source options array (TanStack cache safety)', () => {
|
||||
const source = metadata('subsets', [['latin', 2], ['cyrillic', 9]]);
|
||||
const originalOrder = source.options.map(o => o.value);
|
||||
|
||||
mapFilterMetadataToGroups([source]);
|
||||
|
||||
expect(source.options.map(o => o.value)).toEqual(originalOrder);
|
||||
});
|
||||
|
||||
it('maps every group, preserving group order', () => {
|
||||
const groups = mapFilterMetadataToGroups([
|
||||
metadata('providers', [['google', 1]]),
|
||||
metadata('categories', [['serif', 1]]),
|
||||
]);
|
||||
|
||||
expect(groups.map(g => g.id)).toEqual(['providers', 'categories']);
|
||||
});
|
||||
|
||||
it('returns an empty group list for empty metadata', () => {
|
||||
expect(mapFilterMetadataToGroups([])).toEqual([]);
|
||||
});
|
||||
|
||||
it('yields an empty properties list when a group has no options', () => {
|
||||
const [group] = mapFilterMetadataToGroups([metadata('providers', [])]);
|
||||
|
||||
expect(group.properties).toEqual([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,36 @@
|
||||
import type { FilterMetadata } from '../../api/filters/filters';
|
||||
import type { FilterGroupConfig } from '../../model';
|
||||
|
||||
/**
|
||||
* Map backend filter metadata into the group configs `appliedFilterStore.setGroups`
|
||||
* consumes.
|
||||
*
|
||||
* Inverse direction of `mapAppliedFiltersToParams`: that maps applied selections out
|
||||
* to API params; this maps the API's available-filter catalog in to the UI model.
|
||||
*
|
||||
* Options are ordered by descending font count so the most populated values surface
|
||||
* first. The source array is copied before sorting — `metadata` is TanStack-cached
|
||||
* query data, and `.sort()` mutates in place; sorting the live cache both corrupts it
|
||||
* and, when called from a reactive effect, writes into that effect's own read
|
||||
* dependency (triggering an update loop).
|
||||
*
|
||||
* Every property starts unselected; selection state is owned by the store, not the
|
||||
* backend catalog.
|
||||
*
|
||||
* @param metadata - Available-filter catalog from the filters endpoint
|
||||
* @returns Group configs ready for `setGroups`
|
||||
*/
|
||||
export function mapFilterMetadataToGroups(metadata: FilterMetadata[]): FilterGroupConfig<string>[] {
|
||||
return metadata.map(filter => ({
|
||||
id: filter.id,
|
||||
label: filter.name,
|
||||
properties: [...filter.options]
|
||||
.sort((a, b) => b.count - a.count)
|
||||
.map(opt => ({
|
||||
id: opt.id,
|
||||
name: opt.name,
|
||||
value: opt.value,
|
||||
selected: false,
|
||||
})),
|
||||
}));
|
||||
}
|
||||
@@ -41,7 +41,7 @@ export {
|
||||
* Side-effect import: installs the global appliedFilterStore+sortStore → fontCatalogStore
|
||||
* bridge on first import of this feature barrel. No exports.
|
||||
*/
|
||||
import './store/bindings.svelte';
|
||||
export { startFilterBindings } from './store/bindings.svelte';
|
||||
|
||||
/**
|
||||
* Sorting logic
|
||||
|
||||
+12
-10
@@ -42,8 +42,13 @@ import type {
|
||||
export function createAppliedFilterStore<TValue extends string>(config: FilterConfig<TValue>) {
|
||||
const search = createDebouncedState(config.queryValue ?? '');
|
||||
|
||||
// Create filter instances upfront
|
||||
const groups = $state(
|
||||
// Create filter instances upfront.
|
||||
// `let` (not `const`) so setGroups can REASSIGN the whole array. In-place
|
||||
// `groups.length = 0; groups.push(...)` is forbidden here: push reads the
|
||||
// array's length signal, so a $effect that calls setGroups would both read
|
||||
// and write `groups.length` in one run and re-trigger itself forever
|
||||
// (effect_update_depth_exceeded).
|
||||
let groups = $state(
|
||||
config.groups.map(config => ({
|
||||
id: config.id,
|
||||
label: config.label,
|
||||
@@ -62,14 +67,11 @@ export function createAppliedFilterStore<TValue extends string>(config: FilterCo
|
||||
* Used when dynamic filter data loads from backend
|
||||
*/
|
||||
setGroups(newGroups: FilterGroupConfig<TValue>[]) {
|
||||
groups.length = 0;
|
||||
groups.push(
|
||||
...newGroups.map(g => ({
|
||||
id: g.id,
|
||||
label: g.label,
|
||||
instance: createFilter({ properties: g.properties }),
|
||||
})),
|
||||
);
|
||||
groups = newGroups.map(g => ({
|
||||
id: g.id,
|
||||
label: g.label,
|
||||
instance: createFilter({ properties: g.properties }),
|
||||
}));
|
||||
},
|
||||
/**
|
||||
* Current search query value (immediate, for UI binding)
|
||||
|
||||
@@ -9,52 +9,30 @@
|
||||
* observer, so it lives at module scope, not in any individual widget.
|
||||
*/
|
||||
|
||||
import { fontCatalogStore } from '$entities/Font/model';
|
||||
import { getFontCatalog } from '$entities/Font/model';
|
||||
import { untrack } from 'svelte';
|
||||
import { mapAppliedFiltersToParams } from '../../lib/mapper/mapAppliedFiltersToParams';
|
||||
import { mapFilterMetadataToGroups } from '../../lib/mapper/mapFilterMetadataToGroups';
|
||||
import { appliedFilterStore } from './appliedFilterStore/appliedFilterStore.svelte';
|
||||
import { availableFilterStore } from './availableFilterStore/availableFilterStore.svelte';
|
||||
import { sortStore } from './sortStore/sortStore.svelte';
|
||||
|
||||
$effect.root(() => {
|
||||
/**
|
||||
* Populate appliedFilterStore groups when backend filter metadata resolves.
|
||||
* availableFilterStore is async; until it loads, appliedFilterStore has empty groups
|
||||
* and the UI renders nothing for them.
|
||||
*/
|
||||
$effect(() => {
|
||||
const dynamicFilters = availableFilterStore.filters;
|
||||
export function startFilterBindings(): () => void {
|
||||
const stop = $effect.root(() => {
|
||||
$effect(() => {
|
||||
const dynamicFilters = availableFilterStore.filters;
|
||||
if (dynamicFilters.length > 0) {
|
||||
appliedFilterStore.setGroups(mapFilterMetadataToGroups(dynamicFilters));
|
||||
}
|
||||
});
|
||||
|
||||
if (dynamicFilters.length > 0) {
|
||||
appliedFilterStore.setGroups(
|
||||
dynamicFilters.map(filter => ({
|
||||
id: filter.id,
|
||||
label: filter.name,
|
||||
properties: filter.options.sort((a, b) => b.count - a.count).map(opt => ({
|
||||
id: opt.id,
|
||||
name: opt.name,
|
||||
value: opt.value,
|
||||
selected: false,
|
||||
})),
|
||||
})),
|
||||
);
|
||||
}
|
||||
$effect(() => {
|
||||
const params = mapAppliedFiltersToParams(appliedFilterStore);
|
||||
const sort = sortStore.apiValue;
|
||||
const catalog = getFontCatalog();
|
||||
untrack(() => catalog.setParams({ ...params, sort }));
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* Mirror filter selections + debounced search query + sort into fontCatalogStore params.
|
||||
*
|
||||
* Filters and sort are merged into one setParams call to avoid a startup race:
|
||||
* two separate effects each issued setOptions with a different queryKey on the
|
||||
* first flush, producing an orphaned `?limit=50&offset=0` fetch immediately
|
||||
* followed by the real `?limit=50&sort=popularity&offset=0` fetch.
|
||||
*
|
||||
* untrack the write so fontCatalogStore's internal $state reads don't feed back
|
||||
* into this effect's dependency graph.
|
||||
*/
|
||||
$effect(() => {
|
||||
const params = mapAppliedFiltersToParams(appliedFilterStore);
|
||||
const sort = sortStore.apiValue;
|
||||
untrack(() => fontCatalogStore.setParams({ ...params, sort }));
|
||||
});
|
||||
});
|
||||
return stop; // hand the caller the cleanup
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user