diff --git a/.changeset/fix-swr-refresh-adapter-bypass.md b/.changeset/fix-swr-refresh-adapter-bypass.md new file mode 100644 index 0000000..47a9bc1 --- /dev/null +++ b/.changeset/fix-swr-refresh-adapter-bypass.md @@ -0,0 +1,5 @@ +--- +"ff-serv": patch +--- + +Fix SWR refresh bypassing adapter to call actual lookup diff --git a/packages/serv/src/cache/cache.test.ts b/packages/serv/src/cache/cache.test.ts index 44e8046..bd3ee27 100644 --- a/packages/serv/src/cache/cache.test.ts +++ b/packages/serv/src/cache/cache.test.ts @@ -1,8 +1,32 @@ import { it } from '@effect/vitest'; -import { Duration, Effect, Ref, TestClock } from 'effect'; +import { Clock, Duration, Effect, Option, Ref, TestClock } from 'effect'; import { describe, expect } from 'vitest'; +import type { CacheAdapter, CacheEntry } from './adapter.js'; import { Cache } from './cache.js'; +function makeTestAdapter() { + const store = new Map>(); + const adapter: CacheAdapter = { + get: (key) => + Effect.sync(() => { + const entry = store.get(JSON.stringify(key)); + return entry ? Option.some(entry) : Option.none(); + }), + set: (key, entry, _ttl) => + Effect.sync(() => { + store.set(JSON.stringify(key), entry); + }), + remove: (key) => + Effect.sync(() => { + store.delete(JSON.stringify(key)); + }), + removeAll: Effect.sync(() => { + store.clear(); + }), + }; + return { adapter, store }; +} + describe('Cache', () => { describe('core', () => { it.effect('calls lookup and returns value', () => @@ -349,4 +373,89 @@ describe('Cache', () => { }), ); }); + + describe('adapter', () => { + it.effect('uses adapter data on cold start without calling lookup', () => + Effect.gen(function* () { + const { adapter, store } = makeTestAdapter(); + const now = yield* Clock.currentTimeMillis; + store.set(JSON.stringify(1), { value: 'cached-user-1', storedAt: now }); + + const callCount = yield* Ref.make(0); + const cache = yield* Cache.make({ + ttl: Duration.minutes(5), + lookup: (id: number) => + Ref.update(callCount, (n) => n + 1).pipe( + Effect.map(() => `user-${id}`), + ), + adapter, + }); + + const value = yield* cache.get(1); + expect(value).toBe('cached-user-1'); + expect(yield* Ref.get(callCount)).toBe(0); + }), + ); + + it.effect( + 'SWR refresh calls lookup instead of short-circuiting with adapter', + () => + Effect.gen(function* () { + const { adapter } = makeTestAdapter(); + const callCount = yield* Ref.make(0); + const cache = yield* Cache.make({ + ttl: Duration.minutes(5), + swr: Duration.minutes(10), + lookup: (id: number) => + Effect.gen(function* () { + yield* Ref.update(callCount, (n) => n + 1); + const count = yield* Ref.get(callCount); + return `user-${id}-v${count}`; + }), + adapter, + }); + + yield* cache.get(1); + yield* TestClock.adjust(Duration.minutes(7)); + + // Trigger SWR refresh + yield* cache.get(1); + yield* Effect.yieldNow(); + yield* TestClock.adjust(Duration.zero); + yield* Effect.yieldNow(); + + // lookup must have been called twice (initial + refresh) + expect(yield* Ref.get(callCount)).toBe(2); + }), + ); + + it.effect('adapter updated after SWR refresh', () => + Effect.gen(function* () { + const { adapter, store } = makeTestAdapter(); + const callCount = yield* Ref.make(0); + const cache = yield* Cache.make({ + ttl: Duration.minutes(5), + swr: Duration.minutes(10), + lookup: (id: number) => + Effect.gen(function* () { + yield* Ref.update(callCount, (n) => n + 1); + const count = yield* Ref.get(callCount); + return `user-${id}-v${count}`; + }), + adapter, + }); + + yield* cache.get(1); + yield* TestClock.adjust(Duration.minutes(7)); + + yield* cache.get(1); + yield* Effect.yieldNow(); + yield* TestClock.adjust(Duration.zero); + yield* Effect.yieldNow(); + + const entry = store.get(JSON.stringify(1)); + expect(entry?.value).toBe('user-1-v2'); + }), + ); + }); }); diff --git a/packages/serv/src/cache/cache.ts b/packages/serv/src/cache/cache.ts index 9b4d0f8..0bc0f87 100644 --- a/packages/serv/src/cache/cache.ts +++ b/packages/serv/src/cache/cache.ts @@ -52,13 +52,18 @@ export namespace Cache { : 0; const capacity = adapter?.capacity ?? Number.MAX_SAFE_INTEGER; + // Safe without synchronization — no yield points between has() and add() (cooperative scheduling) + const refreshingKeys = new Set(); + // makeWith uses `timeToLive: (exit) => Duration` — the lookup stores CacheValue // so timeToLive can extract the total window (ttl + swr) from the exit result const inner = yield* EffectCache.makeWith({ capacity, lookup: (key: Key) => Effect.gen(function* () { - if (adapter) { + const isRefreshing = refreshingKeys.has(JSON.stringify(key)); + + if (adapter && !isRefreshing) { const cached = yield* adapter.get(key); if (Option.isSome(cached)) { const now = yield* Clock.currentTimeMillis; @@ -100,9 +105,6 @@ export namespace Cache { }, }); - // Safe without synchronization — no yield points between has() and add() (cooperative scheduling) - const refreshingKeys = new Set(); - const get = (key: Key) => Effect.gen(function* () { const cv = yield* inner.get(key); diff --git a/scripts/patch-exports.ts b/scripts/patch-exports.ts index 95ec634..0bb82fd 100644 --- a/scripts/patch-exports.ts +++ b/scripts/patch-exports.ts @@ -1,65 +1,65 @@ -import { Glob } from "bun" -import { existsSync } from "node:fs" +import { existsSync } from 'node:fs'; +import { Glob } from 'bun'; -const glob = new Glob("*/package.json") +const glob = new Glob('*/package.json'); -const workspaceDirs = ["apps", "packages", "internals"] +const workspaceDirs = ['apps', 'packages', 'internals']; function toDistExport(srcPath: string): { - types: string - import: string - require: string + types: string; + import: string; + require: string; } { - const stripped = srcPath.replace(/^\.\/src\//, "").replace(/\.ts$/, "") + const stripped = srcPath.replace(/^\.\/src\//, '').replace(/\.ts$/, ''); return { types: `./dist/${stripped}.d.ts`, import: `./dist/${stripped}.js`, require: `./dist/${stripped}.cjs`, - } + }; } function patchExports(exports: Record): { - patched: Record - changed: boolean + patched: Record; + changed: boolean; } { - const patched: Record = {} - let changed = false + const patched: Record = {}; + let changed = false; for (const key of Object.keys(exports)) { - const value = exports[key] + const value = exports[key]; if ( - typeof value === "string" && - value.startsWith("./src/") && - value.endsWith(".ts") + typeof value === 'string' && + value.startsWith('./src/') && + value.endsWith('.ts') ) { - patched[key] = toDistExport(value) - changed = true + patched[key] = toDistExport(value); + changed = true; } else { - patched[key] = value + patched[key] = value; } } - return { patched, changed } + return { patched, changed }; } -const root = new URL("..", import.meta.url).pathname.replace(/\/$/, "") +const root = new URL('..', import.meta.url).pathname.replace(/\/$/, ''); for (const dir of workspaceDirs) { - const dirPath = `${root}/${dir}` - if (!existsSync(dirPath)) continue + const dirPath = `${root}/${dir}`; + if (!existsSync(dirPath)) continue; for await (const match of glob.scan(dirPath)) { - const pkgPath = `${dirPath}/${match}` - const pkg = await Bun.file(pkgPath).json() + const pkgPath = `${dirPath}/${match}`; + const pkg = await Bun.file(pkgPath).json(); - if (pkg.publishConfig?.access !== "public") continue - if (!pkg.exports || typeof pkg.exports !== "object") continue + if (pkg.publishConfig?.access !== 'public') continue; + if (!pkg.exports || typeof pkg.exports !== 'object') continue; - const result = patchExports(pkg.exports as Record) - if (!result.changed) continue + const result = patchExports(pkg.exports as Record); + if (!result.changed) continue; - pkg.exports = result.patched - await Bun.write(pkgPath, JSON.stringify(pkg, null, "\t") + "\n") - console.log(`Patched: ${pkg.name} (${pkgPath})`) + pkg.exports = result.patched; + await Bun.write(pkgPath, `${JSON.stringify(pkg, null, '\t')}\n`); + console.log(`Patched: ${pkg.name} (${pkgPath})`); } }