feat: schema-driven SSE with defineSchema, createChannel, and SSEProvider schema prop

[queso]

Summary

• Build pipeline fixes: multi-entrypoint build (index, testing, server) with .d.ts generation, prepare script, server subpath export, fetch type cast for tsc compat
• defineSchema(): shared schema definition with full TypeScript inference for event names, payload types, cache keys, and update strategies
• createChannel(schema): server-side SSE channel with dual Web/Node.js connect signatures, typed broadcast emit, scoped respond emitters, heartbeats, and connection lifecycle management
• SSEProvider schema prop: auto-derives event mappings from schema, mutually exclusive with manual events at the type level
• mockSSE.sendSSE(): testing convenience method wrapping sendRaw with SSE wire format

Test plan

• 531 tests pass (bun test) — 121 new tests across 5 test files
• Lint clean (bun run lint) — 0 errors, 0 warnings
• Build succeeds (bun run build) — JS + .d.ts for all 3 entry points
• Peer dependencies (react, react-dom, swr) are external in bundle

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Schema-driven SSE API, pluggable fetch/custom transports, server-side SSE channel, streaming SSE parser, reconnection/backoff utilities, and new public exports for schema and parser.

• Documentation

  • Expanded README with schema-driven flow, POST/custom transports, testing guidance; added CHANGELOG and transport PRD.

• Chores

  • CI workflow, updated build/prepare scripts, declaration-emission config, and new package export path.

• Tests

  • Extensive test suites and enhanced mock SSE tooling (including sendSSE/sendRaw and fetch interception).

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds schema-driven SSE, a transport abstraction supporting EventSource, fetch-based POST streams, and custom transports; implements a fetch transport, SSE parser/formatters, reconnect utilities, server-side SSE channel API, expanded testing and mocks, multi-entry build with declaration emission, and docs/PRD updates.

Changes

Cohort / File(s)	Summary
CI & Packaging /.github/workflows/ci.yml, package.json, tsconfig.json, tsconfig.emit.json	Adds GitHub Actions CI; multiplatform build now compiles multiple entry points and emits .d.ts via tsconfig.emit.json; adds prepare script; exposes ./server export; excludes tests from TS project.
Core Types & Public API src/types.ts, src/index.ts	Introduces SSETransport, SSERequestOptions, discriminated SSEConfig variants (schema/events/none), schema-related types; exports defineSchema, createSSEParser, SSETransport and new request types.
Schema Utility & Parser src/schema.ts, src/sseParser.ts	Adds defineSchema (frozen, normalized schema) and SSE parser/formatters: createSSEParser, formatSSEEvent, formatSSEData, plus parser interfaces (feed/reset, onRetry/onEvent).
Client: Hook & Provider src/hooks/useSSEStream.ts, src/SSEProvider.tsx	Implements transport-agnostic connection model, composite connection keys/refcounting, schema→events derivation, createTransport (custom/fetch/EventSource) integration, reconnection/backoff, and cleanup adaptations.
Fetch Transport & Reconnect src/fetchTransport.ts, src/reconnect.ts	Implements fetch-based SSE transport (stream reader + parser), JSON body auto-encoding and POST default; adds DEFAULT_RECONNECT and calculateBackoffDelay.
Server Channel src/server/index.ts	New createChannel(schema, options) API for Web and Node SSE paths, broadcast pool, scoped responders, heartbeat, emit/connect/respond/close semantics.
Testing & Mocking src/testing/index.ts, src/__tests__/*	Extends mockSSE to support fetch-based streams, adds sendRaw/sendSSE and URL registration; large suite of new/updated tests covering schema, transports, parser, fetch transport, reconnect, channel, build pipeline, and testing utilities.
Build & Docs README.md, CHANGELOG.md, prd/0001-transport-abstraction.md, src/__tests__/build-pipeline.test.ts	Documents schema-driven SSE and transport PRD, changelog updates, README changes for schema-driven workflows, and adds build-pipeline integration test verifying multi-entry build and declaration outputs.

Sequence Diagram(s)

sequenceDiagram
    participant App as Client App
    participant Hook as useSSEStream / SSEProvider
    participant Resolver as Config Resolver
    participant Factory as Transport Factory
    participant Conn as Transport Connection
    participant Parser as SSE Parser
    participant Server as Server / createChannel

    App->>Hook: mount(url, options/schema)
    Hook->>Resolver: resolve schema → events
    Resolver-->>Hook: resolvedConfig

    Hook->>Factory: createTransport(url, options)
    alt custom transport provided
        Factory-->>Conn: custom SSETransport
    else method/body/headers present
        Factory-->>Conn: fetch-based transport
    else
        Factory-->>Conn: EventSource
    end

    Server->>Conn: emit SSE wire chunks
    Conn->>Parser: feed(chunk)
    Parser-->>Conn: onEvent(parsedEvent)
    Conn->>Hook: dispatch event
    Hook->>App: update state / trigger handlers

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Poem

🐇 I stitched a schema, neat and clear,

opened transports far and near,
a parser listens, streams take flight,
server heartbeats hum through night,
tests and docs — the rabbit cheers.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'feat: schema-driven SSE with defineSchema, createChannel, and SSEProvider schema prop' directly summarizes the main changes by naming the three core features introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/schema-and-channel

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 8

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/__tests__/exports.test.ts (1)

282-289: ⚠️ Potential issue | 🟡 Minor

defineSchema is missing from the expectedRuntimeExports completeness check.

defineSchema is exported from src/index.ts and documented in the CHANGELOG as a new runtime export, but it's absent from the completeness test. This gap means accidental removal of defineSchema would not be caught.

Add to expectedRuntimeExports array

      const expectedRuntimeExports = [
        'SSEProvider',
        'useSSEContext',
        'useSSEStatus',
        'useSSEEvent',
        'useSSEStream',
        'createSSEParser',
+       'defineSchema',
      ]

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/exports.test.ts` around lines 282 - 289, The completeness
test's expectedRuntimeExports array is missing the new runtime export
defineSchema; update the expectedRuntimeExports constant in the exports.test.ts
test (the array containing 'SSEProvider', 'useSSEContext', ...
'createSSEParser') to include 'defineSchema' so the test asserts the exported
runtime API includes defineSchema.

src/SSEProvider.tsx (1)

597-629: ⚠️ Potential issue | 🔴 Critical

Stale closure in unmount cleanup — current connection may leak.

Line 598 captures eventSourceRef.current at the time the effect first runs (mount). If createConnection() is called again later (e.g., due to reconnection or URL change), eventSourceRef.current is updated to the new connection, but the cleanup closure still holds the old (already-closed) reference. On unmount, the current connection is never closed.

Read from the ref inside the cleanup function instead:

🐛 Proposed fix

   useEffect(() => {
-    const connection = eventSourceRef.current
-    const listeners = listenersRef.current
 
     return () => {
       // Clear any pending reconnect timeout
       if (reconnectTimeoutRef.current) {
         clearTimeout(reconnectTimeoutRef.current)
         reconnectTimeoutRef.current = null
       }
 
+      const connection = eventSourceRef.current
+      const listeners = listenersRef.current
+
       if (connection) {
         const wasConnected = connection.readyState !== CLOSED
 
         for (const { type, handler } of listeners) {
           connection.removeEventListener(type, handler)
         }
         listenersRef.current = []
 
         connection.close()
         eventSourceRef.current = null
 
         if (wasConnected) {
           configRef.current.onDisconnect?.()
         }
       }
     }
   }, [])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/SSEProvider.tsx` around lines 597 - 629, The cleanup closure in the
useEffect captures a stale connection reference; instead, inside the returned
cleanup function read the latest eventSourceRef.current (not the
earlier-captured connection) so you close the actual current connection on
unmount; also read listenersRef.current and reconnectTimeoutRef.current inside
cleanup, remove listeners via connection.removeEventListener for each {type,
handler}, clearTimeout(reconnectTimeoutRef.current) and set it to null, call
connection.close(), set eventSourceRef.current = null, and only invoke
configRef.current.onDisconnect() if the current connection's readyState !==
CLOSED to avoid double-calls.

🟡 Minor comments (13)

README.md-558-558 (1)

558-558: ⚠️ Potential issue | 🟡 Minor

Malformed code span — inner backtick breaks the span boundary (MD038).

Backtick code spans don't process backslash escapes, so \` does not escape the backtick — it terminates the span early and leaves data: ${JSON.stringify(data)}\n\n rendered as plain text outside the span.

📝 Proposed fix using double-backtick delimiters

-`sendSSE(data)` is a convenience wrapper that calls `sendRaw(\`data: ${JSON.stringify(data)}\n\n\`)`.
+`sendSSE(data)` is a convenience wrapper that calls ``sendRaw(`data: ${JSON.stringify(data)}\n\n`)``.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@README.md` at line 558, The inline code span for the sendSSE/sendRaw example
is malformed because the inner backtick in `sendRaw(\`...\`)` terminates the
span; update the README sentence to use a code span delimiter that can contain
backticks (e.g., use double-backtick or triple-backtick fencing) so the entire
expression sendRaw(`data: ${JSON.stringify(data)}\n\n`) is rendered as code;
ensure references to sendSSE(data), sendRaw(...), and createSSEParser remain
unchanged and the inner backtick sequence is preserved inside the code span.

.github/workflows/ci.yml-17-17 (1)

17-17: ⚠️ Potential issue | 🟡 Minor

Pin bun-version to a specific version for reproducibility.

Using latest means CI could silently start using a new Bun version with breaking changes between runs, making failures hard to bisect.

-          bun-version: latest
+          bun-version: "1.2.x"   # pin to a known-good minor range

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml at line 17, The workflow currently sets
bun-version: latest which makes CI non-reproducible; update the .github workflow
to pin bun-version to an exact tested Bun release (replace "latest" with the
concrete semantic version you validated, e.g. "1.5.2") so CI uses a fixed
runtime across runs; locate the bun-version key in the workflow YAML and set it
to the specific version string you want to lock to.

CHANGELOG.md-18-19 (1)

18-19: ⚠️ Potential issue | 🟡 Minor

Add channel.isClosed() to the CHANGELOG.md Added section.

isClosed() is implemented in src/server/index.ts and has corresponding tests, but the entry is missing from the CHANGELOG. Add it to the list with the other channel methods (lines 16-19).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` around lines 18 - 19, Add an entry for the new
channel.isClosed() method to the "Added" list in CHANGELOG.md alongside
channel.emit() and channel.close(); locate the Added section in CHANGELOG.md and
insert a bullet like "`channel.isClosed()` to report whether a channel has been
gracefully closed" so the changelog matches the implementation and tests for
isClosed().

src/__tests__/testing-utils-sendSSE.test.ts-21-25 (1)

21-25: ⚠️ Potential issue | 🟡 Minor

Misleading comment — dynamic import() does not bypass ESM module cache.

Standard ESM (and Bun) caches dynamically imported modules; re-importing the same specifier returns the cached instance. The comment "Always re-import to get a fresh module state" is incorrect. Test isolation here is provided by mockSSE.restore() in afterEach, not by module reloading — the comment should reflect that.

📝 Suggested wording fix

  beforeEach(async () => {
    originalFetch = globalThis.fetch
-   // Always re-import to get a fresh module state
+   // Re-importing returns the cached module; isolation is ensured by mockSSE.restore() in afterEach
    mockSSE = (await import('../testing/index.ts')).mockSSE
  })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/testing-utils-sendSSE.test.ts` around lines 21 - 25, Update the
misleading comment in the beforeEach block so it no longer claims dynamic import
bypasses the ESM module cache; instead state that import('../testing/index.ts')
returns the cached module and that test isolation is achieved by calling
mockSSE.restore() in afterEach. Reference the beforeEach that assigns mockSSE
from the dynamic import and the mockSSE.restore() used in teardown to make the
intent clear.

src/__tests__/testing-utils-sendSSE.test.ts-61-69 (1)

61-69: ⚠️ Potential issue | 🟡 Minor

reader may be undefined but is dereferenced without a null guard.

response.body?.getReader() returns ReadableStreamDefaultReader | undefined. Calling reader.read() on undefined produces a TypeError with no context rather than a clear assertion failure. Since tests are excluded from tsc checks (per the CI config), TypeScript won't catch this at compile time.

This same pattern appears throughout the file (lines ~82–90, ~103–110, ~174–181, ~230–239).

🛡️ Suggested fix — use non-null assertion or an explicit guard

-      const reader = response.body?.getReader()
+      const reader = response.body!.getReader()

       // Send via sendSSE
       mock.sendSSE({ type: 'test', value: 42 })

-      const { value } = await reader.read()
+      const { value } = await reader.read()

Or, if you prefer an explicit guard that produces a meaningful failure:

       const reader = response.body?.getReader()
+      if (!reader) throw new Error('Expected readable response body')

Apply the same fix to all repeated occurrences.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/testing-utils-sendSSE.test.ts` around lines 61 - 69, The test
dereferences reader from response.body?.getReader() without null checking, which
can yield a TypeError; update the test to assert reader is present before use
(either add a non-null assertion on response.body!.getReader() or an explicit
guard that throws a clear error) and then call reader.read() and
reader.cancel(); apply the same fix for every occurrence of
response.body?.getReader() / reader.read() / reader.cancel() in this test file
(around the other blocks noted).

prd/0001-transport-abstraction.md-53-54 (1)

53-54: ⚠️ Potential issue | 🟡 Minor

"Server-side SSE implementation or server helpers" is listed as Out of Scope, but the PR includes createChannel.

The PR introduces createChannel(schema) for server-side SSE with typed broadcast/respond emitters, which directly contradicts this out-of-scope item. Consider updating the PRD to reflect the current scope, or noting that server-side helpers were moved in-scope during implementation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@prd/0001-transport-abstraction.md` around lines 53 - 54, The PRD lists
"Server-side SSE implementation or server helpers" as out-of-scope but the code
adds createChannel(schema) and typed emitters (broadcast and respond), so update
the PRD to reflect that server-side SSE helpers are now in-scope: either remove
or change the out-of-scope bullet and add a short section that documents
createChannel(schema), the typed broadcast/respond emitters, their intended
server responsibilities, and any constraints/limitations introduced by bringing
server helpers into scope; reference the created symbols createChannel,
broadcast, and respond so readers can find the implementation and any related
design decisions.

src/__tests__/testing-utils-transport.test.ts-146-170 (1)

146-170: ⚠️ Potential issue | 🟡 Minor

Potential null dereference on reader from optional chaining.

response.body?.getReader() yields reader that could be undefined, yet reader.read() (Line 155) and similar calls throughout this file use it without a null guard. If response.body is ever null, this will throw a TypeError at runtime rather than a clear test failure.

Consider either asserting reader is defined before using it, or removing the optional chaining if the mock guarantees body is always present.

       const reader = response.body?.getReader()
+      expect(reader).toBeDefined()
       const decoder = new TextDecoder()

This pattern recurs at lines 176, 209, 224, 240, 256, 274, 285, 329, 365–366.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/testing-utils-transport.test.ts` around lines 146 - 170, The
tests call response.body?.getReader() which can yield undefined and then call
reader.read() (e.g., in the 'should produce SSE wire format data in the
ReadableStream' test after mockSSE('/api/stream')), risking a null dereference;
update each test (locations around reader usage: reader variable after
fetch('/api/stream') and the other occurrences noted) to explicitly assert/throw
if reader is undefined (e.g., check if (!reader) throw new Error('Expected
response.body to be a ReadableStream')) or replace the optional chaining with a
direct .getReader() when the mock guarantees a body, so subsequent calls to
reader.read() are safe and the test fails with a clear message if body is
missing.

prd/0001-transport-abstraction.md-70-72 (1)

70-72: ⚠️ Potential issue | 🟡 Minor

Duplicate requirement number: two items numbered "9."

Lines 70 and 71 both start with 9.. The SSETransport interface requirement on Line 71 should be numbered 10., and the current 10. on Line 72 should become 11..

 9. The fetch-based path shall track the last received `id:` field and send it as the `Last-Event-ID` header on reconnection, matching native `EventSource` behavior.
-9. The library shall export an `SSETransport` interface as an escape hatch...
-10. `UseSSEStreamOptions` and `SSEConfig` shall accept an optional `transport` property...
+10. The library shall export an `SSETransport` interface as an escape hatch...
+11. `UseSSEStreamOptions` and `SSEConfig` shall accept an optional `transport` property...

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@prd/0001-transport-abstraction.md` around lines 70 - 72, Duplicate numbering:
change the second "9." (the requirement that exports an SSETransport interface
with onmessage/onerror/onopen, close(), readyState,
addEventListener/removeEventListener) to "10.", and increment the following
"10." (the UseSSEStreamOptions and SSEConfig transport precedence requirement)
to "11."; update any cross-references to these requirement numbers if present
and ensure the identifiers SSETransport, UseSSEStreamOptions, and SSEConfig
remain unchanged.

src/__tests__/testing-utils-transport.test.ts-110-131 (1)

110-131: ⚠️ Potential issue | 🟡 Minor

Fetch passthrough test is a tautology — it always passes regardless of behavior.

usedRealFetch is set to true in both the try and catch branches, so expect(usedRealFetch).toBe(true) can never fail. This test doesn't actually verify that the mock didn't intercept the call.

A more meaningful check would inspect properties of the response (e.g., verifying it lacks the mock SSE content-type header) or spy on the original fetch to confirm it was called.

♻️ Suggested improvement

     it('should NOT intercept fetch calls to non-mocked URLs', async () => {
       mockSSE('/api/stream')

-      // This URL is not mocked, so it should go through to the real fetch
-      // We expect it to fail or return a real response, not a mock
-      let usedRealFetch = false
-      const _savedFetch = originalFetch
-      // We can detect passthrough by checking if the original fetch was called
-      // Since non-mocked URLs may fail in test env, we catch the error
-      try {
-        const _response = await fetch('https://example.com/not-mocked')
-        // If we get here, real fetch was used (or mock incorrectly intercepted)
-        // Check that the response is NOT a mock SSE response
-        usedRealFetch = true
-      } catch {
-        // Real fetch may throw in test environment (no network) - that's fine,
-        // it means the call was NOT intercepted by our mock
-        usedRealFetch = true
-      }
-
-      expect(usedRealFetch).toBe(true)
+      // Non-mocked URL: should either throw (no network) or return a non-SSE response
+      try {
+        const response = await fetch('https://example.com/not-mocked')
+        // If it resolved, verify it's NOT our mock (no SSE content-type)
+        const ct = response.headers.get('content-type') ?? ''
+        expect(ct).not.toContain('text/event-stream')
+      } catch {
+        // Real fetch threw (no network in test env) — passthrough confirmed
+      }
     })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/testing-utils-transport.test.ts` around lines 110 - 131, The
test "should NOT intercept fetch calls to non-mocked URLs" is tautological
because usedRealFetch is set true in both try and catch; update the test to
actually verify passthrough by spying on the original fetch or asserting
response headers/body don't match the SSE mock: replace the usedRealFetch
boolean with a spy on global.fetch (or jest.spyOn(window, 'fetch')) before
calling fetch('https://example.com/not-mocked'), call mockSSE('/api/stream') as
before, then assert that the spy was invoked (or that the returned Response
lacks the SSE mock content-type/header or SSE body), and finally restore the
spy; reference mockSSE, originalFetch/fetch and usedRealFetch to locate and
replace the tautological logic.

src/__tests__/fetchTransport.test.ts-339-364 (1)

339-364: ⚠️ Potential issue | 🟡 Minor

Timing-dependent test: removeEventListener relies on interleaving between chunk reads.

This test removes the listener after flushAsync(20) and expects the second chunk (processed after flushAsync(50)) to not reach it. The implementation uses setTimeout(readNext, 25) between reads, so the margin is tight — the removal at ~20ms races with the second read scheduled at ~25ms. In slow CI environments, this could become flaky.

Consider increasing the gap or using a deterministic approach (e.g., mock timers) if this test proves unreliable.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/fetchTransport.test.ts` around lines 339 - 364, The test is
flaky because removeEventListener is racing with the scheduled readNext (which
uses setTimeout 25ms); update the test to make the timing deterministic by
either (A) increasing the wait so removal always happens before the second read
(e.g., call flushAsync(10) then flushAsync(40) to ensure >25ms gap) or (B)
switch to mocked timers (jest.useFakeTimers) and advance timers to control when
readNext runs; reference the test's use of createFetchTransport,
transport.removeEventListener, and flushAsync to locate where to adjust timings
or install fake timers so the second chunk cannot race through.

src/__tests__/channel.test.ts-354-376 (1)

354-376: ⚠️ Potential issue | 🟡 Minor

Possible null dereference on reader1 / reader2.

r1.body?.getReader() could return undefined if body is null, making the subsequent reader1.read() (line 360) and reader1.cancel() (line 373) throw at runtime. While in practice body is always present for streaming responses, using a non-null assertion or an explicit guard would make the test more robust and suppress any TypeScript warnings.

Suggested fix

-      const reader1 = r1.body?.getReader()
-      const reader2 = r2.body?.getReader()
+      const reader1 = r1.body!.getReader()
+      const reader2 = r2.body!.getReader()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/channel.test.ts` around lines 354 - 376, The test may
dereference undefined readers from r1.body?.getReader() / r2.body?.getReader();
ensure reader1 and reader2 are non-null before using them by replacing the
optional getReader calls with explicit non-null assertions or guards (e.g.,
assert r1.body and r2.body exist) so subsequent calls to reader1.read(),
reader2.read(), reader1.cancel(), and reader2.cancel() are safe; update the
setup around the TextDecoder/reader1/reader2 variables to either throw a clear
error if body is missing or use the non-null assertion on getReader to satisfy
TypeScript and avoid runtime null dereference.

src/sseParser.ts-64-69 (1)

64-69: ⚠️ Potential issue | 🟡 Minor

Lines without a colon are skipped — deviation from WHATWG SSE spec.

Per the WHATWG Server-Sent Events spec, a line that does not contain a colon should be processed using the entire line as the field name with an empty string value. For example, a bare data line should append an empty string to the data buffer. The current implementation skips such lines entirely (lines 66–68).

This is unlikely to matter in practice since well-formed SSE streams always use field: value or field:value syntax, and the test suite explicitly validates this "skip" behavior. However, the code comment claiming "per spec" is inaccurate—this behavior actually deviates from the spec.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sseParser.ts` around lines 64 - 69, The parser incorrectly skips lines
without a colon; change the colonIdx === -1 branch so that instead of returning
you treat the whole line string as the field name and the value as an empty
string (e.g., field = line, value = ''), then let the existing field-handling
logic proceed (so a bare "data" appends an empty string to the data buffer).
Update the comment that currently says "skip line per spec" to reflect the
WHATWG behavior. Ensure this change is made in the same function where colonIdx
and line are used so other field-specific code (including "data" handling)
remains unchanged.

src/SSEProvider.tsx-432-434 (1)

432-434: ⚠️ Potential issue | 🟡 Minor

Error message is misleading for non-EventSource transports.

The error string 'EventSource connection error' is hardcoded, but the handler now runs for fetch-based and custom transports as well.

Proposed fix

     connection.onerror = (event: Event) => {
       updateStatus({
-        error: new Error('EventSource connection error'),
+        error: new Error('SSE connection error'),
       })

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/SSEProvider.tsx` around lines 432 - 434, The error text in the
connection.onerror handler is hardcoded to "EventSource connection error" which
is misleading for fetch-based or custom transports; update the onerror logic in
the connection.onerror handler (the one that calls updateStatus) to build and
pass a contextual error message instead—e.g., include the transport identifier
(use an available variable like transportName or derive from
connection.constructor.name/type) and any event message/details if present, so
updateStatus receives a descriptive Error that reflects the actual transport and
error info rather than always saying "EventSource connection error".

🧹 Nitpick comments (24)

package.json (1)

16-19: Consider adding a "node" export condition to the ./server subpath.

Tooling that targets Node.js (e.g., webpack with target: 'node', Rollup) resolves the "node" condition before "import". Without it, the server entry still works at runtime but loses the explicit signal that it is a server-only export.

     "./server": {
+      "node": "./dist/server/index.js",
       "import": "./dist/server/index.js",
       "types": "./dist/server/index.d.ts"
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 16 - 19, Add a "node" export condition to the
"./server" subpath in package.json so Node-targeting bundlers resolve the server
entry correctly; update the "./server" export object (the existing "./server"
key) to include a "node" field alongside "import" and "types" that points to the
same server bundle (e.g., dist/server/index.js) so tools that prefer the "node"
condition pick the server-only entry.

CHANGELOG.md (1)

29-31: Non-standard ### Previous section breaks Keep a Changelog convention.

The spec uses version tags or [Unreleased] as section headers; a freeform Previous header makes automated version-bump tooling (e.g., standard-version, release-please) unable to parse the file correctly. Consider structuring the transport-abstraction entries as a separate [Unreleased] sub-section, or fold them into the existing ### Added block.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@CHANGELOG.md` around lines 29 - 31, The CHANGELOG contains a non-standard
"### Previous" header under the Transport Abstraction section which breaks Keep
a Changelog parsing; replace the "### Previous" header with a proper
"[Unreleased]" subsection or merge the Transport Abstraction entries into the
existing "### Added" block so tooling like standard-version or release-please
can parse the changelog correctly (look for the "### Previous" header and the
"Transport Abstraction" entries to update).

tsconfig.json (1)

28-29: LGTM. Excluding src/__tests__ from the main tsc --noEmit pass is a common pattern that avoids fighting test-specific @ts-expect-error suppressions and bun-test globals in the project typecheck. Note that this means TypeScript type errors inside test files won't be caught by bun run typecheck; bun test transpiles but doesn't type-check. If test type-safety matters, a separate tsconfig.test.json with "include": ["src/__tests__/**/*"] and "noEmit": true could be added as an optional CI step.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tsconfig.json` around lines 28 - 29, The tsconfig currently excludes tests
via the "exclude": ["src/__tests__"] entry which prevents test files from being
type-checked; if you want CI to catch test type errors, add a separate test
tsconfig (e.g., create a tsconfig.test.json) that sets "noEmit": true and
"include": ["src/__tests__/**/*"] and run tsc --project tsconfig.test.json (or
add a dedicated CI step) so tests are type-checked without changing the main
tsconfig; keep the existing "exclude" in the main tsconfig if you want to avoid
test-specific suppressions during local/bundled typecheck.

.github/workflows/ci.yml (1)

15-19: Consider caching bun install to speed up CI.

All packages are re-downloaded on every run. Bun supports the cache: true option in setup-bun (or manual caching of ~/.bun/install/cache) to cut install time.

       - uses: oven-sh/setup-bun@v2
         with:
           bun-version: latest
+          cache: true

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/ci.yml around lines 15 - 19, The workflow currently
re-runs bun install every job; update the setup step using the oven-sh/setup-bun
action to enable caching by adding the cache: true input on the setup-bun
invocation (the same step that uses oven-sh/setup-bun@v2) or alternatively add
an actions/cache step to cache ~/.bun/install/cache before running the bun
install step so bun install reuses cached packages between runs.

src/__tests__/tabVisibility.test.tsx (1)

415-427: _timersBefore is assigned but never read — dead code.

The original intent was likely to assert that the timer count changed after the visibility event (e.g., expect(pendingTimers.size).toBeLessThan(_timersBefore)). Either add the assertion or remove the variable entirely.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/tabVisibility.test.tsx` around lines 415 - 427, _timersBefore
is assigned but never used in the test; either remove the dead assignment or
assert that pendingTimers changed after the visibility events. Update the test
around the dispatchVisibilityChange/advanceTimersByTime block to either delete
the line that sets _timersBefore or add an assertion such as checking
pendingTimers.size is lessThan(_timersBefore) (referencing _timersBefore,
pendingTimers, dispatchVisibilityChange, and advanceTimersByTime), keeping the
existing final assertion on MockEventSource.connectionAttempts.

src/__tests__/reconnection.test.tsx (2)

477-483: _timerCountBefore is declared but never read — dead code.

The intent appears to be a before/after comparison, but the assertion uses pendingTimers.size directly without referencing _timerCountBefore. The underscore prefix signals "intentionally unused", but there is no actual comparison being deferred — the variable can be removed.

🧹 Proposed cleanup

-      const _timerCountBefore = pendingTimers.size
       advanceTimersByTime(10000)

       // No reconnect should have occurred
       expect(MockEventSource.connectionAttempts).toBe(1)
       // No pending timers for reconnection
       expect(pendingTimers.size).toBe(0)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/reconnection.test.tsx` around lines 477 - 483, Remove the dead
variable _timerCountBefore: it is declared but never read, so delete its
declaration in the test around the reconnection assertions; leave the subsequent
calls and assertions intact (advanceTimersByTime(10000),
expect(MockEventSource.connectionAttempts).toBe(1),
expect(pendingTimers.size).toBe(0)) and do not add any new comparisons — simply
drop the unused _timerCountBefore identifier.

---

809-819: _initialAttempts is declared but never read — same dead-code pattern.

The subsequent assertion only checks pendingTimers.size; _initialAttempts is never referenced.

🧹 Proposed cleanup

-      const _initialAttempts = MockEventSource.connectionAttempts
-
       // Simulate failure
       MockEventSource.getLastInstance()?.simulateConnectionFailure()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/reconnection.test.tsx` around lines 809 - 819, The variable
_initialAttempts is declared but never used; remove the dead declaration or use
it in an assertion. Edit the test around MockEventSource.connectionAttempts and
either delete the line that declares _initialAttempts or replace the check by
comparing MockEventSource.connectionAttempts (or _initialAttempts) before and
after simulateConnectionFailure to assert a change; refer to the identifiers
_initialAttempts, MockEventSource.getLastInstance(), simulateConnectionFailure,
and pendingTimers.size to locate the relevant lines.

src/reconnect.ts (1)

6-12: maxAttempts: Number.POSITIVE_INFINITY means indefinite reconnection by default.

A client connecting to a permanently unreachable endpoint will retry forever, potentially causing high load on the server (or on a load balancer that's logging every failed attempt). Most SSE/WebSocket libraries default to a finite ceiling (e.g. 10–15 attempts) and leave infinite retry as an opt-in.

Consider either defaulting to a bounded value (e.g. maxAttempts: 15) or prominently documenting this behaviour in the JSDoc so integrators understand they must set a limit themselves.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/reconnect.ts` around lines 6 - 12, The default reconnect configuration
currently sets DEFAULT_RECONNECT.maxAttempts to Number.POSITIVE_INFINITY which
causes indefinite retries; change the default to a bounded number (e.g. 15) and
update the ReconnectConfig JSDoc to call out that infinite retries must be opted
into explicitly, referencing DEFAULT_RECONNECT and the maxAttempts field so
integrators can find and override it; ensure tests and any places that construct
reconnect configs still behave when maxAttempts is finite.

src/schema.ts (1)

21-31: Object.freeze is shallow — nested event configs remain mutable.

Object.freeze(result) locks the top-level keys of the returned schema, but each per-event value object { ...def, update: ... } is freshly allocated and not frozen. This contradicts the JSDoc's "frozen schema" guarantee:

const schema = defineSchema({ 'user.updated': { key: '/api/users' } })
schema['user.updated'] = {}       // ✅ throws (frozen top-level)
schema['user.updated'].key = 'x' // ❌ silent mutation (unfrozen value)

The Readonly<{…}> in SchemaResult<T> only provides a TypeScript-level top-level constraint — it mirrors the same gap.

🔒 Proposed fix — freeze each event definition too

  for (const eventName of Object.keys(definition)) {
    const def = definition[eventName]
-   result[eventName] = {
+   result[eventName] = Object.freeze({
      ...def,
      update: def?.update ?? 'set',
-   }
+   })
  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/schema.ts` around lines 21 - 31, The returned schema only freezes the
top-level object (result) but not each per-event value, so mutateable event
configs remain; inside the loop that iterates over Object.keys(definition)
create the per-event value (currently assigned from def with update merged) and
call Object.freeze on that value before assigning to result[eventName], then
return Object.freeze(result) as SchemaResult<T>; reference the identifiers
result, definition, eventName, def and the type SchemaResult<T> when
implementing this change (optionally apply a recursive freeze if you need nested
immutability).

src/__tests__/build-pipeline.test.ts (1)

119-131: Build runs at module scope — intentional but worth noting.

execSync('bun run build') executes during test file import, meaning every test run that includes this file triggers a full build. This is fine for CI but may slow local bun test if developers don't filter it out. Consider documenting this in the test's doc comment (or the test filename convention, e.g., build-pipeline.integration.test.ts), so it's easy to exclude in local test scripts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/build-pipeline.test.ts` around lines 119 - 131, The test
currently runs a full build at module import via execSync('bun run build') which
slows unfiltered local test runs; update the test to document this behavior by
adding a top-of-file comment (or renaming the file to an integration-style name)
that explains the intentional module-scope build and advises developers to
filter this test during local runs, and ensure the comment references the
execSync invocation and the buildOutput/buildError variables so readers can
quickly locate the build step.

src/fetchTransport.ts (1)

15-21: isPlainObject won't detect cross-realm objects or Object.create(null).

Object.getPrototypeOf(value) === Object.prototype returns false for objects created with Object.create(null) (no prototype) and for objects from other realms (e.g., iframes). This is acceptable for the typical use case (user passes { key: value } literals), but worth noting if cross-realm scenarios arise.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/fetchTransport.ts` around lines 15 - 21, The isPlainObject helper
(isPlainObject) fails for Object.create(null) and cross-realm objects because it
only tests Object.getPrototypeOf(value) === Object.prototype; update it to treat
objects with a null prototype as plain and to accept cross-realm plain objects
by using a robust check such as verifying typeof value === 'object' && value !==
null and then either checking Object.getPrototypeOf(value) === Object.prototype
|| Object.getPrototypeOf(value) === null, or using
Object.prototype.toString.call(value) === '[object Object]' to correctly detect
plain objects across realms and for prototype-less objects; change the
implementation inside isPlainObject accordingly.

src/types.ts (2)

99-112: method/body/headers co-exist with transport on SSEConfigBase — clarify precedence.

Lines 108–111 add method, body, and headers directly to SSEConfigBase, while line 111 adds transport?: (url: string) => SSETransport. When a user provides a custom transport factory, it's unclear whether method/body/headers are still consumed (e.g., forwarded to the factory) or silently ignored. A brief doc comment on the interaction would prevent misuse — e.g., "When transport is provided, method/body/headers are ignored; pass them directly to your transport factory."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/types.ts` around lines 99 - 112, The SSEConfigBase type currently
includes method, body, and headers alongside a transport factory but lacks
guidance on precedence; update the SSEConfigBase declaration (symbols:
SSEConfigBase, method, body, headers, transport) to document the interaction and
expected behavior—specifically state whether method/body/headers are forwarded
to the transport factory or ignored when transport is provided, and implement a
clear rule in the comment (e.g., "when transport is provided,
method/body/headers are ignored; pass any needed values into your transport
factory") so callers know how to supply request details and maintain consistent
behavior.

---

164-189: SchemaEventDefinition duplicates EventMapping — consider unifying.

SchemaEventDefinition (lines 169–174) has the same fields as EventMapping (lines 47–52): key, update, filter, transform. If the intent is for these to always stay in sync, one could extend the other or share a common base. If they are intentionally decoupled (to allow divergence later), a brief comment noting that would help.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/types.ts` around lines 164 - 189, SchemaEventDefinition duplicates
EventMapping fields; unify them by having SchemaEventDefinition extend
EventMapping (or create a shared base interface used by both) so the field
definitions (key, update, filter, transform) are single-source-of-truth; update
the type signature of SchemaEventDefinition to reference EventMapping (or the
new base) and remove the duplicated property declarations, or if intentional
keep a brief comment above SchemaEventDefinition explaining the deliberate
divergence.

src/server/index.ts (3)

187-225: request parameter is unused — consider leveraging it for AbortSignal.

Currently connectWeb voids the request parameter. For long-lived SSE streams, the client may abort the request. Wiring request.signal to stream cancellation would improve server-side lifecycle management, especially in frameworks that use AbortSignal to signal client disconnects (e.g., Next.js, Remix).

This is not a bug today but worth considering as a future improvement.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/index.ts` around lines 187 - 225, connectWeb currently ignores the
incoming Request; wire request.signal to the stream lifecycle so client
disconnects abort cleanup. Inside connectWeb (function name: connectWeb) use
request.signal to listen for 'abort' (or check signal.aborted) and on abort mark
the WebClient (clientRef) closed, remove it from broadcastPool, and
stopHeartbeat when pool empty—mirror the logic in the stream.cancel() handler;
also remove the abort listener when the stream is cancelled or finished to avoid
leaks. Ensure you reference clientRef, broadcastPool, stopHeartbeat, and the
ReadableStream cancel logic when implementing the cleanup.

---

169-178: Mutating broadcastPool during iteration — inconsistent with emit().

Line 174 deletes from broadcastPool inside a for...of loop over the same Set. While this is technically safe per the ES specification (deleted entries won't be re-visited), it's inconsistent with the approach in emit() (lines 351–360), which properly collects dead clients into a separate array first and then deletes them. The heartbeat callback should use the same pattern for consistency and to avoid confusing future maintainers.

♻️ Suggested: collect dead clients before deleting

     heartbeatTimer = _setInterval(() => {
+      const dead: Client[] = []
       for (const client of broadcastPool) {
         const ok = writeToClient(client, HEARTBEAT_COMMENT)
-        if (!ok) broadcastPool.delete(client)
+        if (!ok) dead.push(client)
+      }
+      for (const client of dead) {
+        broadcastPool.delete(client)
       }
       if (broadcastPool.size === 0) stopHeartbeat()
     }, heartbeatMs)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/index.ts` around lines 169 - 178, The heartbeat loop in
startHeartbeat mutates broadcastPool while iterating (using for...of) which is
inconsistent with emit(); change startHeartbeat to first collect dead clients
into a temporary array (e.g., deadClients) by calling writeToClient(client,
HEARTBEAT_COMMENT) for each client, then after the iteration delete each dead
client from broadcastPool and call stopHeartbeat if size is zero; update
references to heartbeatTimer, _setInterval, writeToClient, HEARTBEAT_COMMENT,
stopHeartbeat and heartbeatMs accordingly to match the emit() pattern.

---

325-378: as Channel assertion masks type-safety for overloaded respond().

The as Channel cast at line 378 is necessary because TypeScript can't verify method overload signatures on object literals. This is a known TS limitation, but it means the compiler won't catch mismatches between the Channel interface overloads and the actual implementation signatures. Consider adding a brief comment noting why the assertion is needed, so future maintainers don't inadvertently weaken the contract.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/server/index.ts` around lines 325 - 378, Add a short explanatory comment
immediately above the return that casts the object to Channel explaining that
the "as Channel" assertion is required due to TypeScript's inability to verify
overloaded method signatures on object literals (specifically for connect() and
respond() overloads), so maintainers understand this is intentional and should
not be removed; reference the overloaded methods connect, respond and the
Channel interface in the comment so it's clear why the cast is present.

src/testing/index.ts (2)

165-185: Patched Request constructor doesn't preserve prototype chain.

The replacement function MockRequest(...) is assigned to globalThis.Request. Code that checks Request.prototype or uses Request as a type discriminator may break. However, since the function returns a real OriginalRequest instance, instanceof OriginalRequest checks will pass correctly. This is acceptable for a test-only utility, but worth a brief comment noting the prototype limitation.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/testing/index.ts` around lines 165 - 185, The MockRequest replacement
assigned to globalThis.Request (function MockRequest) does not preserve the
Request prototype chain even though it returns an OriginalRequest instance; add
a brief inline comment next to the MockRequest assignment (referencing
MockRequest, globalThis.Request, OriginalRequest and registeredUrls) explaining
that this is intentional for tests, that prototype checks on Request.prototype
may not hold, and that instanceof OriginalRequest will still work—this documents
the limitation for future readers.

---

358-374: sendSSE and sendRaw only target fetch streams — document this asymmetry.

sendRaw() and sendSSE() only send to fetch-based streams (not to the EventSource mock), while sendEvent() sends to both. This is by design, but a brief JSDoc on each method would prevent confusion for test authors who might expect sendSSE to also reach EventSource listeners.

Suggested doc comments

+    /** Send raw SSE wire-format text to fetch-based streams only. No-op for EventSource mocks. */
     sendRaw(text: string): void {
       if (mockRegistry.isRestored()) return
       mockRegistry.sendRawToFetchStreams(url, text)
     },

+    /** Send a `data:` SSE message (JSON-encoded) to fetch-based streams only. No-op for EventSource mocks. */
     sendSSE(data: unknown): void {
       if (mockRegistry.isRestored()) return
       mockRegistry.sendRawToFetchStreams(url, formatSSEData(data))
     },

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/testing/index.ts` around lines 358 - 374, Add brief JSDoc comments to the
three exported methods to document their dispatch targets: for sendEvent(event:
SSEEventData) note it forwards to both the EventSource mock (via
instance?._dispatchMessage) and fetch streams
(mockRegistry.sendEventToFetchStreams), for sendRaw(text: string) and
sendSSE(data: unknown) note they only forward to fetch-based streams
(mockRegistry.sendRawToFetchStreams) and do not reach EventSource listeners;
include references to formatSSEData in the sendSSE doc so callers know the data
is formatted before sending.

src/sseParser.ts (1)

18-26: formatSSEEvent and formatSSEData assume single-line JSON output.

Both functions produce data: ${JSON.stringify(payload)}\n\n. If JSON.stringify ever produces multi-line output (e.g., via a custom replacer with indentation, or a toJSON() returning a string with literal newlines), the SSE wire format would be malformed because each line of data needs its own data: prefix.

In practice, default JSON.stringify always produces single-line output, so this is safe for typical usage. Worth a brief doc comment noting the single-line assumption.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/sseParser.ts` around lines 18 - 26, formatSSEEvent and formatSSEData
assume JSON.stringify returns a single-line string; instead handle possible
multi-line payloads by stringifying the payload, splitting the result on newline
characters, and emitting each line with its own "data: " prefix (for
formatSSEEvent emit the single "event: <type>" header once before the data
lines); update the implementations of formatSSEEvent and formatSSEData to
perform this splitting and prefixing so multi-line toJSON/replacer outputs
remain valid SSE, and add a short doc comment noting that both functions now
normalize multi-line JSON by prefixing each line with "data:".

src/__tests__/useSSEStream-transport.test.ts (2)

27-81: Significant duplication of MockEventSource across test files.

This MockEventSource class is nearly identical to the one in SSEProvider-transport.test.tsx. Consider extracting a shared mock into a test utility module to reduce maintenance burden.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/useSSEStream-transport.test.ts` around lines 27 - 81, The
MockEventSource class is duplicated across tests; extract it into a shared test
utility (e.g., export class MockEventSource with constructor, simulateMessage,
simulateError, reset, CONNECTING/OPEN/CLOSED and instances static) and replace
the local class in useSSEStream-transport.test.ts and
SSEProvider-transport.test.tsx with an import from that utility; update tests to
import the same MockEventSource symbol and use its reset() in
beforeEach/afterEach and rely on its simulateMessage/simulateError methods
rather than redefining the class.

---

596-652: Cleanup tests verify .close() works but not that the hook's cleanup effect invokes it.

Since renderToString doesn't run useEffect, these tests only confirm the transport objects are closeable — they don't assert that useSSEStream's unmount cleanup actually calls close(). This is a known limitation of SSR-based testing. Consider adding a client-side render test (e.g., with @testing-library/react) for at least one transport type to cover the full cleanup path.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/useSSEStream-transport.test.ts` around lines 596 - 652, The
tests currently use renderToString which doesn't run useEffect, so they only
verify transports are closeable and not that useSSEStream's cleanup calls close;
add a client-side test that mounts a component using useSSEStream (e.g.,
StreamConsumer) with a real/mocked transport (MockEventSource.instances[0],
mockFetchTransports[0], or createMockTransport()) via `@testing-library/react`'s
render, then call unmount() and assert the transport.close() was invoked and
transport.readyState becomes 2 to cover the hook's cleanup path invoked by
React's effect unmounting.

src/hooks/useSSEStream.ts (1)

109-136: Production code contains test-infrastructure awareness (getMockInstances, isStale).

getMockInstances reads (globalThis.EventSource as { instances?: unknown[] }).instances, a shape that only exists in tests. isStale uses it to detect test-framework resets. This couples production logic to test internals and introduces an unnecessary branch in production.

Consider extracting this staleness check behind a pluggable hook (e.g., __unstable_isStale) or limiting it to a test-only build.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useSSEStream.ts` around lines 109 - 136, The production staleness
check currently embeds test-only logic via getMockInstances and its use inside
isStale; extract that test-framework awareness into a pluggable, optional hook
(e.g., __unstable_isStale) or gate it behind a test-only build flag so
production never reads globalThis.EventSource.instances. Concretely: remove
calls to getMockInstances from isStale and instead call an optional exported
function or global __unstable_isStale(entry) if present; implement
getMockInstances only in tests or provide a test-only module that assigns
__unstable_isStale for test runs so production code only relies on the standard
readyState check and a single optional hook call (symbols: getMockInstances,
isStale, __unstable_isStale).

src/__tests__/SSEProvider-transport.test.tsx (2)

265-314: Fake timer implementation has a subtle issue: fakeSetTimeout return type.

fakeSetTimeout returns a number, but globalThis.setTimeout returns ReturnType<typeof setTimeout> which is Timeout in Node/bun (not a plain number). This could cause type mismatches or runtime issues if the returned ID is passed to the real clearTimeout elsewhere. Since the fake clearTimeout accepts number, this is internally consistent — but watch for cross-boundary calls.

Also, advanceTimersByTime doesn't handle the case where a fired timer's callback schedules a new timer with a delay that also falls within the remaining time — actually, looking again, it does handle this via the while(true) loop that re-scans pendingTimers. This is correct.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/SSEProvider-transport.test.tsx` around lines 265 - 314, The
fake timer API uses number IDs but real environments return ReturnType<typeof
setTimeout> (e.g., Timeout objects); update fakeSetTimeout, fakeClearTimeout,
and the pendingTimers Map key/type to use ReturnType<typeof setTimeout> so types
align with globalThis.setTimeout/clearTimeout, and ensure the ID you generate
and return from fakeSetTimeout matches that type (e.g., create a unique
object/value compatible with ReturnType<typeof setTimeout>); also update any
places that iterate/delete timers to use that ID type (functions:
fakeSetTimeout, fakeClearTimeout, pendingTimers, advanceTimersByTime,
resetTimers).

---

36-128: MockEventSource is duplicated across test files.

This class is nearly identical to the one in useSSEStream-transport.test.ts (and possibly other test files). The SSEProvider-transport version is more fully featured (with simulateNamedEvent, getRegisteredEventTypes, connectionAttempts tracking). Consider consolidating into a shared test utility.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/SSEProvider-transport.test.tsx` around lines 36 - 128,
MockEventSource is duplicated across tests; extract the richer implementation
(the class with simulateNamedEvent, getRegisteredEventTypes, static
connectionAttempts, reset, getLastInstance, etc.) into a shared test utility
module and import it from both SSEProvider-transport.test.tsx and
useSSEStream-transport.test.ts (and any other tests) instead of redefining it;
update the tests to remove the local class, import MockEventSource from the new
shared file, and ensure any test fixtures still call MockEventSource.reset() and
reference MockEventSource.getLastInstance() or .connectionAttempts as before.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

All fetch-transport-selection tests fail in CI — globalThis.fetch mock is ineffective.

The pipeline reports fetchCalls.length is 0 at lines 486, 509, 534, 561, and 589. The comment on lines 27-31 explains why mock.module is avoided, but the fallback strategy — overwriting globalThis.fetch — doesn't work if createFetchTransport captures its fetch reference at module-load time (e.g., const _fetch = globalThis.fetch or via a bundled closure).

Two options to fix:

1. Use config.transport to wrap and observe fetch calls — supply a transport factory that internally calls fetch (which you control), avoiding the module-level capture issue entirely.
2. Accept mock.module in a separate test worker — bun supports per-file test isolation via --preload or separate test entry points to avoid poisoning the module cache for other files.

#!/bin/bash
# Verify whether createFetchTransport captures fetch at module scope
rg -n 'fetch' src/fetchTransport.ts | head -30

🧰 Tools

🪛 GitHub Actions: CI

[error] 486-486: Test assertion failed in SSEProvider-transport.test.tsx: expected fetchCalls.length to be 1 but was 0.

🪛 GitHub Check: test

[failure] 589-589: error: expect(received).toBe(expected)
Expected: 1
Received: 0

  at <anonymous> (/home/runner/work/reactiveSWR/reactiveSWR/src/__tests__/SSEProvider-transport.test.tsx:589:33)

---

[failure] 561-561: error: expect(received).toBe(expected)
Expected: 1
Received: 0

  at <anonymous> (/home/runner/work/reactiveSWR/reactiveSWR/src/__tests__/SSEProvider-transport.test.tsx:561:33)

---

[failure] 534-534: error: expect(received).toBe(expected)
Expected: 1
Received: 0

  at <anonymous> (/home/runner/work/reactiveSWR/reactiveSWR/src/__tests__/SSEProvider-transport.test.tsx:534:33)

---

[failure] 509-509: error: expect(received).toBe(expected)
Expected: 1
Received: 0

  at <anonymous> (/home/runner/work/reactiveSWR/reactiveSWR/src/__tests__/SSEProvider-transport.test.tsx:509:33)

---

[failure] 486-486: error: expect(received).toBe(expected)
Expected: 1
Received: 0

  at <anonymous> (/home/runner/work/reactiveSWR/reactiveSWR/src/__tests__/SSEProvider-transport.test.tsx:486:33)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/SSEProvider-transport.test.tsx` around lines 461 - 593, Tests
are failing because the module under test likely captures global fetch at
import-time (e.g., inside createFetchTransport), so overwriting globalThis.fetch
in tests doesn't trigger fetchCalls; fix by injecting a custom transport via the
SSEProvider config.transport in these tests: create a transport factory that
calls the real/bundled fetch and forwards/records the url/init into the existing
fetchCalls array, then pass it as config.transport when rendering SSEProvider
(instead of relying on globalThis.fetch). Locate references to
createFetchTransport and SSEProvider in the tests and replace those cases with a
config.transport that wraps fetch and pushes calls to fetchCalls so the tests
observe network usage regardless of module-level fetch capture.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/__tests__/SSEProvider-transport.test.tsx (1)

1239-1325: Cleanup tests only verify method existence, not actual cleanup behavior.

All three tests in this describe block use renderToString, which doesn't execute useEffect cleanup. The assertions confirm close and removeEventListener exist as functions, but never verify they're called on unmount. The inline comments (lines 1267-1268) acknowledge this, but the tests provide a false sense of coverage for unmount behavior.

Consider adding a companion test file (or a section in this file) that uses a DOM-based renderer (e.g., @testing-library/react with act() and unmount()) to actually exercise the cleanup path. This would ensure the useEffect teardown calls close() and removeEventListener as expected.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/SSEProvider-transport.test.tsx` around lines 1239 - 1325, The
tests under the "cleanup on unmount" describe block only call renderToString
(which doesn't run useEffect cleanup) and therefore only assert that
SSEProvider's transport (created via transport callback / createMockTransport)
exposes close and removeEventListener, not that SSEProvider actually calls them
on unmount; add a new DOM-based test (using `@testing-library/react` or
react-dom's render/act) that mounts SSEProvider with the same config (transport
-> createMockTransport or a spy/mock transport), then unmount it (or call act(()
=> unmount())) and assert the mock transport.close() and
transport.removeEventListener(...) were actually called; refer to SSEProvider,
createMockTransport, close, removeEventListener, renderToString and ensure the
new test uses unmount/act rather than renderToString so the useEffect teardown
runs.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/__tests__/SSEProvider-transport.test.tsx`:
- Around line 926-960: The test relies on capturedStatus (set inside
StatusCapture which calls useSSEContext) being a mutable object that updates
in-place after transport.simulateOpen; that's brittle if SSEProvider switches to
immutable state. Fix by changing the test to use a DOM render that supports
reactivity (e.g., replace renderToString with a renderer from
testing-library/react and wrap updates in act), then read/capture status after
calling transport.simulateOpen (or re-render/flush effects) so assertions
inspect the up-to-date value from useSSEContext; reference StatusCapture,
capturedStatus, useSSEContext, SSEProvider, transport.simulateOpen and
createMockTransport to locate the relevant code.
- Around line 678-702: The test currently only asserts renderToString doesn't
throw but never verifies onEventError was invoked; update the test to assert
that errorsCaught was populated (e.g., expect(errorsCaught).toHaveLength(1) and
that errorsCaught[0].error.message includes 'Transport factory failed') after
rendering the SSEProvider; if the transport factory is invoked inside a
useEffect (and thus not run during SSR), change the test to perform a DOM render
(use ReactDOM.render or `@testing-library/react`'s render within act and await any
microtasks) and then assert errorsCaught was called, ensuring you reference the
existing errorsCaught array, the SSEProvider component, the transport factory in
SSEConfig, and the onEventError handler when making the assertion.

---

Duplicate comments:
In `@src/__tests__/SSEProvider-transport.test.tsx`:
- Around line 474-607: Tests time out because createFetchTransport reads fetch
at call time, so overwriting globalThis.fetch in beforeEach doesn't affect it;
update each failing test to supply config.transport (instead of relying on
globalThis.fetch) with a small wrapper function that calls the real fetch and
pushes a record into fetchCalls (i.e., a transport that calls fetch(url, init),
records {url, init} into fetchCalls, and returns a stream compatible with
createFetchTransport expectations), then pass that config to SSEProvider in the
renderToString calls so SSEProvider selects createFetchTransport but your
wrapper captures the actual calls; apply this pattern to all five tests
(reference: createFetchTransport, SSEProvider, config.transport, fetchCalls,
globalThis.fetch).

---

Nitpick comments:
In `@src/__tests__/SSEProvider-transport.test.tsx`:
- Around line 1239-1325: The tests under the "cleanup on unmount" describe block
only call renderToString (which doesn't run useEffect cleanup) and therefore
only assert that SSEProvider's transport (created via transport callback /
createMockTransport) exposes close and removeEventListener, not that SSEProvider
actually calls them on unmount; add a new DOM-based test (using
`@testing-library/react` or react-dom's render/act) that mounts SSEProvider with
the same config (transport -> createMockTransport or a spy/mock transport), then
unmount it (or call act(() => unmount())) and assert the mock transport.close()
and transport.removeEventListener(...) were actually called; refer to
SSEProvider, createMockTransport, close, removeEventListener, renderToString and
ensure the new test uses unmount/act rather than renderToString so the useEffect
teardown runs.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/SSEProvider.tsx (1)

612-645: ⚠️ Potential issue | 🟠 Major

Stale closure in unmount cleanup — current connection leaks after reconnection.

The cleanup effect captures eventSourceRef.current at mount time (line 614). If reconnection creates a new transport, the cleanup on unmount will close the initial (already-closed) connection, not the current active one. The active connection and its listeners leak.

Fix: read from refs inside the cleanup function

   useEffect(() => {
-    const connection = eventSourceRef.current
-    const listeners = listenersRef.current
-
     return () => {
       // Clear any pending reconnect timeout
       if (reconnectTimeoutRef.current) {
         clearTimeout(reconnectTimeoutRef.current)
         reconnectTimeoutRef.current = null
       }

+      const connection = eventSourceRef.current
+      const listeners = listenersRef.current
+
       if (connection) {
         // Check if already closed (onerror may have already called onDisconnect)
         const wasConnected = connection.readyState !== CLOSED

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/SSEProvider.tsx` around lines 612 - 645, The cleanup useEffect currently
captures eventSourceRef.current and listenersRef.current at mount time, causing
stale closures that can fail to close the active EventSource after reconnection;
update the cleanup to read eventSourceRef.current, listenersRef.current, and
reconnectTimeoutRef.current inside the returned cleanup function (rather than
using the local consts created at mount) so it always operates on the latest
connection and listeners; ensure you still clear reconnectTimeoutRef, remove
each listener via connection.removeEventListener(type, handler), set
listenersRef.current = [] and eventSourceRef.current = null, and only call
configRef.current.onDisconnect if the actual connection's readyState indicates
it was connected (use the CLOSED constant as before).

🧹 Nitpick comments (4)

src/testing/index.ts (2)

168-185: Request constructor mock: consider documenting the relative-URL limitation.

The patched Request constructor only handles registered mock URLs (relative strings). For any non-registered relative URL, it falls through to the original Request constructor, which may throw in some environments. This is fine for its purpose, but a brief inline comment noting this is intentional would help future maintainers.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/testing/index.ts` around lines 168 - 185, Add a brief inline comment
above the patched Request constructor (the MockRequest replacement that uses
OriginalRequest and registeredUrls) stating that it intentionally only rewrites
URLs for registered relative paths and that non-registered relative URLs are
left to the original Request (which may throw in some environments); reference
MockRequest, OriginalRequest and registeredUrls in the comment so future
maintainers understand the limitation is intentional and not a bug.

---

188-204: originalFetch may be undefined — the optional-chain result is silently cast to Promise<Response>.

If install() is called in an environment where globalThis.fetch is not defined (e.g., older Node without a fetch polyfill), self.originalFetch will be undefined. The optional chain self.originalFetch?.(input, init) returns undefined, which is then cast to Promise<Response> — the caller will get undefined instead of a proper Response or a clear error.

This is a minor edge case since test environments typically have fetch, but it would be confusing to debug.

Suggested defensive check

       if (self.registeredUrls.has(url)) {
         return self.createMockFetchResponse(url)
       }

-      return self.originalFetch?.(input, init) as Promise<Response>
+      if (!self.originalFetch) {
+        return Promise.reject(new Error(`No original fetch available for non-mocked URL: ${url}`))
+      }
+      return self.originalFetch(input, init)
     } as typeof globalThis.fetch

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/testing/index.ts` around lines 188 - 204, The mockFetch override
currently casts self.originalFetch?.(input, init) to Promise<Response> which can
produce undefined if self.originalFetch is missing; update the mock in the
globalThis.fetch override (inside install()) to check whether self.originalFetch
is defined before calling it and if not return a rejected Promise or throw a
clear Error (including context like the requested url) instead of casting
undefined; keep the existing behavior of returning
self.createMockFetchResponse(url) when registeredUrls.has(url) and only fall
back to calling self.originalFetch(input, init) when it exists.

src/__tests__/testing-utils-transport.test.ts (1)

110-131: Unused variable _savedFetch.

Line 116 declares const _savedFetch = originalFetch but it's never used. The underscore prefix suggests it was intentionally kept as dead code, but it can be removed for clarity.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/__tests__/testing-utils-transport.test.ts` around lines 110 - 131, Remove
the unused variable declaration const _savedFetch = originalFetch in the test
"should NOT intercept fetch calls to non-mocked URLs"—it's never referenced;
simply delete that line so only originalFetch (if needed elsewhere) remains and
the test uses fetch directly; ensure no other tests rely on _savedFetch and run
tests to confirm nothing breaks.

src/SSEProvider.tsx (1)

537-550: Synchronous connection creation during render — intentional for SSR but has implications.

Creating a connection synchronously during render is needed for SSR (where useEffect doesn't run). However, note that if config.events changes without a URL change, the named event listeners registered in createConnection (lines 508–534) won't be updated. This is likely acceptable since event schemas are typically static, but worth documenting.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/SSEProvider.tsx` around lines 537 - 550, Rendering currently creates the
connection synchronously (using createConnection when eventSourceRef is null or
urlChanged), but changes to config.events won't update named event listeners;
update the logic so config.events changes also trigger listener reconciliation —
either treat a change in resolvedConfig.events like urlChanged (recreate
connection) or add a useEffect that watches resolvedConfig.events and calls
createConnection or a new updateListeners helper to re-register named listeners
on the existing eventSourceRef; reference createConnection, eventSourceRef,
currentUrlRef, resolvedConfig.url and resolvedConfig.events when making the
change.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Line 567: The MD038 lint warning is caused by nested/backticked code inside
the inline code span; update the README line to avoid nested backticks by either
using a fenced code block or rephrasing the sentence so code spans don’t contain
escaped backticks—for example, present sendSSE(data) and the sendRaw call in a
fenced code block or change the inner template literal representation (used in
the sendRaw example) to a plain text description; locate the sentence
referencing sendSSE, sendRaw(`data: ${JSON.stringify(data)}\n\n`) and
createSSEParser and replace the inline nested-backtick usage with one of those
alternatives.

---

Outside diff comments:
In `@src/SSEProvider.tsx`:
- Around line 612-645: The cleanup useEffect currently captures
eventSourceRef.current and listenersRef.current at mount time, causing stale
closures that can fail to close the active EventSource after reconnection;
update the cleanup to read eventSourceRef.current, listenersRef.current, and
reconnectTimeoutRef.current inside the returned cleanup function (rather than
using the local consts created at mount) so it always operates on the latest
connection and listeners; ensure you still clear reconnectTimeoutRef, remove
each listener via connection.removeEventListener(type, handler), set
listenersRef.current = [] and eventSourceRef.current = null, and only call
configRef.current.onDisconnect if the actual connection's readyState indicates
it was connected (use the CLOSED constant as before).

---

Duplicate comments:
In `@README.md`:
- Around line 183-198: The examples already demonstrate both runtime signatures
for channel.respond: Node.js using channel.respond(req, res) and Web/edge using
const { response, emitter } = channel.respond(request); no code change is
needed—keep these two snippets as-is and ensure the symbols channel.respond,
emitter, response and POST remain consistent with your runtime API surface and
types.

In `@src/__tests__/SSEProvider-transport.test.tsx`:
- Around line 889-923: Remove the stray "[duplicate_comment]" marker in the test
and replace it with a concise in-code comment explaining that the test
intentionally relies on the mutable status ref behavior from SSEProvider
(referencing capturedStatus, StatusCapture, SSEProvider, and the transport
created via createMockTransport) so future readers won't remove or refactor the
pattern; ensure no other duplicate review markers remain in this test file.
- Around line 454-562: Tests previously asserted on fetchCalls which is
unreliable because createFetchTransport captures fetch at module load; update
tests to instead assert that no EventSource was created by checking
MockEventSource.instances.length === 0 to confirm the fetch transport was
selected. Locate the tests referencing SSEProvider and renderToString in the
describe block (tests that construct SSEConfig with method/body/headers) and
remove or replace any assertions on fetchCalls with assertions against
MockEventSource.instances, ensuring createFetchTransport and MockEventSource are
the referenced symbols used to verify transport selection.
- Around line 634-665: The test must ensure transport-factory errors are
reported via the provider's onEventError callback: update the SSEProvider render
test that calls renderToString(createElement(SSEProvider, { config }, ...)) so
the custom transport function (transport) that throws is invoked synchronously
and the onEventError handler captures the error; after render, assert
errorsCaught has at least one entry and that errorsCaught[0].error is an Error
with message "Transport factory failed" to verify createConnection/transport
error propagation.

In `@src/SSEProvider.tsx`:
- Around line 395-422: When createTransport throws, catch the error, call
configRef.current.onEventError with a transport_error event and the error,
install a no-op closed transport object into eventSourceRef.current
(implementing SSETransport shape and readyState = CLOSED) to prevent re-entry,
set currentUrlRef.current = url, and call updateStatus({ connected: false,
connecting: false, error: error instanceof Error ? error : new
Error(String(error))}); locate the handling around createTransport,
eventSourceRef, configRef, currentUrlRef, and updateStatus to apply this change.

In `@src/testing/index.ts`:
- Around line 290-303: No change required: sendEventToFetchStreams correctly
uses formatSSEEvent(event.type, event.payload) to produce the SSE wire format
with an "event:" field and enqueues the encoded chunk into each open
entry.controller for entries from this.fetchStreams.get(url); keep the early
return on missing entries and the closed check on entry.closed as-is.

---

Nitpick comments:
In `@src/__tests__/testing-utils-transport.test.ts`:
- Around line 110-131: Remove the unused variable declaration const _savedFetch
= originalFetch in the test "should NOT intercept fetch calls to non-mocked
URLs"—it's never referenced; simply delete that line so only originalFetch (if
needed elsewhere) remains and the test uses fetch directly; ensure no other
tests rely on _savedFetch and run tests to confirm nothing breaks.

In `@src/SSEProvider.tsx`:
- Around line 537-550: Rendering currently creates the connection synchronously
(using createConnection when eventSourceRef is null or urlChanged), but changes
to config.events won't update named event listeners; update the logic so
config.events changes also trigger listener reconciliation — either treat a
change in resolvedConfig.events like urlChanged (recreate connection) or add a
useEffect that watches resolvedConfig.events and calls createConnection or a new
updateListeners helper to re-register named listeners on the existing
eventSourceRef; reference createConnection, eventSourceRef, currentUrlRef,
resolvedConfig.url and resolvedConfig.events when making the change.

In `@src/testing/index.ts`:
- Around line 168-185: Add a brief inline comment above the patched Request
constructor (the MockRequest replacement that uses OriginalRequest and
registeredUrls) stating that it intentionally only rewrites URLs for registered
relative paths and that non-registered relative URLs are left to the original
Request (which may throw in some environments); reference MockRequest,
OriginalRequest and registeredUrls in the comment so future maintainers
understand the limitation is intentional and not a bug.
- Around line 188-204: The mockFetch override currently casts
self.originalFetch?.(input, init) to Promise<Response> which can produce
undefined if self.originalFetch is missing; update the mock in the
globalThis.fetch override (inside install()) to check whether self.originalFetch
is defined before calling it and if not return a rejected Promise or throw a
clear Error (including context like the requested url) instead of casting
undefined; keep the existing behavior of returning
self.createMockFetchResponse(url) when registeredUrls.has(url) and only fall
back to calling self.originalFetch(input, init) when it exists.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@README.md`:
- Around line 222-235: Change the code fence language from "typescript" to "tsx"
for the block that declares the config: SSEConfig and renders <SSEProvider
config={config}><App /></SSEProvider>, since the snippet contains JSX
(SSEProvider and <App />); update the opening fence to ```tsx so syntax
highlighters and linters correctly parse the JSX.
- Line 559: Update the README API table comment for mock.sendSSE to accurately
describe its behavior: state that sendSSE wraps the provided data in SSE wire
framing (prefixing lines with "data:" and ending with a double newline) and then
delegates to sendRaw, instead of saying it sends "raw JSON data"; reference the
mock.sendSSE and sendRaw symbols so the comment matches the detailed description
elsewhere.

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

src/hooks/useSSEStream.ts (2)

114-116: Magic number 2 for the CLOSED ready state — use a named constant

entry.source.readyState === 2 is non-obvious; 2 mirrors EventSource.CLOSED and the internal CLOSED constant in createFetchTransport.

✨ Proposed refactor

+const READY_STATE_CLOSED = 2  // EventSource.CLOSED / FetchTransport CLOSED

 function isStale(entry: StreamEntry<unknown>): boolean {
-  if (entry.source.readyState === 2) {
+  if (entry.source.readyState === READY_STATE_CLOSED) {
     return true
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useSSEStream.ts` around lines 114 - 116, Replace the magic number
check in isStale by using a named constant for the closed readyState: instead of
comparing entry.source.readyState === 2, reference EventSource.CLOSED (or the
internal CLOSED constant used in createFetchTransport) so the intent is clear;
update the condition in the isStale function (and any related checks of
entry.source.readyState) to use that named symbol (EventSource.CLOSED or CLOSED)
to improve readability and maintain consistency with createFetchTransport.

---

171-178: null as unknown as SSETransport | EventSource bypasses TypeScript's null safety

The cast silences the type checker; if a future branch ever exits without assigning entry.source, the error won't be caught until runtime. Consider restructuring so source is always passed at construction:

✨ Proposed refactor — defer entry creation until source is known

-  const entry: StreamEntry<T> = {
-    source: null as unknown as SSETransport | EventSource,
-    data: undefined,
-    error: undefined,
-    transform,
-    refCount: 0,
-    _instancesRef: getMockInstances(),
-  }
-
   if (options?.transport) {
     try {
       const source = options.transport(url)
-      entry.source = source
-      attachHandlers(source, entry)
+      const entry = makeEntry(source, transform)
+      attachHandlers(source, entry)
+      streams.set(key, entry as StreamEntry<unknown>)
+      return entry
     } catch (err) {
       const noopSource: SSETransport = {
         readyState: 2, onmessage: null, onerror: null, onopen: null,
         close() {}, addEventListener() {}, removeEventListener() {},
       }
-      entry.source = noopSource
-      entry.error = err instanceof Error ? err : new Error(String(err))
+      const entry = makeEntry(noopSource, transform)
+      entry.error = err instanceof Error ? err : new Error(String(err))
+      streams.set(key, entry as StreamEntry<unknown>)
+      return entry
     }
-  } else if ...

Where makeEntry constructs the full StreamEntry<T> given a known source.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useSSEStream.ts` around lines 171 - 178, The current StreamEntry
creation sets source using a null-cast ("null as unknown as SSETransport |
EventSource"), which defeats TS null-safety; instead, remove the null cast and
defer creating the StreamEntry until you have a real source by adding a factory
like makeEntry(source: SSETransport | EventSource): StreamEntry<T> that returns
the object (include transform, refCount, _instancesRef: getMockInstances(), data
and error defaults). Replace places that previously referenced the provisional
entry with calls to makeEntry(...) so entry.source is always set at construction
and TypeScript can enforce non-nullability for source.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/hooks/useSSEStream.ts`:
- Line 59: computeConnectionKey currently mutates the module-level
nonSerializableCounter during render, causing leaked connections under
Strict/Concurrent renders; fix by generating a per-component stable
non-serializable id using React's useRef inside useSSEStream and stop
incrementing the global counter inside computeConnectionKey on every render.
Concretely: in useSSEStream create a ref like const instanceIdRef =
useRef<number>(); if undefined, assign it once from ++nonSerializableCounter (or
a helper that increments the module counter exactly once), then change
computeConnectionKey to accept that ref (or the stable id) and use it for
non-serializable paths instead of ++nonSerializableCounter; update all call
sites of computeConnectionKey inside useSSEStream so they pass the stable ref/id
to avoid side-effects during render and prevent opening streams twice.
- Around line 103-106: The cache key generation in useSSEStream (where method,
body, headers are appended to parts and returned via parts.join('::')) is
brittle because JSON.stringify(headers) preserves insertion order; instead,
canonicalize headers by creating an object with sorted header keys (e.g.,
extract Object.keys(headers).sort() and build a new object with those keys) and
then JSON.stringify that sorted object before pushing to parts so
logically-identical headers produce the same key and reuse the same connection.

---

Nitpick comments:
In `@src/hooks/useSSEStream.ts`:
- Around line 114-116: Replace the magic number check in isStale by using a
named constant for the closed readyState: instead of comparing
entry.source.readyState === 2, reference EventSource.CLOSED (or the internal
CLOSED constant used in createFetchTransport) so the intent is clear; update the
condition in the isStale function (and any related checks of
entry.source.readyState) to use that named symbol (EventSource.CLOSED or CLOSED)
to improve readability and maintain consistency with createFetchTransport.
- Around line 171-178: The current StreamEntry creation sets source using a
null-cast ("null as unknown as SSETransport | EventSource"), which defeats TS
null-safety; instead, remove the null cast and defer creating the StreamEntry
until you have a real source by adding a factory like makeEntry(source:
SSETransport | EventSource): StreamEntry<T> that returns the object (include
transform, refCount, _instancesRef: getMockInstances(), data and error
defaults). Replace places that previously referenced the provisional entry with
calls to makeEntry(...) so entry.source is always set at construction and
TypeScript can enforce non-nullability for source.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Non-serializable body: ++nonSerializableCounter during render leaks live connections under Strict Mode and concurrent renders

computeConnectionKey (called at line 252 during the render body) increments the module-level nonSerializableCounter as a side-effect. React's Strict Mode calls the component function body twice to detect accidentally impure code. Because nonSerializableCounter is not React-managed state, it is not reset between the two invocations. The two calls produce two different keys (e.g. url::1 and url::2), each triggering createStream in the render body, which opens a live network connection and registers it in the module-level streams map. Only the second (committed) render's useEffect fires, so stream url::1 is permanently leaked — its refCount stays at 0 and nothing ever calls entry.source.close(). React may invoke render phase lifecycles more than once before committing, or without committing at all. Ignoring this can lead to a variety of problems including memory leaks and invalid application state.

Concretely: every useSSEStream mount with a non-serializable body leaks one live EventSource/FetchTransport connection in Strict Mode development. In production under concurrent mode (interrupted renders), the same pattern applies.

The fix is to generate the per-component-instance non-serializable key once and stabilize it via useRef, so re-invocations of the same component body don't advance the counter:

🔧 Proposed fix — stable non-serializable key via useRef

 export function useSSEStream<T = unknown>(
   url: string,
   options?: UseSSEStreamOptions<T>,
 ): UseSSEStreamResult<T> {
   const subscribedKeyRef = useRef<string | null>(null)
+  // Stable key for non-serializable bodies — generated once per mount, not per render invocation.
+  const nonSerializableKeyRef = useRef<string | null>(null)
   const transform = options?.transform

-  const key = computeConnectionKey(url, options)
+  const key = computeConnectionKey(url, options, nonSerializableKeyRef)

computeConnectionKey accepts the ref and uses it for non-serializable paths:

 function computeConnectionKey<T>(
   url: string,
   options?: UseSSEStreamOptions<T>,
+  nonSerializableKeyRef?: React.MutableRefObject<string | null>,
 ): string {
   ...
   // Non-serializable bodies -> stable per-instance key, never cross-render reuse
   if (body !== undefined && isNonSerializable(body)) {
-    return `${url}::${++nonSerializableCounter}`
+    if (!nonSerializableKeyRef || nonSerializableKeyRef.current === null) {
+      const newKey = `${url}::ns:${++nonSerializableCounter}`
+      if (nonSerializableKeyRef) nonSerializableKeyRef.current = newKey
+      return newKey
+    }
+    return nonSerializableKeyRef.current
   }

Also applies to: 98-100, 252-264

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useSSEStream.ts` at line 59, computeConnectionKey currently mutates
the module-level nonSerializableCounter during render, causing leaked
connections under Strict/Concurrent renders; fix by generating a per-component
stable non-serializable id using React's useRef inside useSSEStream and stop
incrementing the global counter inside computeConnectionKey on every render.
Concretely: in useSSEStream create a ref like const instanceIdRef =
useRef<number>(); if undefined, assign it once from ++nonSerializableCounter (or
a helper that increments the module counter exactly once), then change
computeConnectionKey to accept that ref (or the stable id) and use it for
non-serializable paths instead of ++nonSerializableCounter; update all call
sites of computeConnectionKey inside useSSEStream so they pass the stable ref/id
to avoid side-effects during render and prevent opening streams twice.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

JSON.stringify(headers) is insertion-order-dependent — identical logical headers with different key ordering produce different cache keys

Two callers passing { Authorization: '...' , 'Content-Type': 'json' } vs { 'Content-Type': 'json', Authorization: '...' } get different composite keys and therefore separate connections. Sort keys before serializing to make the key canonical:

🔧 Proposed fix — canonicalize headers key

-  if (headers !== undefined) parts.push(`headers:${JSON.stringify(headers)}`)
+  if (headers !== undefined) {
+    const sortedHeaders = Object.fromEntries(
+      Object.entries(headers).sort(([a], [b]) => a.localeCompare(b)),
+    )
+    parts.push(`headers:${JSON.stringify(sortedHeaders)}`)
+  }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (method !== undefined) parts.push(`method:${method}`)
	if (body !== undefined) parts.push(`body:${JSON.stringify(body)}`)
	if (headers !== undefined) parts.push(`headers:${JSON.stringify(headers)}`)
	return parts.join('::')
	if (method !== undefined) parts.push(`method:${method}`)
	if (body !== undefined) parts.push(`body:${JSON.stringify(body)}`)
	if (headers !== undefined) {
	const sortedHeaders = Object.fromEntries(
	Object.entries(headers).sort(([a], [b]) => a.localeCompare(b)),
	)
	parts.push(`headers:${JSON.stringify(sortedHeaders)}`)
	}
	return parts.join('::')

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useSSEStream.ts` around lines 103 - 106, The cache key generation
in useSSEStream (where method, body, headers are appended to parts and returned
via parts.join('::')) is brittle because JSON.stringify(headers) preserves
insertion order; instead, canonicalize headers by creating an object with sorted
header keys (e.g., extract Object.keys(headers).sort() and build a new object
with those keys) and then JSON.stringify that sorted object before pushing to
parts so logically-identical headers produce the same key and reuse the same
connection.