Skip to content

fix(fetchium): resolve TopicQuery adapter via subclass-aware lookup - #13

Merged
jimmy-phantom merged 13 commits into
mainfrom
fix/topicquery-adapter-resolution
Apr 28, 2026
Merged

fix(fetchium): resolve TopicQuery adapter via subclass-aware lookup#13
jimmy-phantom merged 13 commits into
mainfrom
fix/topicquery-adapter-resolution

Conversation

@jimmy-phantom

@jimmy-phantom jimmy-phantom commented Apr 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • TopicQuery now assigns static adapter = TopicQueryAdapter so subclasses inherit a runtime value (mirroring RESTQuery/RESTQueryAdapter). The type stays widened to QueryAdapterClass<TopicQueryAdapter> so subclasses can override with concrete adapters whose constructors take arguments.
  • QueryClient.getAdapter() now falls back to an instanceof scan over registered adapters before auto-instantiating, so an abstract base on the query resolves to a consumer-registered concrete subclass.

Why this is needed

Before this change, TopicQuery.adapter was a type-only annotation with no value. That forced every TopicQuery subclass to set static adapter explicitly, which:

  1. Blocks code generators that emit TopicQuery subclasses without knowing which concrete adapter the consumer will register. The generated classes can only target the abstract base.
  2. Forces hand-authored subclasses to write a double-cast (as unknown as typeof TopicQueryAdapter) when their adapter has constructor args, because the override has to match the abstract base shape.

Why subclass-aware lookup is needed for topics but not REST

RESTQueryAdapter is concrete, so consumers register new RESTQueryAdapter(...) directly — the registered instance's .constructor equals the static-adapter key on the query, and exact-match lookup hits.

TopicQueryAdapter is abstract (subscribe/unsubscribe are abstract). Consumers must register a subclass instance (e.g. a WebSocket-backed adapter), so the registered .constructor is the subclass, not the base. Without the subclass-aware fallback, getAdapter(TopicQueryAdapter) misses, falls through to auto-instantiation, and throws because you can't new an abstract class.

The instanceof fallback also makes the REST path correct in the (previously undefined) case where a consumer registers a subclass of RESTQueryAdapter — the old code would silently auto-instantiate a fresh RESTQueryAdapter and ignore the registered instance.

Test plan

  • npm run check-types passes (existing adapter-types.test-d.ts still validates the override surface).
  • All unit + react tests pass in CI (1221 passing on the second attempt; the first attempt hit a flaky react/__tests__/basic.test.tsx:658 "Loading vs 0" timing assertion unrelated to this change — getAdapter's fast-path is unchanged when an exact-match adapter is registered).
  • New unit test in topic-query.test.ts covers a TopicQuery subclass that does not set static adapter, asserting (a) the inherited static value is TopicQueryAdapter and (b) fetchQuery() resolves via the registered subclass instance.

🤖 Generated with Claude Code

jimmy-phantom and others added 13 commits April 27, 2026 17:52
TopicQuery declared `static adapter` as a type-only annotation with no
runtime value, so subclasses had to set it explicitly. That breaks two
patterns:

  1. Generated TopicQuery classes that don't know which concrete adapter
     a consumer will use — they can only target the abstract base.
  2. Hand-authored subclasses had to write
     `static adapter = MyAdapter as unknown as typeof TopicQueryAdapter`
     to satisfy the override.

This change makes TopicQuery assign `static adapter = TopicQueryAdapter`
(mirroring the RESTQuery pattern) and teaches `QueryClient.getAdapter()`
to fall back to a subclass `instanceof` scan before auto-instantiating.

Why both changes are needed: RESTQueryAdapter is concrete, so the
registered instance's `.constructor` equals the static-adapter key on
the query and exact-match lookup hits. TopicQueryAdapter is abstract, so
consumers must register a subclass — the registered constructor is the
subclass, not the base, and exact-match misses. The new instanceof fallback
also makes the REST path correct in the (previously undefined) case where
a consumer registers a subclass of RESTQueryAdapter.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s adapter

The new test in `topic-query.test.ts` relies on the outer `beforeEach`
(~1800 lines above) registering a `MockTopicQueryAdapter` instance on the
QueryClient. Without that context the test reads as magic. Replace the
comment with one that names the inheritance + instanceof-scan path
explicitly and points at the outer registration.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…overrides

After the subclass-aware adapter resolution change, individual TopicQuery
subclasses no longer need to declare `static override adapter` for the
common case of one registered streaming adapter. Update the streaming
page so:

- Inline examples extend `TopicQuery` directly instead of an intermediate
  `MyTopicQuery` base.
- The "Registering the adapter" section explains that the resolution
  happens via inheritance + the QueryClient's instanceof lookup, with an
  opt-in note for disambiguating between multiple registered subclasses.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… note

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
In practice, apps register one streaming adapter per QueryClient — the
multi-adapter override pattern was over-engineering. Replace with a
one-liner pointing at "use a separate QueryClient" if you ever hit the
case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The only justification for "use a separate QueryClient" is the adapter
resolution ambiguity case we already trimmed as hypothetical. Without
that reasoning, the sentence dangles. Remove it; the common-case
explanation stands on its own.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`QueryClient.getAdapter()`'s subclass-aware lookup picks the first
`instanceof` match in registration order. If a consumer registers two
adapters that both satisfy the same lookup base (e.g. two
`TopicQueryAdapter` subclasses on one client), the resolution is silent
and brittle.

In dev builds, scan all registered adapters and throw on more than one
match, naming the conflicting classes. In production, keep the original
fast path: first match wins, exit early. The dev-only branch is gated
behind `if (IS_DEV)` so it tree-shakes out of production bundles.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- `getAdapter()`: collapse the four-case branch in the instanceof scan to
  a single dev-mode ambiguity check plus `match ??= registered` for
  first-match-wins. Also drop a now-redundant inline comment on the
  auto-instantiate fallback (the JSDoc enumerates step 3 and the catch's
  error message covers the failure mode).
- `TopicQuery.ts`: trim the five-line comment on `static override adapter`
  to a single-line note about why the explicit type annotation is load-bearing.

No behavior change. All topic-query tests still pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Captures the "one TopicQueryAdapter subclass per QueryClient" design
intent and pre-warns about the dev-mode ambiguity error so users
structure their setup correctly the first time.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jimmy-phantom
jimmy-phantom requested a review from pzuraq April 28, 2026 15:59
@jimmy-phantom
jimmy-phantom merged commit c024d0a into main Apr 28, 2026
1 check passed
@pzuraq
pzuraq deleted the fix/topicquery-adapter-resolution branch April 28, 2026 16:47
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