From 3e6e4bdfb97b8705bc3e0ddd56926d0520fd89da Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Tue, 13 Jan 2026 15:01:17 -0500 Subject: [PATCH 1/3] fix(kvs): prevent overwriting localStorage during Initial state The kvs atom was writing the default value to storage during the Initial state, before the async load completed. This corrupted any existing stored data, causing values to reset on page reload. The fix removes the code that wrote the default to storage. The original code had a `Result.isSuccess` check around the write, but since Effect.flatten unwraps the Option from KeyValueStore.get, Success always contains a value and we'd return early - making that write unreachable. The Result.value() + Option.isSome() pattern is preserved to correctly handle previousSuccess when a refresh fails. Includes test that verifies storage isn't corrupted by the default value during async loading. --- packages/atom/src/Atom.ts | 4 +-- packages/atom/test/Atom.test.ts | 57 +++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/atom/src/Atom.ts b/packages/atom/src/Atom.ts index 55f10ce..a793bd4 100644 --- a/packages/atom/src/Atom.ts +++ b/packages/atom/src/Atom.ts @@ -1812,9 +1812,7 @@ export const kvs = (options: { if (Option.isSome(value)) { return value.value } - const defaultValue = options.defaultValue() - get.set(setAtom, defaultValue) - return defaultValue + return options.defaultValue() }, (ctx, value: A) => { ctx.set(setAtom, value as any) diff --git a/packages/atom/test/Atom.test.ts b/packages/atom/test/Atom.test.ts index 5d76dc0..9aaba18 100644 --- a/packages/atom/test/Atom.test.ts +++ b/packages/atom/test/Atom.test.ts @@ -3,6 +3,7 @@ import * as Hydration from "@effect-atom/atom/Hydration" import * as Registry from "@effect-atom/atom/Registry" import * as Result from "@effect-atom/atom/Result" import { addEqualityTesters, afterEach, assert, beforeEach, describe, expect, it, test, vitest } from "@effect/vitest" +import * as KeyValueStore from "@effect/platform/KeyValueStore" import { Cause, Either, Equal, FiberRef, Schema, Struct, Subscribable, SubscriptionRef } from "effect" import * as Arr from "effect/Array" import * as Context from "effect/Context" @@ -1501,6 +1502,62 @@ describe("Atom", () => { assert.strictEqual(result.value.b, 4) assert.strictEqual(runs, 2) }) + + describe("kvs", () => { + it("preserves existing value after async load completes", async () => { + vitest.useRealTimers() + // Create an in-memory store with a pre-existing value + const storage = new Map() + storage.set("test-key", JSON.stringify(42)) + + // Create a delayed KeyValueStore to simulate async loading + // Use KeyValueStore.make to get proper forSchema support + const DelayedKVS = Layer.succeed( + KeyValueStore.KeyValueStore, + KeyValueStore.make({ + get: (key) => + Effect.gen(function* () { + yield* Effect.sleep(20) // Short delay to create Initial state window + return Option.fromNullable(storage.get(key)) + }), + getAll: Effect.succeed({}), + set: (key, value) => + Effect.sync(() => { + storage.set(key, value) + }), + remove: (key) => + Effect.sync(() => { + storage.delete(key) + }), + clear: Effect.sync(() => storage.clear()), + size: Effect.sync(() => storage.size) + }) + ) + + const kvsRuntime = Atom.runtime(DelayedKVS) + const atom = Atom.kvs({ + runtime: kvsRuntime, + key: "test-key", + schema: Schema.Number, + defaultValue: () => 0 + }) + + const r = Registry.make() + r.mount(atom) + + // First read during Initial state returns default + let value = r.get(atom) + expect(value).toEqual(0) + + // Wait for async load AND any set effects to complete + await new Promise((resolve) => setTimeout(resolve, 50)) + + // THE KEY ASSERTION: After load completes, storage should still have original value. + // The bug was that the default (0) would be written during Initial state, + // corrupting the storage before the async load could read it. + expect(storage.get("test-key")).toEqual(JSON.stringify(42)) + }) + }) }) interface BuildCounter { From 061af6ef9dc2044b60a68d36dde91a42507495a5 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Tue, 13 Jan 2026 20:41:19 -0500 Subject: [PATCH 2/3] fix(kvs): memoize defaultValue during load Cache the default value in atom state so it is evaluated once while the store is loading. Add a regression test to cover memoization behavior. --- packages/atom/src/Atom.ts | 8 +++++- packages/atom/test/Atom.test.ts | 51 +++++++++++++++++++++++++++++++-- 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/packages/atom/src/Atom.ts b/packages/atom/src/Atom.ts index a793bd4..1cc9f7b 100644 --- a/packages/atom/src/Atom.ts +++ b/packages/atom/src/Atom.ts @@ -1808,11 +1808,17 @@ export const kvs = (options: { return writable( (get) => { get.mount(setAtom) + const cached = get.self() const value = Result.value(get(resultAtom)) if (Option.isSome(value)) { return value.value } - return options.defaultValue() + if (Option.isSome(cached)) { + return cached.value + } + const defaultValue = options.defaultValue() + get.setSelf(defaultValue) + return defaultValue }, (ctx, value: A) => { ctx.set(setAtom, value as any) diff --git a/packages/atom/test/Atom.test.ts b/packages/atom/test/Atom.test.ts index 9aaba18..ea8af2d 100644 --- a/packages/atom/test/Atom.test.ts +++ b/packages/atom/test/Atom.test.ts @@ -1504,6 +1504,54 @@ describe("Atom", () => { }) describe("kvs", () => { + it("memoizes defaultValue while loading empty storage", async () => { + let calls = 0 + const storage = new Map() + + const DelayedKVS = Layer.succeed( + KeyValueStore.KeyValueStore, + KeyValueStore.makeStringOnly({ + get: (key) => + Effect.gen(function*() { + yield* Effect.sleep(20) + return Option.fromNullable(storage.get(key)) + }), + set: (key, value) => + Effect.sync(() => { + storage.set(key, value) + }), + remove: (key) => + Effect.sync(() => { + storage.delete(key) + }), + clear: Effect.sync(() => storage.clear()), + size: Effect.sync(() => storage.size) + }) + ) + + const kvsRuntime = Atom.runtime(DelayedKVS) + const atom = Atom.kvs({ + runtime: kvsRuntime, + key: "default-value-key", + schema: Schema.Number, + defaultValue: () => { + calls++ + return 0 + } + }) + + const r = Registry.make() + r.mount(atom) + + expect(r.get(atom)).toEqual(0) + expect(calls).toEqual(1) + + await vitest.advanceTimersByTimeAsync(50) + + expect(r.get(atom)).toEqual(0) + expect(calls).toEqual(1) + }) + it("preserves existing value after async load completes", async () => { vitest.useRealTimers() // Create an in-memory store with a pre-existing value @@ -1514,13 +1562,12 @@ describe("Atom", () => { // Use KeyValueStore.make to get proper forSchema support const DelayedKVS = Layer.succeed( KeyValueStore.KeyValueStore, - KeyValueStore.make({ + KeyValueStore.makeStringOnly({ get: (key) => Effect.gen(function* () { yield* Effect.sleep(20) // Short delay to create Initial state window return Option.fromNullable(storage.get(key)) }), - getAll: Effect.succeed({}), set: (key, value) => Effect.sync(() => { storage.set(key, value) From f6c7a8fb89afd2e0a54df10e7c1bfe21686940c0 Mon Sep 17 00:00:00 2001 From: Tim Smart Date: Wed, 14 Jan 2026 15:07:00 +1300 Subject: [PATCH 3/3] wip --- packages/atom/src/Atom.ts | 24 ++++++++++++------------ packages/atom/test/Atom.test.ts | 6 +++--- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/atom/src/Atom.ts b/packages/atom/src/Atom.ts index 1cc9f7b..08726e2 100644 --- a/packages/atom/src/Atom.ts +++ b/packages/atom/src/Atom.ts @@ -1802,23 +1802,23 @@ export const kvs = (options: { const resultAtom = options.runtime.atom( Effect.flatMap( KeyValueStore.KeyValueStore, - (store) => Effect.flatten(store.forSchema(options.schema).get(options.key)) + (store) => store.forSchema(options.schema).get(options.key) ) ) return writable( (get) => { get.mount(setAtom) - const cached = get.self() - const value = Result.value(get(resultAtom)) - if (Option.isSome(value)) { - return value.value - } - if (Option.isSome(cached)) { - return cached.value - } - const defaultValue = options.defaultValue() - get.setSelf(defaultValue) - return defaultValue + get.subscribe(resultAtom, (result) => { + if (!Result.isSuccess(result)) return + if (Option.isSome(result.value)) { + get.setSelf(result.value.value) + } else { + const value = Option.getOrElse(get.self(), options.defaultValue) + get.setSelf(value) + get.set(setAtom, value) + } + }, { immediate: true }) + return Option.getOrElse(get.self(), options.defaultValue) }, (ctx, value: A) => { ctx.set(setAtom, value as any) diff --git a/packages/atom/test/Atom.test.ts b/packages/atom/test/Atom.test.ts index ea8af2d..68ca9ab 100644 --- a/packages/atom/test/Atom.test.ts +++ b/packages/atom/test/Atom.test.ts @@ -2,8 +2,8 @@ import * as Atom from "@effect-atom/atom/Atom" import * as Hydration from "@effect-atom/atom/Hydration" import * as Registry from "@effect-atom/atom/Registry" import * as Result from "@effect-atom/atom/Result" -import { addEqualityTesters, afterEach, assert, beforeEach, describe, expect, it, test, vitest } from "@effect/vitest" import * as KeyValueStore from "@effect/platform/KeyValueStore" +import { addEqualityTesters, afterEach, assert, beforeEach, describe, expect, it, test, vitest } from "@effect/vitest" import { Cause, Either, Equal, FiberRef, Schema, Struct, Subscribable, SubscriptionRef } from "effect" import * as Arr from "effect/Array" import * as Context from "effect/Context" @@ -1564,7 +1564,7 @@ describe("Atom", () => { KeyValueStore.KeyValueStore, KeyValueStore.makeStringOnly({ get: (key) => - Effect.gen(function* () { + Effect.gen(function*() { yield* Effect.sleep(20) // Short delay to create Initial state window return Option.fromNullable(storage.get(key)) }), @@ -1593,7 +1593,7 @@ describe("Atom", () => { r.mount(atom) // First read during Initial state returns default - let value = r.get(atom) + const value = r.get(atom) expect(value).toEqual(0) // Wait for async load AND any set effects to complete