fix(components): read the GitHub repos cache lazily, per store - #20
Open
pythonlearner1025 wants to merge 2 commits into
Open
fix(components): read the GitHub repos cache lazily, per store#20pythonlearner1025 wants to merge 2 commits into
pythonlearner1025 wants to merge 2 commits into
Conversation
`allGitHubReposCacheAtom` was built with `atom(githubReposCache.readAll())`, so the localStorage read ran once at module import and became the atom's `init`. `init` belongs to the atom, not to a store, so every store created afterwards started from that import-time snapshot and missed repo lists cached since. `atomWithDefault` evaluates its default per store, on first read, and stays writable -- verified against the pinned jotai 2.17.1 -- so `setWorkspaceReposCacheAtom` is unchanged. It also moves the localStorage read out of module scope, matching the `atomFamily` and `atomWithStorage` atoms beside it. Model: claude-opus-5
`allGitHubReposCacheAtom` was built as `atom(githubReposCache.readAll())`. A primitive atom's `init` is evaluated once — when the module is imported — and that one value is then shared by every store. Any store created after import therefore starts from the import-time snapshot and silently misses repo lists cached since, even though `setWorkspaceReposCacheAtom` wrote them to localStorage. It also puts a localStorage read in module scope, which runs on import whether or not the atom is ever used. `atomWithDefault` evaluates its default lazily and per store, so each store reads the localStorage contents at its own first read. It stays writable, so `setWorkspaceReposCacheAtom` is unchanged. SCOPE, stated honestly: this is a latent store-dependency, not a user-facing bug fix. The OSS composition has exactly one store — `jotaiStore` in `lib/utils.ts`, created at import and passed to the single provider in `main.tsx` — and the only stores built later in this tree are `createTourStore()` and Storybook stories. I could not trace a path from either to a consumer of this atom, so I cannot exhibit an in-app reproduction. What is demonstrated is the staleness itself, which the test pins. Verified against the pinned jotai 2.17.1 rather than assumed: a store created after a write reads the import-time snapshot from `atom(readAll())` and the current contents from `atomWithDefault(() => readAll())`. Model: claude-opus-5
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
1 participant
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.
allGitHubReposCacheAtomis built asatom(githubReposCache.readAll())(packages/components/src/atoms/local-storage-cache.ts:13-15).A primitive atom's
initis evaluated once, when the module is imported, and that single value is then shared by every store. So any store created after import starts from the import-time snapshot and silently misses repo lists cached since — even thoughsetWorkspaceReposCacheAtom(:24-34) wrote them to localStorage. It also puts a localStorage read in module scope, which runs on import whether or not the atom is ever used.atomWithDefaultevaluates its default lazily and per store, so each store reads the localStorage contents at its own first read. It stays writable, sosetWorkspaceReposCacheAtomneeds no change.Scope — this is not a user-facing bug fix
Stated plainly so it isn't oversold: I could not exhibit an in-app reproduction. The OSS composition has exactly one store —
jotaiStoreinlib/utils.ts:23, created at import and passed to the single provider inmain.tsx:160— and the only stores built later in this tree arecreateTourStore()and Storybook stories. I could not trace a path from either to a consumer of this atom.What is demonstrated is the staleness itself. Treat this as removing a latent store-dependency and an import-time side effect, not as fixing a bug anyone has hit here.
Verified, not assumed
I checked jotai's semantics against the pinned 2.17.1 with a standalone probe before writing the fix: a store created after a write reads
{a:1}(the import-time snapshot) fromatom(readAll()), and{a:1,b:2}fromatomWithDefault(() => readAll()).atomWithDefaultstays writable and per-store.Tests
packages/components/tests/atoms-local-storage-cache.test.ts, three cases:githubReposCache.setsees the cached repos, not the import-time snapshotsetWorkspaceReposCacheAtom, into both localStorage and the storeConfirmed in both directions on an idle machine:
3 passedexpected undefined to deeply equal [ { fullName: 'LodyAI/Lody', … } ]andexpected null to deeply equal [ … ]Those are real assertion failures, not timeouts.
What ran, and what did not
Ran: the new test file both ways,
prettier --check(clean),oxlint(0 warnings, 0 errors) on both changed files.Did not run
pnpm check.pnpm installneeds--ignore-scriptsin this environment, andpnpm test:cispansapps/cli, whose nativebetter-sqlite3binding is absent. CI is the authority on the full suite.