feat: two-phase write-through persist for Chrome 146+#434
feat: two-phase write-through persist for Chrome 146+#434
Conversation
Chrome 146+ (March 2026) deprecates the unload event, which this library relied on to persist XOR-split shares. Inspired by ProtonMail's Nov 2025 secureSessionStorage update, this switches to a two-phase approach: Phase 1 (eager): On every mutation (set/delete), debounce-write share1 to window.name immediately — because writing to window.name at pagehide is too late in Chrome/Safari (the browsing context may be frozen). Phase 2 (finalize): On pagehide (with unload fallback), write share2 to sessionStorage, which still works reliably during page teardown. Also adds Playwright e2e tests that verify persistence across real Chromium page refreshes.
actions/cache v1/v2 has been shut down by GitHub, breaking CI. Replaced with built-in caching in actions/setup-node@v4. Also bumped Node from 14.x (EOL) to 22.x and updated coverallsapp/github-action to v2.
Pull Request Test Coverage Report for Build 23556665186Details
💛 - Coveralls |
Install Chromium and run e2e tests after unit tests. Playwright config now explicitly targets only Chromium.
There was a problem hiding this comment.
Pull request overview
This PR updates the library’s browser persistence strategy to handle Chrome 146+ unload deprecation by introducing a two-phase write-through mechanism (eager window.name updates + finalize to sessionStorage on pagehide/unload), and adds Playwright-based e2e coverage for refresh persistence.
Changes:
- Implement two-phase persistence in
SessionKeystore(debounced eager share1 towindow.name, finalize share2 tosessionStorageonpagehidewithunloadfallback). - Add unit tests for two-phase mechanics and new Playwright e2e tests + config/CI wiring.
- Add tooling/scripts for e2e fixture bundling (esbuild) and running the e2e suite.
Reviewed changes
Copilot reviewed 9 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/index.ts |
Reworks persistence to two-phase (eager + finalize) and updates lifecycle event handling. |
src/index.test.ts |
Adds unit tests validating the two-phase persistence behavior. |
e2e/persistence.spec.ts |
Adds Playwright tests for persistence across refresh and session boundaries. |
e2e/fixture/index.html |
Adds a simple browser fixture page for Playwright to exercise persistence. |
playwright.config.ts |
Introduces Playwright config and a local webServer for the fixture. |
package.json |
Adds e2e scripts and dev dependencies (Playwright, esbuild, serve) + ignores e2e in Jest. |
tsconfig.json |
Excludes e2e + Playwright config from TS compilation. |
.github/workflows/ci.yml |
Updates CI to Node 22, adds Playwright install and e2e test execution. |
.github/workflows/cd.yml |
Updates CD to Node 22 with Yarn cache enabled. |
.github/dependabot.yml |
Adds a cooldown for Dependabot update PRs. |
.gitignore |
Ignores e2e fixture output and Playwright test results. |
yarn.lock |
Locks new dev dependencies (Playwright, esbuild, serve and transitive deps). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private _finalize() { | ||
| // Flush any pending debounced save that hasn't fired yet | ||
| if (this.#debounceTimer !== null) { | ||
| clearTimeout(this.#debounceTimer) | ||
| this.#debounceTimer = null | ||
| this.#pendingFinalize = this._save() | ||
| } |
There was a problem hiding this comment.
_finalize() calls _save() when a debounce is still pending, which writes window.name during pagehide/unload. This contradicts the stated motivation (writes to window.name at pagehide may not commit when the browsing context is frozen) and can still drop share1 on fast refreshes. A safer approach is to ensure share1 is written before teardown (e.g., leading-edge write on mutation, or a much shorter flush that runs immediately after mutation), and keep _finalize() limited to writing share2 only.
| // Phase 2: finalize on pagehide (preferred) with unload fallback. | ||
| // Both may fire — _finalize() is idempotent. | ||
| const finalizeHandler = this._finalize.bind(this) | ||
| window.addEventListener('pagehide', finalizeHandler) | ||
| window.addEventListener('unload', finalizeHandler) |
There was a problem hiding this comment.
Each SessionKeystore instance adds pagehide and unload listeners but never removes them. In long-lived apps (or tests that create many instances), this retains instances in memory and can trigger multiple _finalize() runs (and competing writes) for the same storage key. Consider using a shared/static listener per #storageKey, or adding a dispose()/close() API (optionally via AbortController) to remove listeners when a store is no longer used.
| // Wait for debounce to flush window.name (50ms + buffer) | ||
| await page.waitForTimeout(200) |
There was a problem hiding this comment.
Using a fixed waitForTimeout(200) makes the e2e test timing-dependent and potentially flaky under CI load. Prefer waiting on an observable condition (e.g., page.waitForFunction(() => window.name !== '') or a DOM signal) so the test only proceeds once the debounced write has actually happened.
| await page.waitForTimeout(200) | ||
| await page.reload() |
There was a problem hiding this comment.
This waitForTimeout(200) is timing-based and can be flaky. Consider replacing it with a condition-based wait that asserts the debounced persistence has completed (e.g., waiting until window.name becomes non-empty).
| // Wait for debounce | ||
| await page.waitForTimeout(200) | ||
|
|
There was a problem hiding this comment.
This test relies on waitForTimeout(200) for the debounce. To reduce flakes, prefer waiting for window.name to change (phase 1) rather than sleeping for an assumed duration.
The debounce behaviour was removed but function names, comments, and test descriptions still referenced scheduling and debouncing. Renamed _scheduleEagerSave to _writeShare1 and cleaned up all related comments to describe the current synchronous behaviour.
|
Where do we stand with this? |
Tasks
Summary
unload-based persistence with a two-phase write-through approach, inspired by ProtonMail's Nov 2025 secureSessionStorage updatewindow.name— because writing towindow.nameatpagehideis too late in Chrome/Safari (the browsing context may be frozen)pagehide(withunloadfallback), write share2 tosessionStoragepersist()API unchanged for backwards compatibilityCloses #433
Context
Chrome 146+ (March 2026) is deprecating the
unloadevent. The library relied onunloadto persist both XOR shares. ProtonMail discovered that evenpagehideis too late forwindow.namewrites — the browsing context may be frozen and modifications aren't committed. Their solution: writewindow.nameeagerly on every change, defer only thesessionStoragewrite topagehide.Test plan
🤖 Generated with Claude Code