reactjs / reactjs/react.dev

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

Ouverte
#8,506 0 commentaires 1 réaction 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

type: documentation
Langage dominant
JavaScript
Étoiles
11.8k
Forks
7.9k
Merge moyen
1 j 11 h
PR mergées (30 j)
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.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Commencez par la section Fetching Data de la page You Might Not Need an Effect et comparez son exemple avec les indications environnantes sur les gestionnaires d’événements et le partage de logique. Examinez les scénarios proposés concernant l’URL et la pagination, puis ne mettez à jour la documentation que si l’explication distingue systématiquement les événements utilisateur de la synchronisation ; le travail est considéré comme terminé lorsque l’exemple et le texte environnant ne donnent plus de conseils contradictoires.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
javascript, react
Domaine
documentation
Type d'issue
Documentation
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.