Use State not updating as expected-CodePudding

Fairly new to react and trying to build a clone of The Movie Database site. I want this toggle switch to change my api call from movies to tv. It starts working after a couple clicks, but then it throws everything off and it's not displaying the correct items anyway. Not really sure what's going on here...or even why it starts working after two clicks. Anyone know whats up with this?

import React, { useState, useEffect } from "react";
import axios from "axios";
import API_KEY from "../../config";

const Popular = ({ imageUri }) => {
  // GET POPULAR MOVIES
  const [popularMovies, setPopularMovies] = useState("");
  const [genre, setGenre] = useState("movie");
  console.log(genre);

  const getPopular = async () => {
    const response = await axios.get(
      `https://api.themoviedb.org/3/discover/${genre}?sort_by=popularity.desc&api_key=${API_KEY}`
    );
    setPopularMovies(response.data.results);
  };

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

  const listOptions = document.querySelectorAll(".switch--option");
  const background = document.querySelector(".background");

  const changeOption = (el) => {
    let getGenre = el.target.dataset.genre;
    setGenre(getGenre);
    getPopular();

    listOptions.forEach((option) => {
      option.classList.remove("selected");
    });

    el = el.target.parentElement.parentElement;
    let getStartingLeft = Math.floor(
      listOptions[0].getBoundingClientRect().left
    );
    let getLeft = Math.floor(el.getBoundingClientRect().left);
    let getWidth = Math.floor(el.getBoundingClientRect().width);
    let leftPos = getLeft - getStartingLeft;
    background.setAttribute(
      "style",
      `left: ${leftPos}px; width: ${getWidth}px`
    );
    el.classList.add("selected");
  };

  return (
    <section className="container movie-list">
      <div className="flex">
        <div className="movie-list__header">
          <h3>What's Popular</h3>
        </div>
        <div className="switch flex">
          <div className="switch--option selected">
            <h3>
              <a
                data-genre="movie"
                onClick={(e) => changeOption(e)}
                className="switch--anchor"
              >
                In Theaters
              </a>
            </h3>
            <div className="background"></div>
          </div>
          <div className="switch--option">
            <h3>
              <a
                data-genre="tv"
                onClick={(e) => changeOption(e)}
                className="switch--anchor"
              >
                On TV
              </a>
            </h3>
          </div>
        </div>
      </div>
      <div className="scroller">
        <div className="flex flex--justify-center">
          <div className="flex flex--nowrap container u-overScroll">
            {popularMovies &&
              popularMovies.map((movie, idX) => (
                <div key={idX} className="card">
                  <div className="image">
                    <img src={imageUri   "w500"   movie.poster_path} />
                  </div>
                  <p>{movie.title}</p>
                </div>
              ))}
          </div>
        </div>
      </div>
    </section>
  );
};

export default Popular;

CodePudding user response：

You're using the array index as your key prop when you're mapping your array.

You should use an id that is specific to the data that you're rendering. React uses the key prop to know which items have changed since the last render.

In your case you should use the movie id in your key prop instead of the array index.

popularMovies.map((movie) => (
    <div key={movie.id} className="card">
      <div className="image">
        <img src={imageUri   'w500'   movie.poster_path} />
      </div>
      <p>{movie.title}</p>
    </div>
));

Also

You're calling the api directly after setGenre. However state changes aren't immediate. So when you're making your api call you're still sending the last movie genre.

Two ways of fixing this:

You could call your function with the genre directly, and change your function so it handles this case:

getPopular('movie');

Or you could not call the function at all and add genre as a dependency of your useEffect. That way the useEffect will run each time the genre change.

useEffect(() => {
    getPopular();
}, [genre]);

PS: You should consider splitting your code into more component and not interacting with the DOM directly.

To give you an idea of what it could look like, I refactored a bit, but more improvements could be made:

const Popular = ({ imageUri }) => {
    const [popularMovies, setPopularMovies] = useState('');
    const [genre, setGenre] = useState('movie');

    const getPopular = async (movieGenre) => {
        const response = await axios.get(
            `https://api.themoviedb.org/3/discover/${movieGenre}?sort_by=popularity.desc&api_key=${API_KEY}`
        );
        setPopularMovies(response.data.results);
    };

    useEffect(() => {
        getPopular();
    }, [genre]);


    const changeHandler = (el) => {
        let getGenre = el.target.dataset.genre;
        setGenre(getGenre);
    };

    const isMovieSelected = genre === 'movie';
    const isTvSelected = genre === 'tv';
    return (
        <section className="container movie-list">
            <div className="flex">
                <MovieHeader>What's Popular</MovieHeader>
                <div className="switch flex">
                    <Toggle onChange={changeHandler} selected={isMovieSelected}>
                        In Theaters
                    </Toggle>
                    <Toggle onChange={changeHandler} selected={isTvSelected}>
                        On TV
                    </Toggle>
                </div>
            </div>
            <div className="scroller">
                <div className="flex flex--justify-center">
                    <div className="flex flex--nowrap container u-overScroll">
                        {popularMovies.map((movie) => {
                            const { title, id, poster_path } = movie;
                            return (
                                <MovieItem
                                    title={title}
                                    imageUri={imageUri}
                                    key={id}
                                    poster_path={poster_path}
                                />
                            );
                        })}
                    </div>
                </div>
            </div>
        </section>
    );
};

export default Popular;

const Toggle = (props) => {
    const { children, onChange, selected } = props;
    const className = selected ? 'switch--option selected' : 'switch--option';
    return (
        <div className={className}>
            <h3>
                <a
                    data-genre="movie"
                    onClick={onChange}
                    className="switch--anchor"
                >
                    {children}
                </a>
            </h3>
            <div className="background"></div>
        </div>
    );
};
const MovieHeader = (props) => {
    const { children } = props;
    return (
        <div className="movie-list__header">
            <h3>{children}</h3>
        </div>
    );
};

const MovieItem = (props) => {
    const { title, imageUri, poster_path } = props;
    return (
        <div key={idX} className="card">
            <div className="image">
                <img src={imageUri   'w500'   poster_path} />
            </div>
            <p>{title}</p>
        </div>
    );
};

CodePudding user response：

I'm not an expert at all, but if I put myself in your situation, I'll start by

console.log(genre)

every time I click switch, just to be sure that the genre has been taken as is, or something happened to it when clicking