Make FromEventBuffered's bufferSize nullable, defaulting to unbounded - #3
Merged
Conversation
The 0.1.3 fallout: FromEventBuffered's own default of 1 was an arbitrary opinion this wrapper had no business holding, and it was wrong for the wrapper's actual dominant use case (filtering for a specific match among payloads that could arrive in the pre-subscribe gap this method exists to cover) - even I fell into it myself, in puppeteer-sharp's own WaitForTargetAsync, before a code review caught it. bufferSize is now int? = null, where null defers entirely to ReplaySubject's own default (unbounded, matching rxjs's own ReplaySubject(bufferSize = Infinity)) instead of inventing a second, smaller opinionated default. The gap this method targets is normally microseconds; an unbounded buffer only becomes a real memory concern if a BufferedEventSource stays attached a long time without ever being subscribed to, which callers can still avoid by passing an explicit bound for that specific usage. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s
kblok
added a commit
to hardkoded/puppeteer-sharp
that referenced
this pull request
Jul 29, 2026
RxSharp 0.1.4 (pending publish - see hardkoded/ReactiveExtensions-Sharp#3) makes FromEventBuffered's bufferSize nullable, defaulting to ReplaySubject's own unbounded default instead of an opinionated 1. That's the same guarantee EventBufferSize = 16 was working around locally, so it's redundant now - the safe behavior is just the library default. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s
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
1 participant
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.
Follow-up to #1/#2's
FromEventBufferedand the buffer-size bug it caused in puppeteer-sharp (see hardkoded/puppeteer-sharp#3520).bufferSizedefaulted to1— an arbitrary opinion this wrapper had no business holding on its own, and wrong for the method's actual dominant use case: filtering for a specific match among payloads that could arrive in the pre-subscribe gapFromEventBufferedexists to cover. With a buffer of 1, a matching payload can be silently evicted by a later non-matching one before anyone subscribes. Puppeteer-sharp's ownWaitForTargetAsync/WaitForFrameAsynchit exactly this — I wrote both, and still fell into it myself, only caught by an external code review.bufferSizeis nowint? = null, wherenulldefers entirely toReplaySubject's own default (unbounded, matching rxjs's ownReplaySubject(bufferSize = Infinity)) instead ofFromEventBufferedinventing a second, smaller default of its own. The gap this method targets is normally microseconds — an unbounded buffer is only a real memory concern if aBufferedEventSourcestays attached a long time without ever being subscribed to, which a caller can still avoid by passing an explicit bound for that specific usage.Added a test locking in the new default (multiple pre-subscribe values, all buffered and replayed — not just the last one). All 840 tests pass (839 + 1 new).
🤖 Generated with Claude Code
https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s