Skip to content

fix: use ObservableMixins helpers before RxAppBuilder initialization#4410

Merged
glennawatson merged 3 commits into
mainfrom
fix/observable-mixins-initialization
Jul 23, 2026
Merged

fix: use ObservableMixins helpers before RxAppBuilder initialization#4410
glennawatson merged 3 commits into
mainfrom
fix/observable-mixins-initialization

Conversation

@glennawatson

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix.

What is the new behavior?

ObservableMixins no longer forces ReactiveUI initialization. Its pure helpers can be
called before RxAppBuilder...BuildApp() has run:

  • WhereNotNull and the internal FromAsyncWithAllNotifications overloads work without
    any builder setup, because none of them touch RxApp, the dependency resolver, or the
    schedulers.
  • The static ObservableMixins() constructor that called RxAppBuilder.EnsureInitialized()
    has been removed. Every member of the class was verified to be pure, so no per-member
    initialization guard is required.

The nested observable helpers (WhereNotNullObservable, DeferredValueObservable,
FromAsyncObservable) are now internal so their async-notification, error, and
cancellation paths are exercised by real tests through InternalsVisibleTo. The
unreachable linked-token-source disposal race is isolated behind a single
[ExcludeFromCodeCoverage] helper, leaving no dead code.

What is the current behavior?

Calling a pure helper such as ObservableMixins.WhereNotNull(...) before the app was
built threw TypeInitializationException wrapping
InvalidOperationException: ReactiveUI has not been initialized, because the type
initializer called RxAppBuilder.EnsureInitialized() even though nothing in the class
depended on runtime state.

Closes #4360

What might this PR break?

None. Public API is unchanged (the helpers were already returned as interfaces, and the
nested types moved from private to internal, not public). Behavior only changes for
the previously-failing pre-initialization path, which now succeeds.

Checklist

  • I have read the Contribute guide
  • Tests have been added or updated (for bug fixes / features)
  • Docs have been added or updated (for bug fixes / features)
  • Changes target the main branch
  • PR title follows Conventional Commits

Additional information

Coverage: ObservableMixins.cs is at 100% line and 100% branch (77/77 lines), measured
across the full ReactiveUI.Tests suite (1976 tests passing on net9.0).

Sibling-mixin audit (recommended follow-up, not changed here). Every mixin in
src/ReactiveUI.Shared/Mixins carries the same static X() => RxAppBuilder.EnsureInitialized();
pattern. Auditing each:

  • MutableDependencyResolverExtensions - pure; operates only on the passed-in
    IMutableDependencyResolver. Forced init appears unnecessary.
  • MutableDependencyResolverAOTExtensions - pure; operates only on the passed-in resolver.
    Forced init appears unnecessary.
  • ObservedChangedMixins - pure; expression/reflection helpers over IObservedChange, no
    global state. Forced init appears unnecessary.
  • ObservableLoggingMixins - pure with respect to the builder flag; uses Splat's .Log()
    (its own locator, which has a default logger). Forced init appears unnecessary.
  • ReactiveNotifyPropertyChangedMixins - genuinely needs initialization:
    AppLocator.Current.GetServices<ICreatesObservableForProperty>() requires services that
    BuildApp/WithCoreServices register. Keep an init guard (scoped to the member, per the
    no-class-wide-ctor guidance).
  • AutoPersistHelperMixins - genuinely needs initialization: uses
    RxSchedulers.MainThreadScheduler/TaskpoolScheduler and WhenAny, all configured by
    BuildApp. Keep an init guard.

Recommendation: apply the same removal to the four pure mixins in a follow-up, and replace
the class-wide constructor on the two init-dependent mixins with a member-scoped
EnsureInitialized() call rather than a type-initializer side effect.

- Remove the static constructor that forced RxAppBuilder.EnsureInitialized, so
  the pure helpers (WhereNotNull, FromAsyncWithAllNotifications) no longer throw
  InvalidOperationException when used before BuildApp has run
- No member touches RxApp, the dependency resolver, or schedulers, so no
  per-member initialization guard is needed
- Make the nested observable helpers internal so the async-notification overloads,
  error and cancellation paths, and the deferred factory-error path are covered by
  real tests
- Isolate the unreachable linked-source disposal race behind an
  ExcludeFromCodeCoverage helper, leaving no dead code
@glennawatson
glennawatson marked this pull request as ready for review July 22, 2026 22:24
…merge

Main's analyzer suite now applies to this branch's added tests:
- Pass RunContinuationsAsynchronously to every TaskCompletionSource (PSH1302).
- Route captured locals through the state-argument FromAsyncWithAllNotifications
  overload with static lambdas (PSH1011/PSH1000); the parameterless-action test
  keeps its overload via a non-capturing lambda asserting the emitted result.
- Drop redundant explicit type arguments (SST2251); name the '* 2' factor (SST1471).
@glennawatson
glennawatson enabled auto-merge (squash) July 22, 2026 23:09
@glennawatson
glennawatson disabled auto-merge July 22, 2026 23:30
@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.19%. Comparing base (e6088d3) to head (eeb2222).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4410      +/-   ##
==========================================
+ Coverage   93.13%   93.19%   +0.05%     
==========================================
  Files         345      345              
  Lines       14979    14978       -1     
  Branches     1566     1565       -1     
==========================================
+ Hits        13951    13959       +8     
+ Misses        755      748       -7     
+ Partials      273      271       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@glennawatson
glennawatson merged commit c18940e into main Jul 23, 2026
13 checks passed
@glennawatson
glennawatson deleted the fix/observable-mixins-initialization branch July 23, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ObservableMixin requires full initialization

2 participants