timc1 / kbar

fast, portable, and extensible cmd+k interface for your site
https://kbar.vercel.app
MIT License
4.84k stars 185 forks source link

Duplicate actions with useRegisterActions #267

Closed davidkaneda closed 1 year ago

davidkaneda commented 1 year ago

I'm getting every result below repeated 3x in the UI, using the following code. (useMe is a call to useSWR, updates dynamically/async)

  const { me } = useMe();
  const router = useRouter(); // (Next.js router)
  const templateActions = useMemo(() => {
    const actions = [] as Action[];

    actions.push({
      id: "blank-draft",
      name: "Blank draft",
      section: "New",
      parent: "new-template",
      keywords: "new doc draft",
      perform: () => router.push("/edit/"),
      priority: 1,
    });

    if (me?.stripeCustomer?.isActive) {
      actions.push(
        ...Object.entries(templates).map(([key, template]) => ({
          id: key,
          section: "New",
          name: `${template.title}`,
          parent: "new-template",
          perform: () => router.push(`/edit/?template=${key}`),
        }))
      );
    }

    return actions;
  }, [me, router]);

  useRegisterActions(templateActions, [templateActions]);

It feels like the call to unregister in useRegisterActions isn't actually removing them each time the deps change.

davidkaneda commented 1 year ago

Ah, I just looked at the source and see that unregister is just being called on unmount, so not when the deps actually change. Is that intentional?

timc1 commented 1 year ago

unregister is called whenever your dependencies change; it works just as a useEffect would.

davidkaneda commented 1 year ago

templates comes from a static import, there shouldn't be any duplicates in the templateActions output-

davidkaneda commented 1 year ago

Interestingly, the duplication doesn't happen if I remove the "parent" field. "new-template" is a parent action I define statically somewhere else-

davidkaneda commented 1 year ago

Moving the parent action into the useMemo seems to have fixed it-

stale[bot] commented 1 year ago

Hey! This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.