Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions packages/atom/src/Atom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1802,19 +1802,23 @@ export const kvs = <A>(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 value = Result.value(get(resultAtom))
if (Option.isSome(value)) {
return value.value
}
const defaultValue = options.defaultValue()
get.set(setAtom, defaultValue)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be set in some way, as the defaultValue is lazy and should only be evaluated once.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course!

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<A>(), options.defaultValue)
get.setSelf(value)
get.set(setAtom, value)
}
}, { immediate: true })
return Option.getOrElse(get.self<A>(), options.defaultValue)
},
(ctx, value: A) => {
ctx.set(setAtom, value as any)
Expand Down
104 changes: 104 additions & 0 deletions packages/atom/test/Atom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ 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 * 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"
Expand Down Expand Up @@ -1501,6 +1502,109 @@ describe("Atom", () => {
assert.strictEqual(result.value.b, 4)
assert.strictEqual(runs, 2)
})

describe("kvs", () => {
it("memoizes defaultValue while loading empty storage", async () => {
let calls = 0
const storage = new Map<string, string>()

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
const storage = new Map<string, string>()
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.makeStringOnly({
get: (key) =>
Effect.gen(function*() {
yield* Effect.sleep(20) // Short delay to create Initial state window
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: "test-key",
schema: Schema.Number,
defaultValue: () => 0
})

const r = Registry.make()
r.mount(atom)

// First read during Initial state returns default
const 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 {
Expand Down