fix: useStream can now handle multiple streams per component lifetime#20
Draft
ianmacartney wants to merge 1 commit intomainfrom
Draft
fix: useStream can now handle multiple streams per component lifetime#20ianmacartney wants to merge 1 commit intomainfrom
ianmacartney wants to merge 1 commit intomainfrom
Conversation
Fixes #19 Root causes addressed: 1. streamStarted ref was set but never cleared, blocking all future streams - Replaced with activeStreamRef that tracks the current streamId - Acts as both a Strict Mode guard AND allows detecting new streams 2. streamBody and streamEnded were never reset between streams - Now reset both states when a new streamId is detected 3. No AbortController caused stale stream reads to leak - Added AbortController to cleanup in-flight requests when: - Component unmounts - streamId changes to a new value - Errors from aborted fetches are now silently ignored 4. TextDecoder was allocated per chunk (minor perf issue) - Now allocated once per stream and reused The fix enables the real-time streaming UX for multiple consecutive messages in the same component session, rather than falling back to database persistence after the first stream completes. Co-authored-by: Ian Macartney <ianmacartney@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #19
The
useStreamhook was unable to drive more than one stream per component lifetime. After the first stream completed, subsequent calls with a newstreamIdsilently skipped the HTTP POST and fell back to the database query, losing the real-time streaming UX.Root Causes Addressed
1.
streamStartedref was set but never clearedThe original code used a
streamStartedref intended as a React Strict Mode guard, but it permanently blocked all future streams after the first one completed.Fix: Replaced with
activeStreamRefthat tracks the currentstreamId. This serves as both:2.
streamBodyandstreamEndedwere never reset between streamsWhen
streamIdchanged, the state from the previous stream would persist, potentially causing text concatenation issues.Fix: Both states are now reset when a new
streamIdis detected.3. No
AbortControllercaused stale stream reads to leakIf the component re-rendered with a new
streamIdwhile a previous fetch was mid-read, the oldreader.read()loop would continue callingsetStreamBody, mixing text across streams.Fix: Added
AbortControllerto cleanup in-flight requests when:streamIdchanges to a new valueErrors from aborted fetches are silently ignored to avoid false error states.
4. Minor performance improvement
TextDecoderwas being allocated per chunk inside the while loop.Fix: Now allocated once per stream and reused.
Testing
npm run typecheck)npm run lint)npm test)Slack Thread