1
\$\begingroup\$

The code makes unnecessary API calls because there are two useEffect hooks that both fetch data. The first useEffect runs on mount and fetches data for page 1, while the second useEffect runs whenever the page state changes. This causes two API calls when the component first loads, one from each useEffect. To fix this, you can merge them into a single useEffect that runs when page changes, ensuring the API is only called when needed.

What hints could you share about the source like below (what is wrong with the code?):

import { useState, useEffect } from 'react'; import { createRoot } from "react-dom/client"; const fetchTodos = async (page = 1, items = 10) => { const res = await fetch(`/api/todos?items=${items}&page=${page}`); return res.json(); }; function App() { const [todos, setTodos] = useState([]); const [page, setPage] = useState(1); useEffect(function initialFetch() { fetchTodos(1, 10).then(setTodos); }, []); useEffect( function fetchOnPageChange() { fetchTodos(page, 10).then(setTodos); }, [page] ); return ( <div> <h2>Todo List</h2> <table className="table"> <tbody> <tr className="table__row"> <td className="table__row__cell">Title</td> <td className="table__row__cell">Completed</td> </tr> {todos.map((todo) => ( <tr className="table__row"> <td className="table__row__cell">{todo.title}</td> <td className="table__row__cell">{todo.completed ? "Yes" : 'No'}</td> </tr> ))} </tbody> </table> <button onClick={() => setPage(page - 1)}>Previous</button> <button onClick={() => setPage(page + 1)}>Next</button> </div> ); } const root = createRoot(document.getElementById('app')); root.render(<App />); 
\$\endgroup\$
0

    1 Answer 1

    3
    \$\begingroup\$

    The first paragraph (presumably from an LLM) is good advice. Let's apply it!

    Other than that, your component looks reasonable for the most part, with some suggestions and minor fixes below.

    Here's a brief rewrite, with some browserification purely for ease of running (pagination is not expected to work since jsonplaceholder doesn't offer a mock endpoint with pagination), with remarks following:

    <script type="text/babel" defer> const {useState, useEffect} = React; const baseApiUrl = "https://jsonplaceholder.typicode.com"; const fetchTodos = async ({page = 1, items = 10}) => { const res = await fetch(`${baseApiUrl}/todos?page=${page}&items=${items}`); if (!res.ok) { throw Error(res.statusText); } return res.json(); }; const App = () => { const [todos, setTodos] = useState([]); const [page, setPage] = useState(1); useEffect(() => { fetchTodos({page, items: 10}).then(setTodos); }, [page]); return ( <div> <h2>Todo List</h2> <table className="table"> <tbody> <tr className="table__row"> <td className="table__row__cell">Title</td> <td className="table__row__cell">Completed</td> </tr> {todos.map((todo) => ( <tr key={todo.id} className="table__row"> <td className="table__row__cell">{todo.title}</td> <td className="table__row__cell"> {todo.completed ? "Yes" : "No"} </td> </tr> ))} </tbody> </table> <button onClick={() => setPage(page - 1)}>Previous</button> <button onClick={() => setPage(page + 1)}>Next</button> </div> ); }; const root = ReactDOM.createRoot(document.getElementById("app")); root.render(<App />); </script> <script src="https://cdnjs.cloudflare.com/ajax/libs/babel-standalone/6.26.0/babel.min.js"></script> <script src="https://unpkg.com/react@18/umd/react.development.js"></script> <script src="https://unpkg.com/react-dom@18/umd/react-dom.development.js"></script> <div id="app"></div>

    Suggestions:

    1. Handle fetch non-2xx statuses by throwing so the caller can do something about it. As your app grows, you'll likely want to use a library like react-query that handles errors in state nicely. fetch is low-level and would take a lot of wheel reinvention and boilerplate to get it to handle errors nicely in a larger app.
    2. Check the console for warnings. Each child in a list created with map should have a unique key= prop.
    3. Use named parameters for fetchTodos.
    4. Your pages can go to 0, -1, -2... probably not expected. Clamp with Math.max(page, 1) and consider disabling or removing the prev button on page 1.
    5. Use Prettier to format your code.
    \$\endgroup\$
    2
    • \$\begingroup\$Please fix the typo(s) on the await fetch line - I wouldn't have spotted that myself without syntax highlighting. $ is missing from ${} interpolation of page and items\$\endgroup\$CommentedMar 30 at 15:52
    • \$\begingroup\$Thanks, although those parameters are ignored anyway. It's a mock endpoint that doesn't support pagination.\$\endgroup\$
      – ggorlen
      CommentedMar 30 at 15:53

    Start asking to get answers

    Find the answer to your question by asking.

    Ask question

    Explore related questions

    See similar questions with these tags.