Skip to content

refactor!: Align divergent property types with WinUI (sync-gen)#23655

Open
MartinZikmund wants to merge 2 commits into
feature/breakingchangesfrom
dev/mazi/prop-types
Open

refactor!: Align divergent property types with WinUI (sync-gen)#23655
MartinZikmund wants to merge 2 commits into
feature/breakingchangesfrom
dev/mazi/prop-types

Conversation

@MartinZikmund

Copy link
Copy Markdown
Member

GitHub Issue: https://github.com/unoplatform/uno-private/issues/2086 (part of #8339)

PR Type:

💬 Other: API alignment for 7.0 (source/binary-breaking, no behavior change)

What changed? 🚀

The WinAppSDK sync generator (Uno.WinAppSDKSyncGenerator) carried three by-name exceptions in its property-type matcher (SymbolMatchingHelpers.ArePropertiesMatching) that treated a property-type mismatch between the WinAppSDK metadata and Uno's hand-written members as "matching", to avoid a breaking change. For 7.0 (a major) we remove those exceptions so the generated public surface mirrors the WinAppSDK metadata, and align Uno's declared types to match.

The three cases (Content, EasingFunction, ElementName):

  1. Content — the exception was stale. Every WinAppSDK type that declares a Content property already has a matching Uno type (UIElementUIElement, objectobject). Removing the Content clause changes nothing in the generated output; no Uno type change was needed.

  2. DoubleAnimation.EasingFunction: IEasingFunctionEasingFunctionBase. WinAppSDK metadata declares this property (and the sibling ColorAnimation/PointAnimation/Easing*KeyFrame ones) as EasingFunctionBase; the IEasingFunction typing was a WPF holdover. EasingFunctionBase : IEasingFunction, so the internal IAnimation<T> contract and the animator pipeline are unaffected. DoubleAnimation was the only type the exception actually masked — ColorAnimation/PointAnimation have no hand-written public EasingFunction and already emitted the correct EasingFunctionBase stub.

    • SwipeControl assigned the internal, non-EasingFunctionBase LinearEase.Instance to the public property. It now assigns null, which the animator interpolates linearly (_easing = easingFunction ?? LinearEase.Instance) — identical behavior, and closer to WinUI (linear = no easing function).
  3. Binding.ElementName: objectstring. The WinUI IDL declares String ElementName. Uno used object to carry either the element-name string or an ElementNameSubject used for late binding. The subject is relocated to a new Uno-only [EditorBrowsable(Never)] Binding.ElementNameSubject property — structurally identical to the existing public Uno-only CompiledSource seam on the same class. It must be public because the compiled-XAML output lives in the consuming assembly (no access to Uno.UI internals). The XAML code generator, the runtime XAML reader, BindingExpression, and the cross-store binding copy all route the subject through the new property. Binding is the only WinAppSDK type with an ElementName property, so no other declaration is affected.

The by-name exceptions for these three are removed; only the genuine ContentTemplateRoot divergence (native-view typed) is kept.

Validation

  • Regeneration (authoritative) — Ran the sync generator against the pinned reference set (Microsoft.WindowsAppSDK 2.1.3). After the type alignments and with all three exceptions removed, the regenerated Generated/ tree is byte-identical to baseline (empty git diff on *Generated*, exit 0). This proves the members now match the WinAppSDK metadata with no new [NotImplemented] stubs — the result transfers to CI.
  • CompileUno.UI.Skia (net10.0) and Uno.UI.SourceGenerators build clean (0 errors). The SwipeControl consumer break was caught here and fixed.
  • Source-generator testsUno.UI.SourceGenerators.Tests Given_Binding + Given_Parser pass; the 4 XAML goldens with {Binding ElementName=…} now emit ElementNameSubject = _…Subject (updated in this PR).
  • Runtime — Ran the two applicable ElementName binding tests on Skia Desktop — Given_Control.When_Refresh_Setter_BindingOnInvocation_ElementName and Given_ListViewBase.When_Outer_ElementName_Binding — both pass (2/2), confirming the ElementNameSubject relocation resolves {Binding ElementName=…} end-to-end. Given_DoubleAnimation.When_EasingFunction is #if __ANDROID__-gated (native-Android-only assertion) and does not run on Skia; the EasingFunctionBase narrowing is behavior-identical (a null easing is still interpolated linearly) and compile-validated.

Note: the PackageDiffIgnore.xml signatures were authored to match the tool's format; if the CI PackageDiff step reports a slightly different rendering, the exact expected string in its output can be pasted in.

PR Checklist ✅

  • 🧪 Added test / sample (updated source-generator goldens; behavior-preserving so no new runtime test)
  • 📚 Docs have been added/updated (N/A)
  • 🖼️ Validated PR Screenshots Compare Test Run results (N/A — no visual change)
  • ❗ Contains NO breaking changes
  • 👀 Reviewed 2 other open pull requests (optional)

Breaking change impact

  • Binding.ElementName changes from object to string (source + binary break). Code-behind assigning a subject to ElementName no longer compiles (undocumented Uno-internal pattern); assigning a string still compiles and matches WinUI. Precompiled third-party XAML libraries using {Binding ElementName=…} generated against an older Uno.UI must be recompiled — appropriate within the 7.0 breaking-changes window.
  • DoubleAnimation.EasingFunction changes from IEasingFunction to EasingFunctionBase (source + binary break). Callers assigning a custom IEasingFunction that does not derive from EasingFunctionBase no longer compile — this matches WinUI, where EasingFunctionBase is the only valid public value.
  • A new public Binding.ElementNameSubject property is added ([EditorBrowsable(Never)], Uno-only); it is ignored by the sync generator (WinUI has no such member).

MartinZikmund and others added 2 commits July 6, 2026 13:08
Remove the by-name Content/EasingFunction/ElementName exceptions from the
sync generator's property-type matcher (#2086) and align Uno's hand-written
types to the WinAppSDK metadata:

- DoubleAnimation.EasingFunction: IEasingFunction -> EasingFunctionBase
- Binding.ElementName: object -> string; the late-binding subject moves to a
  new Uno-only ElementNameSubject property (set by the XAML codegen and the
  runtime reader, read by BindingExpression), mirroring the CompiledSource seam
- Content: the exception was stale (every type already matched WinUI)

SwipeControl now assigns a null EasingFunction (interpolated linearly by the
animator) instead of the non-EasingFunctionBase LinearEase.Instance.

Regeneration of the sync-generator output is byte-identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbaZY42v4K7Fp3t2GuHnjc
Binding.ElementName (object -> string) and DoubleAnimation.EasingFunction
(IEasingFunction -> EasingFunctionBase) change the public accessor signatures;
add the corresponding 6.6-baseline ignore entries (#2086).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WbaZY42v4K7Fp3t2GuHnjc
Copilot AI review requested due to automatic review settings July 6, 2026 11:13
@github-actions github-actions Bot added area/code-generation Categorizes an issue or PR as relevant to code generation area/build Categorizes an issue or PR as relevant to build infrastructure labels Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MartinZikmund's task in 4m 56s —— View job


Review: refactor!: Align divergent property types with WinUI

  • Gather context and understand the request
  • Read the changed files
  • Review the diff
  • Post inline comments for findings
  • Update summary comment

Overall verdict: ✅ Approve with nit

1 nit (non-blocking), no bugs, no correctness issues.


Cross-cutting observations

DoubleAnimation.EasingFunction (IEasingFunctionEasingFunctionBase) — clean. The explicit interface bridge IEasingFunction IAnimation<float>.EasingFunction => EasingFunction keeps the internal animation pipeline (CPUBoundAnimator) unaffected, the DP type is updated in lockstep (typeof(EasingFunctionBase)), and CPUBoundAnimator.SetEasingFunction already handles nullLinearEase.Instance, so the SwipeControl change is behaviorally safe.

Binding.ElementNameSubject — the separation is correct. [EditorBrowsable(Never)] + public access for cross-assembly XAML gen, with a CompiledSource-parity rationale. All call sites updated consistently (XAML generator, runtime reader, binding expression, binding clone in DependencyPropertyDetailsCollection).

XamlFileGenerator.cs rewrite — the memberName == "ElementName""ElementNameSubject" redirect is only reachable from the new Binding() initializer path (line 4251), so there's no risk of collateral rewriting for other types. Consistent with the existing "_PositionalParameters""Path" pattern.

SymbolMatchingHelpers.cs — the three property-name exceptions removed are no longer needed; only ContentTemplateRoot (native-view divergence) legitimately remains.

PackageDiffIgnore.xml — entries are well-formed, with clear reasons referencing the private issue and breaking-changes window.

Commits — follow Conventional Commits correctly (refactor! for the breaking change, chore(api) for the ignore list).


Nit (inline)

  • XamlObjectBuilder.cs:1205 — The runtime reader sets ElementNameSubject but not ElementName (the string). The binding resolves correctly, but binding.ElementName roundtrips as null for runtime-XAML-created bindings, diverging from WinUI's readable-back guarantee. Suggest adding binding.ElementName = value; alongside AddElementName(value, subject). Non-blocking.

case nameof(Binding.ElementName):
var subject = new ElementNameSubject();
binding.ElementName = subject;
binding.ElementNameSubject = subject;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Binding.ElementName string property is not set anywhere in this handler, so binding.ElementName will be null after the runtime reader parses {Binding ElementName=Foo}. The subject carries the resolution machinery and the binding works correctly, but reading back binding.ElementName returns null instead of "Foo".

WinUI's contract is that a property you write in XAML is readable back from the binding object. To keep that roundtrip consistent, consider also setting the string after the if check:

if (bindingProperty.Value?.ToString() is { } value)
{
    binding.ElementName = value;   // keep public string in sync
    AddElementName(value, subject);
}

This doesn't affect binding resolution (which goes through the subject), but avoids a surprising null for tooling or diagnostic code that inspects binding.ElementName.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns Uno’s public API surface with WinUI metadata by removing special-case property-type mismatches in the WinAppSDK sync generator and updating the affected Uno types (Binding.ElementName, DoubleAnimation.EasingFunction) plus related XAML/codegen plumbing.

Changes:

  • Removed by-name property type “match” exceptions in Uno.WinAppSDKSyncGenerator for Content, EasingFunction, and ElementName (keeping only ContentTemplateRoot divergence).
  • Updated DoubleAnimation.EasingFunction to use EasingFunctionBase and adjusted a consumer (SwipeControl) accordingly.
  • Split Binding.ElementName to a WinUI-parity string plus Uno-only ElementNameSubject, and updated XAML reader, binding engine, source generator, and goldens.

Reviewed changes

Copilot reviewed 9 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Uno.WinAppSDKSyncGenerator/Helpers/SymbolMatchingHelpers.cs Removes stale property-type exceptions so sync-gen requires true type matches.
src/Uno.UI/UI/Xaml/Media/Animation/DoubleAnimation.cs Narrows EasingFunction to EasingFunctionBase and updates DP registration.
src/Uno.UI/UI/Xaml/Markup/Reader/XamlObjectBuilder.cs Routes runtime XAML {Binding ElementName=...} through ElementNameSubject.
src/Uno.UI/UI/Xaml/DependencyPropertyDetailsCollection.cs Hot-reload binding cloning now copies ElementNameSubject.
src/Uno.UI/UI/Xaml/Data/Binding.cs Changes ElementName to string and adds Uno-only ElementNameSubject.
src/Uno.UI/UI/Xaml/Controls/SwipeControl/SwipeControl.Uno.cs Uses null easing for linear interpolation when inertia is used.
src/Uno.UI/DataBinding/BindingExpression.cs Switches element-name sourcing logic from ElementName to ElementNameSubject.
src/SourceGenerators/Uno.UI.SourceGenerators/XamlGenerator/XamlFileGenerator.cs Emits ElementNameSubject = ... instead of ElementName = ... in generated bindings.
src/SourceGenerators/Uno.UI.SourceGenerators.Tests/**/XamlCodeGenerator_*.cs (4 files) Updates golden outputs to use ElementNameSubject.
build/PackageDiffIgnore.xml Adds ignore entries for the intentional API signature changes.
Comments suppressed due to low confidence (1)

src/Uno.UI/UI/Xaml/Markup/Reader/XamlObjectBuilder.cs:1210

  • The runtime XAML reader now populates Binding.ElementNameSubject but never assigns the public Binding.ElementName string. This leaves ElementName unset for {Binding ElementName=...} created via the runtime reader, which is inconsistent with the WinUI API surface and makes debugging/introspection harder. Assign the string value in addition to tracking the subject.
					case nameof(Binding.ElementName):
						var subject = new ElementNameSubject();
						binding.ElementNameSubject = subject;

						if (bindingProperty.Value?.ToString() is { } value)
						{
							AddElementName(value, subject);
						}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 96 to +100
/// Gets or sets the name of the element to use as the binding source for the Binding.
/// </summary>
/// <value>The name of the element, or an ElementNameSubject instance that can be used to monitor the element name instance changes.</value>
public object ElementName { get; set; }
/// <value>The name of the element.</value>
public string ElementName { get; set; }

Comment on lines 76 to 79
newBinding.TargetNullValue = binding.TargetNullValue;
newBinding.ElementName = binding.ElementName;
newBinding.ElementNameSubject = binding.ElementNameSubject;
newBinding.FallbackValue = binding.FallbackValue;
@unodevops

Copy link
Copy Markdown
Contributor

🤖 Your WebAssembly Skia Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-23655/wasm-skia-net9/index.html

@nventive-devops

Copy link
Copy Markdown
Contributor

The build 220796 did not find any UI Test snapshots differences.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/build Categorizes an issue or PR as relevant to build infrastructure area/code-generation Categorizes an issue or PR as relevant to code generation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants