Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Typed Bindings #155

Closed
8 tasks done
brminnick opened this issue Nov 22, 2022 · 8 comments · Fixed by #156
Closed
8 tasks done

[Proposal] Typed Bindings #155

brminnick opened this issue Nov 22, 2022 · 8 comments · Fixed by #156
Assignees
Labels
approved champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved needs discussion The team will aim to discuss this at the next monthly standup proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail

Comments

@brminnick
Copy link
Collaborator

brminnick commented Nov 22, 2022

Feature name

Typed Bindings

Link to discussion

#154 (reply in thread)

Progress tracker

Summary

Typed Bindings are used by XAML Compiled Bindings to improve performance and ensure Type safety.

This Proposal extends the .Bind() extension method by providing the option of using TypedBinding which is the binding engine used by XAML Compiled Bindings to improve performance and ensure Type safety.

// One-way (aka read-only) Binding
new Label().Row(Row.Description).Bind(Label.TextProperty, (StoryModel m) => m.Description)

// Two-way Binding
new Entry().Bind(Entry.TextProperty, (SettingsViewModel vm) => vm.NumberOfTopStoriesToFetch, (SettingsViewModel vm, int text) => vm.NumberOfTopStoriesToFetch = text)

Motivation

The current implementation of .Bind() uses the Binding class which requires reflection when a change to the binding is applied.

This updated implementation brings the option of using TypedBinding with the .Bind() extension method which does not require reflection providing a substantial performance improvement for bindings.

Bindings TypedBinding
Uses Reflection Yes No
Type Safe No Yes

Detailed Design

A POC of this can be found on the Compiled-Bindings branch:

using Microsoft.Maui.Controls.Internals;

namespace CommunityToolkit.Maui.Markup;

/// <summary>
/// TypedBinding Extension Methods for Bindable Objects
/// </summary>
public static class TypedBindingExtensions
{
	/// <summary>Bind to a specified property</summary>
	public static TBindable Bind<TBindable, TBindingContext, TSource>(
		this TBindable bindable,
		BindableProperty targetProperty,
		Func<TBindingContext, TSource> getter,
		Action<TBindingContext, TSource>? setter = null,
		BindingMode? mode = null,
		string? stringFormat = null,
		TBindingContext? source = default) where TBindable : BindableObject
	{
		bindable.SetBinding(targetProperty, new TypedBinding<TBindingContext, TSource>(result => (getter(result), true), setter, null)
		{
			Mode = (setter, mode) switch
			{
				(_, not null) => mode.Value, // Always use the provided mode when given
				(null, null) => BindingMode.OneWay, // When setter is null, binding is read-only; use BindingMode.OneWay to improve performance
				_ => BindingMode.Default // Default to BindingMode.Default
			},
			StringFormat = stringFormat,
			Source = source,
		});

		return bindable;
	}

	/// <summary>Bind to a specified property with inline conversion</summary>
	public static TBindable Bind<TBindable, TBindingContext, TSource, TDest>(
		this TBindable bindable,
		BindableProperty targetProperty,
		Func<TBindingContext, TSource> getter,
		Action<TBindingContext, TSource>? setter = null,
		BindingMode? mode = null,
		Func<TSource?, TDest>? convert = null,
		Func<TDest?, TSource>? convertBack = null,
		string? stringFormat = null,
		TBindingContext? source = default,
		TDest? targetNullValue = default,
		TDest? fallbackValue = default) where TBindable : BindableObject
	{
		var converter = new FuncConverter<TSource, TDest, object>(convert, convertBack);
		bindable.SetBinding(targetProperty, new TypedBinding<TBindingContext, TSource>(result => (getter(result), true), setter, null)
		{
			Mode = (setter, mode) switch
			{
				(_, not null) => mode.Value, // Always use the provided mode when given
				(null, null) => BindingMode.OneWay, // When setter is null, binding is read-only; use BindingMode.OneWay to improve performance
				_ => BindingMode.Default // Default to BindingMode.Default
			},
			Converter = converter,
			StringFormat = stringFormat,
			Source = source,
			TargetNullValue = targetNullValue,
			FallbackValue = fallbackValue
		});

		return bindable;
	}

	/// <summary>Bind to a specified property with inline conversion and conversion parameter</summary>
	public static TBindable Bind<TBindable, TBindingContext, TSource, TParam, TDest>(
		this TBindable bindable,
		BindableProperty targetProperty,
		Func<TBindingContext, TSource> getter,
		Action<TBindingContext, TSource>? setter = null,
		BindingMode? mode = null,
		Func<TSource?, TParam?, TDest>? convert = null,
		Func<TDest?, TParam?, TSource>? convertBack = null,
		TParam? converterParameter = default,
		string? stringFormat = null,
		TBindingContext? source = default,
		TDest? targetNullValue = default,
		TDest? fallbackValue = default) where TBindable : BindableObject
	{
		var converter = new FuncConverter<TSource, TDest, TParam>(convert, convertBack);
		bindable.SetBinding(targetProperty, new TypedBinding<TBindingContext, TSource>(result => (getter(result), true), setter, null)
		{
			Mode = (setter, mode) switch
			{
				(_, not null) => mode.Value, // Always use the provided mode when given
				(null, null) => BindingMode.OneWay, // When setter is null, binding is read-only; use BindingMode.OneWay to improve performance
				_ => BindingMode.Default // Default to BindingMode.Default
			},
			Converter = converter,
			ConverterParameter = converterParameter,
			StringFormat = stringFormat,
			Source = source,
			TargetNullValue = targetNullValue,
			FallbackValue = fallbackValue
		});

		return bindable;
	}
}

Usage Syntax

// One-way (aka read-only) Binding
new Label().Row(Row.Description).Bind(Label.TextProperty, (StoryModel m) => m.Description)

// Two-way Binding
new Entry().Bind(Entry.TextProperty, (SettingsViewModel vm) => vm.NumberOfTopStoriesToFetch, (SettingsViewModel vm, int text) => vm.NumberOfTopStoriesToFetch = text)

Drawbacks

This is an overload to the existing .Bind() method, increasing the number of overloaded methods for .Bind() to 16.

This implementation also ignores TypedBinding's string[] handler constructor parameter. This parameter isn't documented and I'm unsure how it is being used and what use-cases it covers. However, I'm confident we can add support for this parameter in a future update without breaking changes.

Alternatives

TypedBinding can be used currently without C# Markup Extensions

// Two-way Binding
var entry = new Entry();
entry.SetBinding(Entry.TextProperty, new TypedBinding<SettingsViewModel, int>(vm => (vm.NumberOfTopStoriesToFetch, true), (vm, number) => vm.NumberOfTopStoriesToFetch = number, null));

Content = entry;

Unresolved Questions

Should we use a different name for this extension method, like .TypedBind()?

@brminnick brminnick added proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail new labels Nov 22, 2022
@brminnick brminnick self-assigned this Nov 22, 2022
@ghost ghost added champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature pending documentation This feature requires documentation and removed new labels Nov 22, 2022
@brminnick brminnick added new needs discussion The team will aim to discuss this at the next monthly standup and removed champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature pending documentation This feature requires documentation labels Nov 22, 2022
@ghost ghost removed the new label Nov 22, 2022
@brminnick brminnick added champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature pending documentation This feature requires documentation labels Nov 22, 2022
@pictos
Copy link
Member

pictos commented Nov 23, 2022

@brminnick instead of having BindingMode? why not use the Default or make the OneWay as default (I believe it's the default for regular bindings)?

@brminnick
Copy link
Collaborator Author

brminnick commented Nov 23, 2022

instead of having BindingMode? why not use the Default

Good Question! I made BindingMode? mode = null default to null on purpose to slide in a small performance improvement.

Since TypedBinding can have a null setter, we can set the BindingMode to BindingMode.OneWay automatically for the user which improves performance over BindingMode.TwoWay.

Each .Bind() extension method in this proposal uses this logic to set the BindingMode of the binding:

Mode = (setter, mode) switch
{
  (_, not null) => mode.Value, // Always use the provided mode when given
  (null, null) => BindingMode.OneWay, // When setter is null, binding is read-only; use BindingMode.OneWay to improve performance
  _ => BindingMode.Default // Default to BindingMode.Default
},

^ If the user doesn't provide a setter and doesn't provide a BindingMode, then we can safely assume the binding is read-only and set the BindingMode to BindingMode.OneWay.

If the user does provide a setter, and doesn't provide a BindingMode, then we'll default to BindingMode.Default.

or make the OneWay as default (I believe it's the default for regular bindings)?

We can't make the default BindingMode.OneWay because not every binding defaults to BindingMode.OneWay.

https://learn.microsoft.com/dotnet/maui/fundamentals/data-binding/binding-mode?view=net-maui-7.0#two-way-bindings

@brminnick
Copy link
Collaborator Author

Approved in December Standup: https://www.youtube.com/watch?v=tqQhW104UKI

@ghost ghost added approved help wanted This proposal has been approved and is ready to be implemented labels Dec 1, 2022
@brminnick brminnick removed the help wanted This proposal has been approved and is ready to be implemented label Dec 1, 2022
@ghost ghost reopened this Feb 3, 2023
@ghost
Copy link

ghost commented Feb 3, 2023

Reopening Proposal.

Only Proposals moved to the Closed Project Column and Completed Project Column can be closed.

@egvijayanand
Copy link

egvijayanand commented Feb 5, 2023

Have tried this thing previously, as a replacement to bind a nested property without using the nameof() feature as it won't work as expected. For example, nameof(Order.Id) would return only Id which would break the Binding. But didn't continue it actively.

Can this be extended to the BindCommand markup method as well?

And support for default BindableProperty can also be brought in like TextProperty for the Label so that it need not be mentioned in the Bind method again.

@ghost ghost removed the pending documentation This feature requires documentation label Feb 5, 2023
@ghost ghost closed this as completed Feb 5, 2023
@brminnick
Copy link
Collaborator Author

Can this be extended to the BindCommand markup method as well?

Yes! I believe @bijington is working on a Proposal for .BindCommand() support

And support for default BindableProperty can also be brought in like TextProperty for the Label so that it need not be mentioned in the Bind method again.

Maybe. To be honest, I think default bindable properties were a mistake and shouldn’t have been included in CommunityToolkit.Maui; they add very little value and are difficult to maintain.

@egvijayanand
Copy link

Yes! I believe @bijington is working on a Proposal for .BindCommand() support

Great to hear that BindCommand is also taken care of.

Maybe. To be honest, I think default bindable properties were a mistake and shouldn’t have been included in CommunityToolkit.Maui; they add very little value and are difficult to maintain.

It does make it easy to create Bindings in the long run once the developer is familiar with the nature of the control.

When is the documentation planned to get updated on the official site here?

@bijington
Copy link
Contributor

Oh I forgot to add in the proposal for the BindCommand extension! Let me add that now.

I personally prefer the explicitness of defining the property being bound rather than the defaults but I don't feel too strongly about it.

When is the documentation planned to get updated on the official site here?

We believe we have updated all the relevant examples to use the new typed bindings. Once the next proposals are completed then we can update the rest of the docs. Unless you think we have missed some?

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved champion A member of the .NET MAUI Toolkit core team has chosen to champion this feature documentation approved needs discussion The team will aim to discuss this at the next monthly standup proposal A fully fleshed out proposal describing a new feature in syntactic and semantic detail
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants