Repository navigation
App baseline: data layer - #24
Conversation
| const health = useHealth(); | ||
| useRefreshOnFocus(); | ||
|
|
||
| if (health.isPending) { |
There was a problem hiding this comment.
Offline with nothing cached (cold start, or first open of a screen), TanStack pauses the fetch instead of failing it: status stays "pending" with fetchStatus: "paused", so isPending is true forever and this shows LoadingState indefinitely. The spec's "Error, with nothing to show: a message and Retry" never appears offline. Consider treating isPending && fetchStatus === "paused" as an offline/error state (e.g. ErrorState with the network message), ideally in a shared place so every screen gets it.
| isFirstFocus.current = false; | ||
| return; | ||
| } | ||
| void queryClient.refetchQueries({ stale: true, type: "active" }); |
There was a problem hiding this comment.
This filter isn't scoped to the focused screen: it refetches every active stale query in the client, and screens below in the stack stay mounted, so their queries count as active too. Going back from a detail screen refetches the data of the whole stack. refetchQueries also defaults to cancelRefetch: true, so any of those already in flight is aborted and restarted. Scoping it (pass the screen's query keys, or use refetchType/cancelRefetch: false) avoids the extra requests.
| // mounted, so their queries don't refetch on their own. Only stale queries | ||
| // refetch, so quickly moving between screens doesn't. The first focus is | ||
| // skipped: the queries fetch on mount anyway. | ||
| export const useRefreshOnFocus = () => { |
There was a problem hiding this comment.
Altitude: this is a per-screen opt-in that every screen must remember to call (and that tests must mock out), yet its effect is global. TanStack Query v5 has a subscribed option meant for this in React Native: subscribed: useIsFocused() unsubscribes unfocused screens so their queries are no longer "active", and resubscribing on focus refetches when stale. Applying that once (e.g. through a default or a small wrapper the feature hooks use) would remove the hook, the first-focus ref and the cross-screen refetching.
| // fail. Wrongly offline pauses every query; wrongly online only fails | ||
| // them with "Couldn't connect". Unknown counts as online. | ||
| // The listener fires only on changes, hence the initial read. | ||
| void getNetworkStateAsync().then((state) => { |
There was a problem hiding this comment.
Two small issues with the initial read: (1) it races the listener. If the network changes between the call and its resolution, the listener sets the new state first and this older snapshot then overwrites it, leaving onlineManager (and the banner) wrong until the next change. (2) There's no .catch, so a rejection is an unhandled promise rejection. A flag set by the first listener event (skip the initial value if one has arrived) plus a .catch covers both.
| // API_URL overrides the variant's URL, e.g. with a LAN IP for a physical | ||
| // phone. In a development build the config comes from Metro, so | ||
| // restarting Metro picks up a new value without rebuilding. | ||
| apiUrl: process.env.API_URL ?? variant.apiUrl, |
There was a problem hiding this comment.
API_URL overrides the URL for the production variant too. An API_URL left in the environment when running npm run android:prod (or later in CI) silently ships a prod APK pointing at a dev or LAN address, and an http:// one is also blocked there, since only the debug manifest allows cleartext. Also, ?? doesn't catch an empty string, so API_URL= yields "" and the app crashes at launch in config.ts. Consider applying the override only for the development variant and using ||.
| if (response.status === 204) { | ||
| return schema.parse(undefined); | ||
| } | ||
| return schema.parse(await response.json()); |
There was a problem hiding this comment.
A 2xx response that isn't JSON makes response.json() throw a bare SyntaxError. That happens on a captive-portal network, where isConnected is true and the redirect leads to a 200 HTML login page, and with a misconfigured API_URL. It's neither ApiError nor NetworkError, so it isn't retried and the user sees "Something went wrong." instead of "Couldn't connect." Consider wrapping the body read and mapping a parse failure to a dedicated error, or at least to NetworkError.
| if (failureCount >= maxRetries) { | ||
| return false; | ||
| } | ||
| if (error instanceof NetworkError) { |
There was a problem hiding this comment.
Retrying a timed-out NetworkError combines badly with the 15 s timeout. On a connection that drops packets (isConnected true but nothing answers), a first load waits about 15 s x 3 attempts plus 1 s + 2 s of backoff, roughly 48 s of spinner, before ErrorState appears. Consider not retrying timeouts (only immediate connection failures), or a shorter timeout per attempt.
|
|
||
| return ( | ||
| <QueryProvider | ||
| onRefreshError={() => snackbar.show(t("errors.refreshFailed"))} |
There was a problem hiding this comment.
Only the first render's closure is ever used, because the query client is created once, so t and snackbar here are frozen from that render. It works today only because show touches just a ref and a state setter. A language switch, or a later show that reads state, would silently use stale values, and the inline arrow suggests the prop is reactive when it isn't. Calling i18n.t inside, or keeping the latest callback in a ref inside QueryProvider, would make that explicit.
| jest.advanceTimersByTime(15_000); | ||
|
|
||
| await expect(request).rejects.toBeInstanceOf(NetworkError); | ||
| jest.useRealTimers(); |
There was a problem hiding this comment.
jest.useRealTimers() runs only if the preceding await expect(...) passes. If it fails, fake timers leak into the following tests in this file (e.g. the cancellation test), which turns one failure into confusing follow-on failures. Moving it into afterEach (or a try/finally) fixes that.
TanStack Query with its AsyncStorage persister, zod for response schemas, expo-network for connectivity and expo-application for the cache buster. AsyncStorage stays on 2.2.0, the SDK 58 version, rather than npm's 3.x. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The emulator's alias for the Mac's localhost in development, the production API in production, and an API_URL override for a physical phone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
apiFetch parses every response with a zod schema, turns the API's error envelope into an ApiError and a missing response into a NetworkError, and times requests out after 15 seconds. errorMessage picks the copy by code. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The cache is persisted to AsyncStorage for a week and busted by the app version. Queries and mutations retry network and server errors twice, refetch on app focus and reconnect through expo-network, and a failed background refresh shows one "Couldn't refresh" snackbar. The splash screen stays up until the cache is restored. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shown at the top of every Screen while TanStack Query considers the app offline, so it's up exactly while queries are paused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Until features contribute cards, Home shows the API's health, exercising the client, the query cache and the shared loading and error states. The API URL is mocked once for all tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each retry waited the full 15-second timeout again, so a connection that drops packets kept a first load on its spinner for about 48 seconds. Failed connections and server errors are still retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A captive portal answers with a 200 HTML login page. That threw a bare SyntaxError, which wasn't retried and showed a generic message instead of "Couldn't connect." Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Also catch a failed read, which was an unhandled rejection. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The query client is created once, so the callback passed at creation was frozen from the first render. An effect now subscribes to the query cache's error events with the current callback instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
useRefreshOnFocus refetched every stale query in the client, including those of the screens below in the stack, and aborted any already in flight. useApiQuery instead passes useIsFocused() as TanStack Query's subscribed option: a screen out of focus stops counting as active, and regaining focus refetches its stale queries. Lint keeps features off useQuery. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Offline, TanStack Query pauses a query's first fetch instead of failing it, so the screen showed its loading state indefinitely. Screens now check isWaitingForConnection and show ErrorState with an offline message and no Retry, since the query loads by itself once back online. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
3b5431e to
4c9cb99
Compare
An API_URL left in the environment during npm run android:prod would ship a production APK pointing at a dev or LAN address, and an http:// one is blocked there anyway. An empty API_URL also passed through ?? and crashed the app at launch in config.ts; || falls back to the variant's URL instead. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A query whose refresh fails has status "error" but keeps its earlier data. TanStack Query persists only status "success" by default, and the persister rewrites the whole cache on each change, so the first failed refresh deleted the data from storage. The next cold start offline then had nothing to show and said "Couldn't connect." Persist every query that has data instead. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ached Network and server errors now say what to do next, the refresh snackbar says that cached data is shown, and the banner says "cached" rather than "saved". Offline with nothing cached, the banner claimed to show data while the screen said there was none, so the banner now stays hidden until the query cache holds data. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The third App baseline PR: the API client and
ApiError, the query client with its persisted cache and the focus and online wiring, and the offline banner. Design perapp/docs/ARCHITECTURE.md§3, §4, §7 and §10. Home queriesGET /healthto exercise it until features contribute cards.app.config.ts(10.0.2.2:8080in dev,https://life-app-api.lakaniemi.comin prod), with anAPI_URLoverride for a physical phone. Only the dev variant honors the override.apiFetchparses every response with a zod schema and turns the error envelope intoApiError. A request that doesn't reach the API (no response, a 15 s timeout, or a success that isn't JSON) becomesNetworkError.errorMessagepicks copy by code.useApiQuery, which pauses the queries of screens out of focus (lint keeps features offuseQuery). Stale queries refetch on app focus, screen focus and reconnect (expo-network). A failed background refresh shows one "Couldn't refresh" snackbar. Offline with nothing cached, screens say so instead of spinning. The splash screen stays up until the cache is restored.Screen, following TanStack Query's online state.app-shell.mdand the roadmap are updated.Needs a native rebuild (
npm run android): this PR adds AsyncStorage,expo-networkandexpo-application.Testing
npm run lint,npm run typecheckandnpm testpass (44 tests).successqueries by default. Fixed ind9be3fe, with tests.🤖 Generated with Claude Code