[Blazor] Why isn't RenderTreeBuilder using CallerLineNumber for sequence numbers?

[egil]

Hi all,

I understand we need sequence numbers to efficiently diff the render tree, but I am wondering why the CallerLineNumber attribute cannot be used on methods in the RenderTreeBuilder class to get the result we need, without the maintainability issues related to hardcoded sequence numbers.

As I understand it, the compiler will embed the line number at compile time in the assembly, when it sees the CallerLineNumber attribute, thus, performance should be comparable to us manually hardcoding sequence numbers.

Here is a small demonstration of its use, through a bunch of extensions methods I've created:

public static class RenderTreeBuilderExtensions
{
    public static void OpenElement(this RenderTreeBuilder builder, string elementName, [System.Runtime.CompilerServices.CallerLineNumber] int sequence = -1)
    {
        builder.OpenElement(sequence, elementName);
    }

    public static void AddAttribute(this RenderTreeBuilder builder, string name, string value, [System.Runtime.CompilerServices.CallerLineNumber] int sequence = -1)
    {
        builder.AddAttribute(sequence, name, value);
    }

    public static void AddContent(this RenderTreeBuilder builder, RenderFragment fragment, [System.Runtime.CompilerServices.CallerLineNumber] int sequence = -1)
    {
        builder.AddContent(sequence, fragment);
    }
}

That allows me to write code that uses RenderTreeBuilder class like so:

private void Render(RenderTreeBuilder builder)
{
    builder.OpenElement("ul");
    builder.AddAttribute("class", "list-group");
    builder.AddContent(_childContent);
    builder.CloseElement();
}

[danroth27]

@egil Thanks for this suggestion! We'll take a look to see if this is something we can address.

[egil]

A little feedback after using my own extension class for a while.

Extracting line numbers makes the code much less sensitive to being rearranged and refactored, but it also hides the explicit need for sequence numbers need to follow certain rules. If somebody creates helper functions/helper class that calls the RenderTreeBuilder methods, then the line numbers will come from that, and not from the component where the original calls are placed. That can could cause problems, especially since it will work sometimes (by chance) and sometimes it will break diffing.

It might be possible to have a debug mode in RenderTreeBuilder, that detects if sequence numbers are not in ascending order in calls from a RenderFragment, using CallerMemberNameAttribute and CallerFilePathAttribute to track the current RenderFragment. That could help prevent component developers from making obvious mistakes.

Here is my current extension method:

internal static class RenderTreeBuilderExtensions
{
	private const int DEFAULT_SEQUENCE = -1;

	public static void OpenElement(this RenderTreeBuilder builder, string elementName, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.OpenElement(sequence, elementName);
	}

	public static void OpenComponent<TComponent>(this RenderTreeBuilder builder, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE) where TComponent : IComponent
	{
		builder.OpenComponent<TComponent>(sequence);
	}

	public static void AddAttribute(this RenderTreeBuilder builder, string name, string? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(value)) return;

		builder.AddAttribute(sequence, name, value);
	}

	public static void AddEventListener<T>(this RenderTreeBuilder builder, string eventName, EventCallback<T> callback, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(eventName)) return;

		builder.AddAttribute(sequence, eventName, callback);
	}

	public static void AddAttribute(this RenderTreeBuilder builder, string name, bool value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddAttribute(sequence, name, value);
	}

	public static void AddAttribute(this RenderTreeBuilder builder, string name, MulticastDelegate value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddAttribute(sequence, name, value);
	}

	public static void AddAttribute(this RenderTreeBuilder builder, string name, object? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (value is null) return;

		builder.AddAttribute(sequence, name, value);
	}

	public static void AddMultipleAttributes<T>(this RenderTreeBuilder builder, IEnumerable<KeyValuePair<string, T>>? attributes, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (attributes is null) return;

		builder.AddMultipleAttributes<T>(sequence, attributes);
	}

	public static void AddContent(this RenderTreeBuilder builder, string textContent, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddContent(sequence, textContent);
	}

	public static void AddContent(this RenderTreeBuilder builder, RenderFragment fragment, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddContent(sequence, fragment);
	}

	public static void AddMarkupContent(this RenderTreeBuilder builder, string markupContent, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddMarkupContent(sequence, markupContent);
	}

	public static void AddIdAttribute(this RenderTreeBuilder builder, string? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(value)) return;
		builder.AddAttribute(sequence, "id", value);
	}

	public static void AddClassAttribute(this RenderTreeBuilder builder, string? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(value)) return;
		builder.AddAttribute(sequence, "class", value);
	}

	public static void AddRoleAttribute(this RenderTreeBuilder builder, string? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(value)) return;
		builder.AddAttribute(sequence, "role", value);
	}

	public static void AddAriaLabelAttribute(this RenderTreeBuilder builder, string? value, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		if (string.IsNullOrEmpty(value)) return;
		builder.AddAttribute(sequence, "aria-label", value);
	}

	public static void AddAriaHiddenAttribute(this RenderTreeBuilder builder, bool value = true, [CallerLineNumber] int sequence = DEFAULT_SEQUENCE)
	{
		builder.AddAttribute(sequence, "aria-hidden", value);
	}
}

Ps. related to this discussion is the following issue: #11177

[SteveSandersonMS]

I was initially really keen on doing this - it's a very good suggestion!

However, after reflecting on it more, I came to the view that it would be dangerous to do this. If developers aren't aware that sequence numbers exist and are being used, they will not understand why they can't do things like this:

builder.AddContent("Some content");
AnotherMethodInAnotherFile(builder);

... where AnotherMethodInAnotherFile also operates on the builder, but has unrelated line numbers that may be earlier or later than the ones in the first file.

Extracting line numbers makes the code much less sensitive to being rearranged and refactored, but it also hides the explicit need for sequence numbers need to follow certain rules. If somebody creates helper functions/helper class that calls the RenderTreeBuilder methods, then the line numbers will come from that, and not from the component where the original calls are placed.

Yes, exactly!

It might be possible to have a debug mode in RenderTreeBuilder, that detects if sequence numbers are not in ascending order in calls from a RenderFragment, using CallerMemberNameAttribute and CallerFilePathAttribute to track the current RenderFragment. That could help prevent component developers from making obvious mistakes.

I'm not convinced we have a viable proposal here. Adding strange behaviors like "your code may break if you factor out parts of it into a different file" is completely alien to C# developers. If people don't even see the sequence numbers in their own code, they will have no idea why this breaks things or what they are supposed to do about it.

Altogether I'm now of the view that having the sequence numbers appear explicitly is the only safe approach, as it forces developers to recognize what's going on and allow for it.

I'll close this as we're not realistically in a position to do this for 3.0 anyway. We could reconsider in the future if a much clearer and more understandable design emerges. Hope that makes sense!

[egil]

Thanks. Makes sense. I'll keep using my internal extension methods, and see where it lands in the future.