Merge pull request 'fix(recipes): stoppe le scroll infini des sources au lieu de boucler' (#46) from fix/source-scroll-loop into main
All checks were successful
CI / lint (push) Successful in 3m27s
CI / build (push) Successful in 4m22s
CI / intent-service-test (push) Successful in 4m58s
CI / e2e (push) Successful in 13m0s
CI / test (push) Successful in 19m55s

This commit is contained in:
kyuno 2026-09-05 21:00:22 +02:00
commit ec26f32dd8
7 changed files with 339 additions and 37 deletions

View file

@ -66,20 +66,91 @@ function detailUrl(idMeal: string): string {
return `https://www.themealdb.com/meal/${idMeal}`;
}
/** Category-list response shape (`/list.php?c=list`) — same `meals` envelope as everything else, but each row only carries `strCategory`. */
interface TheMealDbCategoryListResponse {
meals: { strCategory: string | null }[] | null;
}
/** Narrows to meals that actually carry a name — a titleless row isn't worth surfacing in a browse list. */
function hasName(meal: TheMealDbMeal): meal is TheMealDbMeal & { strMeal: string } {
return Boolean(meal.strMeal);
}
/** Maps one raw meal (from `/search.php` or `/filter.php`) into the generic list-item shape. */
function toListItem(meal: TheMealDbMeal & { strMeal: string }) {
return {
externalId: meal.idMeal,
title: meal.strMeal,
picture: meal.strMealThumb,
url: detailUrl(meal.idMeal),
};
}
/**
* Name search (`/search.php?s=`) — the only mode TheMealDB's free tier
* offers for a non-empty query. One response holds every match, so there's
* never a next page.
*/
async function searchByName(query: string): Promise<RecipeSourceListResult> {
try {
const data = await fetchTheMealDb<TheMealDbMealsResponse>(
`/search.php?s=${encodeURIComponent(query)}`,
);
return { items: (data.meals ?? []).filter(hasName).map(toListItem), nextCursor: null };
} catch (err) {
throw err; // see fetchTheMealDb()'s catch comment above
}
}
/**
* "Browse everything" (an empty/omitted query). TheMealDB's free tier has
* no catalog-wide listing endpoint — `/search.php?s=` only answers with a
* fixed ~25-meal sample — so this walks the category list instead:
* `/list.php?c=list` for the categories, then `/filter.php?c={category}`
* for one category at a time, exposing **one category per cursor step** so
* the infinite scroll keeps pulling real, distinct pages instead of
* stalling on that sample (issue #38). The category list is re-fetched on
* each step rather than cached in module state — it's a single small
* request, it keeps this adapter stateless like the others, and the walk
* is paced one step per scroll anyway.
*/
async function listByCategory(cursor: string | null | undefined): Promise<RecipeSourceListResult> {
try {
const parsed = Number.parseInt(cursor ?? "0", 10);
const startAt = Number.isNaN(parsed) || parsed < 0 ? 0 : parsed;
const categoryList = await fetchTheMealDb<TheMealDbCategoryListResponse>("/list.php?c=list");
const categories = (categoryList.meals ?? [])
.map((row) => row.strCategory)
.filter((name): name is string => Boolean(name));
const category = categories[startAt];
if (category === undefined) {
// Cursor has walked past the last category — the clean end of the list.
return { items: [], nextCursor: null };
}
const data = await fetchTheMealDb<TheMealDbMealsResponse>(
`/filter.php?c=${encodeURIComponent(category)}`,
);
const nextCursor = startAt + 1 < categories.length ? String(startAt + 1) : null;
return { items: (data.meals ?? []).filter(hasName).map(toListItem), nextCursor };
} catch (err) {
throw err; // see fetchTheMealDb()'s catch comment above
}
}
/**
* TheMealDB (themealdb.com) — a free, public recipe API (no scraping: the
* publisher's own structured JSON, hence `official: true`). The first real
* `RecipeSourceAdapter` implementation, proving the generic contract
* (recipe-source-adapter.ts) end to end against a live source.
*
* `list()` is search-only — TheMealDB has no dedicated "browse everything"
* endpoint on its free tier. An omitted `query` searches for an empty
* string, which TheMealDB happens to answer with a small default sample
* (~25 meals) rather than nothing — close enough to this contract's
* "omitted `query` means browse everything" convention
* (`RecipeSourceListParams.query`) to lean on as-is, though it's a fixed
* sample, not the whole catalog. Search isn't paginated either — one
* response holds every match, so `nextCursor` is always `null`.
* `list()` has two modes (see the helpers above): a non-empty `query` is a
* name search (`/search.php?s=`, single unpaginated response,
* `nextCursor: null`); an empty/omitted `query` — this contract's "browse
* everything" — walks the category list one category per cursor step,
* since the free tier has no real catalog-wide endpoint.
*/
export const theMealDbAdapter: RecipeSourceAdapter<TheMealDbMeal> = {
key: SOURCE_KEY,
@ -93,22 +164,8 @@ export const theMealDbAdapter: RecipeSourceAdapter<TheMealDbMeal> = {
async list(params: RecipeSourceListParams): Promise<RecipeSourceListResult> {
try {
const query = params.query ?? "";
const data = await fetchTheMealDb<TheMealDbMealsResponse>(
`/search.php?s=${encodeURIComponent(query)}`,
);
const meals = data.meals ?? [];
return {
items: meals
.filter((meal): meal is TheMealDbMeal & { strMeal: string } => Boolean(meal.strMeal))
.map((meal) => ({
externalId: meal.idMeal,
title: meal.strMeal,
picture: meal.strMealThumb,
url: detailUrl(meal.idMeal),
})),
nextCursor: null,
};
const query = (params.query ?? "").trim();
return query.length > 0 ? await searchByName(query) : await listByCategory(params.cursor);
} catch (err) {
throw err; // see fetchTheMealDb()'s catch comment above
}

View file

@ -16,6 +16,28 @@ function stubFetch(body: unknown, status = 200) {
globalThis.fetch = (async () => new Response(JSON.stringify(body), { status })) as typeof fetch;
}
/**
* Route-aware variant of {@link stubFetch} for the empty-query category
* walk, which fires several different requests (`/list.php?c=list`, then a
* `/filter.php?c=…`) within a single `list()` call. Matches by URL
* substring; an unstubbed path throws rather than silently returning junk.
* Returns the list of requested URLs, so a test can assert *which*
* endpoint was hit (e.g. category walk vs. `/search.php`).
*/
function stubFetchByUrl(routes: Record<string, unknown>, status = 200): string[] {
const calls: string[] = [];
globalThis.fetch = (async (input: string | URL | Request) => {
const url = typeof input === "string" ? input : input.toString();
calls.push(url);
const route = Object.keys(routes).find((fragment) => url.includes(fragment));
if (route === undefined) {
throw new Error(`stubFetchByUrl: no route matches ${url}`);
}
return new Response(JSON.stringify(routes[route]), { status });
}) as typeof fetch;
return calls;
}
const baseMeal: TheMealDbMeal = {
idMeal: "52795",
strMeal: "Chicken Handi",
@ -109,6 +131,113 @@ describe("theMealDbAdapter", () => {
expect((err as RecipeSourceFetchError).cause).to.be.instanceOf(Error);
}
});
// Empty/omitted query = "browse everything". The free tier has no
// catalog-wide endpoint, so the adapter walks categories one per
// cursor step (issue #38) rather than settling for `/search.php?s=`'s
// fixed ~25-meal sample.
describe("empty query — walks categories", () => {
const CATEGORIES = {
meals: [{ strCategory: "Beef" }, { strCategory: "Chicken" }, { strCategory: "Dessert" }],
};
it("returns the first category's meals, cursor pointing at the next category", async () => {
stubFetchByUrl({
"/list.php?c=list": CATEGORIES,
"/filter.php?c=Beef": {
meals: [
{ idMeal: "1", strMeal: "Beef Pie", strMealThumb: "https://img.test/1.jpg" },
{ idMeal: "2", strMeal: "Beef Stew", strMealThumb: null },
],
},
});
const result = await theMealDbAdapter.list({});
expect(result.items).to.deep.equal([
{
externalId: "1",
title: "Beef Pie",
picture: "https://img.test/1.jpg",
url: "https://www.themealdb.com/meal/1",
},
{
externalId: "2",
title: "Beef Stew",
picture: null,
url: "https://www.themealdb.com/meal/2",
},
]);
expect(result.nextCursor).to.equal("1");
});
it("follows the cursor into the next category", async () => {
const calls = stubFetchByUrl({
"/list.php?c=list": CATEGORIES,
"/filter.php?c=Chicken": {
meals: [{ idMeal: "3", strMeal: "Chicken Handi", strMealThumb: null }],
},
});
const result = await theMealDbAdapter.list({ cursor: "1" });
expect(calls.some((u) => u.includes("/filter.php?c=Chicken"))).to.equal(true);
expect(result.items.map((i) => i.externalId)).to.deep.equal(["3"]);
expect(result.nextCursor).to.equal("2");
});
it("stops (null cursor) on the last category", async () => {
stubFetchByUrl({
"/list.php?c=list": CATEGORIES,
"/filter.php?c=Dessert": {
meals: [{ idMeal: "9", strMeal: "Flan", strMealThumb: null }],
},
});
const result = await theMealDbAdapter.list({ cursor: "2" });
expect(result.nextCursor).to.be.null;
});
it("returns an empty final page once the cursor is past the last category", async () => {
stubFetchByUrl({ "/list.php?c=list": CATEGORIES });
const result = await theMealDbAdapter.list({ cursor: "3" });
expect(result.items).to.deep.equal([]);
expect(result.nextCursor).to.be.null;
});
it("treats a blank query the same as no query — category walk, never /search.php", async () => {
const calls = stubFetchByUrl({
"/list.php?c=list": CATEGORIES,
"/filter.php?c=Beef": {
meals: [{ idMeal: "1", strMeal: "Beef Pie", strMealThumb: null }],
},
});
await theMealDbAdapter.list({ query: " " });
expect(calls.some((u) => u.includes("/search.php"))).to.equal(false);
expect(calls.some((u) => u.includes("/list.php?c=list"))).to.equal(true);
});
it("skips a titleless meal in a category page, like search does", async () => {
stubFetchByUrl({
"/list.php?c=list": CATEGORIES,
"/filter.php?c=Beef": {
meals: [
{ idMeal: "1", strMeal: null, strMealThumb: null },
{ idMeal: "2", strMeal: "Beef Stew", strMealThumb: null },
],
},
});
const result = await theMealDbAdapter.list({});
expect(result.items.map((i) => i.externalId)).to.deep.equal(["2"]);
});
});
});
describe("fetchDetail", () => {

View file

@ -56,6 +56,18 @@ Feature: Browsing external recipe sources
And I should see the source item "Beef Wellington"
And I should not see "Voir plus"
Scenario: Walks several pages with distinct cursors, then stops, without re-requesting a page
Given the recipe catalog contains nothing
And the sources reference list has options
And the household has enabled TheMealDB
And browsing TheMealDB returns three pages, each addressed by its own cursor
When I visit "/recettes"
And I click the button "TheMealDB"
Then I should see the source item "Page 1 Item 1"
When I scroll the source list to the bottom 5 times
Then the third page's items should have loaded
And no browse cursor should have been requested more than twice
Scenario: Offers a retry when loading the next page fails
Given the recipe catalog contains nothing
And the sources reference list has options

View file

@ -132,6 +132,82 @@ Given("browsing TheMealDB returns two pages of items", () => {
});
});
/**
* Three full pages, each addressed by a distinct `cursor` value the mock
* actually parses (not just "does the URL contain `cursor=`"). Pages are
* deliberately large enough to overflow the list's own scroll container so
* the sentinel starts below the fold — each `scrollTo("bottom")` then
* produces a real IntersectionObserver transition, the only thing that
* makes the observer fire again. Records every cursor it serves so the
* scenario can assert none was requested twice — the tell-tale of the
* issue #38 loop, where a stale closure re-requests the same page forever
* instead of advancing.
*/
const THREE_PAGE_SIZE = 15;
const threePageCursorsRequested: string[] = [];
function threePagePage(pageNo: number, nextCursor: string | null) {
return {
items: Array.from({ length: THREE_PAGE_SIZE }, (_, index) => ({
externalId: `p${pageNo}-${index}`,
title: `Page ${pageNo} Item ${index + 1}`,
picture: null,
url: `https://www.themealdb.com/meal/p${pageNo}-${index}`,
alreadyImported: false,
recipeId: null,
})),
nextCursor,
};
}
Given("browsing TheMealDB returns three pages, each addressed by its own cursor", () => {
threePageCursorsRequested.length = 0;
const pages: Record<string, ReturnType<typeof threePagePage>> = {
"": threePagePage(1, "cursor-2"),
"cursor-2": threePagePage(2, "cursor-3"),
"cursor-3": threePagePage(3, null),
};
cy.intercept("GET", "**/sources/theMealDb/browse*", (req) => {
const cursor = new URL(req.url).searchParams.get("cursor") ?? "";
threePageCursorsRequested.push(cursor);
req.reply({ statusCode: 200, body: pages[cursor] ?? { items: [], nextCursor: null } });
});
});
When("I scroll the source list to the bottom {int} times", (times: number) => {
for (let attempt = 0; attempt < times; attempt += 1) {
cy.get(".source-items-column .recipe-table-wrap").scrollTo("bottom", {
ensureScrollable: false,
});
// IntersectionObserver callbacks land on their own task, and the next
// page's fetch + merge then needs a render — give both a beat before
// the next scroll (which needs fresh content below it to have anything
// new to reveal).
cy.wait(350);
}
});
Then("the third page's items should have loaded", () => {
// Page 3's marker sits ~30 rows down inside the list's own overflow
// container — scroll it in before asserting, like `common.steps.ts`'s
// ingredient-row check does. With the stale-closure loop it never loads
// at all: `handleLoadMore` keeps re-requesting cursor-2.
cy.contains(".recipe-table__name", "Page 3 Item 1").scrollIntoView().should("be.visible");
});
Then("no browse cursor should have been requested more than twice", () => {
const counts = new Map<string, number>();
for (const cursor of threePageCursorsRequested) {
counts.set(cursor, (counts.get(cursor) ?? 0) + 1);
}
// React StrictMode double-invokes effects in the e2e dev server (see
// recipes.cy.ts), so the panel's initial fetch + one-page-ahead prefetch
// can each fire twice — hence "twice", not "once". The issue #38 loop
// blows well past that: every scroll re-requests the same stale cursor.
const offenders = [...counts.entries()].filter(([, n]) => n > 2);
expect(offenders, `cursor request counts: ${JSON.stringify([...counts])}`).to.have.length(0);
});
// The panel prefetches the next page as soon as page 1 is on screen (before
// anyone's actually waited on it), so the *first* request for it is that
// prefetch — this is what actually fails "once", not a request triggered by

View file

@ -236,11 +236,22 @@ export function RecipeSourcesPanel({
.then(({ items, nextCursor }) => {
nextPagePrefetchRef.current = null;
isLoadingMoreRef.current = false;
setBrowseState((prev) =>
prev.status === "loaded"
? { status: "loaded", items: [...prev.items, ...items], nextCursor }
: prev,
);
setBrowseState((prev) => {
if (prev.status !== "loaded") return prev;
// Merge deduplicated by `externalId`, and treat "this page added
// nothing genuinely new" as the end of the list even if the
// source still returned a non-null `nextCursor`. Belt-and-braces
// with `SourceItemTable`'s latest-ref fix against the issue #38
// loop (a stale cursor re-fetching the same page forever), and a
// guard against a source whose own end-of-catalog heuristic is
// imperfect (e.g. 750g's "page with no cards").
const seen = new Set(prev.items.map((item) => item.externalId));
const freshItems = items.filter((item) => !seen.has(item.externalId));
if (freshItems.length === 0) {
return { status: "loaded", items: prev.items, nextCursor: null };
}
return { status: "loaded", items: [...prev.items, ...freshItems], nextCursor };
});
setLoadMoreStatus("idle");
})
.catch(() => {

View file

@ -45,12 +45,24 @@ export function SourceItemTable({
const { t } = useTranslation();
const scrollContainerRef = useRef<HTMLDivElement | null>(null);
const sentinelRef = useRef<HTMLTableRowElement | null>(null);
// Keeps the observer callback pointed at the *current* render's
// `onLoadMore` without re-subscribing the observer on every render.
// `RecipeSourcesPanel` re-declares `handleLoadMore` — and the
// `browseState.nextCursor` it closes over — on every render; the effect
// below only re-runs when `hasMore` flips (which it doesn't during a
// normal scroll). Calling `onLoadMore` directly would therefore pin the
// observer to the very first render's closure forever, re-requesting
// that stale cursor page after page so `nextCursor` never reaches its
// real `null` and the list loops (issue #38). Reading through a ref
// reassigned each render fixes that while keeping the "subscribe once"
// intent.
const onLoadMoreRef = useRef(onLoadMore);
onLoadMoreRef.current = onLoadMore;
// biome-ignore lint/correctness/useExhaustiveDependencies: `onLoadMore` is deliberately excluded — RecipeSourcesPanel passes a fresh function identity on every render, and re-subscribing the observer on every single render (rather than only when hasMore actually flips) would be wasteful busywork for no behavioral difference.
useEffect(() => {
const root = scrollContainerRef.current;
const sentinel = sentinelRef.current;
if (!hasMore || !onLoadMore || !root || !sentinel) return;
if (!hasMore || !root || !sentinel) return;
// `rootMargin` starts loading the next page slightly before the
// sentinel actually reaches the visible edge — combined with
@ -59,7 +71,7 @@ export function SourceItemTable({
// need it, not just "as soon as" they do.
const observer = new IntersectionObserver(
(entries) => {
if (entries.some((entry) => entry.isIntersecting)) onLoadMore();
if (entries.some((entry) => entry.isIntersecting)) onLoadMoreRef.current?.();
},
{ root, rootMargin: "200px 0px" },
);

View file

@ -559,11 +559,16 @@ pas encore de ligne `Source` — jamais une erreur).
- **`the-meal-db.ts`** — `key: "theMealDb"`, `official: true`, `locale: "en"`.
API publique gratuite (`https://www.themealdb.com/api/json/v1/${API_KEY}`,
`THE_MEAL_DB_API_KEY` env var, défaut `"1"` = clé de test partagée
documentée par TheMealDB). `list()` n'a qu'une recherche (`/search.php?s=`),
pas de vrai "tout parcourir" côté gratuit — une requête vide renvoie un
petit échantillon fixe (~25 recettes), non paginé (`nextCursor` toujours
`null`). `parse()` reconstruit les ingrédients depuis les paires plates
`strIngredient1..20`/`strMeasure1..20`.
documentée par TheMealDB). `list()` a deux modes : une `query` non vide est
une recherche par nom (`/search.php?s=`, réponse unique non paginée,
`nextCursor: null`) ; une `query` vide/absente ("tout parcourir") **déroule
la liste des catégories** — `/list.php?c=list` une fois, puis
`/filter.php?c={catégorie}` une catégorie à la fois, le `cursor` étant
l'index de catégorie (une catégorie = une "page"). Le palier gratuit n'a
pas de vrai endpoint catalogue global ; `/search.php?s=` (vide) ne renvoie
qu'un échantillon fixe (~25 recettes), donc le scroll infini s'arrêtait
aussitôt (issue #38). `parse()` reconstruit les ingrédients depuis les
paires plates `strIngredient1..20`/`strMeasure1..20`.
- **`json-ld-recipe.ts`** — `key: "jsonLdRecipe"`, `official: false`,
scraper générique schema.org/`Recipe` (extraction regex des blocs
`<script type="application/ld+json">`, gère objet nu / tableau de types