ramuk
ramuk

Reputation: 47

Clear state on selecting dropdown

Completely new to React and I'm building a small Pokedex based on Pokeapi. I'm able to fetch Pokemon, its corresponding data, render it successfully.

I'm having a dropdown wherein the users can select the Pokemon range (start and end no.) and the Pokemon list should update automatically. The problem is, the previous range is not getting cleared and the current range is added with the previous range. But when I allow all the Pokemon in a range to load completely and select the second range, I'm not facing this error. Quickly toggling between the ranges causes this error. Code snippets below.

The thing is allPokemons is getting set to empty when I select a dropdown item, but the same isn't reflecting in the render

GIF demonstrating my issue : Pokedex error

Codesandbox link : here

State:

constructor(props) {
        super(props);
        this.state = {
            allPokemons: [],    
            limit: 151,
            offset: 0,
            regions: [
                {
                 name: "Kanto",
                 limit: 151,
                 offset: 0,
                },
                {
                 name: "Johto",
                 limit: 100,
                 offset: 151,
                },
                ]      
        }
}

Fetching Pokemon list and corresponding data:

getAllPokemons = async () => {
    const response = await axios.get(`https://pokeapi.co/api/v2/pokemon?limit=${this.state.limit}&offset=${this.state.offset}`).catch((err) => console.log("Error:", err));
    this.getPokemonData(response.data.results);

}

getPokemonData = async (result) => {

    this.setState({
        allPokemons : [],
    })

    var response;
    for (var i = 0; i < result.length; i++) {
        response = await axios.get(`https://pokeapi.co/api/v2/pokemon/${result[i].name}`).catch((err) => console.log("Error:", err));            
        this.setState({
            allPokemons: [...this.state.allPokemons,response.data],               
        })
    }       
}

Dropdown handle change function

handleChangeRegions = (event) => {

    this.setState({
        allPokemons : [],
    })

    for (var i = 0; i < this.state.regions.length; i++) {
        if (this.state.regions[i].name === event.target.value) {
            this.setState({
                limit : this.state.regions[i].limit,
                offset : this.state.regions[i].offset,
                allPokemons : [],
            },()=>{
                this.getAllPokemons();  
            })

            break;
        }
    }
 }

Render component

Object.keys(this.state.allPokemons).map((item, index) =>
<Pokemon
    key={index}
    id={this.state.allPokemons[item].id}
    name={this.state.allPokemons[item].name}
    type={this.state.allPokemons[item].types}
/>
)

Dropdown component

<select value={this.state.valueregion} onChange={this.handleChangeRegions}>
{this.state.regions.map((region) => (
    <option value={region.name}>{region.name}&nbsp;({region.offset + 1}-{region.limit + region.offset})</option>
))}
</select>

I need to know the reason for this issue and solutions are much appreciated!

Upvotes: 1

Views: 159

Answers (1)

Essameldeen Youssef
Essameldeen Youssef

Reputation: 395

well a couple of notes,

  1. in your Render Component as you named it, setting the key to index of the array is a really bad practice , this will result in issues with the rendered items and specifically with onclick handlers and so forth, use the unique id returned from the api response instead as you use it in id (not sure why you are using the id but you need to set the key) , more info here
  2. You are using setState alot, in handleChangeRegions you reset the state of the allPokemons key and then you set it again with values from the dropdown while you can just pass the values from the drop down to the next function and that will do the api call , you only need to use the state when you have something that changes on the screen a changing variable that should cause your component to re-render
  3. furthermore, you are then setting the state everytime with the new response, causing un-necessary re-renders, just save the content from the api response in an array then pass it to the state after all the requests have passed

this might not be the solution but it should at least give you some hints to narrow down the problem. if you can re produce the code in a code sandbox that would be extremely helpful in finding out the issue not for the full code but you can replace the Pokemon component for example with a display of the id to ease us debugging the code.

EDIT

Since the question is now updated and the original problem is resolved i checked your sandbox, you are issuing individual requests with each one waiting for the one before it

for (var i = 0; i < result.length; i++) {
  response = await axios
    .get(`https://pokeapi.co/api/v2/pokemon/${result[i].name}`)
    .catch((err) => console.log("Error:", err));
  pokemonArr.push(response.data);
}

and thats your culprit right there, each request is being sent and then you are waiting for that request to process for each pokemon.. you are basically sending 200 requests but not in parallel but sequentially since you are using async await.

replace that part with the following

await Promise.all(
  result.map(pokemonItem => {
    return axios.get(`https://pokeapi.co/api/v2/pokemon/${pokemonItem.name}`)
      .then(result => {
        pokemonArr.push(result.data);
      });
  })
);
this.setState({
  allPokemons:pokemonArr
})

Promise.all() sends all of the requests together and processes their response together along with the await statement before it to only wait until all the requests are finished before setting the state and it voila! works like a charm.

a few more comments now here tho would be:

  1. Usually you send the api calls in the componentDidMount() lifecycle hook not in the comopnentWillMount()
  2. but I think you know that already, sending 200 requests to access the data for each individual pokemon and display them in a list is a poor design for the backend service .. i know its not yours but a case like this you won't usually meet in real life.

the only thing you might find weird now is that your list will not be sorted .. since alot of requests are being sent in parallel and we dont know which request will be handled first.. a simple sort function would solve the issue and you can find one here

I have updated your code with the working solution here Happy hacking and welcome to SO!

Upvotes: 1

Related Questions