eBay / nice-modal-react

A modal state manager for React.
https://ebay.github.io/nice-modal-react
MIT License
1.96k stars 110 forks source link

feat: flatten dependencies in useModal #54

Closed janczizikow closed 2 years ago

janczizikow commented 2 years ago

Right now functions returned from useModal hook change their identity after they are dispatched. This happens because in useReducer hook used to store the state of modals, will always return in a new object whenever an action is dispatched.

e.g.:

export const reducer = (
  state: NiceModalStore = initialState,
  action: NiceModalAction
): NiceModalStore => {
  switch (action.type) {
    case "nice-modal/show": {
      const { modalId, args } = action.payload;
      return {
        ...state,
        [modalId]: {
          ...state[modalId],
          id: modalId,
          args,
          // If modal is not mounted, mount it first then make it visible.
          // There is logic inside HOC wrapper to make it visible after its first mount.
          // This mechanism ensures the entering transition.
          visible: !!ALREADY_MOUNTED[modalId],
          delayVisible: !ALREADY_MOUNTED[modalId],
        },
      };
    }
    // ... some other actions
    default:
      return state;
  }
};

This state for an individual modal is then consumed in useModal hook and passed as a dependency to useMemo hook:

export function useModal(modal?: any, args?: any): any {
  const modals = useContext(NiceModalContext); // the value of modals is state of the reducer
  // ...

  const modalInfo = modals[mid]; // we access the state of a modal by id

  // ...

  return useMemo<NiceModalHandler>(
    () => ({
      id: mid,
      args: modalInfo?.args,
      visible: !!modalInfo?.visible,
      keepMounted: !!modalInfo?.keepMounted,
      show: (args?: Record<string, unknown>) => show(mid, args),
      hide: () => hide(mid),
      remove: () => remove(mid),
      resolve: (args?: unknown) => {
        modalCallbacks[mid]?.resolve(args);
        delete modalCallbacks[mid];
      },
      reject: (args?: unknown) => {
        modalCallbacks[mid]?.reject(args);
        delete modalCallbacks[mid];
      },
      resolveHide: (args?: unknown) => {
        hideModalCallbacks[mid]?.resolve(args);
        delete hideModalCallbacks[mid];
      },
    }),
    [mid, modalInfo], // <- not much will be memoized because modalInfo is going to change whenever we dispatch an action :(
  );
}

So to come back to the original example calling show() will cause modalInfo to change which in return will create a new function for show.

In this PR I just flattened out the dependencies of modalInfo. This way we don't compare an object to object with ===, but primitive values (booleans)

supnate commented 2 years ago

Thanks @janczizikow for this fantastic improvement!

supnate commented 2 years ago

Published this in v1.2.2.

janczizikow commented 2 years ago

@supnate Thank you for the cool library and super quick merge and release 💪 谢谢!