Skip to content

fix(web): reset scroll restoration when sessionStorage quota hits#707

Merged
tiann merged 3 commits into
tiann:mainfrom
heavygee:fix/web-scroll-restoration-hard-reset
May 26, 2026
Merged

fix(web): reset scroll restoration when sessionStorage quota hits#707
tiann merged 3 commits into
tiann:mainfrom
heavygee:fix/web-scroll-restoration-hard-reset

Conversation

@heavygee
Copy link
Copy Markdown
Contributor

@heavygee heavygee commented May 26, 2026

Summary

  • TanStack Router keeps scroll restoration state in memory; when sessionStorage.setItem failed we could prune the JSON on disk but the in-RAM map stayed huge, so the very next scroll persist threw again and the UI still hit the error boundary (fix(web): TanStack scroll restoration fills sessionStorage and can crash the SPA #708).
  • On any persist failure for tsr-scroll-restoration-v1_3, after prune+retry fails (or JSON is invalid), clear storage and call scrollRestorationCache.set(() => ({})) when the guarded target is real window.sessionStorage (skipped in unit tests that use a mock Storage).
  • Treat any write failure on that key as recoverable (not only QuotaExceededError / DOMException-shaped quota errors).

Test plan

  • cd web && bun run test src/lib/scrollStorageGuard.test.ts (11 tests)
  • cd web && bun run typecheck
  • Manual: long session chat, scroll, navigate until storage is stressed; confirm no uncaught setItem / no white-screen error boundary

Issues

Fixes #708

TanStack keeps scroll state in RAM; pruning only the JSON blob did not stop
repeat quota throws. On persist failure for the scroll key, clear the library
cache when guarding real sessionStorage. Treat any write error on that key
(not only QuotaExceededError-shaped) as recoverable.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Successful prune still leaves TanStack cache oversized — when the trimmed retry succeeds, the persisted JSON shrinks but scrollRestorationCache keeps the full in-memory map, so the next TanStack persist serializes the huge state again and re-enters the quota path. Evidence web/src/lib/scrollStorageGuard.ts:102.
  • [Major] Hard reset can recurse through the guarded setter — scrollRestorationCache.set persists through sessionStorage.setItem, which is already replaced by wrappedSetItem; if even the {} write fails, it re-enters the same hard-reset path until stack overflow. Evidence web/src/lib/scrollStorageGuard.ts:36.

Summary

  • Review mode: initial
  • The patch does not fully clear TanStack Router memory on the common successful-prune path, and the failure reset path can recursively call itself.
  • Coverage gap: tests use a mock Storage, so they skip the real-sessionStorage cache reset branch.

Testing

  • Not run (automation): bun is not installed in this runner.

HAPI Bot

Comment thread web/src/lib/scrollStorageGuard.ts
Comment thread web/src/lib/scrollStorageGuard.ts Outdated
Exercise TanStack scrollRestorationCache reset on real sessionStorage,
mock-storage isolation, and generic storage write failures for tiann#708.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • [Major] Successful prune still leaves TanStack cache oversized — the new hard reset only runs when parsing or the trimmed retry fails, but when originalSetItem accepts the trimmed JSON the closed-over scrollRestorationCache.state remains the full oversized object. TanStack’s next scroll update mutates and serializes that same large state again, so the UI can hit the same storage exception immediately after a “successful” recovery. Evidence web/src/lib/scrollStorageGuard.ts:102.
  • [Major] Hard reset can recurse through the guarded setter — scrollRestorationCache.set(() => ({})) writes via the captured sessionStorage.setItem, which is already replaced by wrappedSetItem; if the {} write throws, the wrapper re-enters hardResetScrollRestorationPersistedState before the fallback can run. Evidence web/src/lib/scrollStorageGuard.ts:36.

Summary

  • Review mode: follow-up after new commits
  • The follow-up added tests, but the production guard still has the two failure modes from the previous bot review. The current tests mock scrollRestorationCache.set as a pure function, so they do not exercise TanStack’s real setter writing back through sessionStorage.setItem.

Testing

  • Not run (automation): bun is not installed in this runner.

HAPI Bot

Comment thread web/src/lib/scrollStorageGuard.ts
Comment thread web/src/lib/scrollStorageGuard.ts Outdated
After a successful storage prune, align TanStack's in-memory
scrollRestorationCache with the trimmed payload. Temporarily unwrap
sessionStorage.setItem when writing through the library cache so hard
reset cannot recurse through the guard (Codex PR review tiann#707).

Co-authored-by: Cursor <cursoragent@cursor.com>
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Findings

  • None.

Summary

  • Review mode: follow-up after new commits
  • No issues found in the latest PR diff. The previous two bot findings are addressed in web/src/lib/scrollStorageGuard.ts: the successful prune path now syncs TanStack's in-memory cache, and cache writes temporarily restore the original setItem to avoid recursive guarded writes. Residual risk: this still depends on TanStack Router's private tsr-scroll-restoration-v1_3 storage key and scrollRestorationCache internals.

Testing

  • Not run (automation): bun is not installed in this runner.
  • Reviewed contributor-reported coverage in web/src/lib/scrollStorageGuard.test.ts for successful prune cache sync and unwrapped hard reset writes.

HAPI Bot

@tiann tiann merged commit cfc8508 into tiann:main May 26, 2026
6 checks passed
heavygee added a commit to heavygee/hapi that referenced this pull request May 28, 2026
Document how tiann#716 was verified: Vitest positive repro of the tiann#707 unwrap
race, plus Playwright dev harnesses for live A/B stress (same pattern as
scripts/dev/read-hapi-web.mjs from tiann#544).

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(web): TanStack scroll restoration fills sessionStorage and can crash the SPA

2 participants