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

.changeset/urql-loader-data-guards.md

@@ -0,0 +1,25 @@
+---
+'@accounter/client': patch
+---
+
+Simplify how the charge, business and contracts screens read their route loader data.
+
+All three 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. The guard is now removed and the disable with it.
+
+`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. The variable, its import and the `|| loaderData` / `!!loaderData` branches are deleted
+rather than rewritten.
+
+`charge.tsx` also drops a mount effect that re-executed a query already unpaused under the identical
+condition. Contrary to what the removal might suggest, this was not costing a second request: urql
+dedupes the re-execution against the still-in-flight operation, which a new test pins down by
+counting the operations a scripted `fetch` actually receives. The effect was redundant, not
+expensive, and the test now guards that the count stays at one.
+
+That test is written against `createMemoryRouter` rather than `MemoryRouter`, because
+`useLoaderData` throws outside a data router — which is also why the `try/catch` existed and why it
+is safe to remove only for components that are exclusively route elements.

packages/client/src/components/screens/businesses/business.tsx

@@ -18,14 +18,8 @@ import { AccounterLoader } from '../../common/index.js';
 export const BusinessScreen = (): ReactElement => {
   const { businessId } = useParams<{ businessId: string }>();
 
-  // Try to get loader data (will be available when navigating via router)
-  let loaderData: BusinessScreenQuery | undefined;
-  try {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    loaderData = useLoaderData() as BusinessScreenQuery;
-  } catch {
-    // No loader data - fallback to query
-  }
+  // Present when the route's loader ran; `undefined` on a route without one.
+  const loaderData = useLoaderData() as BusinessScreenQuery | undefined;
 
   // Only fetch if we don't have loader data
   const [{ data, fetching }, fetchBusiness] = useQuery({

packages/client/src/components/screens/businesses/clients/contracts/contracts.tsx

@@ -1,9 +1,8 @@
 import { useContext, type ReactElement } from 'react';
-import { useLoaderData } from 'react-router-dom';
 import { useQuery } from 'urql';
 import { AccounterLoader } from '@/components/common/index.js';
 import { ContractsTable } from '@/components/contracts/index.js';
-import { ContractsScreenDocument, type ContractsScreenQuery } from '@/gql/graphql.js';
+import { ContractsScreenDocument } from '@/gql/graphql.js';
 import { UserContext } from '@/providers/index.js';
 
 // eslint-disable-next-line @typescript-eslint/no-unused-expressions -- used by codegen
@@ -20,28 +19,14 @@ export const ContractsScreen = (): ReactElement => {
   const { userContext } = useContext(UserContext);
   const adminId = userContext?.context.adminBusinessId;
 
-  // Try to get loader data (will be available when navigating via router)
-  let loaderData: ContractsScreenQuery | undefined;
-  try {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    loaderData = useLoaderData() as ContractsScreenQuery;
-  } catch {
-    // No loader data - fallback to query
-  }
-
-  // Only fetch if we don't have loader data
-  const [{ data, fetching }] = useQuery({
+  const [{ data: contractsData, fetching: isLoading }] = useQuery({
     query: ContractsScreenDocument,
-    pause: !adminId || !!loaderData,
+    pause: !adminId,
     variables: {
       adminId: adminId ?? '',
     },
   });
 
-  // Use loader data if available, otherwise use query data
-  const contractsData = loaderData || data;
-  const isLoading = !loaderData && fetching;
-
   if (isLoading && !contractsData?.contractsByAdmin) {
     return <AccounterLoader />;
   }

packages/client/src/components/screens/charges/__tests__/charge.test.tsx

@@ -0,0 +1,83 @@
+// @vitest-environment happy-dom
+
+import { act } from 'react';
+import { createRoot, type Root } from 'react-dom/client';
+import { RouterProvider, createMemoryRouter } from 'react-router-dom';
+import { Client, Provider, fetchExchange } from 'urql';
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import { Charge } from '../charge.js';
+
+(globalThis as unknown as { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
+
+/** Builds an urql client backed by a scripted, request-recording fetch. */
+function makeClient(operations: string[]): Client {
+  const mockFetch = (async (_url: string, init: RequestInit) => {
+    const body = JSON.parse(String(init.body)) as { operationName: string };
+    operations.push(body.operationName);
+    const payload = JSON.stringify({ data: { charge: null } });
+    return {
+      status: 200,
+      ok: true,
+      headers: new Headers({ 'Content-Type': 'application/json' }),
+      text: async () => payload,
+      json: async () => JSON.parse(payload),
+    } as unknown as Response;
+  }) as unknown as typeof fetch;
+
+  return new Client({
+    url: 'http://localhost/graphql',
+    exchanges: [fetchExchange],
+    fetch: mockFetch,
+    // Keep every operation on POST so the harness can read the body.
+    preferGetMethod: false,
+  });
+}
+
+let container: HTMLDivElement;
+let root: Root;
+
+beforeEach(() => {
+  container = document.createElement('div');
+  document.body.append(container);
+  root = createRoot(container);
+});
+
+afterEach(async () => {
+  await act(async () => {
+    root.unmount();
+    await Promise.resolve();
+  });
+  container.remove();
+});
+
+describe('Charge screen', () => {
+  it('fetches the charge exactly once on mount', async () => {
+    // The screen used to pair an already-unpaused `useQuery` with a mount effect
+    // that re-executed it under the identical condition. That never cost a
+    // second request — urql dedupes the re-execution against the operation still
+    // in flight — so this count was already 1 before the effect was removed. The
+    // assertion is here to keep it that way, not to record a fix.
+    const operations: string[] = [];
+    const client = makeClient(operations);
+
+    // A *data* router, not `MemoryRouter`: `useLoaderData` throws outside one,
+    // and in the app these screens are only ever reached as route elements of
+    // `createBrowserRouter`. The route deliberately declares no loader, which
+    // is what leaves `useLoaderData` undefined and lets the query run.
+    const router = createMemoryRouter(
+      [{ path: '/charges/:chargeId', element: <Charge /> }],
+      { initialEntries: ['/charges/charge-1'] },
+    );
+
+    await act(async () => {
+      root.render(
+        <Provider value={client}>
+          <RouterProvider router={router} />
+        </Provider>,
+      );
+      await Promise.resolve();
+    });
+
+    expect(operations.filter(name => name === 'ChargeScreen')).toHaveLength(1);
+  });
+});

packages/client/src/components/screens/charges/charge.tsx

@@ -1,4 +1,4 @@
-import { useEffect, type ReactElement } from 'react';
+import type { ReactElement } from 'react';
 import { useLoaderData, useParams } from 'react-router-dom';
 import { useQuery } from 'urql';
 import { ChargesTable } from '@/components/charges/charges-table.js';
@@ -23,30 +23,17 @@ export const Charge = ({ chargeId }: Props): ReactElement => {
   const { chargeId: chargeIdFromUrl } = useParams<{ chargeId: string }>();
   const id = chargeId || chargeIdFromUrl;
 
-  // Try to get loader data (will be available when navigating via router)
-  let loaderData: ChargeScreenQuery | undefined;
-  try {
-    // eslint-disable-next-line react-hooks/rules-of-hooks
-    loaderData = useLoaderData() as ChargeScreenQuery;
-  } catch {
-    // No loader data - component used outside router context (e.g., as child component)
-  }
+  // Present when the route's loader ran; `undefined` on a route without one.
+  const loaderData = useLoaderData() as ChargeScreenQuery | undefined;
 
-  // Only fetch if we don't have loader data and need to fetch (prop-based usage)
-  const [{ data, fetching }, fetchCharge] = useQuery({
+  const [{ data, fetching }] = useQuery({
     query: ChargeScreenDocument,
     pause: !id || !!loaderData,
     variables: {
       chargeId: id ?? '',
     },
   });
 
-  useEffect(() => {
-    if (id && !loaderData) {
-      fetchCharge();
-    }
-  }, [id, loaderData, fetchCharge]);
-
   // Use loader data if available, otherwise use query data
   const chargeData = loaderData || data;
   const isLoading = !loaderData && fetching;