dotnet / maui

.NET MAUI is the .NET Multi-platform App UI, a framework for building native device applications spanning mobile, tablet, and desktop.
https://dot.net/maui
MIT License
22.28k stars 1.76k forks source link

[Proposal] Source generator for `[QueryProperty]` #20466

Open simonrozsival opened 9 months ago

simonrozsival commented 9 months ago

Motivation

ShellContent applies query properties via reflection. Since we can't fix this code just with [DynamicallyAccesseMebers(...)] attributes, we need a different solution.

The idea is to source-generate code equivalent to the reflection-based code. This would make this functionality not only trimmable and AOT-compatible, but also a bit faster.

Source generator specification

Example

// app code
namespace MyNamespace
{
    [QueryProperty("id", "Id")]
    [QueryProperty("first_name", "FirstName")]
    [QueryProperty("last_name", "LastName")]
    [QueryProperty("image", "Image")]
    internal partial class MyViewModel
    {
        public int Id { get; private set; }
        public string FirstName { get; private set; } = null!;
        public string? LastName { get; private set; }
        public Image? Image { get; private set; }
    }
}
// <auto-generated />
namespace MyNamespace
{
    partial class MyViewModel : global::Microsoft.Maui.Controls.IQueryAttributable
    {
        public void ApplyQueryAttributes(global::System.Collections.Generic.IDictionary<string, object?> query)
        {
            if (query.TryGetValue("id", out object? id) && id is not null)
            {
                // Unclear: what to do when the input can't be converted to the target type?
                // - `ChangeType` should throw System.FormatException, that's identical to the current behavior
                // - `ChangeType` can lose information - for example it will allow changing double into int. Is that expected?
                this.Id = id is int _id ? _id : (int)global::System.Convert.ChangeType(id, typeof(int));
            }
            else
            {
                // Unclear: throw when not nullable? should we add overload with `oldQuery` to `IQueryAttributable?
                this.Id = default;
            }

            if (query.TryGetValue("first_name", out object? first_name) && first_name is not null)
            {
                this.FirstName = global::System.Net.WebUtility.UrlDecode((string)first_name);
            }
            else
            {
                // Unclear: Throw since it's a non-nullable reference type? Assign `null!` or `string.Empty`?
                this.FirstName = string.Empty;
            }

            if (query.TryGetValue("last_name", out object? last_name) && last_name is not null)
            {
                this.LastName = global::System.Net.WebUtility.UrlDecode((string)last_name);
            }
            else
            {
                this.LastName = null;
            }

            if (query.TryGetValue("image", out object? image) && image is not null)
            {
                this.Image = (Image)image;
            }
            else
            {
                this.Image = null;
            }
        }
    }
}

Notes and questions

mattleibow commented 9 months ago

One potential issue I see in:

interface IQueryAttributable
{
    void ApplyQueryAttributes(IDictionary<string, object?> query);
    void ApplyQueryAttributes(IDictionary<string, object?> query, IDictionary<string, object?>? oldQuery) => ApplyQueryAttributes(query);
}

If we add the new member, we don't break code now, but then we are going to always need both members implemented. If I want the new API with 2 params, we can implement it, but we will still need the old API.

I wonder if a new interface is better, and also not use a plain dictionary but rather some ApplyQueryAttributesEventArgs type thing:

class ApplyQueryAttributesEventArgs
{
    public IDictionary<string, object?> NewQuery { get; }
    public IDictionary<string, object?>? OldQuery { get; }

    // potentially new members to help
    public bool IsFirstBinding { get; }
}