Skip to content

feat: reactive getConfig() - #25

Merged
jimmy-phantom merged 8 commits into
mainfrom
jimmy/reactive-getconfig
May 18, 2026
Merged

feat: reactive getConfig()#25
jimmy-phantom merged 8 commits into
mainfrom
jimmy/reactive-getconfig

Conversation

@jimmy-phantom

@jimmy-phantom jimmy-phantom commented May 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Replaces PR #20's per-fetch subscribe rebuild with a signalium-native reactive model. getConfig() re-runs only when the response values it actually reads have changed, not on every fetch. For poll-like subscribers this collapses steady-state rebuilds to zero; for stateful subscribers (websockets, EventSource) it eliminates the reconnect-per-fetch cost.

User-facing API

import { reactiveSignal } from 'signalium';

class GetPrice extends RESTQuery {
  path = '/price';
  result = { value: t.number };

  getConfig() {
    const ok = reactiveSignal(() => {
      this.responseNotifier.consume();
      return this.response?.ok;
    }).value;

    return {
      subscribe: poll({ interval: ok ? 1000 : 5000 }),
    };
  }
}

Two patterns at the read site:

  • Plain (non-reactive): this.response?.X. Read once when getConfig() first evaluates, never re-read. Use when no reactivity is needed.
  • Reactive: reactiveSignal(() => { this.responseNotifier.consume(); return this.response?.X; }).value. Re-evaluates after every fetch (when responseNotifier fires). reactiveSignal filters propagation via Object.is on the thunk's return value, so extract a primitive (e.g. response.ok, response.status) rather than returning a fresh object or the Response itself, otherwise getConfig re-runs on every fetch even when nothing meaningful changed.

The framework adds nothing beyond responseNotifier; consume() + plain this.response reads is the user pattern. Headers and other derived values use the same shape (this.response?.headers.get('etag') inside the thunk).

Framework changes

  • ctx.response stays a plain Response | undefined field. New ctx.responseNotifier (signalium Notifier) initialized in createExecutionContext.
  • RESTQueryAdapter.executeRequest writes both ctx.response = fetchResponse and ctx.responseNotifier.notify() after each fetch.
  • this.config and this.retryConfig on QueryInstance are getters backed by _resolvedOptions = reactiveSignal(() => resolveOptions(ctx)). The signal's dep tracking filters propagation; cached value returned when no tracked signal changed.
  • reconcileSubscription ref-checks against lastSubscribeFn before tearing down. Identical-subscribe reads no-op.
  • lastSubscribeFn cleared on deactivate and on paramsDidChange so those paths always rebuild.

Test plan

  • Two new failing-repro tests in poll.test.ts (honors a state-dependent interval after first fetch resolves, stops polling when getConfig() switches subscribe to undefined after an error) pass with surgical behavior verified.
  • Full test suite: 1232 passed, 2 skipped.

Footgun worth flagging

A user can write reactiveSignal(() => this.response?.ok).value and forget the consume() call. The derived signal would compute once with no deps and stay cached forever, silently producing stale config. Mitigation candidates: documentation convention ("always call this.responseNotifier.consume() at the top of a reactiveSignal thunk that reads this.response") and an optional lint rule.

🤖 Generated with Claude Code

jimmy-phantom and others added 7 commits May 15, 2026 21:57
Lets a query make subscribe (or any config field) react to response
state surgically. The framework exposes a Notifier on the execution
context that fires after each fetch; user code wraps reads of
this.response in reactiveSignal + responseNotifier.consume() to opt
into per-value reactive tracking.

  class GetPrice extends RESTQuery {
    getConfig() {
      const ok = reactiveSignal(() => {
        this.responseNotifier.consume();
        return this.response?.ok;
      }).value;

      return { subscribe: poll({ interval: ok ? 1000 : 5000 }) };
    }
  }

getConfig re-runs only when ok actually transitions, not on every
fetch. The derived signal's default Object.is equals filters
propagation; the outer resolveOptions wrapper stays clean when no
deps changed; setupSubscription sees the same subscribe ref and
short-circuits.

Framework changes:

- ctx.response stays a plain field. New ctx.responseNotifier (Notifier)
  is initialized in createExecutionContext.
- RESTQueryAdapter.executeRequest writes both ctx.response and
  ctx.responseNotifier.notify() after each fetch.
- this.config and this.retryConfig on QueryInstance become getters
  backed by a reactiveSignal wrapping resolveOptions.
- setupSubscription ref-checks against lastSubscribeFn before tearing
  down, so identical-subscribe reads no-op.
- lastSubscribeFn cleared on deactivate and on paramsDidChange so
  rebuilds happen in those cases regardless of ref.

Three lifecycle tests in query-stream.test.ts converted from
getConfig() returning inline closures to static config = ..., since
their getConfig usage was incidental (the tests verify subscribe
lifecycle, not dynamic config). The static form gives a stable
subscribe ref so the new behavior's ref check naturally short-circuits.

Two new tests in poll.test.ts cover the dynamic-interval and
stop-on-error patterns end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Inline arrow functions inside getConfig() lexically capture `this`,
so a cached config from a previous ctx will read stale params (and
other ctx fields) when invoked under a new ctx. The framework was
caching the resolved-options signal across ctx changes, so a query
whose params changed kept seeing the old ctx through the cached
subscribe closure.

Replace _resolvedSignal whenever getOrCreateExecutionContext builds
a new ctx. Future reads pick up the new signal and re-call getConfig
with the new ctx as `this`, so inline arrows see the new params.

This removes the previous test diagnostic conversion to static
config: getConfig() returning an inline arrow with this.params access
now works correctly across param changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reflects the post-reactive-getConfig role of the function: not a one-shot
setup but a reconcile against possibly-changed config. Also drops a
vestigial cast on ctx.responseNotifier in the REST adapter and refreshes
the _resolvedSignal header comment to name the reactivity contract
(upstream notifier, param-change replacement).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Drop the redundant inline comment at the reassignment site (the field
header already explains it). Refresh the field header to name the
mechanism accurately (memoization, not closure capture) and use "such
as" rather than "typically" since third-party use cases are open-ended.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the broken dynamic-poll-interval example with the canonical
reactiveSignal + responseNotifier.consume() pattern, add a Notifier row
to the RESTQuery instance properties table, and note in the methods
table that getConfig is reactive. Also adds a minor changeset.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jimmy-phantom jimmy-phantom changed the title feat(getConfig): reactive subscribe via responseNotifier feat: reactive getConfig() May 16, 2026
@jimmy-phantom
jimmy-phantom marked this pull request as ready for review May 16, 2026 15:33
@jimmy-phantom
jimmy-phantom requested a review from pzuraq May 16, 2026 15:33
The prior "implementations that never read reactive state continue to
work" line was only true for getConfig that doesn't read mutable
execution-context state at all. Direct this.response reads (the
canonical pre-existing pattern) silently return stale config under the
new memoization model. Spell out the failure mode and the fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jimmy-phantom
jimmy-phantom merged commit 088a23b into main May 18, 2026
1 check passed
@jimmy-phantom
jimmy-phantom deleted the jimmy/reactive-getconfig branch May 18, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants