From 2524959f59f72ce968624f3235e0b64ba30a88b6 Mon Sep 17 00:00:00 2001 From: Sean Lynch Date: Sat, 2 Nov 2024 23:31:23 -0400 Subject: [PATCH] fix(MultiSelect): Resolve infinite loop when using `mode="immediate"` and Svelte 5 (#508) * feat(selectionStore): Add `setSelection()` method to override selection (or (re-)initialize after creation) * fix(MultiSelect): Resolve infinite loop when using `mode="immediate"` and Svelte 5. Resovles #507 --- .changeset/hot-stingrays-move.md | 5 +++ .../src/lib/components/MultiSelect.svelte | 23 +++++----- .../src/lib/stores/selectionStore.ts | 43 ++++++++++++------- 3 files changed, 45 insertions(+), 26 deletions(-) create mode 100644 .changeset/hot-stingrays-move.md diff --git a/.changeset/hot-stingrays-move.md b/.changeset/hot-stingrays-move.md new file mode 100644 index 000000000..404af3b4e --- /dev/null +++ b/.changeset/hot-stingrays-move.md @@ -0,0 +1,5 @@ +--- +'svelte-ux': patch +--- + +fix(MultiSelect): Resolve infinite loop when using `mode="immediate"` and Svelte 5 diff --git a/packages/svelte-ux/src/lib/components/MultiSelect.svelte b/packages/svelte-ux/src/lib/components/MultiSelect.svelte index 6f487669e..24ca3f1d2 100644 --- a/packages/svelte-ux/src/lib/components/MultiSelect.svelte +++ b/packages/svelte-ux/src/lib/components/MultiSelect.svelte @@ -3,7 +3,8 @@ import { type ComponentProps, createEventDispatcher } from 'svelte'; import { flip } from 'svelte/animate'; - import { get, partition } from 'lodash-es'; + import { get } from 'svelte/store'; + import { partition, isEqual } from 'lodash-es'; import { mdiMagnify } from '@mdi/js'; @@ -17,6 +18,7 @@ import selectionStore from '../stores/selectionStore.js'; import uniqueStore from '../stores/uniqueStore.js'; import { cls } from '../utils/styles.js'; + import changeStore from '../stores/changeStore.js'; export let options: MenuOption[]; export let value: TValue[] = []; @@ -82,15 +84,11 @@ onChange(); } - $: if (mode === 'immediate' && $selection) { - applyChange(); - } - export let searchText = ''; let applying = false; // Partition options based on if they initially selected, which will be displayed at the top - $: [selectedOptions, unselectedOptions] = partition(options, (x) => value.includes(x.value)); + $: [selectedOptions, unselectedOptions] = partition(options, (o) => value.includes(o.value)); // Filter by search text function applyFilter(option: MenuOption, searchText: string) { @@ -105,10 +103,15 @@ $: filteredSelectedOptions = selectedOptions.filter((x) => applyFilter(x, searchText)); $: filteredUnselectedOptions = unselectedOptions.filter((x) => applyFilter(x, searchText)); - $: selection = selectionStore({ - initial: selectedOptions.map((x) => x.value), - max, - }); + const selection = selectionStore({ max }); + // Only "subscribe" to value changes (not `$selection`) to fix correct `value` / topological ordering. Should be simplified with Svelte 5 + $: get(selection).setSelected(value); + + // Immediately apply only if changed + const changed = changeStore(selection); + $: if (mode === 'immediate' && !isEqual($selection.selected, $changed.previous?.selected)) { + applyChange(); + } $: isSelectionDirty = dirtyStore(selection); $: indeterminateStore = uniqueStore(indeterminateSelected); diff --git a/packages/svelte-ux/src/lib/stores/selectionStore.ts b/packages/svelte-ux/src/lib/stores/selectionStore.ts index fc7f871ad..e4a39f3dd 100644 --- a/packages/svelte-ux/src/lib/stores/selectionStore.ts +++ b/packages/svelte-ux/src/lib/stores/selectionStore.ts @@ -22,12 +22,14 @@ export default function selectionStore(props: SelectionProps = {}) { const max = props.max; return derived([selected, all], ([$selected, $all]) => { - function isSelected(value: T) { - return $selected.has(value); - } - - function isDisabled(value: T) { - return !isSelected(value) && isMaxSelected(); + function setSelected(values: T[]) { + selected.update(($selected) => { + if (max == null || values.length < max) { + return new Set(values); + } else { + return $selected; + } + }); } function toggleSelected(value: T) { @@ -49,6 +51,22 @@ export default function selectionStore(props: SelectionProps = {}) { }); } + function toggleAll() { + let values: T[]; + if (isAllSelected()) { + // Deselect all (within current `all`, for example page/filtered result) + values = [...$selected].filter((v) => !$all.includes(v)); + } else { + // Select all (`new Set()` will dedupe) + values = [...$selected, ...$all]; + } + selected.set(new Set(values)); + } + + function isSelected(value: T) { + return $selected.has(value); + } + function isAllSelected() { return $all.every((v) => $selected.has(v)); } @@ -61,16 +79,8 @@ export default function selectionStore(props: SelectionProps = {}) { return max != null ? $selected.size >= max : false; } - function toggleAll() { - let values: T[]; - if (isAllSelected()) { - // Deselect all (within current `all`, for example page/filtered result) - values = [...$selected].filter((v) => !$all.includes(v)); - } else { - // Select all (`new Set()` will dedupe) - values = [...$selected, ...$all]; - } - selected.set(new Set(values)); + function isDisabled(value: T) { + return !isSelected(value) && isMaxSelected(); } function clear() { @@ -85,6 +95,7 @@ export default function selectionStore(props: SelectionProps = {}) { return { selected: single ? (selectedArr[0] ?? null) : selectedArr, + setSelected, toggleSelected, isSelected, isDisabled,