Transition to Tanstack Query - #159
Closed
spencerrlongg wants to merge 54 commits into
Closed
Conversation
general.ok had no equivalent in Snipe-IT's lang files, so it won't get a free Crowdin TM match; mobile.* is reserved for strings unique to this app
pushes source strings to Crowdin on every push to develop; pulling translations back is handled by Crowdin's native GitHub integration (configured on crowdin.com), same split as the main app
installed i18next's own types only support 'v4' (the default) for compatibilityJSON; 'v3' isn't a valid option anymore. Also splits audit_session_count into _one/_other CLDR suffixes so Crowdin can present separate plural slots to languages that need them (Russian, Arabic, Polish, etc.) instead of forcing one string for every count
…b integration Skipping Crowdin's native GitHub repo integration by choice, not because it's blocked. This Action replicates the pull/PR side of that integration via the CLI Action instead, triggered on push to develop plus manual dispatch.
aa-ER (Afar/Eritrea) is the locale slot the snipe-it Crowdin project uses for its In-Context localization pseudo-language, not a real translation target. Its strings are all crwdns/crwdne placeholder markers. Excluding it going forward via --exclude-language=aa in crowdin-download.yml.
aa-ER is the locale slot the snipe-it Crowdin project uses for its In-Context localization pseudo-language, not a real translation target -- its strings are crwdns/crwdne placeholder markers, not actual Afar text. Excluding it so future pulls don't ship that file.
--exclude-language=aa passed through download_translations_args had no effect -- aa-ER still came through on the next pull (verified via a manual workflow_dispatch run). Using crowdin.yml's per-file excluded_target_languages instead, since it's config the project itself respects on both push and pull, rather than a runtime flag that may not be honored by whatever CLI version the Action's Docker image bundles.
aa-ER isn't a real project target language; it's Crowdin's synthetic
In-Context pseudo-language pack. Referencing 'aa' in
excluded_target_languages made the project reject it outright
("Project doesn't have 'aa' language(s)"), failing every
crowdin-upload.yml run on push to develop. Reverting; the real fix
needs to strip the pseudo-language file after download instead of
trying to configure it away project-side.
Same issue as before -- aa-ER is Crowdin's In-Context pseudo-language slot, not a real translation. The exclude-language CLI flag didn't stick (see #151 for the real fix via crowdin.yml), so stripping it manually from this PR.
i18n/index.js only ever registered en-US as a resource, so pulled translations had nothing loading them regardless of device language. Statically import and register all 75 real locale files (skipping the aa-ER pseudo-language pack). Also add supportedLocales to the expo-localization plugin config in app.json, since iOS only reports languages an app has explicitly declared via CFBundleLocalizations -- without it, getLocales() never returns anything but English no matter what the device is set to. Verified against the actual plugin source (no transformation, passes codes straight into Info.plist) and confirmed all 75 locale codes are valid BCP-47 tags. Device locale resolution now tries an exact tag match first, then falls back to a base-language match (e.g. de-US -> de-DE) before defaulting to en-US, since real devices commonly report a language/region combination that doesn't exactly match one of our translated locale files.
Prefix matching returned the first resource key sharing a base language, which is wrong when several variants of one language ship. Five base languages have multiple variants (en, es, zh, pt, de), and the zh case was a real bug: Traditional Chinese devices matched zh-CN and were served Simplified. Add a local Expo module wrapping the platform APIs built for exactly this: Bundle.preferredLocalizations(from:) on iOS and LocaleListCompat.getFirstMatch() on Android. Both apply the OS's own CLDR resolution. Apple's TN2418 warns against hand-rolling this. Prefix matching stays as the fallback for web, which has no native counterpart, and for a platform reporting no match at all. An earlier attempt used @formatjs/intl-localematcher, but it needs Intl.Locale, which Hermes does not implement -- match() threw, and a catch turned that into a silent fallback to English. Polyfilling it also carries a documented startup regression (formatjs#4276), so the native route is both correct and cheaper here. No catch is wrapped around the native call on purpose: if the module is not linked, requireNativeModule throws a clear error rather than quietly degrading. Also anchor the android/ and ios/ gitignore rules to the repo root. Unanchored, they matched modules/*/android and modules/*/ios too and silently excluded this module's native source from the commit.
create-expo-module scaffolds a standalone publishable package, so it
emitted an MIT LICENSE carrying Expo's own copyright line ("650
Industries, Inc.") and a podspec full of placeholder metadata. The
license asserted Expo's copyright over code written here and has no
purpose in a --local module that never leaves this repo; removed it.
Filled in the podspec's summary, description, author and homepage.
react-native-web isn't a dependency, so the web target can't build regardless and the .web.ts stub was dead code. Removed it along with the stale comment in i18n/index.js citing web as a reason for the fallback path -- that path now only covers the platform reporting no match.
It was the only .ts file in the project and typescript isn't a dependency, so `expo run:android` refused to build and demanded typescript@~6.0.3 be installed. Babel transpiles TS without a tsconfig, which is what I checked originally, but the Expo CLI separately gates on the dependency existing as soon as it sees a .ts file. Plain JS keeps the project TypeScript-free; the type information moves to a comment. Also ignore Gradle output under modules/*/android/build. Anchoring the android/ and ios/ rules to the repo root stopped them covering local modules, so a build directory became trackable and 110 generated files were briefly staged along with the source.
LocaleList.getFirstMatch is not the counterpart to iOS's preferredLocalizations(from:). It returns one of the *user's* locales rather than one of the supported ones, so it cannot name a translation file: a zh-Hant device made it return "zh-Hant", which matches no resource key, and the prefix fallback then picked the first zh-* key (zh-CN) and served Simplified Chinese to a Traditional reader -- exactly the bug this module exists to fix. android.icu.util.LocaleMatcher would be the real equivalent but is not in Android's public SDK (absent from android.jar in both API 35 and 36). ULocale.addLikelySubtags is available, and is the same CLDR "maximize" primitive Intl.Locale.maximize() provided in the abandoned JS approach. Maximizing both sides fills in implied scripts (zh-Hant -> zh-Hant-TW, zh-CN -> zh-Hans-CN), so matching on language+script+region, then language+script, then language picks Traditional for Traditional. Every ULocale method used is API 24 per the SDK's own api-versions.xml, matching Expo's minSdk, so no version guard is needed. Verified on an API 36 emulator with the per-app locale set to zh-Hant: renders 資產/最近操作活動/稽核 (Traditional) where it previously showed 资产/最近活动/盘点 (Simplified).
Tapping "Audit Due" or "Audit Overdue" on the home screen sent you to the audit list, which always opened on the Due tab. Both badges now pass a tab param and the list screen seeds its initial tab from it. Closes #125
Shared groundwork for the TanStack Query migration (Phase 0, step 1). expo-network is also required by the audit offline-caching plan.
30s staleTime for entity reads, gcTime >= staleTime, and a retry predicate that skips 4xx so permission and validation errors surface immediately instead of being retried.
makeRequest previously resolved a 403 as null, so a permission denial and a valid empty response were indistinguishable once TanStack starts caching resolved values as successful data. Introduces a typed PermissionDeniedError carrying the permissionKey and status, and converts the six call sites that depended on the old null-check convention in the same commit: assets edit/create, accessories edit, consumables checkout, and both audit-count fetches in AuditDashboardCard (which hides the dashboard card on denial).
Wraps the app in QueryClientProvider with DevTools attached, and wires TanStack's onlineManager to expo-network and focusManager to AppState so refetchOnReconnect/refetchOnWindowFocus behave correctly on native, where they're no-ops by default.
TanStack's documented screen-focus pattern: skips the initial mount via firstTimeRef, then refetches stale active queries on subsequent focus. Uses expo-router's useFocusEffect per this repo's existing convention.
assetKeys.detail/list and customFieldKeys.all give the detail, edit, checkin, checkout, and list screens a shared, typo-proof vocabulary for querying and invalidating the asset cache.
A reanimated pulsing placeholder plus an AssetDetailSkeleton composition mirroring the real Section/DetailRow geometry, so the cold-open layout doesn't collapse while the first asset query resolves.
Splits the /hardware/{id} and /fields Promise.allSettled into two
independent useQuery calls with separate staleTime, so a /fields
denial no longer risks the asset render and both queries share cache
with the edit screen. Cached data renders instantly on repeat visits;
only a cold, cacheless open shows AssetDetailSkeleton instead of
blanking the whole screen on every edit-then-back navigation. Replaces
the useFocusEffect refetch-on-focus with useRefreshOnFocus, and wires
RefreshControl to refetch instead of a local loading/refreshing pair.
Hydrates from the same assetKeys.detail(id)/customFieldKeys.all cache as the detail screen, so opening the editor after viewing an asset is instant. The existing Burnt success/error toasts move into the mutation's onSuccess/onError; on success the detail and list caches are invalidated and navigation uses router.dismissTo instead of router.replace so the detail screen below is reused rather than replaced with a fresh, cache-cold instance.
create.jsx has no detail screen below it to dismiss to, so dismissTo falls back to its documented replace behavior here — kept in step with the other assets mutation handlers for consistency.
Moves the POST /hardware/{id}/checkin call to useMutation, invalidates
the asset detail and list caches on success, and swaps router.replace
for router.dismissTo so returning to the detail screen reuses its
cache instead of mounting a fresh, cache-cold instance.
Same shape as the checkin migration: POST /hardware/{id}/checkout as a
useMutation, invalidates the asset detail and list caches on success,
and uses router.dismissTo to return to the detail screen.
getNextPageParam continues the existing offset-25 scheme, and placeholderData: keepPreviousData means changing search text or filters no longer blanks the list while the new page loads. Deletes the setData([]) calls that ran on every focus/search/filter change, so scroll position and loaded pages now survive navigating to a detail screen and back — replaced by useRefreshOnFocus for the focus-refetch behavior those calls used to provide.
RefreshControl's refreshing prop was bound directly to isFetching/ isRefetching, so any background refetch (focus refetch, post-mutation invalidation) triggered the native spinner and reserved its layout space even though the user never pulled down. On iOS this can also render as a frozen, non-animating spinner since the native control never received an actual pull gesture. Track refresh state locally and only flip it from the RefreshControl's own onRefresh handler.
Swap RN's Image for expo-image on the asset detail and list screens, with a 200ms fade transition and an onLoadStart/onLoadEnd pair driving a small ActivityIndicator overlay while each image loads. List rows track their own load state independently so scrolling doesn't cross- trigger neighboring spinners. cachePolicy is set explicitly to memory-disk per the project's expo-image caching convention.
Wires a __DEV__-gated button that clears the TanStack Query cache and expo-image's memory/disk caches in one tap, for testing cold-start and loading-indicator behavior without reinstalling the app. Guards the image-cache clear with a 5s timeout via Promise.race so a native-module hiccup can never leave the button spinning indefinitely.
queryClient.refetchQueries()/invalidateQueries() reject their returned promise when a refetch fails, unlike a passive useQuery observer which stores the error in query state instead of throwing. useRefreshOnFocus and the asset checkin/checkout/edit success handlers called these without awaiting or catching them, so a failed background refresh surfaced as an unhandled rejection (visible as a raw AxiosError in RN's global handler) instead of failing silently like the rest of these background-sync paths.
The asset detail, edit, checkin, and checkout screens hardcoded paddingTop: insets.top + 44 as a fixed guess at the native header height, with no contentInsetAdjustmentBehavior set. That guess drifts after repeated navigation transitions, producing a growing gap between the header and content. Switch to the convention used everywhere else in the app: contentInsetAdjustmentBehavior="automatic" with paddingTop: 0 on iOS (letting the OS compute the inset correctly) and insets.top + 56 on Android.
Member
|
I thought we were going to use CrowdIn's OTA language stuff? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Drafting