reactjs / reactjs/react.dev

[Suggestion]: Wrong example in fetching data section on You Might not need a Effect Page

Open
#8,506 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

type: documentation
Dominant language
JavaScript
Stars
11.8k
Forks
7.9k
Avg merge
1d 11h
Merged PRs (30d)
11

Description

Summary

In the Fetching Data Section of You might not need an effect is solving for different scenario with different problem altogether which will a create a confusion whether it requires a effect or not

Page

https://react.dev/learn/you-might-not-need-an-effect#fetching-data

Details

The Fetching Data Section of You might not need an effect is solving a different scenario with a different problem altogether.

In this code block:

function SearchResults({ query }) {
  const [results, setResults] = useState([]);
  const [page, setPage] = useState(1);
  useEffect(() => {
    // 🔴 Avoid: Fetching without cleanup logic
    fetchResults(query, page).then(json => {
      setResults(json);
    });
  }, [query, page]);
  function handleNextPageClick() {
    setPage(page + 1);
  }
  // ...
}

The documentation mentions that:

This might seem like a contradiction with the earlier examples where you needed to put the logic into the event handlers! However, consider that it's not the typing event that's the main reason to fetch. Search inputs are often prepopulated from the URL, and the user might navigate Back and Forward without touching the input.

It doesn't matter where page and query come from. While this component is visible, you want to keep results synchronized with data from the network for the current page and query. This is why it's an Effect.

This encourages people to add page as a dependency in the useEffect and also categorize the page change event as an effect altogether. But page changes are user events (wrt to the context like the results are shown in a table), and the documentation should encourage people to fetch data inside the change handler rather than using an effect (similar to how we encourage handling POST request on a form).

Regarding the URL scenario: let's say the user goes to page 3, navigates to a different page, and comes back again. The URL page param should be read inside the effect callback like this:

function SearchResults({ query }) {
  const [results, setResults] = useState([]);
  const [page, setPage] = useState(new URLSearchParams(window.location.search).get("page"));
  
  useEffect(() => {
    const urlSearchParams = new URLSearchParams(window.location.search);
    const currentPage = urlSearchParams.get("page");
    fetchResults(query, currentPage).then(json => {
      setResults(json);
    });
  }, [query]);
  
  function handleNextPageClick() {
    setPage(page + 1);
    fetchResults(query, page + 1).then(json => {
      setResults(json);
    });
  }
  // ...
}

This way, page can be removed as a dependency altogether, and the effect only runs whenever the query changes while always getting the latest page param value. When the page is changed by the user, the fetch should be invoked inside handleNextPageClick since page change is an event.

The earlier example will result in more confusion, and people will add things that should be inside the event handler into the effect with dependencies, making Event handlers sharing logic obsolete too. This also make the people aware on avoiding the "setState triggers effect" pattern for user events and follows the same principle on removing effects and using events properly as the article intended.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Fetching Data section of the You Might Not Need an Effect page and compare its example with the surrounding guidance on event handlers and sharing logic. Review the proposed URL and pagination scenarios, then update the documentation only if the explanation consistently distinguishes user events from synchronization; done means the example and surrounding text no longer give conflicting advice.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, react
Domain
documentation
Issue type
Documentation
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.