One big issue with your app is you are literally sending hundreds of requests to the poke api when someone lands on your page (two requests for each pokemon). That is inconceivable for a real app and even here it's likely that it's taking a toll on the poke api server(s).
You normally want a single requests for multiple pokemons (like "https://pokeapi.co/api/v2/pokemon?offset=20&limit=20", the offset and limit parameters are useful for pagination).
Apart from that, the code itself has room for improvement, among other things:
the variables entries, filteredEntries... at the top level don't belong there. In react, if the UI depends on some data, this data should be in state. Now your app may seem to be working but you're actually lucky that the UI somehow remains in sync with the data, as only changes to state or props trigger a rerender (UI update). At the top level of a react app, you normally find static data which is not related to the UI.
you should follow the principle of minimal state (reactjs.org/docs/thinking-in-react...) i.e store the minimal amount of data in state and do not store data that can be computed. For instance here you'd store the entries in state but you wouldn't store the filtered entries. Instead, store the filter itself and compute the filtered entries from the data and the filter. That way it's much easier to keep everything in sync, otherwise you might update the filter in state but forget to update the filtered data, etc.
promises in js resolves asynchronously and here there's no telling which one will resolve last (even if the request for pokemon 199 is fired last, the response might come back earlier than pokemon 1), hence the setMainLoading fn is flawed. Here to check that all the data has been loaded you could verify that each entry has been populated, or increase a counter when every response come so when it reaches 199 you know everything is loaded.
Thank you for taking such an indepth look into my code, it's exactly the kind of feedback I need as well as providing solutions. I see now that I need to be more careful when handling api calls and include pagination, as well as be smarter when it comes to seeing if something has loaded or not. I will try to implement these changes!
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
One big issue with your app is you are literally sending hundreds of requests to the poke api when someone lands on your page (two requests for each pokemon). That is inconceivable for a real app and even here it's likely that it's taking a toll on the poke api server(s).
You normally want a single requests for multiple pokemons (like
"https://pokeapi.co/api/v2/pokemon?offset=20&limit=20", theoffsetandlimitparameters are useful for pagination).Apart from that, the code itself has room for improvement, among other things:
the variables
entries,filteredEntries... at the top level don't belong there. In react, if the UI depends on some data, this data should be in state. Now your app may seem to be working but you're actually lucky that the UI somehow remains in sync with the data, as only changes to state or props trigger a rerender (UI update). At the top level of a react app, you normally find static data which is not related to the UI.you should follow the principle of minimal state (reactjs.org/docs/thinking-in-react...) i.e store the minimal amount of data in state and do not store data that can be computed. For instance here you'd store the entries in state but you wouldn't store the filtered entries. Instead, store the filter itself and compute the filtered entries from the data and the filter. That way it's much easier to keep everything in sync, otherwise you might update the filter in state but forget to update the filtered data, etc.
promises in js resolves asynchronously and here there's no telling which one will resolve last (even if the request for pokemon 199 is fired last, the response might come back earlier than pokemon 1), hence the
setMainLoadingfn is flawed. Here to check that all the data has been loaded you could verify that each entry has been populated, or increase a counter when every response come so when it reaches 199 you know everything is loaded.Thank you for taking such an indepth look into my code, it's exactly the kind of feedback I need as well as providing solutions. I see now that I need to be more careful when handling api calls and include pagination, as well as be smarter when it comes to seeing if something has loaded or not. I will try to implement these changes!