Move assignment cache to IndexedDB, scoped by clientToken#188
Move assignment cache to IndexedDB, scoped by clientToken#188leoromanovsky wants to merge 3 commits intomainfrom
Conversation
…tToken Replace the localStorage-based assignment cache (exposure deduplication) with IndexedDB, consistent with the flags cache. Storage is now scoped by clientToken via buildStorageKeySuffix(), so different apps on the same origin no longer share state. - New IndexedDBAssignmentCache: same dd-flagging DB / configurations store, in-memory Map mirror for fast synchronous set(), fire-and-forget persistence to IndexedDB. - Simplified factory: IndexedDB → memory-only (Chrome storage and localStorage dropped). - Updated provider to pass clientToken instead of hardcoded key.
c40284b to
9998cf0
Compare
There was a problem hiding this comment.
Pull request overview
This PR changes the browser SDK’s exposure/assignment deduplication cache persistence to use IndexedDB (shared with the flags cache) and scopes the persisted data by clientToken, preventing cross-app collisions on the same origin.
Changes:
- Introduces
IndexedDBAssignmentCacheand updatesassignmentCacheFactoryto use IndexedDB (or fall back to memory-only). - Updates the OpenFeature provider to pass
clientTokeninto the assignment cache factory. - Updates/adds tests to use
fake-indexeddband to reset IndexedDB between test runs; removes Chrome storage assignment cache code.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/browser/src/cache/indexeddb-assignment-cache.ts | Adds new IndexedDB-backed assignment cache with an in-memory mirror and fire-and-forget persistence. |
| packages/browser/src/cache/assignment-cache-factory.ts | Simplifies factory to choose between IndexedDB-backed hybrid cache and memory-only cache. |
| packages/browser/src/openfeature/provider.ts | Switches provider to create the exposure cache using clientToken. |
| packages/browser/src/cache/helpers.ts | Removes Chrome storage helpers; keeps localStorage + IndexedDB availability helpers. |
| packages/browser/src/cache/chrome-storage-async-map.ts | Removes Chrome storage async map implementation. |
| packages/browser/src/cache/chrome-storage-assignment-cache.ts | Removes Chrome storage assignment cache implementation. |
| packages/browser/test/openfeature/exposures.spec.ts | Resets IndexedDB between tests instead of clearing localStorage; adds fake-indexeddb setup. |
| packages/browser/test/cache/indexeddb-assignment-cache.spec.ts | Adds coverage for IndexedDBAssignmentCache behavior and token isolation. |
| packages/browser/test/cache/hybrid-assignment-cache.spec.ts | Updates hybrid cache test to hydrate serving cache from persistent IndexedDB store. |
| packages/browser/test/cache/assignment-cache-factory.spec.ts | Updates factory tests for IndexedDB availability and memory-only fallback. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Delete orphaned localStorage and Chrome storage cache files and helpers. - Extract shared openDB() and IndexedDB constants into indexeddb-store.ts. - Batch persist() writes via queueMicrotask so rapid set() calls coalesce into a single IDB transaction per tick. - Move structuredClone polyfill and fake-indexeddb setup to shared jest setup file. - Replace inline require() with top-level import in exposures spec. - Add cross-instance persistence test for the page-reload scenario.
| const store = tx.objectStore(STORE_NAME) | ||
| const request = store.get(this.storageKey) | ||
| request.onsuccess = () => resolve(request.result as [string, string][] | undefined) | ||
| request.onerror = () => reject(request.error) |
There was a problem hiding this comment.
minor: you could also add tx.onabort = () => reject(tx.error) in case the transaction is aborted
greghuels
left a comment
There was a problem hiding this comment.
Nothing major, but one thing to note is that the local exposure deduplication cache might be wiped out after users upgrade. I don't think that's blocking though, and a migration from localStorage to indexeddb might be overkill.
| if (Array.isArray(entries)) { | ||
| this.mirror.clear() | ||
| for (const [k, v] of entries) { | ||
| this.mirror.set(k, v) | ||
| } | ||
| return entries | ||
| } |
There was a problem hiding this comment.
Potential race condition between
- the fire-and-forget
setmethod schedules persist ofthis.mirrorviaqueueMicrotask - when
this.mirror.clear()ingetEntriesdestroys unpersisted writes
which would cause data loss. The getEntries() method treats IndexedDB as the source of truth and overwrites the mirror, but the mirror may contain entries that were queued for persistence but haven't been written yet.
I haven't looked much further, so it's possible that these methods are used in a way that this race condition isn't a concern, but perhaps worth taking a second look.
There was a problem hiding this comment.
Thank you for the careful review; will take another look at this.
Yes we're starting fresh. |
Motivation
The assignment cache (exposure deduplication) persists to localStorage with a hardcoded key (
eppo-assignment-dd-of-browser), meaning different apps on the same origin share state. This PR replaces localStorage with IndexedDB (consistent with the flags cache) and scopes the storage key byclientToken.Changes
Replaced the localStorage and Chrome storage assignment cache backends with a single IndexedDB implementation that shares the same
dd-flaggingdatabase andconfigurationsobject store used by the flags cache. The storage key is scoped byclientTokenso different apps on the same origin no longer collide.Simplified the assignment cache factory from three backends (Chrome storage, localStorage, memory) down to two (IndexedDB, memory). Deleted the Chrome storage and localStorage cache implementations along with their helpers since nothing references them anymore.
Extracted the shared
openDB()function and IndexedDB constants intoindexeddb-store.tsto eliminate duplication between the flags cache and assignment cache.Moved the
structuredClonepolyfill andfake-indexeddb/autoregistration into a shared jest setup file instead of repeating them in every test.Design decisions
IndexedDB over localStorage. IndexedDB is already used by the flags cache, works in Chrome extensions (unlike localStorage in some contexts), and has a higher storage quota. Using the same database for both caches means one fewer storage API to depend on.
In-memory mirror with fire-and-forget persistence. The
set()call writes synchronously to an in-memoryMapand schedules persistence to IndexedDB without blocking the caller. This means exposure dedup checks stay fast (theHybridAssignmentCachereads from the in-memory serving store) and a slow or failing IndexedDB write never puts the provider into an error state.Microtask-batched writes. Rapid
set()calls within the same tick (e.g., many flag evaluations during a single render) coalesce into one IDB transaction viaqueueMicrotask. This avoids opening a connection per event and writing progressively larger snapshots.Scoped by
clientTokenonly. The assignment cache key usesassignments-${buildStorageKeySuffix(clientToken)}rather than including the targeting key or evaluation context. Assignment dedup is per-app, not per-user — if a user changes, the exposure cache is cleared by the provider when the precomputed configcreatedAtchanges. Adding context to the key would fragment the cache unnecessarily.Chrome storage dropped entirely. The Chrome storage backend was ported from the Eppo SDK for a single extension customer. IndexedDB works in Chrome extensions too, so the extra code path adds complexity without benefit.