persist: Dexie migration Promises silently overwrite saved state with defaults #3577
Unanswered
Tnalxmsk
asked this question in
Bug report
Replies: 3 comments 5 replies
|
I feel that's on userland which means you need a custom storage |
1 reply
|
I'm actually fine to use isPromiseLike instead of instanceof Promise, if that helps. |
4 replies
This comment was marked as duplicate.
This comment was marked as duplicate.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Bug Description
In
zustand@5.0.15, returning a Dexie Promise directly frompersist'smigratecallback can overwrite saved data with the initial defaults, even when the migration resolves successfully.Steps to reproduce
{ drafts: [] }, andpartializeconfigured to persistdrafts.migrate, return a Dexie database operation's Promise that resolves to the saved state.Actual behavior
The stored record changes from:
{ "state": { "drafts": [{ "id": "1", "text": "My saved draft" }] }, "version": 1 }to:
{ "state": { "drafts": [] }, "version": 2 }Hydration reports no error. Because the version has advanced, recreating the store does not rerun the migration. No user edits or additional
setStatecalls are needed to trigger the overwrite.Expected behavior
Await the migration result before merging and persisting it. If migration rejects, preserve the original stored data and version and report the error.
Investigation
The migration branch checks
migration instanceof Promise. Dexie Promises do not pass that check, so the unresolved Promise is treated as the migrated state. Wrapping the same operation in nativePromise.resolve(...)preserves the data.Dexie's
PromiseExtended<T>extendsPromise<T>, and this callback type-checks against Zustand'smigratetype without a cast. A native Promise from another realm reproduces the same failure.I verified this with
dexie@4.4.5, including a real browser using IndexedDB and localStorage. The linked Node reproduction uses fake-indexeddb and fails an assertion on the published Zustand version.Related context: #1505 discussed native Promise assumptions in a React Native mocking report, and #2827 covered an older ordinary async-migration regression. This reproduction demonstrates the Dexie/cross-realm data-loss path in 5.0.15; same-realm native Promises work correctly.
Fix prepared
I have prepared a fix and regression tests on this branch:
https://github.com/Tnalxmsk/zustand/tree/fix/persist-promise-migration
The fix preserves native Promise detection and recognizes other migration results through the fuller Promise interface (
then,catch,finally, and a stringSymbol.toStringTag) before awaiting them. It remains scoped to migration results.The regression tests cover successful and rejected migrations, storage/version preservation, store recreation, and synchronous states containing ordinary actions named
then,catch, orfinally. Those synchronous states still hydrate immediately without invoking their actions. All 233 tests, type checks, lint, formatting, and build pass locally. I also verified the fix with actual Dexie operations in a browser using IndexedDB and localStorage.One design trade-off for review: structural checks cannot distinguish a Promise from a state object that deliberately exposes the entire checked interface. This change does not add support for arbitrary then-only objects or change the other Promise checks in storage handling.
Since PR creation is currently limited to collaborators, could a maintainer open a PR from this branch? I'm happy to address review feedback.
Reproduction Link
https://github.com/Tnalxmsk/zustand/tree/e3f3c106dd60c1ee7a8d8bd7128cdc756345eb10/reproductions/persist-promise-migration
All reactions