Skip to content

Blazor API Review: RenderTreeBuilder #12415

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

Closed
rynowak opened this issue Jul 21, 2019 · 6 comments · Fixed by #12460
Closed

Blazor API Review: RenderTreeBuilder #12415

rynowak opened this issue Jul 21, 2019 · 6 comments · Fixed by #12460
Assignees
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed

Comments

@rynowak
Copy link
Member

rynowak commented Jul 21, 2019

Summary

RenderTreeBuilder is a low level API used by the compiler for .razor components, and used in expert scenarios for C# components.

It's critical that we get the design of the RenderTreeBuilder and its methods right, because there's no conceivable way we could change anything here. It's always possible for us to add however, and we don't generally need to optimize for simplicity or clear names.

APIs

namespace Microsoft.AspNetCore.Components.RenderTree
{
    public partial class RenderTreeBuilder : System.IDisposable
    {
        public const string ChildContent = "ChildContent";
        public RenderTreeBuilder(Microsoft.AspNetCore.Components.Rendering.Renderer renderer) { }
        public void AddAttribute(int sequence, in Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame frame) { }
        public void AddAttribute(int sequence, string name, Microsoft.AspNetCore.Components.EventCallback value) { }
        public void AddAttribute(int sequence, string name, System.Action value) { }
        public void AddAttribute(int sequence, string name, bool value) { }
        public void AddAttribute(int sequence, string name, System.Func<System.Threading.Tasks.Task> value) { }
        public void AddAttribute(int sequence, string name, System.MulticastDelegate value) { }
        public void AddAttribute(int sequence, string name, object value) { }
        public void AddAttribute(int sequence, string name, string value) { }
        public void AddAttribute<T>(int sequence, string name, Microsoft.AspNetCore.Components.EventCallback<T> value) { }
        public void AddComponentReferenceCapture(int sequence, System.Action<object> componentReferenceCaptureAction) { }
        public void AddContent(int sequence, Microsoft.AspNetCore.Components.MarkupString markupContent) { }
        public void AddContent(int sequence, Microsoft.AspNetCore.Components.RenderFragment fragment) { }
        public void AddContent(int sequence, object textContent) { }
        public void AddContent(int sequence, string textContent) { }
        public void AddContent<T>(int sequence, Microsoft.AspNetCore.Components.RenderFragment<T> fragment, T value) { }
        public void AddElementReferenceCapture(int sequence, System.Action<Microsoft.AspNetCore.Components.ElementReference> elementReferenceCaptureAction) { }
        public void AddMarkupContent(int sequence, string markupContent) { }
        public void AddMultipleAttributes(int sequence, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes) { }
        public void Clear() { }
        public void CloseComponent() { }
        public void CloseElement() { }
        public Microsoft.AspNetCore.Components.RenderTree.ArrayRange<Microsoft.AspNetCore.Components.RenderTree.RenderTreeFrame> GetFrames() { throw null; }
        public void OpenComponent(int sequence, System.Type componentType) { }
        public void OpenComponent<TComponent>(int sequence) where TComponent : Microsoft.AspNetCore.Components.IComponent { }
        public void OpenElement(int sequence, string elementName) { }
        public void SetKey(object value) { }
        public void SetUpdatesAttributeName(string updatesAttributeName) { }
        void System.IDisposable.Dispose() { }
    }
}
@blowdart blowdart added the area-blazor Includes: Blazor, Razor Components label Jul 21, 2019
@rynowak rynowak self-assigned this Jul 21, 2019
@rynowak rynowak added this to the 3.0.0-preview9 milestone Jul 21, 2019
@rynowak
Copy link
Member Author

rynowak commented Jul 21, 2019

The naming here follows a general pattern - we should make sure we think this is right:

  • There are methods for appending a single item Add...
  • There are methods for opening closing a scoped construct Open.../Close...
  • There are methods for modifying the current frame Set...

This seems like a reasonable naming structure. Outliers:

  • Clear and GetFrames for getting the result and resetting
  • Dispose used to return memory to the pool and it's optional to call

Some suggestions for improvement:

  • Make it sealed
  • Remove the constructor (it doesn't need the renderer)
  • Remove AddAttribute(int sequence, string name, System.Action value) and AddAttribute(int sequence, string name, System.Func<System.Threading.Tasks.Task> value) (unused)

The justification for the last one is that the compiler does something more sophisticated now. Removing this API doesn't block anyone from using delegates, they will go through AddAttribute(int sequence, string name, System.MulticastDelegate value).

@rynowak
Copy link
Member Author

rynowak commented Jul 21, 2019

@SteveSandersonMS

@danroth27 danroth27 mentioned this issue Jul 21, 2019
20 tasks
@SteveSandersonMS
Copy link
Member

All the suggestions sound good to me.

@rynowak
Copy link
Member Author

rynowak commented Jul 23, 2019

One other topic - is RenderTree a good namespace for this to live in forever? Right now what's there is all of the actual render frame types, which we want to discourage people from using. Rendering would be a better namespace.

@SteveSandersonMS
Copy link
Member

Namespace RenderTree->Rendering makes sense. I would totally endorse that.

rynowak pushed a commit that referenced this issue Jul 24, 2019
@rynowak rynowak added the Done This issue has been fixed label Jul 25, 2019
rynowak pushed a commit that referenced this issue Jul 25, 2019
@rynowak
Copy link
Member Author

rynowak commented Jul 25, 2019

Following up on the namespace part separately.

@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components Done This issue has been fixed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants