Skip to content

fix(MultiSelect): Resolve infinite loop when using mode="immediate" and Svelte 5 #508

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hot-stingrays-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte-ux': patch
---

fix(MultiSelect): Resolve infinite loop when using `mode="immediate"` and Svelte 5
23 changes: 13 additions & 10 deletions packages/svelte-ux/src/lib/components/MultiSelect.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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<TValue>[];
export let value: TValue[] = [];
Expand Down Expand Up @@ -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<TValue>, searchText: string) {
Expand All @@ -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);
Expand Down
43 changes: 27 additions & 16 deletions packages/svelte-ux/src/lib/stores/selectionStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ export default function selectionStore<T>(props: SelectionProps<T> = {}) {
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) {
Expand All @@ -49,6 +51,22 @@ export default function selectionStore<T>(props: SelectionProps<T> = {}) {
});
}

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));
}
Expand All @@ -61,16 +79,8 @@ export default function selectionStore<T>(props: SelectionProps<T> = {}) {
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() {
Expand All @@ -85,6 +95,7 @@ export default function selectionStore<T>(props: SelectionProps<T> = {}) {

return {
selected: single ? (selectedArr[0] ?? null) : selectedArr,
setSelected,
toggleSelected,
isSelected,
isDisabled,
Expand Down