Sources externes : le scroll infini boucle au lieu de s'arrêter + revue de la couverture en recherche vide #38

New issue

Closed

opened 2026-09-05 16:15:19 +02:00 by kyuno · 0 comments

kyuno commented

2026-09-05 16:15:19 +02:00

Owner

Contexte

Deux demandes liées, dans le flux "parcourir une source externe" (onglets
RecipeSourcesPanel.tsx, apps/web/src/features/recipes/sources/) :

Un vrai bug : le scroll infini boucle indéfiniment au lieu de s'arrêter
proprement une fois le catalogue épuisé.
Une revue : pour les 4 sources déjà enregistrées, quelle est la meilleure
façon de remonter le maximum de recettes quand la recherche est vide
("parcourir tout") ?

Deux explorations dédiées (front / back) ont permis de localiser précisément
la cause du bug plutôt que de la supposer, et de caractériser le comportement
réel de chaque source en recherche vide plutôt que de deviner (y compris une
vérification en direct de Marmiton, non couverte par l'exploration initiale).

Partie 1 — Le bug de bouclage

Cause exacte

apps/web/src/features/recipes/sources/SourceItemTable.tsx:49-68 — l'effet
qui crée l'IntersectionObserver a pour unique dépendance [hasMore] :
onLoadMore en est délibérément exclu (biome-ignore justifié par "éviter
un réabonnement inutile à chaque rendu").

// biome-ignore lint/correctness/useExhaustiveDependencies: `onLoadMore` est
// délibérément exclu — RecipeSourcesPanel passe une nouvelle référence de
// fonction à chaque rendu...
useEffect(() => {
  const root = scrollContainerRef.current;
  const sentinel = sentinelRef.current;
  if (!hasMore || !onLoadMore || !root || !sentinel) return;

  const observer = new IntersectionObserver(
    (entries) => {
      if (entries.some((entry) => entry.isIntersecting)) onLoadMore();
    },
    { root, rootMargin: "200px 0px" },
  );
  observer.observe(sentinel);
  return () => observer.disconnect();
}, [hasMore]);

hasMore (browseState.nextCursor !== null, RecipeSourcesPanel.tsx:316)
reste true sur toute la durée d'un scroll qui progresse normalement —
l'observer n'est donc créé qu'une seule fois et capture pour toujours
la référence onLoadMore (= handleLoadMore, RecipeSourcesPanel.tsx:214-255)
de ce rendu-là. Or handleLoadMore est une fonction re-déclarée à chaque
rendu (pas de useCallback), qui fige browseState.nextCursor par
valeur au moment de sa création :

// RecipeSourcesPanel.tsx:214-222
function handleLoadMore() {
  if (browseState.status !== "loaded" || !browseState.nextCursor) return;
  if (isLoadingMoreRef.current) return;
  isLoadingMoreRef.current = true;
  const cursor = browseState.nextCursor; // figé à la valeur de CE rendu
  ...

Séquence du bug : après le tout premier chargement automatique
(closure fraîche, fonctionne), chaque intersection suivante ré-exécute la
même closure figée, qui redemande le même curseur périmé — jamais la
vraie valeur courante de browseState.nextCursor. Le merge
items: [...prev.items, ...items] (RecipeSourcesPanel.tsx:241) n'ayant
aucune déduplication, la liste visible s'allonge en boucle avec les mêmes
éléments. nextCursor ne peut alors mécaniquement jamais atteindre le vrai
null final via ce chemin, hasMore ne repasse jamais à false, la
sentinelle (rendue tant que hasMore, SourceItemTable.tsx:110-114) reste
dans le DOM, et l'observer — jamais recréé — continue de se déclencher :
boucle infinie.

Fait notable : l'effet de préchargement "une page d'avance"
(nextPagePrefetchRef, RecipeSourcesPanel.tsx:183-212) a lui un tableau de
dépendances correct et complet ([browseState, sourceKey, debouncedSearch])
— il avance bien vers le vrai curseur courant à chaque fois. Mais ses
résultats ne sont jamais consommés par le scroll automatique bloqué sur
l'ancienne closure : il tourne dans le vide en parallèle du bug.

Pourquoi aucun test ne l'a vu

Il faut au moins 3 pages pour que le bug se manifeste (le tout premier
déclenchement automatique utilise toujours une closure fraîche — c'est le
second déclenchement automatique, qui suppose qu'un premier chargement a
déjà réussi, qui utilise la closure périmée). Le seul scénario Cypress
existant (recipe-sources.feature, "Loads further pages automatically, with
no load more button") ne simule que 2 pages, et son mock détecte "c'est
la dernière page" via req.url.includes("cursor=") plutôt que la valeur
réelle du curseur — il ne peut donc pas distinguer "a bien avancé de la page
2 à la page 3" de "a redemandé la page 2 en boucle".

Fix recommandé

Pattern "latest ref" — garde [hasMore] comme seule dépendance (donc pas de
réabonnement inutile, l'intention d'origine du biome-ignore reste
respectée), mais l'observer appelle toujours la closure la plus fraîche :

// SourceItemTable.tsx
const onLoadMoreRef = useRef(onLoadMore);
onLoadMoreRef.current = onLoadMore; // réassigné à chaque rendu, sans effet dédié

useEffect(() => {
  ...
  const observer = new IntersectionObserver((entries) => {
    if (entries.some((entry) => entry.isIntersecting)) onLoadMoreRef.current?.();
  }, { root, rootMargin: "200px 0px" });
  ...
}, [hasMore]); // inchangé

Garde-fou complémentaire (répond directement à "je veux garder l'auto
scroll tant que de nouvelles recettes sont pas affichées") — dans
handleLoadMore (RecipeSourcesPanel.tsx), dédupliquer le merge par
externalId et traiter "cette page n'a ajouté aucun élément réellement
nouveau" comme un arrêt local, même si le serveur a renvoyé un nextCursor
non nul. Protège aussi contre un futur souci côté backend (l'heuristique de
fin de liste d'un adaptateur qui se tromperait), pas seulement contre ce bug
précis.

Test à ajouter : le mock Cypress doit simuler ≥3 pages réelles avec des
curseurs distincts effectivement vérifiés (pas juste "contient cursor=") —
sinon la même régression pourrait revenir sans qu'aucun test ne la voie,
comme cette fois.

Partie 2 — Maximiser les recettes par source en recherche vide

Source	Aujourd'hui, requête vide	Constat
TheMealDB	`search.php?s=` (vide) → échantillon fixe (~25 repas), `nextCursor` codé en dur à `null` (`the-meal-db.ts:110`)	Plafond artificiel, pas une limite réelle de l'API — TheMealDB expose d'autres endpoints jamais utilisés ici
Marmiton	`recherche.aspx?aqt=` (vide) — vérifié en direct : renvoie une vraie page de résultats (recettes les mieux notées : "Blanquette de veau", "La Vraie Moussaka Grecque"...), pagine ensuite via `&page=N`	Fonctionne déjà correctement ; fin de liste via 404, déjà documentée dans le code comme heuristique imparfaite mais opérationnelle
750g	`dernieres-recettes.htm?page=N` — pagine réellement ; commentaire du code : vérifié en direct jusqu'à la page 500 avec du contenu distinct à chaque fois	Pas de plafond connu, mais la détection de fin ("page sans aucune carte") pourrait en théorie ne jamais se déclencher si le site affiche un jour des cartes "suggestions" au-delà de la vraie fin — risque non confirmé, pas un bug constaté
Manger Bouger	Même chemin de code que pour une vraie recherche, `page=N` + `hasMorePages` renvoyé par le serveur lui-même	Le mieux implémenté des 4 — signal de fin fiable, pas de plafond artificiel

Seul vrai problème identifié : TheMealDB. Son list() n'a aucune branche
dédiée à la requête vide — il tape search.php?s= comme pour n'importe
quelle recherche, qui renvoie un échantillon fixe côté TheMealDB lui-même
(documenté dans le code, mais sans mention "vérifié en direct" contrairement
aux commentaires équivalents de 750g/Manger Bouger — potentiellement
approximatif).

Amélioration proposée : quand query est vide, énumérer par
catégorie plutôt que de rester sur search.php?s= — list.php?c=list
(une fois) pour obtenir la liste des catégories, puis
filter.php?c={catégorie} pour chacune, en traitant une catégorie = une
"page" du curseur existant (cursor = index de catégorie dans la liste).
Préféré à une énumération par lettre (search.php?f=a..z, ~26 appels) :
filter.php renvoie déjà exactement la forme dont RecipeSourceListItem a
besoin (id/nom/vignette), sans le détail complet que search.php renvoie
inutilement pour un simple listing (le détail complet reste de toute façon
récupéré séparément via fetchDetail/lookup.php au moment de l'import,
quelle que soit la richesse de list()).

Point d'attention : la clé API utilisée (THE_MEAL_DB_API_KEY ?? "1") est
documentée dans le code comme "une clé de test publique, partagée, gratuite
pour le développement" — dérouler l'énumération une catégorie à la fois (au
rythme du scroll, via le curseur) plutôt qu'en un seul burst reste
raisonnable pour ne pas la solliciter d'un coup.

750g/Marmiton/Manger Bouger : aucun changement de code requis pour cette
demande.

Fichiers à toucher

apps/web/src/features/recipes/sources/SourceItemTable.tsx — pattern
"latest ref" ci-dessus.
apps/web/src/features/recipes/sources/RecipeSourcesPanel.tsx — merge
dédupliqué par externalId + arrêt local si une page n'ajoute rien de
nouveau.
apps/web/cypress/e2e/recipe-sources.feature +
apps/web/cypress/support/step_definitions/recipe-sources.ts — nouveau
scénario à ≥3 pages avec curseurs distincts effectivement vérifiés.
apps/api/src/sources/the-meal-db.ts — list() : branche "requête vide"
→ énumération par catégorie décrite ci-dessus.
apps/api/test/recipe-sources/the-meal-db.test.ts — nouveaux cas pour
cette branche (aujourd'hui aucun test n'appelle list() avec une requête
vide du tout — seule lacune de couverture identifiée côté TheMealDB ;
750g.test.ts a déjà un bloc dédié à ce cas, manger-bouger.test.ts
l'exerce indirectement puisque son code ne distingue pas requête vide/non
vide).

Hors scope

Généraliser le garde-fou de déduplication à d'autres listes à scroll
infini de l'app — aucune autre identifiée à ce jour, SourceItemTable est
la seule.
Durcir l'heuristique "page sans carte" de 750g contre le risque théorique
ci-dessus — non confirmé comme un vrai bug, à réévaluer si ça se produit
réellement.

🤖 Filed by Claude Code

## Contexte Deux demandes liées, dans le flux "parcourir une source externe" (onglets `RecipeSourcesPanel.tsx`, apps/web/src/features/recipes/sources/) : 1. Un vrai bug : le scroll infini boucle indéfiniment au lieu de s'arrêter proprement une fois le catalogue épuisé. 2. Une revue : pour les 4 sources déjà enregistrées, quelle est la meilleure façon de remonter le maximum de recettes quand la recherche est vide ("parcourir tout") ? Deux explorations dédiées (front / back) ont permis de localiser précisément la cause du bug plutôt que de la supposer, et de caractériser le comportement réel de chaque source en recherche vide plutôt que de deviner (y compris une vérification en direct de Marmiton, non couverte par l'exploration initiale). ## Partie 1 — Le bug de bouclage ### Cause exacte `apps/web/src/features/recipes/sources/SourceItemTable.tsx:49-68` — l'effet qui crée l'`IntersectionObserver` a pour unique dépendance `[hasMore]` : `onLoadMore` en est délibérément exclu (`biome-ignore` justifié par "éviter un réabonnement inutile à chaque rendu"). ```tsx // biome-ignore lint/correctness/useExhaustiveDependencies: `onLoadMore` est // délibérément exclu — RecipeSourcesPanel passe une nouvelle référence de // fonction à chaque rendu... useEffect(() => { const root = scrollContainerRef.current; const sentinel = sentinelRef.current; if (!hasMore || !onLoadMore || !root || !sentinel) return; const observer = new IntersectionObserver( (entries) => { if (entries.some((entry) => entry.isIntersecting)) onLoadMore(); }, { root, rootMargin: "200px 0px" }, ); observer.observe(sentinel); return () => observer.disconnect(); }, [hasMore]); ``` `hasMore` (`browseState.nextCursor !== null`, `RecipeSourcesPanel.tsx:316`) reste `true` sur toute la durée d'un scroll qui progresse normalement — l'observer n'est donc créé **qu'une seule fois** et capture **pour toujours** la référence `onLoadMore` (= `handleLoadMore`, `RecipeSourcesPanel.tsx:214-255`) de ce rendu-là. Or `handleLoadMore` est une fonction re-déclarée à chaque rendu (pas de `useCallback`), qui fige `browseState.nextCursor` **par valeur** au moment de sa création : ```tsx // RecipeSourcesPanel.tsx:214-222 function handleLoadMore() { if (browseState.status !== "loaded" || !browseState.nextCursor) return; if (isLoadingMoreRef.current) return; isLoadingMoreRef.current = true; const cursor = browseState.nextCursor; // figé à la valeur de CE rendu ... ``` **Séquence du bug** : après le tout premier chargement automatique (closure fraîche, fonctionne), chaque intersection suivante ré-exécute la **même** closure figée, qui redemande **le même curseur périmé** — jamais la vraie valeur courante de `browseState.nextCursor`. Le merge `items: [...prev.items, ...items]` (`RecipeSourcesPanel.tsx:241`) n'ayant aucune déduplication, la liste visible s'allonge en boucle avec les mêmes éléments. `nextCursor` ne peut alors mécaniquement jamais atteindre le vrai `null` final via ce chemin, `hasMore` ne repasse jamais à `false`, la sentinelle (rendue tant que `hasMore`, `SourceItemTable.tsx:110-114`) reste dans le DOM, et l'observer — jamais recréé — continue de se déclencher : boucle infinie. Fait notable : l'effet de préchargement "une page d'avance" (`nextPagePrefetchRef`, `RecipeSourcesPanel.tsx:183-212`) a lui un tableau de dépendances correct et complet (`[browseState, sourceKey, debouncedSearch]`) — il avance bien vers le vrai curseur courant à chaque fois. Mais ses résultats ne sont jamais consommés par le scroll automatique bloqué sur l'ancienne closure : il tourne dans le vide en parallèle du bug. ### Pourquoi aucun test ne l'a vu Il faut **au moins 3 pages** pour que le bug se manifeste (le tout premier déclenchement automatique utilise toujours une closure fraîche — c'est le *second* déclenchement automatique, qui suppose qu'un premier chargement a déjà réussi, qui utilise la closure périmée). Le seul scénario Cypress existant (`recipe-sources.feature`, "Loads further pages automatically, with no load more button") ne simule que **2 pages**, et son mock détecte "c'est la dernière page" via `req.url.includes("cursor=")` plutôt que la valeur réelle du curseur — il ne peut donc pas distinguer "a bien avancé de la page 2 à la page 3" de "a redemandé la page 2 en boucle". ### Fix recommandé Pattern "latest ref" — garde `[hasMore]` comme seule dépendance (donc pas de réabonnement inutile, l'intention d'origine du `biome-ignore` reste respectée), mais l'observer appelle toujours la closure la plus fraîche : ```tsx // SourceItemTable.tsx const onLoadMoreRef = useRef(onLoadMore); onLoadMoreRef.current = onLoadMore; // réassigné à chaque rendu, sans effet dédié useEffect(() => { ... const observer = new IntersectionObserver((entries) => { if (entries.some((entry) => entry.isIntersecting)) onLoadMoreRef.current?.(); }, { root, rootMargin: "200px 0px" }); ... }, [hasMore]); // inchangé ``` **Garde-fou complémentaire** (répond directement à "je veux garder l'auto scroll tant que de nouvelles recettes sont pas affichées") — dans `handleLoadMore` (`RecipeSourcesPanel.tsx`), dédupliquer le merge par `externalId` et traiter "cette page n'a ajouté aucun élément réellement nouveau" comme un arrêt local, même si le serveur a renvoyé un `nextCursor` non nul. Protège aussi contre un futur souci côté backend (l'heuristique de fin de liste d'un adaptateur qui se tromperait), pas seulement contre ce bug précis. **Test à ajouter** : le mock Cypress doit simuler ≥3 pages réelles avec des curseurs distincts effectivement vérifiés (pas juste "contient cursor=") — sinon la même régression pourrait revenir sans qu'aucun test ne la voie, comme cette fois. ## Partie 2 — Maximiser les recettes par source en recherche vide | Source | Aujourd'hui, requête vide | Constat | |---|---|---| | **TheMealDB** | `search.php?s=` (vide) → échantillon fixe (~25 repas), `nextCursor` **codé en dur à `null`** (`the-meal-db.ts:110`) | Plafond artificiel, pas une limite réelle de l'API — TheMealDB expose d'autres endpoints jamais utilisés ici | | **Marmiton** | `recherche.aspx?aqt=` (vide) — **vérifié en direct** : renvoie une vraie page de résultats (recettes les mieux notées : "Blanquette de veau", "La Vraie Moussaka Grecque"...), pagine ensuite via `&page=N` | Fonctionne déjà correctement ; fin de liste via 404, déjà documentée dans le code comme heuristique imparfaite mais opérationnelle | | **750g** | `dernieres-recettes.htm?page=N` — pagine réellement ; commentaire du code : vérifié en direct jusqu'à la page 500 avec du contenu distinct à chaque fois | Pas de plafond connu, mais la détection de fin ("page sans aucune carte") pourrait en théorie ne jamais se déclencher si le site affiche un jour des cartes "suggestions" au-delà de la vraie fin — risque non confirmé, pas un bug constaté | | **Manger Bouger** | Même chemin de code que pour une vraie recherche, `page=N` + `hasMorePages` renvoyé par le serveur lui-même | Le mieux implémenté des 4 — signal de fin fiable, pas de plafond artificiel | **Seul vrai problème identifié : TheMealDB.** Son `list()` n'a aucune branche dédiée à la requête vide — il tape `search.php?s=` comme pour n'importe quelle recherche, qui renvoie un échantillon fixe côté TheMealDB lui-même (documenté dans le code, mais sans mention "vérifié en direct" contrairement aux commentaires équivalents de 750g/Manger Bouger — potentiellement approximatif). **Amélioration proposée** : quand `query` est vide, énumérer par **catégorie** plutôt que de rester sur `search.php?s=` — `list.php?c=list` (une fois) pour obtenir la liste des catégories, puis `filter.php?c={catégorie}` pour chacune, en traitant **une catégorie = une "page"** du curseur existant (`cursor` = index de catégorie dans la liste). Préféré à une énumération par lettre (`search.php?f=a..z`, ~26 appels) : `filter.php` renvoie déjà exactement la forme dont `RecipeSourceListItem` a besoin (id/nom/vignette), sans le détail complet que `search.php` renvoie inutilement pour un simple listing (le détail complet reste de toute façon récupéré séparément via `fetchDetail`/`lookup.php` au moment de l'import, quelle que soit la richesse de `list()`). Point d'attention : la clé API utilisée (`THE_MEAL_DB_API_KEY ?? "1"`) est documentée dans le code comme "une clé de test publique, partagée, gratuite pour le développement" — dérouler l'énumération une catégorie à la fois (au rythme du scroll, via le curseur) plutôt qu'en un seul burst reste raisonnable pour ne pas la solliciter d'un coup. **750g/Marmiton/Manger Bouger** : aucun changement de code requis pour cette demande. ## Fichiers à toucher - `apps/web/src/features/recipes/sources/SourceItemTable.tsx` — pattern "latest ref" ci-dessus. - `apps/web/src/features/recipes/sources/RecipeSourcesPanel.tsx` — merge dédupliqué par `externalId` + arrêt local si une page n'ajoute rien de nouveau. - `apps/web/cypress/e2e/recipe-sources.feature` + `apps/web/cypress/support/step_definitions/recipe-sources.ts` — nouveau scénario à ≥3 pages avec curseurs distincts effectivement vérifiés. - `apps/api/src/sources/the-meal-db.ts` — `list()` : branche "requête vide" → énumération par catégorie décrite ci-dessus. - `apps/api/test/recipe-sources/the-meal-db.test.ts` — nouveaux cas pour cette branche (aujourd'hui aucun test n'appelle `list()` avec une requête vide du tout — seule lacune de couverture identifiée côté TheMealDB ; `750g.test.ts` a déjà un bloc dédié à ce cas, `manger-bouger.test.ts` l'exerce indirectement puisque son code ne distingue pas requête vide/non vide). ## Hors scope - Généraliser le garde-fou de déduplication à d'autres listes à scroll infini de l'app — aucune autre identifiée à ce jour, `SourceItemTable` est la seule. - Durcir l'heuristique "page sans carte" de 750g contre le risque théorique ci-dessus — non confirmé comme un vrai bug, à réévaluer si ça se produit réellement. --- 🤖 Filed by [Claude Code](https://claude.com/claude-code)