huntabyte / cmdk-sv

cmdk, but for Svelte ✨
https://cmdk-sv.com
MIT License
423 stars 18 forks source link

Bug: Search items not always returned to original order after searching #32

Closed Not-Jayden closed 7 months ago

Not-Jayden commented 8 months ago

I've noticed the Command.Item components will be render in a seemingly random order sometimes when searching and resetting the filter.

Reproduction:

  1. Use the Raycast menu on cmdk-sv.com
  2. Type a random letter (e.g. "I") and press backspace repeatedly in quick succession until the order changes.
  3. The "unfiltered" results result is sorted by some seemingly random order

Screen Recording 2023-11-18 at 9 52 28 pm

Not-Jayden commented 8 months ago

If it helps, my current workaround for solving this is doing:

{#key inputValue === ''}
    <CommandMenu />
{/key}

so the component is created and destroyed, but that definitely feels like a hack.

huntabyte commented 8 months ago

This only occurs when you do that in rapid succession, correct? If so, we likely have some sort of race condition which is messing with the DOM sorting, almost as if it's still in the process of sorting, interrupted, and then sorts based on the interruption.

I will look into this when I get the chance, thanks for posting your workaround. It definitely makes sense that it works, and if we aren't able to identify an alternative fix, we can implement that within the library so you aren't stuck doing it in your codebase.

Not-Jayden commented 8 months ago

@huntabyte Nope, I incorrectly assumed that was the case at first but it's actually reliably unordered when items are filtered out by a letter and re-added.

What I did figure out is what is happening with the "random" order, is actually the included results from the previous input value are moved to the top, and the filtered results are moved to the bottom.

Screen Recording 2023-11-19 at 2 45 23 am

My hunch is using the DOM .appendChild() method to reorder elements when sorting breaks Svelte's ability to keep track of the original DOM order/where the node should be inserted back, but I haven't investigated too much further.

Not-Jayden commented 8 months ago

I'll put up a PR up now for the {#key} fix, feel free to close if you can think of a better solution :)