Skip to content

Conversation

@stephenquan
Copy link
Contributor

@stephenquan stephenquan commented Dec 5, 2025

Description of Change

This PR enhances the BindableProperty source generator to be initializer-aware and modernizes RatingView to use property initializers instead of attribute-provided defaults. It also aligns unit tests and expected codegen outputs with the new behavior.

Linked Issues

PR Checklist

  • Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Changes adhere to coding standard
  • Documentation created or updated: https://github.com/MicrosoftDocs/CommunityToolkit/pulls - TBA

Additional information

Key Outcomes

✅ Initializer-Aware Codegen

When a partial property has an initializer (e.g.,
public partial string Text { get; set; } = "Initial Value";), the generator:

  • Emits an initializer property access guard:
    bool __initializing<Property> to allow access to the partial property initializer
  • Generates a default value creator:
    __createDefault<Property>(BindableObject) that toggles the guard, reads the initializer-backed value, and returns it safely.
  • Updates the BindableProperty getter to return the partial property initializer when requested, otherwise it returns the standard BindableProperty value
    • Getter:
      get => __initializing<Property> ? field : (T)GetValue(PropertyProperty);
  • Automatically wires __createDefault<Property> into BindableProperty.Create(...) when an initializer exists; falls back to attribute DefaultValueCreatorMethodName otherwise.

✅ Cleaner Control Code in RatingView

Defaults now come from property initializers (via RatingViewDefaults) rather than attributes:

  • Removes several default-value helper methods (e.g., CreateDefaultFillColor, CreateDefaultShapePadding).
  • CustomShapePath clarified as nullable (string?).

✅ Test & Model Alignment
  • Unit tests updated across common, edge, generic, and nested scenarios to validate initializer-aware generation.
  • BindablePropertyModel extended with HasInitializer so the generator reliably selects the initializer path.

Benefits

  • Idiomatic .NET defaults: Respect property initializers as the source of truth for defaults.
  • Less boilerplate: Fewer default-value creator methods and attribute defaults scattered across controls.

Copilot AI review requested due to automatic review settings December 5, 2025 07:37
Copy link
Contributor

Copilot AI left a comment

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 enhances the BindableProperty source generator to recognize and respect C# property initializers, eliminating the need for attribute-based default values and separate default-value creator methods. Properties with initializers now have their defaults captured safely via generated guard flags and creator methods, while RatingView demonstrates the cleaner pattern by replacing attribute defaults with inline initializers.

Key Changes

  • Source generator now detects property initializers and generates guard flags (__initializing<Property>) and default-value creator methods (__createDefault<Property>)
  • Generated properties synchronize the backing field with BindableProperty values in both getter and setter
  • RatingView modernized to use property initializers (= RatingViewDefaults.X) instead of attribute parameters, removing boilerplate default-creator methods

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
RatingView.shared.cs Migrates 13 properties from attribute defaults to initializers; removes 4 helper methods; makes CustomShapePath nullable
Records.cs Adds HasInitializer boolean parameter to BindablePropertyModel record
BindablePropertyAttributeSourceGenerator.cs Implements initializer detection, guard-flag generation, and conditional default-creator wiring
BindablePropertyModelTests.cs Adds test case for models with initializers and updates existing tests with new parameter
IntegrationTests.cs Updates all expected code to include guard flags and field synchronization
EdgeCaseTests.cs Updates expected output for keywords, enums, nullables, arrays, namespaces, and literals
CommonUsageTests.cs Updates expectations for basic usage, inheritance, callbacks, accessibility, and adds new initializer test

Copilot AI review requested due to automatic review settings December 5, 2025 07:59
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@stephenquan
Copy link
Contributor Author

stephenquan commented Dec 5, 2025

@TheCodeTraveler, looks like I tried to update the CommonUsageTests.cs at the same time as you. I tried to push a last limit minute change to only apply the initializer modifications if HasInitializer is true which means much of the pre-initializer tests should be reverted to how they were.

@TheCodeTraveler
Copy link
Collaborator

This is awesome!!

This the perfect use case for the file modifier!

We can use the file modifier on the __initializing[PropertyName] field since there's no reason for its scope to be accessible outside of the generated *.g.cs file.

For example:

file bool __initializingText = false;

Copilot AI review requested due to automatic review settings December 5, 2025 08:31
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

@stephenquan
Copy link
Contributor Author

@TheCodeTraveler I tried the file modifier, but it doesn't appear to work for properties.

Copilot AI review requested due to automatic review settings December 5, 2025 22:44
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Copilot AI review requested due to automatic review settings December 5, 2025 23:23
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Copilot AI review requested due to automatic review settings December 6, 2025 00:09
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Copilot AI review requested due to automatic review settings December 8, 2025 18:48
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings December 8, 2025 20:01
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

TheCodeTraveler
TheCodeTraveler previously approved these changes Dec 8, 2025
Copy link
Collaborator

@TheCodeTraveler TheCodeTraveler left a comment

Choose a reason for hiding this comment

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

Thanks Stephen!

FYI - I made two updates to the source generator. Let me know what you think of the implementation!

1. Handle the scenario when both a property initializer and a DefaultValueCreatorMethodName are provided

In this scenario, we will ignore the partial property initializer, instead using DefaultValueCreatorMethod. This mimic's how .NET MAUI works today when creating a BindableProperty: .NET initializes the property when the class is initialized using the provided partial property initializer, then .NET MAUI overrides it, setting the BindableProperty default value using DefaultValueCreator

BindableProperty With Both Initializer and DefaultValueCreatorMethod

public partial class TestView : View
{
    [BindableProperty(DefaultValueCreatorMethodName = nameof(CreateDefaultText))]
    public partial string Text { get; set; } = "Initial Value";

	static string CreateDefaultText(BindableObject bindable)
	{
		return "Initial Value";
	}
}

Generated Code

// <auto-generated>
// See: CommunityToolkit.Maui.SourceGenerators.Internal.BindablePropertyAttributeSourceGenerator
#pragma warning disable
#nullable enable
namespace {{defaultTestNamespace}};
public partial class {{defaultTestClassName}}
{
    /// <summary>
    /// Backing BindableProperty for the <see cref = "Text"/> property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("Text", typeof(string), typeof({{defaultTestNamespace}}.{{defaultTestClassName}}), null, Microsoft.Maui.Controls.BindingMode.OneWay, null, null, null, null, CreateDefaultText);
    public partial string Text { get => false ? field : (string)GetValue(TextProperty); set => SetValue(TextProperty, value); }
}

2. Move Out-of-Scope Generated Types to file static class

I apologize for misunderstanding the file keyword earlier! I didn't realize the keyword could only be used on a class.

I updated the source generator to write bool __initializing* and object CreateDefault* in a file static class so that they are not accessible or discoverable by developers and that no developer accidentally references or changes the values to these.

BindableProperty Using Initializer

// <auto-generated>
// See: CommunityToolkit.Maui.SourceGenerators.Internal.BindablePropertyAttributeSourceGenerator
#pragma warning disable
#nullable enable
namespace {{defaultTestNamespace}};
public partial class {{defaultTestClassName}}
{
    /// <summary>
    /// Backing BindableProperty for the <see cref = "Text"/> property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("Text", typeof(string), typeof({{defaultTestNamespace}}.{{defaultTestClassName}}), null, Microsoft.Maui.Controls.BindingMode.OneWay, null, null, null, null, __BindablePropertyInitHelpers.CreateDefaultText);
    public partial string Text { get => __BindablePropertyInitHelpers.IsInitializingText ? field : (string)GetValue(TextProperty); set => SetValue(TextProperty, value); }

    /// <summary>
    /// Backing BindableProperty for the <see cref = "CustomDuration"/> property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty CustomDurationProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("CustomDuration", typeof(System.TimeSpan), typeof({{defaultTestNamespace}}.{{defaultTestClassName}}), null, Microsoft.Maui.Controls.BindingMode.OneWay, null, null, null, null, __BindablePropertyInitHelpers.CreateDefaultCustomDuration);
    public partial System.TimeSpan CustomDuration { get => __BindablePropertyInitHelpers.IsInitializingCustomDuration ? field : (System.TimeSpan)GetValue(CustomDurationProperty); set => SetValue(CustomDurationProperty, value); }
}

Generated Code

public partial class TestName
{
    /// <summary>
    /// Backing BindableProperty for the <see cref = "Text"/> property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty TextProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("Text", typeof(string), typeof({{defaultTestNamespace}}.{{defaultTestClassName}}), null, Microsoft.Maui.Controls.BindingMode.OneWay, null, null, null, null, __BindablePropertyInitHelpers.CreateDefaultText);
    public partial string Text { get => __BindablePropertyInitHelpers.IsInitializingText ? field : (string)GetValue(TextProperty); set => SetValue(TextProperty, value); }

    /// <summary>
    /// Backing BindableProperty for the <see cref = "CustomDuration"/> property.
    /// </summary>
    public static readonly global::Microsoft.Maui.Controls.BindableProperty CustomDurationProperty = global::Microsoft.Maui.Controls.BindableProperty.Create("CustomDuration", typeof(System.TimeSpan), typeof({{defaultTestNamespace}}.{{defaultTestClassName}}), null, Microsoft.Maui.Controls.BindingMode.OneWay, null, null, null, null, __BindablePropertyInitHelpers.CreateDefaultCustomDuration);
    public partial System.TimeSpan CustomDuration { get => __BindablePropertyInitHelpers.IsInitializingCustomDuration ? field : (System.TimeSpan)GetValue(CustomDurationProperty); set => SetValue(CustomDurationProperty, value); }
}

file static class __BindablePropertyInitHelpers
{
  public static bool IsInitializingText = false;
  public static object CreateDefaultText(global::Microsoft.Maui.Controls.BindableObject bindable)
  {
      IsInitializingText = true;
      var defaultValue = ((TestView)bindable).Text;
      IsInitializingText = false;
      return defaultValue;
  }

  public static bool IsInitializingCustomDuration = false;
  public static object CreateDefaultCustomDuration(global::Microsoft.Maui.Controls.BindableObject bindable)
  {
      IsInitializingCustomDuration = true;
      var defaultValue = ((TestView)bindable).CustomDuration;
      IsInitializingCustomDuration = false;
      return defaultValue;
  }
}

Copy link
Contributor

@bijington bijington left a comment

Choose a reason for hiding this comment

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

Just a couple of thoughts from me

public partial class {{defaultTestClassName}} : View
{
[BindablePropertyAttribute(DefaultValueCreatorMethodName = nameof(CreateDefaultText))]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we report a warning here? It feels like it could be nice to warn the developer that their initial value will be ignored.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yup! Before we promote [BindableProperty] to stable, we'll create a slew of Analyzers, similar to the CommunityToolkit.Mvvm Analyzers to help make using these dummy-proof. It'll include things this, generating a compiler error when missing the partial modifier, generating a compiler error when the provided method signatures are incorrect, etc.

My plan is to start working on the initializers in the new year after we've publish the first preview release of [BindableProperty].

ne0rrmatrix
ne0rrmatrix previously approved these changes Dec 8, 2025
Copy link
Member

@ne0rrmatrix ne0rrmatrix left a comment

Choose a reason for hiding this comment

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

Looks good to me. I hope we can merge this soon. Just one question I do have. Is there any tests we can run or create that will validate that the generators will or will not generate invalid code that is not caught by the compiler? Has anyone even tested the idea of deliberately writing code that tests this out? Maybe irrelevant but I just thought this might poke an idea from someone about any future issues?

@TheCodeTraveler TheCodeTraveler dismissed stale reviews from ne0rrmatrix and themself via fc82b82 December 8, 2025 22:01
TheCodeTraveler
TheCodeTraveler previously approved these changes Dec 8, 2025
@stephenquan
Copy link
Contributor Author

stephenquan commented Dec 8, 2025

@TheCodeTraveler, thanks for the improvements. It makes perfect sense. It makes the implementation way better!

@stephenquan
Copy link
Contributor Author

A quick question about __BindablePropertyInitHelpers: since it’s generated as a static class within the namespace, wouldn’t that risk clashing with other __BindablePropertyInitHelpers instances in the same namespace?

One suggestion would be to mangle the class name to include the owning type, for example: __{ClassName}_BindablePropertyInitHelpers.

Copilot AI review requested due to automatic review settings December 9, 2025 01:22
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI review requested due to automatic review settings December 9, 2025 01:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@TheCodeTraveler
Copy link
Collaborator

TheCodeTraveler commented Dec 9, 2025

A quick question about __BindablePropertyInitHelpers: since it’s generated as a static class within the namespace, wouldn’t that risk clashing with other __BindablePropertyInitHelpers instances in the same namespace?

One suggestion would be to mangle the class name to include the owning type, for example: __{ClassName}_BindablePropertyInitHelpers.

Good question! Since you had already implemented partial property initializers in RatingView, I ran a test where I implemented [BindableProperty] on AvatarView using partial property initializers and found that there was not a namespace collision between the two generated file static class files. The file modifier must prevent these collisions 🤷

That said, it still feels kinda icky to have multiple classes in multiple generated files use the same name. Plus, having unique class names will make analyzing stack traces and debugging easier for future us. So, I still moved forward with your suggestion, updating the file static class name to __{ClassName}BindablePropertyInitHelpers.

Good call! Thanks for the suggestion 🙌

@TheCodeTraveler TheCodeTraveler merged commit 81c5187 into CommunityToolkit:main Dec 9, 2025
16 of 17 checks passed
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.

4 participants