App.tsx holds the whole catalogue-fetching machine — extract it as a useCatalogue hook #98

New Issue

2026-08-21T15:57:03-05:00

bermudalamb commented

2026-08-21 15:57:03 -05:00

App.tsx is 326 lines, and roughly sixty of them are one cohesive concern that has nothing to do with laying out a page: fetching the catalogue for the current filters, debouncing it, and negotiating with the session before it can ask.

What is actually tangled

Six pieces of state, four callbacks and two effects all serve a single job:

items, loading, failed, options        state
filters, filterKey                     derived from the URL
awaitingAuth, needsFavoritesAuth       derived from filters + session
load, reload, handleRetry              the request
the debounce effect                    when to run it

None of it is about rendering a header, a footer, a filter drawer or an auth modal — which is what the remaining two hundred lines do. The component is a page and a data layer sharing one scope.

This is the same shape #81 already dealt with once: App.tsx was over the cognitive-complexity limit at 22 and came under it by extracting Catalogue. That took out the rendering half. The state half is still here.

The extraction

A useCatalogue(filters) hook in its own module returning { items, loading, failed, needsFavoritesAuth, reload, retry }. App keeps the URL as the source of filter truth — that part genuinely belongs to the page — and passes them in.

What that buys, beyond a shorter file:

It becomes testable. There is no frontend unit suite today (#72), but a hook with a clear input and output is the first thing worth testing when there is one. The behaviour it holds is not trivial: a request that must not fire before the session resolves, a debounce, and a failure that must not be rendered as an empty shop.
The auth negotiation stops being invisible. awaitingAuth and needsFavoritesAuth encode a rule the comments explain at length — that firing early would 401 and show a signed-in customer an outage banner. Inside a named hook that rule has somewhere to live.
App reads as what it is, a page that composes a header, a catalogue, a drawer and a modal.

One oddity worth fixing while in there

load currently round-trips the filters through a string and back:

const filterKey = filtersToSearchParams(filters).toString();

const load = useCallback(() => {
  return fetchItems(filtersFromSearchParams(new URLSearchParams(filterKey)))
  ...
}, [filterKey]);

Serialise, then immediately re-parse. The intent is sound and worth keeping — depending on a string rather than the filters object means load stays stable when filters are value-equal, which is what stops the debounce effect re-firing on every render. But the re-parse is a workaround for the dependency, not something the request needs, and it deserves either a comment saying so or a shape where the memoized filters can be used directly.

Not in scope

The other large files — Customers.tsx at 451 lines and Admin.tsx at 388 — have the same smell and are deliberately left alone. This issue is about the one that was asked about, and lumping three refactors together would make all three harder to review.

Verification

The end-to-end suite already covers what this must not break: the storefront listing, the filters, the favorites-requires-sign-in prompt, and the failure banner that must never be rendered as an empty catalogue. No behaviour change is intended, so a green suite is the bar.

Severity

Low. Nothing is broken; this is maintainability, and specifically about making the least-covered file in the project (see #79) into something that can be covered at all.

Found during a React best-practices review.

`App.tsx` is 326 lines, and roughly sixty of them are one cohesive concern that has nothing to do with laying out a page: fetching the catalogue for the current filters, debouncing it, and negotiating with the session before it can ask. ## What is actually tangled Six pieces of state, four callbacks and two effects all serve a single job: ``` items, loading, failed, options state filters, filterKey derived from the URL awaitingAuth, needsFavoritesAuth derived from filters + session load, reload, handleRetry the request the debounce effect when to run it ``` None of it is about rendering a header, a footer, a filter drawer or an auth modal — which is what the remaining two hundred lines do. The component is a page and a data layer sharing one scope. This is the same shape #81 already dealt with once: `App.tsx` was over the cognitive-complexity limit at 22 and came under it by extracting `Catalogue`. That took out the *rendering* half. The state half is still here. ## The extraction A `useCatalogue(filters)` hook in its own module returning `{ items, loading, failed, needsFavoritesAuth, reload, retry }`. `App` keeps the URL as the source of filter truth — that part genuinely belongs to the page — and passes them in. What that buys, beyond a shorter file: - **It becomes testable.** There is no frontend unit suite today (#72), but a hook with a clear input and output is the first thing worth testing when there is one. The behaviour it holds is not trivial: a request that must not fire before the session resolves, a debounce, and a failure that must not be rendered as an empty shop. - **The auth negotiation stops being invisible.** `awaitingAuth` and `needsFavoritesAuth` encode a rule the comments explain at length — that firing early would 401 and show a signed-in customer an outage banner. Inside a named hook that rule has somewhere to live. - **`App` reads as what it is**, a page that composes a header, a catalogue, a drawer and a modal. ## One oddity worth fixing while in there `load` currently round-trips the filters through a string and back: ```tsx const filterKey = filtersToSearchParams(filters).toString(); const load = useCallback(() => { return fetchItems(filtersFromSearchParams(new URLSearchParams(filterKey))) ... }, [filterKey]); ``` Serialise, then immediately re-parse. The intent is sound and worth keeping — depending on a *string* rather than the filters object means `load` stays stable when filters are value-equal, which is what stops the debounce effect re-firing on every render. But the re-parse is a workaround for the dependency, not something the request needs, and it deserves either a comment saying so or a shape where the memoized `filters` can be used directly. ## Not in scope The other large files — `Customers.tsx` at 451 lines and `Admin.tsx` at 388 — have the same smell and are deliberately left alone. This issue is about the one that was asked about, and lumping three refactors together would make all three harder to review. ## Verification The end-to-end suite already covers what this must not break: the storefront listing, the filters, the favorites-requires-sign-in prompt, and the failure banner that must never be rendered as an empty catalogue. No behaviour change is intended, so a green suite is the bar. ## Severity Low. Nothing is broken; this is maintainability, and specifically about making the least-covered file in the project (see #79) into something that can be covered at all. Found during a React best-practices review.

bermudalamb added this to the Code Quality and Hardening 2 project 2026-08-21 16:08:38 -05:00

bermudalamb self-assigned this 2026-08-21 16:09:01 -05:00

bermudalamb referenced this issue from a commit

2026-08-24 08:57:02 -05:00

refactor(storefront): extract the catalogue machine from App.tsx as useCatalogue (#98)

bermudalamb referenced this issue from a commit

2026-08-24 09:01:00 -05:00

refactor(storefront): extract the catalogue machine from App.tsx as useCatalogue (#98)

bermudalamb referenced this issue

2026-08-24 09:04:13 -05:00

refactor(storefront): extract the catalogue machine from App.tsx as useCatalogue (#98) #153

bermudalamb added reference feature/98-use-catalogue