refactor(client): drop the dead loader-data guards in three screens

[gilgardosh]

Step 5 of 10 in the urql quick-wins sequence — tracking doc in #4437.

⚠️ Stacked on #4444 (step 4). Retarget to main once that merges.

What changed

The charge, business and contracts screens each wrapped useLoaderData() in a try/catch with an // eslint-disable-next-line react-hooks/rules-of-hooks, guarding against being rendered outside a data router. None of them can be — each is only ever a lazy route element of createBrowserRouter, and none has a JSX call site anywhere in the app (verified by grep).

contracts.tsx goes further. Its route declares no loader at all — only chargeLoader and businessLoader exist — so loaderData was always undefined there and every branch reading it was unreachable. Deleted rather than rewritten.

charge.tsx also loses a mount effect that re-executed a query already unpaused under the identical condition.

Two corrections to what I claimed when planning this

Both came out of writing the test first, and both are worth reading before approving:

1. The effect was not costing a second request. I said in the plan that it caused "a duplicate network round-trip on every mount." It doesn't — urql dedupes the re-execution against the still-in-flight operation. The new test establishes this by counting the operations a scripted fetch actually receives: it was already 1 before the change. So this is a redundancy cleanup, not a performance fix, and the test's job is to hold that count at one across the removal.

2. The try/catch was not entirely dead code. useLoaderData() genuinely throws outside a data router. My first draft of the test used MemoryRouter — which is not a data router — and the suite went red with useLoaderData must be used within a data router. The test was modelling an unsupported configuration, not finding a bug.

Rewritten against createMemoryRouter + RouterProvider, which is what these components actually live inside. That is also precisely why removing the guard is safe only for components that are exclusively route elements — which is the condition I verified for all three.

Testing

New charge.test.tsx using the scripted-fetch real-Client harness from charges-table-refetch.test.tsx. The route deliberately declares no loader, which is what leaves useLoaderData undefined and lets the query run at all.

yarn test:client   48 files, 373 passed, 0 skipped
yarn lint          0 errors
tsc --noEmit       clean

Note for later

useRouteLoaderData is deliberately not used — no route in router/config.tsx declares an id, and it would need one.

Separately, worth a follow-up: since the :chargeId route always has a loader, charge.tsx's useQuery is permanently paused in production, and its chargeId prop has no callers. The whole query path there looks unreachable. Out of scope here, but I've noted it in the tracking doc.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TbrGL3NndzRiEwJkwHxnbm

---

Generated by Claude Code

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Simplifies route-only screen data loading and removes redundant query re-execution.

Changes:

• Removes try/catch loader guards from charge and business screens.
• Removes unreachable loader-data handling from contracts.
• Adds a charge request-count regression test and changeset.

File summaries

File	Description
packages/client/src/components/screens/charges/charge.tsx	Updated as part of this pull request.
packages/client/src/components/screens/charges/tests/charge.test.tsx	Updated as part of this pull request.
packages/client/src/components/screens/businesses/clients/contracts/contracts.tsx	Updated as part of this pull request.
packages/client/src/components/screens/businesses/business.tsx	Updated as part of this pull request.
.changeset/urql-loader-data-guards.md	Updated as part of this pull request.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.