refactor!: drop duplicate sinks for Primitives 5.7.0 concretes#4387
Merged
Conversation
Bump ReactiveUI.Primitives to 5.7.0 and delete in-repo observable sinks that the package now provides as concrete signal classes. - Remove ReactiveUI.Internal.SingleValueObservable<T> (+ Void/True/False singletons), SyncExecuteObservable<T>, StartObservable<T> - Use the seam-neutral Primitives concretes instead: ReturnSignal, StartSignal and ImmutableReturnRxVoidSignal - Import ReactiveUI.Primitives.Advanced (lean) / ReactiveUI.Primitives.Reactive.Advanced (shim) via ReactiveShim.props and the test Directory.Build.props so the names resolve under both seams - Trim the removed public types from the ReactiveUI.Core PublicAPI baselines - Drop the obsolete SingleValueObservable unit test BREAKING CHANGE: the public types ReactiveUI.Internal.SingleValueObservable<T> and ReactiveUI.Internal.SyncExecuteObservable<T> are removed; construct the equivalent ReactiveUI.Primitives.Advanced signal classes instead.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4387 +/- ##
==========================================
- Coverage 93.07% 93.05% -0.02%
==========================================
Files 343 339 -4
Lines 14924 14882 -42
Branches 1570 1567 -3
==========================================
- Hits 13890 13848 -42
Misses 770 770
Partials 264 264 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The near-identical EventHandler / EventHandler<TEventArgs> command-binding overloads in the android and WinForms binders were copy-pasted, which SonarCloud flagged as new duplication on the Primitives 5.7.0 PR. Hoist the shared logic into one private static helper per binder instead. - FlexibleCommandBinder: new BindEvent centralizes the command-parameter subscription, handler attach/detach and enabled-property sync. ForEvent (both overloads) and BindCommandToObject now only supply the typed handler wiring via a small closure. - CreatesWinformsCommandBinding: new BindToHandler does the same for its two overloads, including the Component Enabled-property sync. - Net ~57 fewer lines of platform code with identical behaviour (WinForms command-binding tests stay green).
The SonarCloud scan runs on windows-latest, so it executes the core, WPF,
WinUI and WinForms tests — that platform code stays measured (the WinForms
command binder reads ~99%). It cannot run the android/apple per-OS code under
ReactiveUI/Platforms/{android,apple-common,ios,mac,tvos,uikit-common,
mobile-common}, which needs an emulator, device or mac runner, so those files
always report 0% coverage and drag the new-code coverage gate down.
Exclude exactly those per-OS directories from coverage only. Platforms/net and
Platforms/netstandard2.0 stay measured (the core tests exercise them), and
every excluded file is still fully analysed for bugs, smells and duplication —
this is not a duplication exclusion.
The constructor kicked off the initial navigation-stack reconciliation with a fire-and-forget Task.Run, mutating MAUI navigation state from a thread-pool thread. That raced with any push happening right after construction: the background SyncNavigationStacksAsync could interleave its own PushAsync with the caller's, leaving CurrentPage pointing at the wrong page. It surfaced as the intermittent RoutedViewHostTest.InvalidateCurrentViewModel_MatchingViewModelType_ UpdatesViewModel failure on the slower macOS CI runner. Marshal the initial sync onto RxSchedulers.MainThreadScheduler instead — the same pattern the rest of the class already uses (OnNavigateRequested). UI navigation is now only ever mutated on the UI thread and the initial sync is serialized with later navigation rather than racing it. Verified: full ReactiveUI.Maui.Tests suite 202/202; RoutedViewHostTest stressed 20x with zero failures.
|
ChrisPulman
approved these changes
Jun 26, 2026
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
2 participants
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.
What kind of change does this PR introduce?
Refactor + dependency update. Bumps
ReactiveUI.Primitivesto 5.7.0 and removes in-repo observable sinks that the package now ships as concrete signal classes.What is the new behavior?
ReactiveUI.Primitivesupdated 5.6.0 → 5.7.0.SingleValueObservable<T>/.Void/.True/.False→ReturnSignal<T>(value, Sequencer.Immediate)/ImmutableReturnRxVoidSignal.InstanceSyncExecuteObservable<T>→StartSignal<T>(fn, Sequencer.Immediate)StartObservable<T>→StartSignal<T>(fn, scheduler)ReturnSignal,StartSignal,ImmutableReturnRxVoidSignal) are used, so the source still compiles under both the lean and.Reactive(System.Reactive interop) seams.ReactiveShim.propsand the testDirectory.Build.propsimportReactiveUI.Primitives.Advanced(lean) /ReactiveUI.Primitives.Reactive.Advanced(shim) for unqualified resolution.ReactiveUI.CorePublicAPI baselines updated to drop the removed public types.What is the current behavior?
ReactiveUI carried its own
ReactiveUI.Internal.SingleValueObservable<T>,SyncExecuteObservable<T>andStartObservable<T>sinks that duplicated functionality now provided byReactiveUI.Primitives.What might this PR break?
Breaking: the public types
ReactiveUI.Internal.SingleValueObservable<T>andReactiveUI.Internal.SyncExecuteObservable<T>are removed. They lived in theReactiveUI.Internalnamespace and were intended for internal use; consumers depending on them should construct the equivalentReactiveUI.Primitives.Advancedsignal classes (ReturnSignal,StartSignal). No runtime behavior change.Checklist
mainbranchAdditional information
Verified with
-warnaserror:ReactiveUI.Core, leanReactiveUI(net8.0 + net10.0-android),ReactiveUI.Reactive(net8.0 + net10.0-android), and all four Apple TFMs (net10.0-ios/tvos/macos/maccatalyst) for both the lean and.Reactiveleaves.ReactiveUI.Testsnet8.0 — 1966 passed, 0 failed; the.Reactivetest leaf compiles.