feat: use IndexedDB for snapshots instead of sessionStorage#16346
feat: use IndexedDB for snapshots instead of sessionStorage#16346Rich-Harris wants to merge 11 commits into
Conversation
|
Install the latest version of pnpm add https://pkg.svelte.dev/@sveltejs/kit/c/b31721fc50ca7595365774451cc6758961852020Open in |
🦋 Changeset detectedLatest commit: b31721f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
I had a memory of indexedDB not working on Firefox in private browsing windows, but it looks like it got fixed a few years ago, luckily. The need for a secure context is what's mostly concerning me now, if there isn't some sort of a fallback. There are various reasons during development why someone would be accessing their own site on something other than |
|
We could fall back to |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
…oss sessions, and `init()` loads every historical snapshot into memory on every hydration, because IndexedDB (unlike the former per-tab `sessionStorage`) persists across tab-close/restart with no cleanup path. This commit fixes the issue reported at packages/kit/src/runtime/client/snapshot-storage.svelte.js:36 ## Bug confirmed (still present) `packages/kit/src/runtime/client/snapshot-storage.svelte.js` uses a shared, persistent IndexedDB store (`indexedDB.open(SNAPSHOT_KEY, 1)`) for snapshots but never scopes or prunes entries by browsing session. Because IndexedDB (unlike the former per‑tab `sessionStorage`) persists across tab‑close/restart: * In `client.js` `start`, a fresh page load with no `history.state` sets `current_history_index = current_navigation_index = Date.now();`, so each session uses a **unique** numeric key base. * `set(index, value)` → `put(index, value)` writes each captured snapshot keyed by that index. Snapshots may now contain large `File`/`Blob` objects. * The only removal path is `truncate(index)`, which deletes forward‑history entries of the *current* session. Entries under previous sessions' unique `Date.now()`‑based keys never match a current index, so they are never deleted → **unbounded growth** and quota exhaustion risk. * `init()` loads the **entire** store into the in‑memory `snapshots` map on every hydration, so startup cost grows monotonically. The `efb62fc` sessionStorage fallback only added resiliency for when IndexedDB is *unavailable*; the normal path is unchanged, so the issue remains. ## Fix (reworked to use the Web Locks API) The earlier `MAX_SESSIONS` last‑active‑timestamp heuristic was rejected as kludgy. This revision uses `navigator.locks` to determine which sessions are *actually* still alive, so there is no arbitrary retention count and no timestamp bookkeeping. * **Session id** — generated once (`crypto.randomUUID()` with a non‑secure‑context fallback) and stored in `sessionStorage` under `SNAPSHOT_KEY:session` via the existing `storage` helper with identity parse/stringify. It survives reloads (so reload‑restore works) but is discarded when the tab closes. * **Liveness lock** — the first time the session id is resolved, we fire‑and‑forget acquire an exclusive Web Lock `session:<id>` whose callback returns a promise that never resolves, so the lock is held for the tab's lifetime and released automatically by the browser on close/crash. The `request` promise is **not** awaited (it never settles); we only attach `.catch()` so a failed acquisition just leaves pruning conservative. * **Compound keys** — snapshots are stored under `[session_id, index]`, so each entry is attributable to a session. The DB stays at **version 1** — the store has no `keyPath` (out‑of‑line keys), so switching from a plain number key to an array key needs no schema migration or second object store. * **Pruning** — `init()` calls `navigator.locks.query()`, collects the ids of all held `session:*` locks into an `active` set that **always explicitly includes the current session** (guarding against `query()` running before this tab's own lock is granted), then iterates the store: loads only the current session's entries into memory, and deletes any entry whose session id isn't in `active` (this also removes legacy non‑namespaced number keys). * **Graceful degradation** — `get_active_sessions()` returns `null` when `navigator.locks`/`query` is unavailable (older browsers / insecure context). In that case `init()` still loads only the current session's snapshots into memory but **does not prune** anything, since staleness can't be proven — we never delete data we can't confirm is dead. * **Fallback intact** — the `efb62fc` sessionStorage fallback (`init()` catch, `set()`, `truncate()`) and the `import * as storage from './session-storage.js'` alias are reused unchanged. ### Residual edge case A tab that is mid‑startup and hasn't yet been granted its `session:*` lock could, in principle, have its snapshots pruned by another tab's concurrent `init()`. This is acceptable in practice: a fresh session rarely has persisted snapshots yet, and its id is always included in its own `active` set, so it never prunes itself. Co-authored-by: Vercel <vercel[bot]@users.noreply.github.com> Co-authored-by: Rich-Harris <hello@rich-harris.dev>
|
/autofix |
| snapshots[index] = $state.snapshot(value); | ||
|
|
||
| void put(index, snapshots[index]); | ||
| storage.set(SNAPSHOT_KEY, snapshots); |
There was a problem hiding this comment.
Should this only be done when put fails instead?
|
|
||
| // a File is not JSON-serializable, so this only works because the snapshot | ||
| // backend uses the structured clone algorithm (IndexedDB) | ||
| await expect(page.locator('[data-testid="info"]')).toHaveText('hello.txt|11'); |
There was a problem hiding this comment.
This one seems to pass regardless because the File is already in-memory. Not sure how to test this better...
Co-authored-by: Tee Ming <chewteeming01@gmail.com>
Nic-Polumeyv
left a comment
Nic-Polumeyv
left a comment
There was a problem hiding this comment.
The snapshots docs (30-advanced/65-snapshots.md) still say the data "must be serializable as JSON so that it can be persisted to sessionStorage", worth updating to whatever the new contract is, since the happy path and the fallback now accept different things (see inline). A smaller nit, put() warns on DataCloneError only, so quota failures stay silent even in dev.
| snapshots[index] = $state.snapshot(value); | ||
|
|
||
| void put(index, snapshots[index]); | ||
| storage.set(SNAPSHOT_KEY, snapshots); |
There was a problem hiding this comment.
This can throw synchronously for values IndexedDB accepts. session-storage.js runs stringify(value) outside its try, and JSON.stringify throws on BigInt and circular references, both fine under structured clone. capture_snapshot runs unguarded inside navigate() (client.js:1836), so a snapshot containing such a value fails the navigation instead of just losing the fallback copy. Same applies to truncate below. Alternatively the stringify call could move inside the try in session-storage.js, whose only other caller is SCROLL_KEY.
| storage.set(SNAPSHOT_KEY, snapshots); | |
| try { | |
| storage.set(SNAPSHOT_KEY, snapshots); | |
| } catch { | |
| // best-effort, JSON.stringify rejects some structured-clonable values (BigInt, circular refs) | |
| } |
| new Promise((resolve, reject) => { | ||
| const transaction = db.transaction(STORE, 'readwrite'); | ||
| const store = transaction.objectStore(STORE); | ||
| const request = store.openCursor(); |
There was a problem hiding this comment.
start() awaits init() before hooks.init and hydration, and this cursor materializes every session's stored values, so a new tab deserializes other tabs' snapshots (potentially multi-megabyte Files) just to prune their keys. getAll(IDBKeyRange.bound([current], [current, []])) would load only this session's values, and the prune could iterate openKeyCursor() instead, or run after hydration entirely.
| * | ||
| * @returns {string} | ||
| */ | ||
| function get_session() { |
There was a problem hiding this comment.
Duplicating a tab copies sessionStorage, so the duplicate starts with the same session id and the two tabs then overwrite each other's [session, index] entries. The per-tab sessionStorage map kept duplicates independent after the copy. Since a request for a held lock just queues, the duplicate could detect this at startup with { ifAvailable: true } and mint a fresh id.
4 participants
This replaces the
sessionStoragepersistence mechanism for snapshots with IndexedDB. Doing so gives us a much larger quota, and means that we can serialize more things, such asFileobjects. If my hunch in #12409 (comment) is correct, this means people are less likely to reach forhistory.statein some circumstances, which means people are less likely to wantpage.stateto be restored on reload.Caveat: IndexedDB requires
https:(except onlocalhost/127.0.0.1). I think that's probably fine, honestly.Possible follow-ups:
we could presumably do the same thing for
page.state. That would be a more complete alternative to feat: use devalue for pushState/replaceState #14129, with the additional benefit of the larger quotaI think we can improve upon the
snapshotAPI. It's annoying to have to export it from a+page.sveltecomponent. The requirement exists for a good reason — we need to be able to guarantee that the snapshot belongs to a specific component, across page reloads — but the ergonomics suck. It occurs to me that we could get the same guarantee in the common case by generating a stack trace, which tells us the callsite.It wouldn't be bulletproof — if you had multiple instances of a component that called
snapshot, or called it via an intermediate layer, we would see duplicates (that we could error on), and if you deployed a new version with different chunks that resulted in a different stack trace, the snapshot wouldn't survive reload. But we could let you specify an optionalidto solve those issuesPlease don't delete this checklist! Before submitting the PR, please make sure you do the following:
Tests
pnpm testand lint the project withpnpm lintandpnpm checkChangesets
pnpm changesetand following the prompts. Changesets that add features should beminorand those that fix bugs should bepatch. Please prefix changeset messages withfeat:,fix:, orchore:.Edits