skip to Main Content

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.

2

Answers


  1. You should add signatura to your hooks dependencies array and separate out the logic you use for fetching vs filtering the photos:

    useEffect(() => {
      dispatch(getPhotos());
    }, [dispatch]);
    
    useEffect(() => {
      const photoIndex = photos.findIndex((photo, index) => {
        console.log(index, photo.signatura, signatura);
        return photo.signatura === signatura;
      });
    
      console.log(photoIndex, photos, signatura);
      dispatch(setSelectedPhotoIndex(photoIndex));
    
    }, [dispatch, signatura, photos]);
    

    This will ensure that the code used to filter by signatura will run whenever the photos selector updates, preventing any race conditions from occurring.

    On a side note, I would suggest installing eslint and using the react hooks rule to catch these types of bugs for you

    Login or Signup to reply.
  2. 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.

    Login or Signup to reply.
Please signup or login to give your own answer.
Back To Top
Search