[DRAFT] PoC: TanStack Query Option B — UIMessenger-enforced background ownership + UI-side staleTime support#41102
[DRAFT] PoC: TanStack Query Option B — UIMessenger-enforced background ownership + UI-side staleTime support#41102
UIMessenger-enforced background ownership + UI-side staleTime support#41102Conversation
`fetchQuery` with `staleTime: 0` is intentional for controller-driven push data: each state change executes the `queryFn` (which returns the state directly), storing it in the background `QueryClient` cache. `BaseDataService` then auto-publishes `cacheUpdate` with `DehydratedState`. This is the fetch ownership boundary — the controller decides when to update, not the UI.
…ery` Uses `hydrate()` instead of `setQueryData` so the `DehydratedState` from `BaseDataService:cacheUpdate` carries cache metadata (`dataUpdatedAt`, `staleTime`) from the background — UI cache reflects background timing semantics without re-fetching. `useCurrencyRatesQuery` is a passive cache reader (`queryFn: () => undefined`); `isFetching` stays false.
… enforcement Per-route gating instead of global subscription prevents components from silently depending on events their route hasn't declared. Undeclared event subscriptions are a compile-time type error (via `useMessenger` generics) and a runtime throw (via delegated capabilities check).
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
✨ Files requiring CODEOWNER review ✨👨🔧 @MetaMask/core-extension-ux (1 files, +264 -0)
📜 @MetaMask/policy-reviewers (12 files, +84 -0)
Tip Follow the policy review process outlined in the LavaMoat Policy Review Process doc before expecting an approval from Policy Reviewers. |
|
@metamaskbot update-policies |
|
Policies updated. 🧠 Learn how: https://lavamoat.github.io/guides/policy-diff/#what-to-look-for-when-reviewing-a-policy-diff 👀 lavamoat/browserify/beta/policy.json changes differ from main/policy.json policy changes |
Builds ready [bd5fb82]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
bf23bc6 to
150cc56
Compare
Builds ready [150cc56]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
…tate subscriptions `useSyncExternalStore` adapter for background messenger events. Controller state is not server state — the background owns cadence, the UI renders the latest value. No TQ involvement on the UI side.
Currency rates are controller state pushed from background — not server state the UI fetches. Replace no-op `queryFn` + `staleTime` with direct messenger event subscription via `useSyncExternalStore`.
…drate()` path No longer needed — controller state flows through `useControllerState` over messenger events. Simplify `QueryClient` to standard TQ defaults; per-query cache policy via `get*QueryOptions`.
…outes `createRouteWithLayout` is a temporary v5-to-v6 bridge being deprecated. Route-level messenger scoping now uses `RouteWithMessenger` as a layout route element with `Outlet` — idiomatic React Router v6.
…i-endpoint disambiguation
150cc56 to
61d47e9
Compare
Builds ready [61d47e9]
⚡ Performance Benchmarks
🌐 Dapp Page Load BenchmarksCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
`createControllerStore` subscribes at module scope, bypassing the UIMessenger hierarchy. The hook now accepts an `event` parameter and calls `useMessenger()` to validate that the current route has declared the event in its `RouteWithMessenger` capabilities — enforcing the three-tier messenger boundary at the React layer.
Builds ready [9aa181c]
⚡ Performance Benchmarks
Dapp page load benchmarks: data not available. Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
PoC: TanStack Query Option B
corediffSummary
queryFnscope lint rule enforce the same boundary asOmitKeyof, without strippingstaleTimefrom the UI type signature.staleTime(5s swaps, 30s portfolio) on the same hook andqueryKey— a type error under Option A'sOmitKeyof.Option A vs Option B
fb/rq-poc)OmitKeyofstripsqueryFn+staleTimestaleTimequeryFnqueryFnfromget*QueryOptionsfetchQuery+ proxy routing per endpointget*QueryOptionsdirect, zero lead timecreateUIQueryClient->hydrate()BaseDataService->cacheUpdate->useSyncExternalStoreBaseDataServicecore-backendusageget*QueryOptionswrapped in proxy;staleTimestrippedget*QueryOptionsconsumed directly.staleTime/select/retrypreservedOwnership boundary enforcement
The central question between Option A and Option B: how is the fetch ownership boundary enforced?
Option A:
OmitKeyofBackground owns all fetches. UI receives a
QueryClientwithstaleTime: 0andOmitKeyofstrippingstaleTime/queryFnfrom the type signature. This enforces ownership — but as a blunt instrument. StrippingstaleTimeis a side effect of strippingqueryFn, not an independent constraint.Option B: UIMessenger capability gating + lint rule
Two independent, composable mechanisms — each enforcing one axis:
1. UIMessenger capability gating (Axis 2 — fetch ownership)
Per-controller messenger configs declare which events are exposed to the UI. Routes declare which events they can subscribe to. Components use
useMessenger()with compile-time and runtime validation against the route's declared capabilities.The enforcement hierarchy:
shared/messenger-config/*.ts) — team-owned declaration of UI-exposed events usingDataServiceEventstypes fromBaseDataService. Adding/removing an event here controls access at compile time and runtime.RouteWithMessenger) — scopes event access per route.RouteWithMessengercreates a child messenger that only delegates declared events.useMessenger({ events })) — validates requested events against the route's delegated capabilities. Undeclared event = type error + runtime throw.useControllerStatecallsuseMessenger()internally, so every controller-state subscription is validated against the route's declared capabilities. The underlyingcreateControllerStoresubscribes at module scope for efficiency, but the React hook enforces the messenger hierarchy at render time.This follows the three-tier messenger hierarchy from decisions#126:
RootMessenger→UIMessenger→RouteMessenger.2.
queryFnscope lint rule (Axis 2 — fetch authorship)no-restricted-importsinui/hooks/**requiresqueryFnviaget*QueryOptionsfrom@metamask/core-backend. Blocks unauthorized direct-API calls in UI hooks — without restrictingstaleTimeor other cache policy.Together: UIMessenger gates event subscriptions (controller access boundary). Lint rule gates
queryFnauthorship (fetch ownership boundary). These are the same constraintsOmitKeyofenforces — but as independent mechanisms that don't couple Axis 1 to Axis 2.Per-screen vs per-client cache policy
Per-client divergence (extension vs mobile) is solvable under both options via data service constructor config. The constraint unique to Option A is per-screen divergence within the same client: the data service instance is shared across all screens, so a single
staleTimegoverns every consumer.Examples: gas estimates (activity list 30s vs confirmation 3s), token balances (dashboard 30s vs send confirmation 0),
selectfor re-render scope (home: all tokens, swap: only source/dest),refetchOnMount: 'always'for confirmation screens.staleTimeis type-stripped byOmitKeyof; other options remain available butstaleTime: 0breaks the conditional interaction system (e.g.,refetchOnMount: truealways fires because data is always stale).What background proxy uniquely provides
After accounting for
core-backend(shared query definitions), constructor config (per-client divergence), andpersistQueryClient(cache persistence):OmitKeyofis compile-time; lint rules can beeslint-disabledpersistQueryClientThese are operational concerns. The ADR question: do they justify
staleTime: 0breaking the conditional refetch system,core-backendpass-through freshness undermined, and split-process staleness indirection?Staleness indirection
The ADR identifies a structural problem with background-owned fetches: when the
QueryClientthat detects staleness is not the one that owns thequeryFn, twostaleTimevalues interact opaquely.useSpotPricesQuery)useCurrencyRatesQuery)queryFnownerQueryClientdirectlyCurrencyRateDataService)staleTime, single processstaleTimeOverrideper componentOption A's proxy pattern produces the indirection for all queries. Option B eliminates it for UI-direct queries (structurally impossible, single owner) and replaces it with transparent push semantics for controller-driven queries (
isFetching: false,dataUpdatedAtreflects background timing viaDehydratedState).Architecture
flowchart TB subgraph UIDirectFetch["UI-direct: server state (useQuery)"] direction TB A1["useSpotPricesQuery(staleTimeOverride?)"] A2["useQuery(apiClient.prices.getV3SpotPricesQueryOptions(...))"] A3["price.api.cx.metamask.io"] A1 --> A2 --> A3 end subgraph BackgroundSync["Controller state (useSyncExternalStore)"] direction TB B1["CurrencyRateController:stateChange"] B2["CurrencyRateDataService.fetchQuery()"] B3["@tanstack/query-core (background-internal)"] B4["cacheUpdate messenger event"] B5["useSyncExternalStore → component"] B1 --> B2 --> B3 --> B4 --> B5 end style B3 fill:#f5f5f5,stroke:#ccc,stroke-dasharray: 5 5Two tools for two problems. TQ on the background side is an implementation detail — its only UI-visible output is the messenger event.
TQ usage by layer
@tanstack/query-core— internalfetchQueryfor caching, deduplication, gc.staleTime/gcTime/retryare background-internal. PublishescacheUpdatemessenger events.@tanstack/react-query— full APIuseQuery/useInfiniteQuerywithget*QueryOptionsfrom@metamask/core-backend. Call-sitestaleTime,refetchOnFocus,select,retry.useSyncExternalStoreover messenger events. NoqueryFn, nostaleTime, no TQ cache policy. Background owns cadence.useControllerStatevalidates route-level messenger capabilities viauseMessenger().Background-side TQ guidelines
Data services use
@tanstack/query-coreas internal infrastructure. The UI does not consume background TQ artifacts directly.queryKeyis internal. The messenger event name (e.g.,CurrencyRateDataService:cacheUpdate) is the contract between background and UI — not the queryKey.staleTime/gcTime/retryare background-internal cache policy. They govern how the background caches and refreshes data. They do not leak to the UI.fetchQueryvsprefetchQuery: UsefetchQuerywhen the caller needs the result (e.g., controller consuming the data). UseprefetchQuerywhen the goal is just to populate the cache for latercacheUpdateevents.queryOptionsfrom data services. If the UI needs to fetch the same data directly (display-only context), it usesget*QueryOptionsfrom@metamask/core-backend— a separate code path with its ownqueryFn.Per-controller vs multi-endpoint data services
The ADR identifies a packaging choice for background data services: per-controller (
TokenRatesDataService) vs multi-endpoint (@metamask/core-backend). Under Option A, where all data routes through the background, these are competing organizational models for the same concern.This PoC resolves the tension: they serve different data categories.
useControllerState(useSyncExternalStore)useQuery(get*QueryOptions(...))@tanstack/query-core— internalPer the ADR: "A per-controller data service can consume core-backend's exported
get*QueryOptionswhile owning its ownQueryClientand lifecycle — using core-backend as a query definition library rather than a runtime service." This PoC makes that concrete: core-backend is the library, per-controller data services are the runtime — and they don't overlap.core-backendalignment@metamask/core-backendexportsget*QueryOptionsfor 40+ endpoints with{ queryKey, queryFn, staleTime, gcTime }. ItsFetchOptionspass-through preservesstaleTime,select,retryat the call site. Option B consumes this directly:Option A wraps the same
get*QueryOptionsin a proxyqueryFnand stripsstaleTimeviaOmitKeyof. The pass-through thatcore-backendpreserves is removed at the consumption layer.Existing usage on
mainand mobile confirms the full API surface is needed:activity-v2usesuseInfiniteQuery,select,keepPreviousData,refetchOnMount,prefetchInfiniteQuery.selectandrefetchOnMountare available under Option A (not stripped byOmitKeyof), butstaleTime: 0breaks conditional behavior —refetchOnMount: truealways fires.metamask-mobiletest: header integration test for contract interaction #25981 usesstaleTime: 10_000, full status API, controller-boundaryqueryFn. Call-sitestaleTime, stripped under Option A.Option B is a superset: full TQ API at call site, plus controller state via
useSyncExternalStore, plus per-componentstaleTimeOverride.Model selection: UI-direct vs controller-driven
When adding a new data hook, use this decision tree:
Rule of thumb: default to UI-direct (
useQuery). Cross-client reuse is automatic —@metamask/core-backendis the shared query options package, consumed directly by both extension and mobile. Upgrade to controller state only when an independent background ownership criterion is met (controller dependency, process persistence, externally driven cadence).Shared queryKey staleTime behavior
When multiple
useQueryobservers share aqueryKeywith differentstaleTimevalues, TQ uses the shortest staleTime for refetch decisions. Per-componentstaleTimeOverride(e.g., 5s swap vs 30s portfolio) achieves differentiated freshness only when the components are on separate routes (mutually exclusive mounts). When co-mounted, the shorter staleTime governs both.Changelog
CHANGELOG entry:
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist