Search code examples
javascriptreactjsreact-hooksreact-router-domredux-toolkit

How do I make my carousel respond to url changes?


I have a gallery, that opens individual images in a carousel. Unfortunately, I can't make it respond to URL path changes.

const { signatura } = useParams();

const selectedPhotoIndex = useSelector(selectSelectedPhotoIndex);
console.log(selectedPhotoIndex);
const photos = useSelector(selectPhotos);

useEffect(() => {
  const fetchPhotos = () => {
    dispatch(getPhotos());

    const photoIndex = photos.findIndex((photo, index) => {
      console.log(index, photo.signatura, signatura);

      return photo.signatura === signatura;
    });

    console.log(photoIndex, photos, signatura);
    dispatch(setSelectedPhotoIndex(photoIndex));
  };

  fetchPhotos();
}, [dispatch]);

const selectedPhoto = photos[selectedPhotoIndex];

if (!selectedPhoto) {
  return <div> Photo not found </div>;
}

Slice:

  setSelectedPhotoIndex: (state, action) => {
    state.selectedPhotoIndex = action.payload;
  },
  setSelectedPhoto: (state, action) => {
    state.selectedPhoto = action.payload;
  },
}

...
export const selectSelectedPhoto = state => {
  const selectedPhotoIndex = state.gallery.selectedPhotoIndex;
  const photos = state.gallery.photos;

  if (selectedPhotoIndex >= 0 && selectedPhotoIndex < photos.length) {
    return photos[selectedPhotoIndex];
  }

  return null;
};

I have tried making my useEffect dependent on the signatura path parameter from useParams:

  ...
  fetchPhotos();
}, [dispatch, signatura]);

as well as setting the selectedPhoto directly

dispatch(setSelectedPhoto(photo.signatura));

None of those seem to work when editing the URL manually.

The reproducible example is here.


Solution

  • Issue

    From what I can tell you have a single side-effect that is doing too much. It dispatches an action to get all the photos while at the same time it tries to read the current/initial photos value selected from state to compute a selected photo index based on the signatura route path parameter.

    Because of the way Javascript closures work, the photos value in the fetchPhotos function won't ever be the updated photos that were fetched. In other words, it's an issue of a stale closure.

    Solution

    Split the logic of fetching the photos from the logic of computing a derived selected photo index value by using two useEffect hooks.

    const { signatura } = useParams();
    
    const photos = useSelector(selectPhotos);
    const selectedPhotoIndex = useSelector(selectSelectedPhotoIndex);
    
    useEffect(() => {
      dispatch(getPhotos());
    }, [dispatch]);
    
    useEffect(() => {
      const photoIndex = photos.findIndex(
        (photo) => photo.signatura === signatura
      );
    
      dispatch(setSelectedPhotoIndex(photoIndex));
    }, [dispatch, photos, signatura]);
    
    const selectedPhoto = photos[selectedPhotoIndex];
    

    In my opinion you don't need the selectedPhotoIndex state at all. Recall that above I called this a derived value, e.g. a value derived from the current photos state and the current signatura route path parameter. It's a general React anti-pattern to store derived "state" in state.

    Here's a suggested example skipping the stored selectedPhotoIndex state:

    const { signatura } = useParams();
    
    const photos = useSelector(selectPhotos);
    
    useEffect(() => {
      dispatch(getPhotos());
    }, [dispatch]);
    
    const selectedPhoto = photos.find(
      (photo) => photo.signatura === signatura
    );
    

    You could, if you wanted to, memoize the selected photo value using the useMemo hook:

    const selectedPhoto = React.useMemo(() => photos.find(
      (photo) => photo.signatura === signatura
    ), [photos, signatura]);
    

    In fact, it's a bit of a "rule of thumb" that when you catch yourself creating a useState/useEffect or useSelector/useEffect "coupling" that you are implementing the anti-pattern and should just compute and use the derived value directly in the component.