mantine icon indicating copy to clipboard operation
mantine copied to clipboard

Cannot register spotlight action in an effect

Open ramiel opened this issue 2 years ago • 6 comments

What package has an issue

@mantine/spotlight

Describe the bug

If you try to register new actions in an useEffect, this doesn't work. If you pass spotlight as the useEffect dependency, this is run forever. also if you limit to registerActions and removeActions which probably are incorrectly created and are not callbacks created through useCallback.

The idea is this:

const spotlight = useSpotlight();

  useEffect(() => {
    console.log("register");
    spotlight.registerActions([
      {
        id: "act1",
        title: "Action",
        onTrigger: () => {}
      }
    ]);

    return () => {
      console.log("remove");
      spotlight.removeActions(["act1"]);
    };
    // Try to correctly pass spotlight as dependency here
  }, []);

An use case for this is to register and de-register actions based on external events. e.g. an action to mute/unmute the microphone based on the mute state.

In which browser did the problem occur

Any

If possible, please include a link to a codesandbox with the reproduced problem

https://codesandbox.io/s/reverent-leftpad-6i6fkz

Do you know how to fix the issue

No

Are you willing to participate in fixing this issue and create a pull request with the fix

No

Possible fix

This probably happen because everytime the children of SpotlightProvider change, the initial actions are re-set

ramiel avatar May 12 '22 10:05 ramiel

Same here. We're using a custom hook that looks almost identical to your example. We're using it to update the search results depending on the current page.

I tried logging the registeredActions and it really seems like the Provider overrides the actions to the default value once the children change. Although I wasn't able to pinpoint the exact source of the issue yet.

Array [ {…} ]
Array [ {…} ]
Array(57) [ {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, {…}, … ]
Array [ {…} ]
Array [ {…} ]

pgrones avatar May 13 '22 07:05 pgrones

Found a workaround for our particular usecase, but I have no idea how stable this really is. It also runs quite a few times before settling down.

export const useSearchActions = (actions: SpotlightAction[]) => {
  const { actions: registeredActions, removeActions, registerActions } = useSpotlight();

  useEffect(() => {
    // Clear if the passed actions are empty
    if (!actions.length) removeActions(registeredActions.map(a => a.id!));
  }, []);

  useEffect(() => {
    // Only update actions if there is a difference between the registered ones and the passed ones
    // This check prevents endless loops
    const hasDifference = !!actions.filter(a => !registeredActions.some(r => r.id === a.id)).length;

    if (hasDifference) {
      removeActions(registeredActions.map(a => a.id!));
      registerActions(actions);
    }
  }, [actions]);
};

pgrones avatar May 13 '22 08:05 pgrones

Same here. We're using a custom hook that looks almost identical to your example. We're using it to update the search results depending on the current page.

I tried logging the registeredActions and it really seems like the Provider overrides the actions to the default value once the children change. Although I wasn't able to pinpoint the exact source of the issue yet.

The cause should be the useUpdate hook in the SpotlightProvider that reset the actions. I'm not entirely sure what should be the logic. Since exists a registerActions, I feel like that update is useless...but I don't know

ramiel avatar May 13 '22 12:05 ramiel

Fixed in 4.2.5, actions can now be registered in useEffect

rtivital avatar May 15 '22 09:05 rtivital

This is still not working. To reproduce the problem you need to re-render SpotlightProvider after the actions have been registered. I updated the codesandbox to use latest mantine package https://codesandbox.io/s/reverent-leftpad-6i6fkz

ramiel avatar May 15 '22 09:05 ramiel

PR #1085 introduced behavior to update the actions whenever the actions prop to SpotlightProvider changes.

@ramiel because you are passing an unmemoized array as the actions prop, when the provider re-renders it thinks your actions have changed and updates actions with the empty array. Dependency checks are always shallow comparisons; the two empty arrays from each re-render are NOT equal. Memoizing the array fixes your sandbox example: https://codesandbox.io/s/vibrant-jennings-ioy752

ercgrat avatar Jul 18 '22 01:07 ercgrat

Fixed in 5.1.3

rtivital avatar Aug 11 '22 15:08 rtivital