From 6910a4ed1defd5596168a1f53ba9eae476a1e188 Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Thu, 7 May 2026 16:45:58 -0700 Subject: [PATCH 1/6] test(fetchium): failing tests for dynamic getConfig() subscribe getConfig() re-evaluates on every fetch and produces a fresh subscribe value, but setupSubscription in QueryResult only consults config.subscribe at activation, params change, or when no subscriber exists. Once a subscriber is running, later subscribe values from getConfig() are ignored. Adds two failing tests: - A state-dependent interval (poll interval depending on this.response.ok) is captured at activation with response undefined, and never rebuilds once response.ok becomes true. - subscribe: undefined returned from getConfig() after a 404 does not stop the running subscriber. The existing "getConfig subscribe" tests pass because they return a constant poll() value, never exercising the cross-fetch change. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/fetchium/src/__tests__/poll.test.ts | 66 ++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/packages/fetchium/src/__tests__/poll.test.ts b/packages/fetchium/src/__tests__/poll.test.ts index a517a96..c88b5d5 100644 --- a/packages/fetchium/src/__tests__/poll.test.ts +++ b/packages/fetchium/src/__tests__/poll.test.ts @@ -137,6 +137,72 @@ describe('poll() factory', () => { expect(callCount).toBeGreaterThanOrEqual(3); }); }); + + it('honors a state-dependent interval after first fetch resolves', async () => { + const { client, mockFetch } = getClient(); + let callCount = 0; + mockFetch.get('/dynamic-interval', () => ({ n: ++callCount })); + + class GetDynamicInterval extends RESTQuery { + path = '/dynamic-interval'; + result = { n: t.number }; + + getConfig() { + // Before first fetch: response is undefined → falsy branch (5000ms). + // After first fetch: response.ok === true → fast branch (100ms). + return { + subscribe: poll({ interval: this.response?.ok ? 100 : 5000 }), + }; + } + } + + await testWithClient(client, async () => { + const relay = fetchQuery(GetDynamicInterval); + await relay; + const callsAfterFirst = callCount; + + // Once response.ok is true, getConfig() returns poll({ interval: 100 }). + // 350ms should yield at least 2 additional polls. + await sleep(350); + + expect(callCount).toBeGreaterThanOrEqual(callsAfterFirst + 2); + }); + }); + + it('stops polling when getConfig() switches subscribe to undefined after a 404', async () => { + const { client, mockFetch } = getClient(); + let callCount = 0; + // First call: 200 OK. + mockFetch.get('/maybe-gone', () => ({ n: ++callCount })); + // Subsequent calls: 404 (route reuse falls through to this last-match route). + mockFetch.get('/maybe-gone', () => ({ n: ++callCount }), { status: 404 }); + + class GetMaybeGone extends RESTQuery { + path = '/maybe-gone'; + result = { n: t.number }; + + getConfig() { + // Stop polling once a 404 has been observed. + return { + subscribe: this.response?.status === 404 ? undefined : poll({ interval: 100 }), + }; + } + } + + await testWithClient(client, async () => { + const relay = fetchQuery(GetMaybeGone); + await relay; + + // Let the 100ms poll tick at least once so a 404 lands. + await sleep(250); + const callsAtTerminal = callCount; + expect(callsAtTerminal).toBeGreaterThan(1); + + // After the 404, getConfig() returns subscribe: undefined → polling should stop. + await sleep(400); + expect(callCount).toBe(callsAtTerminal); + }); + }); }); describe('Multiple independent polls', () => { From 54a26e3ad30fa9ff5d7906d56fcf029b26ee5e4b Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Fri, 8 May 2026 09:48:05 -0700 Subject: [PATCH 2/6] fix(fetchium): rebuild subscriber when getConfig() subscribe changes setupSubscription previously only consulted config.subscribe at activation, on params change, or when no subscriber was running. Once a subscriber was active, later subscribe values from getConfig() on each refetch were ignored, so patterns like subscribe: poll({ interval: this.response?.ok ? 100 : 5000 }) subscribe: this.response?.status === 404 ? undefined : poll({ ... }) had no effect after the first activation. Changes: - setupSubscription tracks the last-installed subscribe ref and rebuilds on change. The post-fetch path always invokes it (the previous unsubscribe === undefined guard skipped the case where the new value is undefined and a running subscriber should be torn down). - runQuery re-resolves options after the fetch resolves so getConfig() observes the freshly assigned this.response before setupSubscription runs. - poll() is memoized by interval, so re-evaluating getConfig() with a stable interval returns the same subscribe reference and incurs no rebuild. - Param-change rebuilds clear lastSubscribeFn before calling setupSubscription, since the running subscriber captured the old params. Three lifecycle tests in query-stream.test.ts that used getConfig() to scope an inline closure for test counters are converted to static config = { subscribe(...) {...} }. Their config values never depended on per-fetch state; the getConfig() form was incidental and would now cause per-fetch rebuilds (the documented trade-off of choosing the dynamic form). Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/getconfig-subscribe-rebuild.md | 5 ++ packages/fetchium/src/QueryResult.ts | 17 ++++-- packages/fetchium/src/__tests__/poll.test.ts | 35 ++++++++++++ .../src/__tests__/query-stream.test.ts | 54 +++++++++---------- .../fetchium/src/subscriptions/polling.ts | 18 ++++++- 5 files changed, 93 insertions(+), 36 deletions(-) create mode 100644 .changeset/getconfig-subscribe-rebuild.md diff --git a/.changeset/getconfig-subscribe-rebuild.md b/.changeset/getconfig-subscribe-rebuild.md new file mode 100644 index 0000000..7028cbd --- /dev/null +++ b/.changeset/getconfig-subscribe-rebuild.md @@ -0,0 +1,5 @@ +--- +"fetchium": patch +--- + +`getConfig()` returning a different `subscribe` value mid-session now rebuilds the running subscriber. Previously, `setupSubscription` only consulted `config.subscribe` at activation, on params change, or when no subscriber was running, so patterns like `subscribe: poll({ interval: this.response?.ok ? 100 : 5000 })` and `subscribe: this.response?.status === 404 ? undefined : poll(...)` had no effect after the first activation. `poll()` is now memoized by interval, so a stable interval re-evaluated on every fetch returns the same subscribe reference and incurs no rebuild. diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index 32582a8..fa1e0a2 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -33,6 +33,7 @@ export class QueryInstance { private params: QueryParams | undefined = undefined; private unsubscribe?: () => void = undefined; + private lastSubscribeFn: QueryConfigOptions['subscribe'] = undefined; private _relayState: RelayState> | undefined = undefined; private _isActive: boolean = false; @@ -112,6 +113,7 @@ export class QueryInstance { this.unsubscribe?.(); this.unsubscribe = undefined; + this.lastSubscribeFn = undefined; const gcTime = this.config?.gcTime ?? DEFAULT_GC_TIME; this.queryClient.gcManager.schedule(this.queryKey, gcTime, GcKeyType.Query); @@ -165,6 +167,8 @@ export class QueryInstance { } } } else if (paramsDidChange) { + // Force rebuild: the running subscriber captured the old params. + this.lastSubscribeFn = undefined; this.setupSubscription(); this.runDebounced(); } @@ -264,10 +268,13 @@ export class QueryInstance { } private setupSubscription(): void { + const subscribeFn = this.config?.subscribe; + if (subscribeFn === this.lastSubscribeFn) return; + this.unsubscribe?.(); this.unsubscribe = undefined; + this.lastSubscribeFn = subscribeFn; - const subscribeFn = this.config?.subscribe; if (!subscribeFn) return; const ctx = this._executionCtx; @@ -318,9 +325,11 @@ export class QueryInstance { const result = this.applyData(freshData, true); this.saveQueryMetadata(); - if (this.unsubscribe === undefined) { - this.setupSubscription(); - } + // getConfig() can read this.response; the pre-fetch resolve in + // getOrCreateExecutionContext saw it as undefined. Resolve again so + // setupSubscription sees the post-fetch value. + this.resolveAndApplyOptions(); + this.setupSubscription(); return result; }, diff --git a/packages/fetchium/src/__tests__/poll.test.ts b/packages/fetchium/src/__tests__/poll.test.ts index c88b5d5..f85e00c 100644 --- a/packages/fetchium/src/__tests__/poll.test.ts +++ b/packages/fetchium/src/__tests__/poll.test.ts @@ -206,6 +206,41 @@ describe('poll() factory', () => { }); describe('Multiple independent polls', () => { + it('should tick independently when two queries share the same interval', async () => { + const { client, mockFetch } = getClient(); + let aCount = 0; + let bCount = 0; + + mockFetch.get('/poll-shared-a', () => ({ n: ++aCount })); + mockFetch.get('/poll-shared-b', () => ({ n: ++bCount })); + + class GetSharedA extends RESTQuery { + path = '/poll-shared-a'; + result = { n: t.number }; + config = { subscribe: poll({ interval: 100 }) }; + } + + class GetSharedB extends RESTQuery { + path = '/poll-shared-b'; + result = { n: t.number }; + config = { subscribe: poll({ interval: 100 }) }; + } + + await testWithClient(client, async () => { + const relayA = fetchQuery(GetSharedA); + const relayB = fetchQuery(GetSharedB); + await relayA; + await relayB; + + await sleep(250); + + // Each query refetches its own path on its own ticker, even though + // both share the same memoized poll() reference. + expect(aCount).toBeGreaterThanOrEqual(2); + expect(bCount).toBeGreaterThanOrEqual(2); + }); + }); + it('should tick independently with different intervals', async () => { const { client, mockFetch } = getClient(); let fastCount = 0; diff --git a/packages/fetchium/src/__tests__/query-stream.test.ts b/packages/fetchium/src/__tests__/query-stream.test.ts index e7f0b53..79e01bb 100644 --- a/packages/fetchium/src/__tests__/query-stream.test.ts +++ b/packages/fetchium/src/__tests__/query-stream.test.ts @@ -295,16 +295,14 @@ describe('Query Stream Option', () => { posts: t.array(t.entity(Post)), }; - getConfig() { - return { - subscribe: (onEvent: (event: MutationEvent) => void) => { - subscribeCount++; - return () => { - unsubscribeCount++; - }; - }, - }; - } + config = { + subscribe: (_onEvent: (event: MutationEvent) => void) => { + subscribeCount++; + return () => { + unsubscribeCount++; + }; + }, + }; } // First activation @@ -439,17 +437,15 @@ describe('Query Stream Option', () => { items: t.array(t.entity(Item)), }; - getConfig() { - return { - subscribe: (onEvent: (event: MutationEvent) => void) => { - const sub = { channelId: this.params.channelId, unsubscribed: false }; - subscriptions.push(sub); - return () => { - sub.unsubscribed = true; - }; - }, - }; - } + config = { + subscribe(this: any, _onEvent: (event: MutationEvent) => void) { + const sub = { channelId: this.params.channelId, unsubscribed: false }; + subscriptions.push(sub); + return () => { + sub.unsubscribed = true; + }; + }, + }; } const channelSignal = signal('ch-1'); @@ -507,15 +503,13 @@ describe('Query Stream Option', () => { items: t.array(t.entity(Item)), }; - getConfig() { - return { - subscribe: (onEvent: (event: MutationEvent) => void) => { - subscribeCount++; - latestOnEvent = onEvent; - return () => {}; - }, - }; - } + config = { + subscribe: (onEvent: (event: MutationEvent) => void) => { + subscribeCount++; + latestOnEvent = onEvent; + return () => {}; + }, + }; } const channelSignal = signal('ch-1'); diff --git a/packages/fetchium/src/subscriptions/polling.ts b/packages/fetchium/src/subscriptions/polling.ts index d8b631a..4e9a177 100644 --- a/packages/fetchium/src/subscriptions/polling.ts +++ b/packages/fetchium/src/subscriptions/polling.ts @@ -16,10 +16,21 @@ function clampInterval(interval: number): number { return interval; } -export function poll(config: PollConfig): (this: any, onEvent: (event: MutationEvent) => void) => () => void { +type PollSubscribe = (this: any, onEvent: (event: MutationEvent) => void) => () => void; + +// Memoize so re-evaluating `getConfig()` with the same interval returns a +// stable reference. Without this, the canonical use case +// (`subscribe: poll({ interval: this.response?.ok ? 100 : 5000 })`) would +// tear down and rebuild the subscriber on every fetch in steady state. +const pollCache = new Map(); + +export function poll(config: PollConfig): PollSubscribe { const interval = clampInterval(config.interval); - return function subscribe(this: any, _onEvent: (event: MutationEvent) => void): () => void { + let subscribe = pollCache.get(interval); + if (subscribe !== undefined) return subscribe; + + subscribe = function (this: any, _onEvent: (event: MutationEvent) => void): () => void { let active = true; let timer: ReturnType | undefined; @@ -47,4 +58,7 @@ export function poll(config: PollConfig): (this: any, onEvent: (event: MutationE } }; }; + + pollCache.set(interval, subscribe); + return subscribe; } From 72e1963a41db6c3df095057bf9677e415102320b Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Fri, 8 May 2026 10:11:55 -0700 Subject: [PATCH 3/6] remove poll() memoization Caching `poll()` output by interval was an asymmetric optimization: re-evaluating `poll()` inside `getConfig()` on every fetch returned a stable ref so steady-state polling didn't churn, but inline subscribe closures inside `getConfig()` always returned fresh refs and rebuilt on every fetch. The cache made poll-using queries cheaper than arbitrary user closures for the same shape of code, which is a distinction users would have to learn. Removed. Now both produce a fresh ref each `getConfig()` call and both incur the same cheap per-fetch rebuild (clearTimeout + setTimeout + one closure alloc), with no timing impact since the next tick is scheduled `interval` ms after fetch resolution either way. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/getconfig-subscribe-rebuild.md | 2 +- packages/fetchium/src/subscriptions/polling.ts | 18 ++---------------- 2 files changed, 3 insertions(+), 17 deletions(-) diff --git a/.changeset/getconfig-subscribe-rebuild.md b/.changeset/getconfig-subscribe-rebuild.md index 7028cbd..586bde3 100644 --- a/.changeset/getconfig-subscribe-rebuild.md +++ b/.changeset/getconfig-subscribe-rebuild.md @@ -2,4 +2,4 @@ "fetchium": patch --- -`getConfig()` returning a different `subscribe` value mid-session now rebuilds the running subscriber. Previously, `setupSubscription` only consulted `config.subscribe` at activation, on params change, or when no subscriber was running, so patterns like `subscribe: poll({ interval: this.response?.ok ? 100 : 5000 })` and `subscribe: this.response?.status === 404 ? undefined : poll(...)` had no effect after the first activation. `poll()` is now memoized by interval, so a stable interval re-evaluated on every fetch returns the same subscribe reference and incurs no rebuild. +`getConfig()` returning a different `subscribe` value mid-session now rebuilds the running subscriber. Previously, `setupSubscription` only consulted `config.subscribe` at activation, on params change, or when no subscriber was running, so patterns like `subscribe: poll({ interval: this.response?.ok ? 100 : 5000 })` and `subscribe: this.response?.status === 404 ? undefined : poll(...)` had no effect after the first activation. diff --git a/packages/fetchium/src/subscriptions/polling.ts b/packages/fetchium/src/subscriptions/polling.ts index 4e9a177..d8b631a 100644 --- a/packages/fetchium/src/subscriptions/polling.ts +++ b/packages/fetchium/src/subscriptions/polling.ts @@ -16,21 +16,10 @@ function clampInterval(interval: number): number { return interval; } -type PollSubscribe = (this: any, onEvent: (event: MutationEvent) => void) => () => void; - -// Memoize so re-evaluating `getConfig()` with the same interval returns a -// stable reference. Without this, the canonical use case -// (`subscribe: poll({ interval: this.response?.ok ? 100 : 5000 })`) would -// tear down and rebuild the subscriber on every fetch in steady state. -const pollCache = new Map(); - -export function poll(config: PollConfig): PollSubscribe { +export function poll(config: PollConfig): (this: any, onEvent: (event: MutationEvent) => void) => () => void { const interval = clampInterval(config.interval); - let subscribe = pollCache.get(interval); - if (subscribe !== undefined) return subscribe; - - subscribe = function (this: any, _onEvent: (event: MutationEvent) => void): () => void { + return function subscribe(this: any, _onEvent: (event: MutationEvent) => void): () => void { let active = true; let timer: ReturnType | undefined; @@ -58,7 +47,4 @@ export function poll(config: PollConfig): PollSubscribe { } }; }; - - pollCache.set(interval, subscribe); - return subscribe; } From 7b842a2543b57ca28852ff4aec909647f0466ec8 Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Fri, 8 May 2026 10:15:57 -0700 Subject: [PATCH 4/6] generalize test name for undefined-subscribe-after-error case The test still uses a 404 to trigger the transition, but the behavior under test is `subscribe: undefined` after any error response, not specifically 404. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/fetchium/src/__tests__/poll.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/fetchium/src/__tests__/poll.test.ts b/packages/fetchium/src/__tests__/poll.test.ts index f85e00c..99e9d28 100644 --- a/packages/fetchium/src/__tests__/poll.test.ts +++ b/packages/fetchium/src/__tests__/poll.test.ts @@ -169,7 +169,7 @@ describe('poll() factory', () => { }); }); - it('stops polling when getConfig() switches subscribe to undefined after a 404', async () => { + it('stops polling when getConfig() switches subscribe to undefined after an error', async () => { const { client, mockFetch } = getClient(); let callCount = 0; // First call: 200 OK. From 8552ea6f7018d351c48ea981e2e78f936fe7b709 Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Fri, 8 May 2026 10:20:47 -0700 Subject: [PATCH 5/6] expand comment on post-fetch resolveAndApplyOptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Spell out the chain — adapter populates this.response, getConfig() may branch on it, the pre-fetch resolve saw it as undefined — so a reader following runQuery doesn't have to reverse-engineer why the resolve is called twice. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/fetchium/src/QueryResult.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index fa1e0a2..413d8f5 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -325,9 +325,12 @@ export class QueryInstance { const result = this.applyData(freshData, true); this.saveQueryMetadata(); - // getConfig() can read this.response; the pre-fetch resolve in - // getOrCreateExecutionContext saw it as undefined. Resolve again so - // setupSubscription sees the post-fetch value. + // adapter.send just populated this.response on the execution context. + // getConfig() implementations may branch on it to choose subscribe + // (e.g. poll interval based on response.ok), so the earlier resolve + // (via getOrCreateExecutionContext, before the fetch) produced a + // config computed against an undefined this.response. Re-resolve + // here so setupSubscription installs the response-aware subscribe. this.resolveAndApplyOptions(); this.setupSubscription(); From f6b480897928814205b352b9c99ddba25e5328e6 Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Fri, 8 May 2026 10:22:40 -0700 Subject: [PATCH 6/6] remove redundant same-interval poll test The test existed to verify that two queries sharing a memoized poll() reference each got independent per-invocation state. With the cache removed, two queries calling poll(100) get distinct refs, and the existing different-intervals test already covers per-query independence. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/fetchium/src/__tests__/poll.test.ts | 35 -------------------- 1 file changed, 35 deletions(-) diff --git a/packages/fetchium/src/__tests__/poll.test.ts b/packages/fetchium/src/__tests__/poll.test.ts index 99e9d28..3d5af76 100644 --- a/packages/fetchium/src/__tests__/poll.test.ts +++ b/packages/fetchium/src/__tests__/poll.test.ts @@ -206,41 +206,6 @@ describe('poll() factory', () => { }); describe('Multiple independent polls', () => { - it('should tick independently when two queries share the same interval', async () => { - const { client, mockFetch } = getClient(); - let aCount = 0; - let bCount = 0; - - mockFetch.get('/poll-shared-a', () => ({ n: ++aCount })); - mockFetch.get('/poll-shared-b', () => ({ n: ++bCount })); - - class GetSharedA extends RESTQuery { - path = '/poll-shared-a'; - result = { n: t.number }; - config = { subscribe: poll({ interval: 100 }) }; - } - - class GetSharedB extends RESTQuery { - path = '/poll-shared-b'; - result = { n: t.number }; - config = { subscribe: poll({ interval: 100 }) }; - } - - await testWithClient(client, async () => { - const relayA = fetchQuery(GetSharedA); - const relayB = fetchQuery(GetSharedB); - await relayA; - await relayB; - - await sleep(250); - - // Each query refetches its own path on its own ticker, even though - // both share the same memoized poll() reference. - expect(aCount).toBeGreaterThanOrEqual(2); - expect(bCount).toBeGreaterThanOrEqual(2); - }); - }); - it('should tick independently with different intervals', async () => { const { client, mockFetch } = getClient(); let fastCount = 0;