From 6609d60e3821777bde3bae61f40941e304eb8b35 Mon Sep 17 00:00:00 2001 From: Sebastian Lorenz Date: Sat, 1 Aug 2026 18:43:04 +0000 Subject: [PATCH 1/3] Add reproduction for platform-browser/BrowserKeyValueStore issue --- .../BrowserKeyValueStoreTransaction.test.ts | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts diff --git a/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts b/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts new file mode 100644 index 00000000000..37a39ba383f --- /dev/null +++ b/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts @@ -0,0 +1,73 @@ +import * as BrowserKeyValueStore from "@effect/platform-browser/BrowserKeyValueStore" +import * as IndexedDb from "@effect/platform-browser/IndexedDb" +import { assert, it } from "@effect/vitest" +import * as Effect from "effect/Effect" +import * as Layer from "effect/Layer" +import * as Result from "effect/Result" +import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore" +import { IDBKeyRange } from "fake-indexeddb" + +it.effect("does not report a write before its transaction commits", () => { + const db = { + objectStoreNames: { contains: () => true }, + close() {}, + transaction() { + const transaction = { + error: null as unknown, + onabort: null as null | (() => void), + objectStore() { + return { + put() { + const request = { + readyState: "pending", + result: undefined, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.onsuccess?.() + transaction.error = new DOMException("Commit failed", "AbortError") + transaction.onabort?.() + }) + return request + } + } + } + } + return transaction + } + } + const indexedDB = { + open() { + const request = { + readyState: "pending", + result: undefined as unknown, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void), + onupgradeneeded: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.result = db + request.onsuccess?.() + }) + return request + } + } + const layer = BrowserKeyValueStore.layerIndexedDb({ database: "transaction_repro" }).pipe( + Layer.provide(Layer.succeed( + IndexedDb.IndexedDb, + IndexedDb.make({ indexedDB: indexedDB as IDBFactory, IDBKeyRange }) + )) + ) + + return Effect.gen(function*() { + const store = yield* KeyValueStore.KeyValueStore + const result = yield* Effect.result(store.set("key", "value")) + yield* Effect.yieldNow + assert.isTrue(Result.isFailure(result), "the aborted transaction was reported as successful") + }).pipe(Effect.provide(layer)) +}) From 389d5a81b9e384171a274ffd4c64ee9707dcf744 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Mon, 3 Aug 2026 10:40:02 +1200 Subject: [PATCH 2/3] Fix IndexedDB key-value transaction completion --- .../src/BrowserKeyValueStore.ts | 58 ++++++++++++++++--- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/packages/platform-browser/src/BrowserKeyValueStore.ts b/packages/platform-browser/src/BrowserKeyValueStore.ts index 72840e3968c..dd63cbffe3b 100644 --- a/packages/platform-browser/src/BrowserKeyValueStore.ts +++ b/packages/platform-browser/src/BrowserKeyValueStore.ts @@ -74,8 +74,11 @@ export const layerIndexedDb = (options?: { return KeyValueStore.make({ clear: Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest({ method: "clear", message: "Failed to clear backing store" }, () => store.clear()) + return idbWriteRequest( + db, + { method: "clear", message: "Failed to clear backing store" }, + (store) => store.clear() + ) }), get: (key: string) => Effect.map( @@ -103,10 +106,10 @@ export const layerIndexedDb = (options?: { ), set: (key: string, value: string | Uint8Array) => Effect.asVoid(Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest( + return idbWriteRequest( + db, { method: "set", message: "Failed to set value in backing store", key }, - () => store.put({ key, value }) + (store) => store.put({ key, value }) ) })), size: Effect.suspend(() => { @@ -118,10 +121,10 @@ export const layerIndexedDb = (options?: { }), remove: (key: string) => Effect.asVoid(Effect.suspend(() => { - const store = getKvsEntriesStore(db, "readwrite") - return idbRequest( + return idbWriteRequest( + db, { method: "remove", message: "Failed to remove value from backing store", key }, - () => store.delete(key) + (store) => store.delete(key) ) })) }) @@ -171,6 +174,45 @@ const idbRequest = ( )) }) +const idbWriteRequest = ( + db: IDBDatabase, + failArgs: { method: string; message: string; key?: string }, + evaluate: (store: IDBObjectStore) => IDBRequest +): Effect.Effect => + Effect.callback((resume) => { + const transaction = db.transaction(entriesStoreName, "readwrite") + const request = evaluate(transaction.objectStore(entriesStoreName)) + let result: A + let done = false + + const fail = (cause: unknown) => { + if (done) return + done = true + resume(Effect.fail(new KeyValueStore.KeyValueStoreError({ ...failArgs, cause }))) + } + + if (request.readyState === "done") { + result = request.result + } else { + request.onsuccess = () => { + result = request.result + } + request.onerror = () => fail(request.error) + } + + transaction.oncomplete = () => { + if (done) return + done = true + resume(Effect.succeed(result!)) + } + transaction.onerror = () => fail(transaction.error) + transaction.onabort = () => fail(transaction.error) + + return Effect.sync(() => { + if (!done) transaction.abort() + }) + }) + const getKvsEntriesStore = (db: IDBDatabase, mode: IDBTransactionMode) => { const transaction = db.transaction(entriesStoreName, mode) return transaction.objectStore(entriesStoreName) From 5b0231791a036759d56aeb4c215cce6633dd9c63 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Mon, 3 Aug 2026 11:32:46 +1200 Subject: [PATCH 3/3] Address IndexedDB key-value review feedback --- .changeset/brave-keys-commit.md | 5 ++ .../test/BrowserKeyValueStore.test.ts | 70 +++++++++++++++++- .../BrowserKeyValueStoreTransaction.test.ts | 73 ------------------- 3 files changed, 74 insertions(+), 74 deletions(-) create mode 100644 .changeset/brave-keys-commit.md delete mode 100644 packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts diff --git a/.changeset/brave-keys-commit.md b/.changeset/brave-keys-commit.md new file mode 100644 index 00000000000..f9358594f9b --- /dev/null +++ b/.changeset/brave-keys-commit.md @@ -0,0 +1,5 @@ +--- +"@effect/platform-browser": patch +--- + +Fix IndexedDB-backed key-value writes to wait for transaction commit before reporting success. diff --git a/packages/platform-browser/test/BrowserKeyValueStore.test.ts b/packages/platform-browser/test/BrowserKeyValueStore.test.ts index eabe5636765..f4ca6281fbc 100644 --- a/packages/platform-browser/test/BrowserKeyValueStore.test.ts +++ b/packages/platform-browser/test/BrowserKeyValueStore.test.ts @@ -1,8 +1,11 @@ import * as BrowserKeyValueStore from "@effect/platform-browser/BrowserKeyValueStore" import * as IndexedDb from "@effect/platform-browser/IndexedDb" -import { describe } from "@effect/vitest" +import { assert, describe, it } from "@effect/vitest" import { Layer } from "effect" import { testLayer } from "effect-test/unstable/persistence/KeyValueStore.test" +import * as Effect from "effect/Effect" +import * as Result from "effect/Result" +import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore" import { IDBKeyRange, indexedDB } from "fake-indexeddb" describe("KeyValueStore / layerLocalStorage", () => testLayer(BrowserKeyValueStore.layerLocalStorage)) @@ -20,4 +23,69 @@ describe("KeyValueStore / layerIndexedDb", () => { Layer.provide(layerFakeIndexedDb) ) ) + + it.effect("does not report a write before its transaction commits", () => { + const db = { + objectStoreNames: { contains: () => true }, + close() {}, + transaction() { + const transaction = { + error: null as unknown, + onabort: null as null | (() => void), + objectStore() { + return { + put() { + const request = { + readyState: "pending", + result: undefined, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.onsuccess?.() + transaction.error = new DOMException("Commit failed", "AbortError") + transaction.onabort?.() + }) + return request + } + } + } + } + return transaction + } + } + const failingIndexedDb = { + open() { + const request = { + readyState: "pending", + result: undefined as unknown, + error: null, + onsuccess: null as null | (() => void), + onerror: null as null | (() => void), + onupgradeneeded: null as null | (() => void) + } + queueMicrotask(() => { + request.readyState = "done" + request.result = db + request.onsuccess?.() + }) + return request + } + } + const layer = BrowserKeyValueStore.layerIndexedDb({ database: "transaction_repro" }).pipe( + Layer.provide(Layer.succeed( + IndexedDb.IndexedDb, + IndexedDb.make({ indexedDB: failingIndexedDb as unknown as IDBFactory, IDBKeyRange }) + )) + ) + + return Effect.gen(function*() { + const store = yield* KeyValueStore.KeyValueStore + const result = yield* Effect.result(store.set("key", "value")) + yield* Effect.yieldNow + assert.isTrue(Result.isFailure(result), "the aborted transaction was reported as successful") + }).pipe(Effect.provide(layer)) + }) }) diff --git a/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts b/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts deleted file mode 100644 index 37a39ba383f..00000000000 --- a/packages/platform-browser/test/BrowserKeyValueStoreTransaction.test.ts +++ /dev/null @@ -1,73 +0,0 @@ -import * as BrowserKeyValueStore from "@effect/platform-browser/BrowserKeyValueStore" -import * as IndexedDb from "@effect/platform-browser/IndexedDb" -import { assert, it } from "@effect/vitest" -import * as Effect from "effect/Effect" -import * as Layer from "effect/Layer" -import * as Result from "effect/Result" -import * as KeyValueStore from "effect/unstable/persistence/KeyValueStore" -import { IDBKeyRange } from "fake-indexeddb" - -it.effect("does not report a write before its transaction commits", () => { - const db = { - objectStoreNames: { contains: () => true }, - close() {}, - transaction() { - const transaction = { - error: null as unknown, - onabort: null as null | (() => void), - objectStore() { - return { - put() { - const request = { - readyState: "pending", - result: undefined, - error: null, - onsuccess: null as null | (() => void), - onerror: null as null | (() => void) - } - queueMicrotask(() => { - request.readyState = "done" - request.onsuccess?.() - transaction.error = new DOMException("Commit failed", "AbortError") - transaction.onabort?.() - }) - return request - } - } - } - } - return transaction - } - } - const indexedDB = { - open() { - const request = { - readyState: "pending", - result: undefined as unknown, - error: null, - onsuccess: null as null | (() => void), - onerror: null as null | (() => void), - onupgradeneeded: null as null | (() => void) - } - queueMicrotask(() => { - request.readyState = "done" - request.result = db - request.onsuccess?.() - }) - return request - } - } - const layer = BrowserKeyValueStore.layerIndexedDb({ database: "transaction_repro" }).pipe( - Layer.provide(Layer.succeed( - IndexedDb.IndexedDb, - IndexedDb.make({ indexedDB: indexedDB as IDBFactory, IDBKeyRange }) - )) - ) - - return Effect.gen(function*() { - const store = yield* KeyValueStore.KeyValueStore - const result = yield* Effect.result(store.set("key", "value")) - yield* Effect.yieldNow - assert.isTrue(Result.isFailure(result), "the aborted transaction was reported as successful") - }).pipe(Effect.provide(layer)) -})