1

I'm looking for feedback on this customer React hook. I'm wondering:

  1. Does this look like a proper use of custom React hooks?
  2. Is there a better way to switch between different API endpoints based upon the prop that is passed in? I'm looking to do something like:
<MovieGrid typeOfMovies={"popular"} />

and

<MovieGrid typeOfMovies={"upcoming"} />
  1. Do you have any feedback or recommendations on anything you see. Thank you!

The code I've provided does indeed work. But since hooks a relatively new I don't feel totally confident I'm using them right.

Here's what I've got:


import React, { useState, useEffect } from "react";

function useFetchMovies(typeOfMovies) {
  const [movieData, setMovieData] = useState([]);


  const movieEndpointURL = () => {
    if (typeOfMovies === "upcoming") {
      return `https://api.themoviedb.org/3/movie/upcoming?api_key={API_KEY}&language=en-US&page=1&region=US`;
    } else if (typeOfMovies === "popular") {
      return `https://api.themoviedb.org/3/movie/popular?api_key={API_KEY}&language=en-US&page=1&region=US`;
    }
  };

  const fetchMovieData = async () => {
    try {
      const res = await fetch(movieEndpointURL());
      const movies = await res.json();
      setMovieData(movies.results);
      console.log(movies.results);
    } catch (err) {
      console.log(err);
    }
  };

  useEffect(() => {
    fetchMovieData();
  }, []);

  return [movieData, setMovieData];
}

export { useFetchMovies };
Emile Bergeron
  • 17,074
  • 5
  • 83
  • 129
Macintosh007
  • 55
  • 1
  • 7
  • 1
    Does this actually `fetchMovieData` when `typeOfMovies` change ? It looks to me the current use of useEffect would fetch after first render (like old `componentDidMount`) and not on prop changes later. – Jokester Mar 29 '19 at 01:54
  • @Jokester, you know what, you're totally right. It doesn't update. Hmmmm. – Macintosh007 Mar 29 '19 at 03:41
  • I recommend [this article](https://overreacted.io/a-complete-guide-to-useeffect/) to understand the new hook APIs (and the ideas behind them) :) – Jokester Mar 29 '19 at 03:47
  • including [typeOfMovies] in useEffect should make it work when the prop changes – Rahil Ahmad Mar 29 '19 at 04:33
  • @jokester thank you so much for the recommended article. I just finished reading the article! It was a long read, but SO helpful. – Macintosh007 Mar 29 '19 at 05:27

1 Answers1

1

Your useFetchMovies seems to be correct expect the part that when typeOfMovies changes new data will not be fetched because when the useEffect first runs on mount of component it will refer to the fetchMoviesData that was initially created along with its closure and when the useFetchMovies hook is called again a new function is created which isn't referenced by the useEffect.

In order to make it word correctly you should pass typeOfMovies as the second argument to useEffect like

useEffect(() => {
    fetchMovieData();
}, [typeOfMovies]);
Shubham Khatri
  • 270,417
  • 55
  • 406
  • 400
  • That worked! Thank you so much for the feedback and kindness. It's all starting to make sense. – Macintosh007 Mar 29 '19 at 13:47
  • Do you think using the IF statements to switch between API endpoints is fine? Or would a switch statement or something else be preferred? – Macintosh007 Mar 29 '19 at 13:47
  • you don't actually need if statments you could simply do it using dynamic evaluation like `\`https://api.themoviedb.org/3/movie/${typeOfMovies}?api_key={API_KEY}&language=en-US&page=1&region=US\`;` – Shubham Khatri Mar 29 '19 at 17:16