@uppy/golden-retriever: Store metaData in IndexedDB#6362
Conversation
🦋 Changeset detectedLatest commit: caa0d8a The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
Pull request overview
This PR updates @uppy/golden-retriever to persist its recovery snapshot (metadata/state) in IndexedDB (with localStorage fallback) to avoid localStorage quota failures (notably with large Transloadit assemblies, per #6280). It also adds supporting DB schema changes and browser regression tests to validate the new persistence behavior and fallback semantics.
Changes:
- Add an IndexedDB-backed metadata store and select it by default when IndexedDB is available, falling back to the existing localStorage-backed store otherwise.
- Extend the existing
uppy-blobsIndexedDB database with a newstateobject store and expire it viacleanup(), including closing connections onversionchange. - Add/adjust browser tests to cover IndexedDB persistence, localStorage fallback, and non-cloneable values behavior; include a changeset for release notes.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/@uppy/golden-retriever/src/MetaDataStore.ts | Makes localStorage persistence best-effort by swallowing setItem failures. |
| packages/@uppy/golden-retriever/src/IndexedDBStore.ts | Adds state store + version bump and cleans up expired files and state entries. |
| packages/@uppy/golden-retriever/src/IndexedDBMetaDataStore.ts | New IndexedDB-backed metadata store with JSON serialization to avoid structured-clone failures. |
| packages/@uppy/golden-retriever/src/index.ts | Chooses IndexedDB vs localStorage metadata backend and makes restore async + failure-safe. |
| packages/@uppy/golden-retriever/src/goldenRetriever.browser.test.ts | Adds regression tests for quota/unavailable storage and IndexedDB persistence behavior. |
| .changeset/shy-knives-read.md | Declares a minor release with a note about the persistence backend change. |
Comments suppressed due to low confidence (1)
packages/@uppy/golden-retriever/src/index.ts:434
- With the IndexedDB metadata backend,
#restore()now yields immediately onawait this.#metaDataStore.load(). Because thestate-updatelistener is registered right away, a fast useraddFile()can persist an empty/partial snapshot before restore finishes loading the previous one, overwriting the recovery state and preventing restore. One way to avoid this is to delay persistence listeners until after restore has attempted to load/apply the previous session.
install(): void {
this.#restore().catch((err) => {
// Restore is best-effort: a failure here must not break the plugin.
this.uppy.log(
'[GoldenRetriever] Could not restore from a previous session',
'warning',
)
this.uppy.log(err)
})
this.uppy.on('state-update', this.#handleStateUpdate)
this.uppy.on('restore-confirmed', this.#handleRestoreConfirmed)
this.uppy.on('restore:plugin-data-changed', this.#handlePluginDataChanged)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // The recovery snapshot now lives in IndexedDB's `state` store; clearing it | ||
| // (plus localStorage for the fallback path) isolates tests from each other. | ||
| async function clearPersistedState() { | ||
| localStorage.clear() | ||
| const db = await connect(DB_NAME) | ||
| await new Promise<void>((resolve) => { | ||
| const tx = db.transaction([STATE_STORE_NAME], 'readwrite') | ||
| tx.objectStore(STATE_STORE_NAME).clear() | ||
| tx.oncomplete = tx.onerror = () => resolve() | ||
| }) | ||
| db.close() | ||
| } |
There was a problem hiding this comment.
yeah this can be added , but we didn't cleared it prior to this PR ( previously also we only cleared the metadata store like now ) and tests run fine despite this.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This PR fixes #6280, Adds IndexedDB as MetaDataStore falling back to
localStoragewhen IndexedDB isn't available.AI Disclaimer : AI Used
Changes
IndexedDBMetaDataStore.ts: IndexedDB-backedMetaDataStore; syncget/setvia in-memory cache, asyncload/save.IndexedDBStore.ts: add astatestore to the existinguppy-blobsDB (v3→4, additive); expire it incleanup(); close onversionchange.index.ts: choose IndexedDB vs localStorage; restore is async and failure-safe.MetaDataStore.ts:setItemwrapped in try/catch so the fallback can't throw either.Notes for reviewers
JSON.stringify'd beforeput()(not stored raw): structured clone throws on non-cloneable values; JSON drops them like localStorage did. Storing raw froze the snapshot and ghosted files.Manual Testing : Manually Tested with large Assemblies