refactor(cache): move the cache lifecycle inside useCachedResource - #29614
Draft
chrisnojima wants to merge 1 commit into
Draft
refactor(cache): move the cache lifecycle inside useCachedResource#29614chrisnojima wants to merge 1 commit into
chrisnojima wants to merge 1 commit into
Conversation
This was referenced Sep 9, 2026
chrisnojima
added this pull request to stack #29617
September 9, 2026 19:40
useCachedResource resets whatever cache object it is handed while disabled. That is an ordering constraint on the caller stated in no type, so eight consumers each answered "which cache object do I hold this render?" with the same ~40-line scaffold: a module-scope map, a registerExternalResetter, an invalidation listener Set, a useXCacheMap gate, a debounce, several useEngineActionListeners, and a recycler inside load(). The hook now takes a namespace and a key instead of a cache object. The namespace owns the map and the sign-out reset. `key: undefined` means the instance has no entry: it shares nothing and resets nothing, so the shadow hazard is unrepresentable rather than something each caller has to gate against - chat/conversation/team-hooks gated on forceLocalCache alone, not on !enabled || !validTeamID like its sibling, and could seed and reset a shared entry an instance had no business holding. invalidateOn folds the engine listeners, the invalidation Sets and the debounce into one trigger. The debounce runs BEFORE the epoch is allocated, so a burst that coalesces into one reload is one event rather than several that supersede each other; an invalidate() broadcast carries its own epoch through so every consumer of it shares a single rpc. recycle moves the identity recyclers out of load(), where they had to reach into the cache object to see the previous value. Team channels and chat team members now coalesce their reload bursts the way the team and the teams list already did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015rccpV5nLxxC5opF5xzrz7
chrisnojima
force-pushed
the
nojima/HOTPOT-arch-08-cached-resource
branch
from
September 11, 2026 01:47
c64a92d to
b2fed16
Compare
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
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.
Problem
util/use-cached-resource.tsxis a genuinely deep module, but it resets whatever cache object it is handed whenenabledis false — an ordering constraint on the caller, stated in no type. So every consumer had to answer "which cache object do I hold this render?" before it could call the hook.Six consumers each hand-rolled the same ~40-line scaffold: a module-scope cache map, a
registerExternalResetterfor sign-out, an invalidation-listenerSet, a cache-map gate, adebounce(2000, {leading, trailing}), 3–5useEngineActionListeners, and a recycler insideload(). A seventh (teams/common/activity.tsx) opted out entirely.chat/conversation/team-hooks.tsxgated only onforceLocalCache, not on!enabled || !validTeamID.Change
cacheKey: undefinedmeans no shared entry, so the shadow-instance hazard becomes structurally unrepresentable rather than merely corrected. The namespace owns the map and the resetter;invalidateOnfolds the ad-hoc listener sets into one coalesced, epoch-allocating trigger.Ten consumers de-scaffolded. Gone: 3 cache-map gates, 4 hand-rolled debounces, 2 invalidation
Sets, 8registerExternalResetters, 1 hand-rolled LRU, 2 in-load()recyclers, ~19useEngineActionListenercalls.Did the
team-hooksbug reproduce?The divergence is real, but a test driving it through the public API passed —
useChatTeamMembersexposes noenabledparameter, so the two flags can only disagree for an invalid teamID whose shared entry no loader uses. A latent hazard, not a live bug. What did reproduce against the pre-fix code: handing the shared cache to a disabled instance resetloadedAtto 0 and caused a second RPC inside the stale window. That now lives as a test: "a disabled instance never touches the shared entry".Regressions caught in review, each mutation-checked
effect: 'clear'didn't cancel the queued debounced reload, so a deleted or left team was re-fetched up to 2s later and the cleared entry repopulated. A new regression for team-channels and chat-team-members, whose old reloads were undebounced. NowclearNowcancels first.namespace.invalidate()broadcasts were routed through the same debounce, so after creating a channel the flash-to-empty lasted up to 2s instead of one round trip. Broadcasts now go straight through — they are already one event carrying one epoch. (Deliberate deviation from "fold the listener Sets into the debounced trigger"; the debounce-before-epoch rule still holds for the engine path it was protecting.)[cache, cacheKey], not only on unmount.cacheKeywas silently discarded whennamespacewas omitted; duplicateinvalidateOntypes double-subscribed. Both latent, both fixed.util/use-rpc-load.tsxis untouched, not deleted. Stopped deliberately rather than half-migrate 16 call sites across 12 modules. The blockers are semantic:enabled: falsemeans "skip auto-load, keep data" there and "go inert and blank" inuseCachedResource(differs atdevice-revoke,bot/install,featured-bots,incoming-share);when: 'manual'has no equivalent (settings/chat,git/row,devices/index);setData,errorandonResultare all in live use. Its own PR, with its own gate.util/recycle.tsxconsolidation (the optional part) is skipped. Therecycleoption exists and the two in-load()recyclers use it; the other four are untouched.Pre-existing limitation, neither introduced nor fixed
On an engine notification, N mounted consumers of one entry each allocate their own epoch and supersede each other — N RPCs for one event. Only the
invalidate()path shares an epoch. The old hand-rolled code behaved identically.Validation
lint:allclean —0 bailed out, 0 whole-props deps, tsc clean both projects.jest --runInBand— 231 suites / 2257 tests (baseline 230 / 2236).