Skip to content

refactor: keep one ComponentModelFallbackConverter for all platforms - #4435

Open
glennawatson wants to merge 3 commits into
mainfrom
refactor/share-component-model-fallback-converter
Open

refactor: keep one ComponentModelFallbackConverter for all platforms#4435
glennawatson wants to merge 3 commits into
mainfrom
refactor/share-component-model-fallback-converter

Conversation

@glennawatson

@glennawatson glennawatson commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Refactor. Deletes a duplicated file. No build configuration change.

What is the new behavior?

ComponentModelFallbackConverter has one definition instead of two identical ones.

  • It lives in ReactiveUI.Shared/Bindings/Converter/, beside the ComponentModelConversion helper both copies already delegated to.
  • Neither project file needed an entry for it. ReactiveUI and ReactiveUI.Reactive already compile ReactiveUI.Shared/**/*.cs wholesale, so an existing glob picks it up. Both .csproj files are byte-identical to main.
  • [Preserve(AllMembers = true)] is what allows one file to serve every platform. It is ReactiveUI's own attribute and is inert off mobile, so the mobile linker still sees the converter and the desktop targets are unaffected at runtime. That attribute was the only meaningful difference between the two copies.

What is the current behavior?

Platforms/net/ComponentModelFallbackConverter.cs and Platforms/mobile-common/ComponentModelFallbackConverter.cs are byte-equivalent apart from that [Preserve] attribute and an unused using System.Diagnostics; on the desktop copy. The duplication scanner reports the pair at 66.7 percent and 25 percent.

What might this PR break?

The declared public API of the desktop targets changes, in one way: the type now carries [Preserve(AllMembers = true)] where it previously did not. Keeping the attribute is what lets a single file serve mobile, so the desktop targets inherit it. Preserve is ReactiveUI's own attribute and is inert away from the mobile linker, so nothing changes at runtime - but it is part of the surface and the baselines now record it.

  • 22 baselines updated: ReactiveUI and ReactiveUI.Reactive across net8/9/10/11, net462/472/481 and the four -windows targets.
  • The Apple and Android baselines already recorded the attribute and are unchanged.
  • Otherwise the same type, in the same namespace, is emitted on every target framework as before - only the file it is compiled from moved.

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

The diff is one file moving, one being deleted, and the [Preserve] line appearing in the 22 baselines that did not already have it.

Verified with a clean reactiveui.slnx Release build on a real Windows host: 0 errors, with ReactiveUI building 22 target frameworks and ReactiveUI.Reactive 21 - ios, tvos, maccatalyst, macos, android, Windows desktop, WinUI and .NET Framework included. Re-verified after #4428 landed with dotnet build reactiveui.slnx -c Release -t:Rebuild -warnaserror on Linux: 0 warnings, 0 errors.

@glennawatson
glennawatson force-pushed the refactor/share-component-model-fallback-converter branch from fbfd27f to 496efcb Compare August 13, 2026 01:54
- The desktop and mobile platform groups each carried an identical copy,
  differing only in the mobile copy's Preserve attribute.
- The survivor moves into ReactiveUI.Shared, which ReactiveUI and
  ReactiveUI.Reactive both already compile wholesale, so neither project
  file needs an entry for it.
- Preserve(AllMembers) is ReactiveUI's own attribute and is inert off
  mobile, so one file serves every platform.
@glennawatson
glennawatson force-pushed the refactor/share-component-model-fallback-converter branch from 496efcb to e8c76d3 Compare August 13, 2026 01:56
@glennawatson glennawatson changed the title refactor: fold the duplicate fallback converters into one file refactor: keep one ComponentModelFallbackConverter for all platforms Aug 13, 2026
@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.89%. Comparing base (749b862) to head (5710233).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4435   +/-   ##
=======================================
  Coverage   93.89%   93.89%           
=======================================
  Files         359      359           
  Lines       15104    15104           
  Branches     1568     1568           
=======================================
  Hits        14182    14182           
  Misses        679      679           
  Partials      243      243           

☔ 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.

Sharing one file across every platform gives the desktop targets the
[Preserve(AllMembers = true)] the mobile copy already carried, which is a
public API change the baselines have to state.

- Regenerate the 22 affected baselines: ReactiveUI and ReactiveUI.Reactive
  across net8/9/10/11, net462/472/481 and the four -windows targets.
- The Apple and Android baselines already record the attribute and are
  unchanged.
@sonarqubecloud

Copy link
Copy Markdown

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.

2 participants