Spike: deep-module restructuring of the storage-atom persistence layer
- Dominant language
- TypeScript
- Stars
- 481
- Forks
- 108
- Avg merge
- 6d 8h
- Merged PRs (30d)
- 5
Description
## Goal
The persistence stack under `src/core/StateProvider/` has a clean base (`writeQueue` → `createWriteThroughAtom`) but splays at the top into shallow, divergent surface. Investigate how to restructure it into **deep modules** — small interfaces, substantial hidden implementation — and produce a recommendation + sliced plan.
Three coupled pain points motivate this:
1. **Two divergent factories for one idea.** `atomWithLocalForage(key, default, reconcile?)` and `createSessionScopedAtom({key, default, codec})` both seed-before-return, write through the queue, and wrap `createWriteThroughAtom`. They differ only in *storage policy* (where the value lives, the cross-tab strategy, serialization). Callers must know which factory to pick, and the two have different parameter shapes for the same job.
2. **`storageAtoms.ts` is a god-list with an ordering footgun.** A positional `Promise.all` whose array order must match the destructure order (the exact "circular-dep regression" its own DEV NOTE warns about). Adding a key means editing three places, and per-tab vs. shared-vs-reconciled scope is invisible at the call site.
3. **Per-concept knowledge is scattered.** A single concept (e.g. graph-view-layout) has its key+default+codec in one file, wiring in `storageAtoms.ts`, migration in `migrateUserLayout.ts`, and hooks in another — no single definition of "the persisted entry."
Underlying all three: **cross-tab semantics are three unnamed mechanisms** (merge / isolate / overwrite), chosen implicitly by factory + optional arg. A reader can't answer "what happens when two tabs edit this?" without tracing code.
**Dependency note:** everything here is in-process or local-substitutable (fake-indexeddb, in-memory sessionStorage — both already injectable), so each option is deepenable by merging and testing *through the interface*; no network ports needed. The one genuine seam is the **storage backend** (localForage / sessionStorage / in-memory), which already has 2+ adapters.
## Options to evaluate
### Option 1 — One `persistentAtom`, storage policy as a port
Collapse the two factories into one interface; make the *policy* a swappable strategy (a genuine port — three real adapters).
```ts
type StoragePolicy = {
seed(key: string, fallback: T): Promise; // how to read the initial value
persist(key: string, value: T): void; // how a change is written
};
function persistentAtom(key: string, defaultValue: T, policy: StoragePolicy)
// named adapters — the cross-tab semantics, finally named:
sharedValue() // blind overwrite (scalars)
sharedMap() // reconcile-by-key merge
perTab(codec: SessionCodec) // sessionStorage + localForage breadcrumb
```
`atomWithLocalForage` and `createSessionScopedAtom` *become* adapters behind the port and stop being public surface. Lowest risk, high depth gain. Does not fix the positional tuple in `storageAtoms.ts`.
### Option 2 — Declarative storage registry
Replace the positional `Promise.all` with a keyed descriptor map; generate the atoms from it.
```ts
const storage = await defineStorage({
graphViewLayout: { key: "graph-view-layout", default: …, policy: perTab(codec), migrate: migrateUserLayout },
schema: { key: "schema", default: new Map(), policy: sharedMap() },
showDebugActions: { key: "showDebugActions", default: false, policy: sharedValue() },
// …
});
// storage.graphViewLayout is the fully-preloaded atom; ordering footgun gone.
```
`defineStorage` owns migration ordering, parallel preload, and the key registry behind one call. One entry = one concept, all its facts (key, default, scope, codec, migration) together; scope is visible at a glance. Builds on Option 1's port. Trades named imports for named-key access (churny rename across consumers).
### Option 3 — Per-concept stores (`keyedStore` / `valueStore`)
Deepen *past the atom* into the operations callers actually perform. `userPreferences.ts` hand-rolls `new Map(prev).set(...)` / `delete` twice; `schema`, `configuration`, and `graph-sessions` repeat the same keyed-collection dance.
```ts
const userVertexStyles = createKeyedStore({
key: "user-vertex-styles",
policy: sharedMap(),
});
// userVertexStyles.useEntry(type) -> [value, upsert, remove]
// userVertexStyles.allAtom, .upsert(k, v), .remove(k)
```
Highest depth *for keyed collections* — kills the duplicated upsert/remove/get-with-default logic across ~4 consumers. Narrower (helps Maps a lot, scalars/per-tab singletons less); best as a complement to Option 1, not a replacement.
### Option 4 — "Persisted entry" as one concept (registry + operations)
The ambitious unification of 2 + 3: declare a concept once and get its atom *and* its typed hooks.
```ts
const graphViewLayout = defineEntry({ key, default: …, policy: perTab(codec), migrate });
// -> { atom, useValue, useSetValue }
const userVertexStyles = defineKeyedEntry({ key: "user-vertex-styles", policy: sharedMap() });
// -> { allAtom, useEntry, upsert, remove }
```
`storageAtoms.ts` becomes a registry of entries; `graphViewLayout.ts` / `userPreferences.ts` shrink to feature logic on top. Deepest single concept in the codebase — "a persisted thing" spanning storage → reconciliation → React. Largest blast radius; risks an over-general framework if the per-concept hooks vary more than they appear to. North star, not the next PR.
## Recommendation (starting point — validate during the spike)
Preferred sequence: **1 → 3, hold 2 and 4.**
- **Option 1 first — high ROI, low risk.** Almost pure win: it names the cross-tab semantics (the currently-invisible, dangerous thing), turns the two factories into adapters behind one port, and passes the deletion test cleanly — three callers would otherwise re-derive the seed logic. The `createSessionScopedAtom` just shipped in PR #1875 is really Option 1's `perTab` adapter waiting to be named as such, so this is partly a consolidation of existing work.
- **Option 3 second — concrete ROI.** Removes real, current duplication (the Map upsert/remove dance in `userPreferences`, `schema`, `configuration`, `graph-sessions`). Leverage is measurable, not speculative. Best done after Option 1 so it builds on the policy port.
- **Option 2 — weakest ROI relative to churn.** The positional tuple is ugly but it's one localized footgun, and named-key access trades one import style for another across every consumer. Do it only if the tuple actually bites again.
- **Option 4 — north star, not the next PR.** Only earns its keep *after* 1 and 3 prove the policy port and store shapes are stable. Jumping straight here risks a framework nobody needed (YAGNI). Highest payoff, highest risk.
ROI summary:
| Option | Depth gain | Blast radius | Risk | When |
| --- | --- | --- | --- | --- |
| 1 — `persistentAtom` + policy port | High | Small (2 factories + call sites) | Low | First |
| 3 — per-concept keyed stores | High (for Maps) | Medium (~4 consumers) | Low–med | Second |
| 2 — declarative registry | Medium | Medium–high (import-style rename) | Medium | Only if tuple bites |
| 4 — unified persisted-entry | Highest | Largest | High (over-generalization) | After 1 + 3 prove stable |
## Expected Outcome
- A validated (or revised) version of the Recommendation above — confirm the sequence and ROI against a deeper look at the call sites.
- A design-it-twice pass on the chosen option's primary interface (e.g. the `StoragePolicy` port) comparing 2–3 radically different shapes on depth, locality, and seam placement.
- A sliced, low-risk rollout plan (one slice per option/collection) with blast-radius notes per slice.
- An explicit **go/no-go vs. #1852**: this spike restructures *within* the Jotai+localForage model; #1852 proposes moving persistence *out* into TanStack Query. They are competing bets on the same problem space and should not both be built — reconcile or pick one.
## Non-goals
- Changing the on-disk storage shape (zero migration).
- Implementing any option — this spike produces the recommendation and plan only.
- Re-deciding the cross-tab reconciliation strategy itself (covered by the `per-key-diff-merge` ADR); this is about *where that logic lives*, not what it does.
## Related Issues
- Originated from the layout-atom session-scoping work (PR #1875 follow-up)
- Related to #1852 (competing persistence-architecture spike — reconcile)
- Context: #1820 (cross-tab clobber bug), #689 (Persisted and Shared User Settings)
> [!IMPORTANT]
> Internal only — this issue is maintained by the core team and is not accepting external contributions.
Contributor guide
Research direction
Start by reading src/core/StateProvider/, especially the writeQueue, createWriteThroughAtom, atomWithLocalForage, createSessionScopedAtom, storageAtoms.ts, and the Map-using consumers. Compare the proposed policy and keyed-store shapes against those call sites, then record a validated sequence, design alternatives, rollout slices, blast-radius notes, and a go/no-go decision against #1852.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 20/100