From d2c791d65684ac4b65fe8932bf024ba2c922bef4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dar=C3=ADo=20Kondratiuk?= Date: Wed, 29 Jul 2026 09:50:05 -0300 Subject: [PATCH] Make FromEventBuffered's bufferSize nullable, defaulting to unbounded 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 Claude-Session: https://claude.ai/code/session_01K92eapPm8e7mX7puT4cF4s --- src/RxSharp/Extras/FromEventBuffered.cs | 30 ++++++++----------- src/RxSharp/RxSharp.csproj | 6 ++-- .../Extras/FromEventBufferedExtrasTests.cs | 20 +++++++++++++ 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/src/RxSharp/Extras/FromEventBuffered.cs b/src/RxSharp/Extras/FromEventBuffered.cs index caf3aa0..dc8a05b 100644 --- a/src/RxSharp/Extras/FromEventBuffered.cs +++ b/src/RxSharp/Extras/FromEventBuffered.cs @@ -69,21 +69,24 @@ public static partial class RxExtensions /// Called on , with the same handler, to remove it from the event. /// Converts an callback into the event's actual delegate shape. /// - /// The maximum number of most-recent payloads kept for replay. Defaults to 1 - fine if the caller only - /// cares about the single most recent payload, but a real risk if the caller filters for a specific match - /// among possibly-several payloads that could arrive in the pre-subscribe gap this method exists to cover: - /// with the default of 1, a matching payload can be silently evicted from the buffer by a later - /// non-matching one before anyone subscribes to see it. Pass a larger value whenever more than one payload - /// could plausibly arrive before subscription and only some of them are what the caller is looking for. + /// The maximum number of most-recent payloads kept for replay. Defaults to , which + /// defers entirely to 's own default (unbounded) - this wrapper has no + /// buffering opinion of its own beyond what the primitive it's built on already does. A caller filtering + /// for a specific match among payloads that could arrive in the pre-subscribe gap this method exists to + /// cover needs more than 1: with too small a buffer, a matching payload can be silently evicted by a later + /// non-matching one before anyone subscribes to see it. Pass an explicit bound only if this source stays + /// attached for a long time without being subscribed to and unbounded growth is a real concern for that + /// specific usage - the gap this method targets is normally microseconds, not something an unbounded + /// buffer meaningfully grows during. /// /// A handle exposing the buffered payloads as an observable, and detaching the handler on disposal. public static BufferedEventSource FromEventBuffered( Action addHandler, Action removeHandler, Func, TDelegate> conversion, - int bufferSize = 1) + int? bufferSize = null) { - var subject = new ReplaySubject(bufferSize); + var subject = bufferSize is { } size ? new ReplaySubject(size) : new ReplaySubject(); var handler = conversion(subject.OnNext); addHandler(handler); return new BufferedEventSource(subject, () => removeHandler(handler)); @@ -93,18 +96,11 @@ public static BufferedEventSource FromEventBufferedThe type of the event's payload. /// Called immediately, with the handler to add to the event. /// Called on , with the same handler, to remove it from the event. - /// - /// The maximum number of most-recent payloads kept for replay. Defaults to 1 - fine if the caller only - /// cares about the single most recent payload, but a real risk if the caller filters for a specific match - /// among possibly-several payloads that could arrive in the pre-subscribe gap this method exists to cover: - /// with the default of 1, a matching payload can be silently evicted from the buffer by a later - /// non-matching one before anyone subscribes to see it. Pass a larger value whenever more than one payload - /// could plausibly arrive before subscription and only some of them are what the caller is looking for. - /// + /// See 's parameter of the same name. /// A handle exposing the buffered payloads as an observable, and detaching the handler on disposal. public static BufferedEventSource FromEventBuffered( Action> addHandler, Action> removeHandler, - int bufferSize = 1) + int? bufferSize = null) => FromEventBuffered, TEventArgs>(addHandler, removeHandler, onNext => (_, args) => onNext(args), bufferSize); } diff --git a/src/RxSharp/RxSharp.csproj b/src/RxSharp/RxSharp.csproj index e3bf595..d09bcff 100644 --- a/src/RxSharp/RxSharp.csproj +++ b/src/RxSharp/RxSharp.csproj @@ -16,9 +16,9 @@ https://github.com/hardkoded/ReactiveExtensions-Sharp https://github.com/hardkoded/ReactiveExtensions-Sharp README.md - 0.1.3 - 0.1.3.0 - 0.1.3.0 + 0.1.4 + 0.1.4.0 + 0.1.4.0 diff --git a/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs b/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs index 5aa8491..4ea679b 100644 --- a/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs +++ b/test/RxSharp.Tests/Extras/FromEventBufferedExtrasTests.cs @@ -67,6 +67,26 @@ public void ShouldDeliverToEveryIndependentSubscriber() Assert.That(second, Is.EqualTo(new[] { "shared-target" })); } + [Test] + public void ShouldBufferEveryPreSubscribeValueByDefault() + { + // Locks in the default: null defers to ReplaySubject's own default (unbounded), not a small opinionated + // number - a matching payload must never be evicted by a later non-matching one before subscription. + var manager = new FakeTargetManager(); + using var source = RxExtensions.FromEventBuffered( + h => manager.TargetCreated += h, + h => manager.TargetCreated -= h); + + manager.RaiseTargetCreated("one"); + manager.RaiseTargetCreated("two"); + manager.RaiseTargetCreated("three"); + + var received = new List(); + source.AsObservable().Subscribe(received.Add); + + Assert.That(received, Is.EqualTo(new[] { "one", "two", "three" })); + } + [Test] public void ShouldOnlyReplayUpToTheRequestedBufferSize() {