pacocoursey / cmdk

Fast, unstyled command menu React component.
https://cmdk.paco.me
MIT License
9.42k stars 266 forks source link

App freezes when Items inside List changes based on async query #103

Closed justkahdri closed 6 months ago

justkahdri commented 1 year ago

I'm combining React Query and Meilisearch with cmdk and the app freezes if I type something on the Command.Input that doesn't give any results on the query. The bug doesn't happen if I change the Command.Item for a common div tag. Any ideas on what could I be doing wrong?

Source code:

export const Search = () => {
  const [open, setOpen] = React.useState(false)
  const [search, setSearch] = React.useState('')

  const { data: results, isLoading } = useQuery(
    ['docs', search],
    async (context) => {
      const client = new MeiliSearch({
        host: 'http://127.0.0.1:7700',
        apiKey: 'myMasterKey'
      })
      // An index is where the documents are stored.
      const index = client.index('movies')

      const queryResults = await index.search(context.queryKey[1], { limit: 5 })
      return queryResults
    }
  )

  // Toggle the menu when ⌘K is pressed
  React.useEffect(() => {
    const down = (e: KeyboardEvent) => {
      if (e.key === 'k' && e.metaKey) {
        setOpen((open) => !open)
      }
    }

    document.addEventListener('keydown', down)
    return () => document.removeEventListener('keydown', down)
  }, [])

  return (
    <>
      <button onClick={() => setOpen(true)}>
        Search docs
      </button>
      <Command.Dialog
        shouldFilter={false}
        open={open}
        onOpenChange={setOpen}
        label="Global Command Menu"
      >
        <Command.Input
          value={search}
          onValueChange={setSearch}
        />
        <Command.List>
          {isLoading && <Command.Loading>Fetching words…</Command.Loading>}
          {results?.hits.map((hit) => (
            <Command.Item key={hit.id} value={hit.title}>
              <h4>{hit.title}</h4>
              <span>{hit.overview}</span>
            </Command.Item>
          ))}
          <Command.Empty>No results found.</Command.Empty>
        </Command.List>
      </Command.Dialog>
    </>
  )
}
joaom00 commented 1 year ago

Can you provide a minimal reproduction in a sandbox?

justkahdri commented 1 year ago

Not really, I couldn't make it work on a sandbox at all.

sagarchoudhary96 commented 1 year ago

The issue seems to be when render a large value in the Command.Item. i have a subtitle Column being rendered which was showing lot of test data for each of the result, which was causing the issue. here also i think maybe the hit.overview contains a large set of text and here when there are many results to show the app runs out of memory as they all are being rendered and attached to the DOM and are always present there.

nkeat12 commented 1 year ago

Anyone have any ideas on solution here?

joaom00 commented 1 year ago

Are the query results too large? if so maybe virtualizing the list can solve it.

sagarchoudhary96 commented 1 year ago

yep, virtualisation seems to be the only option here in this case. if there could be some config to enable it out of the bo it will be great as we might have Grouped command items as well so need to have virtualization works in all the cases properly. Or maybe just can add example for it to the Docs so that it's easily available for the user to implement.

justkahdri commented 1 year ago

Are the query results too large?

The list itself is not long because the limit on the query is set to 5 so at worst cases is an Array of 5 elements. hit.overview is at most two paragraphs of plain text. I don't know if there is a rough estimation of what could be a "large result"?

Clarity-89 commented 1 year ago

Anyone got an example of virtualizing the options list? I managed to do that with react-window but it breaks the filtering functionality:

import { FixedSizeList as List } from 'react-window';

<Command>
  <CommandInput placeholder={placeholder} />
  <CommandEmpty>No results found.</CommandEmpty>
  <CommandList className={'max-h-[200px] overflow-auto'}>
    <List
      itemSize={40}
      height={200}
      itemCount={options.length}
      width={286}
    >
      {({ index, style }) => {
        const option = options[index];
        return (
          <CommandItem
            style={style}
            key={option.value}
            value={option.value}
            onSelect={(currentValue, ...rest) => {
              onChange(currentValue);
              setOpen(false);
            }}
          >
            <Check
              className={cn(
                'mr-2 h-4 w-4',
                value === option.value ? 'opacity-100' : 'opacity-0',
              )}
            />
            {option.label}
          </CommandItem>
        );
      }}
    </List>
  </CommandList>
</Command>
donferi commented 1 year ago

Has anyone figured out how to fix this? 🤔. My API returns also limited results (3-5 items) but with enough searches cmdk dies. Not sure what I'm doing wrong.

This seems to only be happening when using react-query 🤔

donferi commented 1 year ago

Ok I figured out, two issues I'm seeing. First is that even if I'm setting shouldFilter={false} it still runs the filter function (even if it doesn't use the results).

If I do shouldFilter={false} and pass a dummy filter like filter={() => 1} the issue is completely gone.

The second issue is that command-score apparently is incredibly slow when using things like aaaaaaaaaaaaaaaaaaaaaaaa as I was using.

skovy commented 1 year ago

Confirming the observed behavior seen by @donferi. I have a very simple list (2 static items with a few words each) but when testing with asdf asdf asdf asdf ... or aaaaaaaaaaaaa around characters 30-50 it starts becoming noticeably slow then hangs the tab. In my case, I also didn't need any filter so setting filter={() => 1} (in addition to shouldFilter={false}) fixed it for me.

leweyse commented 8 months ago

@Clarity-89 and for anyone looking into Virtualized lists, I think this example will help: https://github.com/oaarnikoivu/shadcn-virtualized-combobox/blob/main/src/components/virtualized-combobox.tsx

This example was shared in this issue: https://github.com/shadcn-ui/ui/issues/1127#issuecomment-1779825280

pacocoursey commented 6 months ago

The second issue is that command-score apparently is incredibly slow when using things like aaaaaaaaaaaaaaaaaaaaaaaa as I was using.

This is now fixed.

@justkahdri Are you still seeing performance issues? A reproduction would be really helpful if so. I would like to improve the virtualization support, but let's close this issue in favor of one about virtualization instead if that's the main point here.

justkahdri commented 6 months ago

@pacocoursey No, it's working well now. Thanks for the support!