Skip to content
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

fix: large list performance #55

Merged
merged 5 commits into from
Mar 23, 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/chilly-eagles-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"cmdk-sv": patch
---

Fixed bug where page would crash when a large list was rendered
1 change: 0 additions & 1 deletion .eslintrc.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ module.exports = {
],
'svelte/no-target-blank': 'error',
'svelte/no-immutable-reactive-statements': 'error',
'svelte/prefer-style-directive': 'error',
'svelte/no-reactive-literals': 'error',
'svelte/no-useless-mustaches': 'error',
'svelte/button-has-type': 'off',
Expand Down
157 changes: 90 additions & 67 deletions src/lib/cmdk/command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
omit,
generateId,
toWritableStores,
isHTMLElement,

Check warning on line 9 in src/lib/cmdk/command.ts

View workflow job for this annotation

GitHub Actions / Lint

'isHTMLElement' is defined but never used. Allowed unused vars must match /^_/u
isUndefined,
kbd,
removeUndefined
removeUndefined,
effect
} from '$lib/internal/index.js';

const NAME = 'Command';
Expand Down Expand Up @@ -108,19 +109,50 @@
const commandEl = writable<HTMLDivElement | null>(null);

const options = toWritableStores(omit(withDefaults, 'value', 'ids'));
const { shouldFilter, loop, filter, label } = options;

let $allItems = get(allItems);
let $allGroups = get(allGroups);
let $allIds = get(allIds);

let shouldFilter = get(options.shouldFilter);
let loop = get(options.loop);
let label = get(options.label);
let filter = get(options.filter);

effect(options.shouldFilter, ($shouldFilter) => {
shouldFilter = $shouldFilter;
});

effect(options.loop, ($loop) => {
loop = $loop;
});
effect(options.filter, ($filter) => {
filter = $filter;
});
effect(options.label, ($label) => {
label = $label;
});

effect(allItems, (v) => {
$allItems = v;
});
effect(allGroups, (v) => {
$allGroups = v;
});
effect(allIds, (v) => {
$allIds = v;
});

const context: Context = {
value: (id, value) => {
if (value !== get(allIds).get(id)) {
if (value !== $allIds.get(id)) {
allIds.update(($allIds) => {
$allIds.set(id, value);
return $allIds;
});
state.update(($state) => {
$state.filtered.items.set(id, score(value, $state.search));
const sortedState = sort($state, get(shouldFilter));
return sortedState;
return $state;
});
}
},
Expand All @@ -139,17 +171,14 @@
return $allGroups;
});
}

state.update(($state) => {
const $shouldFilter = get(shouldFilter);
const filteredState = filterItems($state, $shouldFilter);
const sortedState = sort(filteredState, $shouldFilter);
const filteredState = filterItems($state, shouldFilter);

if (!sortedState.value) {
if (!filteredState.value) {
const value = selectFirstItem();
sortedState.value = value ?? '';
filteredState.value = value ?? '';
}
return sortedState;
return filteredState;
});

return () => {
Expand Down Expand Up @@ -194,24 +223,23 @@
};
},
filter: () => {
return get(shouldFilter);
return shouldFilter;
},
label: get(label) || props['aria-label'] || '',
label: label || props['aria-label'] || '',
commandEl,
ids,
updateState
};

function updateState<K extends keyof State>(key: K, value: State[K], preventScroll?: boolean) {
const $shouldFilter = get(shouldFilter);
state.update((curr) => {
if (Object.is(curr[key], value)) return curr;
curr[key] = value;

if (key === 'search') {
const filteredState = filterItems(curr, $shouldFilter);
const filteredState = filterItems(curr, shouldFilter);
curr = filteredState;
const sortedState = sort(curr, $shouldFilter);
const sortedState = sort(curr, shouldFilter);
curr = sortedState;
tick().then(() =>
state.update((curr) => {
Expand All @@ -230,9 +258,7 @@
}

function filterItems(state: State, shouldFilterVal?: boolean): State {
const $shouldFilter = shouldFilterVal ?? get(shouldFilter);
const $allItems = get(allItems);
const $allIds = get(allIds);
const $shouldFilter = shouldFilterVal ?? shouldFilter;
if (!state.search || !$shouldFilter) {
state.filtered.count = $allItems.size;
return state;
Expand All @@ -252,7 +278,7 @@
}

// Check which groups have at least 1 item shown
for (const [groupId, group] of get(allGroups)) {
for (const [groupId, group] of $allGroups) {
for (const itemId of group) {
const rank = state.filtered.items.get(itemId);
if (rank && rank > 0) {
Expand All @@ -266,7 +292,7 @@
}

function sort(state: State, shouldFilterVal?: boolean) {
const $shouldFilter = shouldFilterVal ?? get(shouldFilter);
const $shouldFilter = shouldFilterVal ?? shouldFilter;
if (!state.search || !$shouldFilter) {
return state;
}
Expand All @@ -275,20 +301,19 @@

// sort groups
const groups: [string, number][] = [];
const $allGroups = get(allGroups);

state.filtered.groups.forEach((value) => {
for (const value of state.filtered.groups) {
const items = $allGroups.get(value);
if (!items) return;
if (!items) continue;
// get max score of the group's items
let max = 0;
items.forEach((item) => {
for (const item of items) {
const score = scores.get(item);
if (isUndefined(score)) return;
if (isUndefined(score)) continue;
max = Math.max(score, max);
});
}
groups.push([value, max]);
});
}

// Sort items within groups to bottom
// sort items outside of groups
Expand All @@ -297,41 +322,39 @@
if (!rootEl) return state;
const list = rootEl.querySelector(LIST_SELECTOR);

// Sort the items
getValidItems(rootEl)
.sort((a, b) => {
const valueA = a.getAttribute(VALUE_ATTR) ?? '';
const valueB = b.getAttribute(VALUE_ATTR) ?? '';
return (scores.get(valueA) ?? 0) - (scores.get(valueB) ?? 0);
})
.forEach((item) => {
const group = item.closest(GROUP_ITEMS_SELECTOR);
const closest = item.closest(`${GROUP_ITEMS_SELECTOR} > *`);
if (isHTMLElement(group)) {
if (item.parentElement === group) {
group.appendChild(item);
} else {
if (!isHTMLElement(closest)) return;
group.appendChild(closest);
}
const validItems = getValidItems(rootEl).sort((a, b) => {
const valueA = a.getAttribute(VALUE_ATTR) ?? '';
const valueB = b.getAttribute(VALUE_ATTR) ?? '';
return (scores.get(valueA) ?? 0) - (scores.get(valueB) ?? 0);
});

for (const item of validItems) {
const group = item.closest(GROUP_ITEMS_SELECTOR);
const closest = item.closest(`${GROUP_ITEMS_SELECTOR} > *`);
if (group) {
if (item.parentElement === group) {
group.appendChild(item);
} else {
if (!isHTMLElement(list)) return;
if (item.parentElement === list) {
list.appendChild(item);
} else {
if (!isHTMLElement(closest)) return;
list.appendChild(closest);
}
if (!closest) continue;
group.appendChild(closest);
}
});
} else {
if (item.parentElement === list) {
list?.appendChild(item);
} else {
if (!closest) continue;
list?.appendChild(closest);
}
}
}

groups.sort((a, b) => b[1] - a[1]);

for (const group of groups) {
const el = rootEl.querySelector(`${GROUP_SELECTOR}[${VALUE_ATTR}="${group[0]}"]`);
el?.parentElement?.appendChild(el);
}

groups
.sort((a, b) => b[1] - a[1])
.forEach((group) => {
const el = rootEl.querySelector(`${GROUP_SELECTOR}[${VALUE_ATTR}="${group[0]}"]`);
if (!isHTMLElement(el)) return;
el.parentElement?.appendChild(el);
});
return state;
}

Expand All @@ -345,7 +368,7 @@

function score(value: string | undefined, search: string) {
const lowerCaseAndTrimmedValue = value?.toLowerCase().trim();
const filterFn = get(filter);
const filterFn = filter;
if (!filterFn) {
return lowerCaseAndTrimmedValue ? defaultFilter(lowerCaseAndTrimmedValue, search) : 0;
}
Expand All @@ -371,15 +394,15 @@
const rootEl = rootElement ?? document.getElementById(ids.root);
if (!rootEl) return [];
return Array.from(rootEl.querySelectorAll(VALID_ITEM_SELECTOR)).filter(
(el): el is HTMLElement => isHTMLElement(el)
(el): el is HTMLElement => (el ? true : false)
);
}

function getSelectedItem(rootElement?: HTMLElement) {
const rootEl = rootElement ?? document.getElementById(ids.root);
if (!rootEl) return;
const selectedEl = rootEl.querySelector(`${VALID_ITEM_SELECTOR}[aria-selected="true"]`);
if (!isHTMLElement(selectedEl)) return null;
if (!selectedEl) return;
return selectedEl;
}

Expand All @@ -399,7 +422,7 @@
// get item at this index
let newSelected = items[index + change];

if (get(loop)) {
if (loop) {
if (index + change < 0) {
newSelected = items[items.length - 1];
} else if (index + change === items.length) {
Expand Down Expand Up @@ -482,9 +505,9 @@
break;
case kbd.ENTER: {
e.preventDefault();
const item = getSelectedItem();
const item = getSelectedItem() as HTMLElement;
if (item) {
item.click();
item?.click();
}
}
}
Expand Down
1 change: 1 addition & 0 deletions src/lib/cmdk/components/Command.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
});

function syncValueAndState(value: string | undefined) {
console.log('sync value and state');

Check warning on line 46 in src/lib/cmdk/components/Command.svelte

View workflow job for this annotation

GitHub Actions / Lint

Unexpected console statement
if (value && value !== $stateStore.value) {
$stateStore.value = value;
}
Expand Down
23 changes: 12 additions & 11 deletions src/lib/cmdk/components/CommandInput.svelte
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<script lang="ts">
import { derived } from 'svelte/store';
import { derived, get } from 'svelte/store';
import { ITEM_SELECTOR, VALUE_ATTR, getCtx, getState } from '../command.js';
import { addEventListener, isBrowser, isHTMLInputElement } from '$lib/internal/index.js';
import { addEventListener, isBrowser } from '$lib/internal/index.js';
import type { InputEvents, InputProps } from '../types.js';
import { sleep } from '$lib/internal/helpers/sleep.js';

Expand All @@ -14,7 +14,7 @@
const valueStore = derived(state, ($state) => $state.value);

export let autofocus: $$Props['autofocus'] = undefined;
export let value: $$Props['value'] = $search;
export let value: $$Props['value'] = get(search);
export let asChild: $$Props['asChild'] = false;

export let el: HTMLElement | undefined = undefined;
Expand All @@ -33,15 +33,16 @@
if (autofocus) {
sleep(10).then(() => node.focus());
}
if (asChild) {
const unsubEvents = addEventListener(node, 'change', (e) => {
const currTarget = e.currentTarget as HTMLInputElement;
state.updateState('search', currTarget.value as string);
});

const unsubEvents = addEventListener(node, 'change', (e) => {
if (!isHTMLInputElement(e.target)) return;
state.updateState('search', e.target.value);
});

return {
destroy: unsubEvents
};
return {
destroy: unsubEvents
};
}
}

$: handleValueUpdate(value);
Expand Down
37 changes: 37 additions & 0 deletions src/routes/sink/+page.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
<script lang="ts">
import { Command } from '$lib/index.js';

// prettier-ignore
const ten_first_names = ["John", "Doe", "Jane", "Smith", "Michael", "Brown", "William", "Johnson", "David", "Williams"];
// prettier-ignore
const ten_middle_names = ["James", "Lee", "Robert", "Michael", "David", "Joseph", "Thomas", "Charles", "Christopher", "Daniel"];
// prettier-ignore
const ten_last_names = ["Smith", "Johnson", "Williams", "Brown", "Jones", "Garcia", "Miller", "Davis", "Rodriguez", "Martinez"];

const names = ten_first_names
.map((first) => {
return ten_middle_names.map((middle) => {
return ten_last_names.map((last) => {
return `${first} ${middle} ${last}`;
});
});
})
.flat(2)
.slice(500);
</script>

<div style:padding="16px">
<Command.Root loop>
<Command.Input placeholder="Search items..." />
<Command.Empty>No item found.</Command.Empty>

<Command.List
class="h-[var(--cmdk-list-height)]"
style="height: 200px; overflow-y: auto; max-width: 300px;"
>
{#each names as txt (txt)}
<Command.Item value={txt}>{txt}</Command.Item>
{/each}
</Command.List>
</Command.Root>
</div>
Loading