dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
34.81k stars 9.84k forks source link

Idea: Stateless Blazor components #54547

Open egil opened 3 months ago

egil commented 3 months ago

To improve the performance of Blazor apps, the recommendation is to create reusable render fragments.

For components that qualify, the Razor compiler could create static reusable render fragments instead of regular instantiable components, that are made available as any other components for usage.

For example, the following ChatMessage.razor component:

<div class="chat-message">
    <span class="author">@Message.Author</span>
    <span class="text">@Message.Text</span>
</div>

@code {
    [Parameter]
    public ChatMessage? Message { get; set; }
}

This could be transpiled into the following:

public static class ChatMessage
{
  public readonly static RenderFragment<ChatMessage?> Fragment = builder => message =>
  {
    builder.OpenElement(0, "div");
    builder.AddAttribute(1, "class", "chat-message");

    builder.OpenElement(2, "span");
    builder.AddAttribute(3, "class", "author");
    builder.AddContent(4, message.Author);
    builder.CloseElement();

    builder.OpenElement(5, "span");
    builder.AddAttribute(6, "class", "text");
    builder.AddContent(7, message.Text);
    builder.CloseElement();

    builder.CloseElement();
  }
}

The key is that the usage of the "static ChatMessage" component should be the same as if it was a regular component, i.e.:

<ChatMessage Message=@message />

@code {
  private ChatMessage? message = ... ;
}

Benefits

Performance is the key motivator, and enabling it without having to write components in a different way than normal.

It will enable performance-cautious users to break their larger components into smaller, easier-maintained components, without impacting performance.

To be valuable, it should be seamless, meaning the user should not decide, e.g. through an attribute, whether they want a static component or a normal component, and they should be used similarly.

What components would qualify

I think it should be safe to create a static component if the following apply:

  1. Component do not have instance fields defined.
  2. Component do not have any life-cycle methods explicitly defined (i.e. constructor, SetParameterAsync, Dispose, DisposeAsync).
  3. Component do not have async code.
  4. Component can have parameters and services injected into them.

Methods, both static and instance methods, are OK, since they can be converted to local functions inside the generated render fragment.

Static fields and constants can just be included in the generated static class where the render fragment is defined, so that should be fine too.

Any parameters and/or injected services would be passed as arguments to the generated render fragment.

Since render fragments are not async, async code is not allowed, and users would have to have to do any async stuff outside and pass the resulting data in.

It may be that the compiler needs to mark the static components with an attribute or similar to mark them as static components for the editor or the compiler itself when static components are used in other components.

A more complex example:

@inject TimeProvider tp

<div class="chat-message">
  <span class="time">@ToLocal(Message.Time, tp)</span>
  <span class="author">@Message?.Author ?? "Unknown"</span>
  <span class="text">@Message?.Text ?? "No message content"</span>
  <button @onclick="LikeMessage">Like</button>
</div>

@code {
  private static int LikeMultiplier = 42;

  [Parameter]
  public ChatMessage? Message { get; set; }

  [Parameter]
  public EventCallback<int> OnLike { get; set; }

  private Task LikeMessage()
  {
    var like = Message.Likes + LikeMultiplier;
    return OnLike.InvokeAsync(like);
  }

  private static DateTimeOffset ToLocal(DateTimeOffset time, TimeProvider timeProvider) 
    => TimeZoneInfo.ConvertTime(time, timeProvider.LocalTimeZone)
}

Would be compiled into the following:

using Microsoft.AspNetCore.Components;
using System.Threading.Tasks;

public static class ChatMessage
{
  private static readonly int LikeMultiplier = 42;

  public readonly static RenderFragment<ChatMessage?, EventCallback<int>, TimeProvider> Fragment = builder => (message, onLike, tp) =>
  {
    builder.OpenElement(0, "div");
    builder.AddAttribute(1, "class", "chat-message");

    builder.OpenElement(2, "span");
    builder.AddAttribute(3, "class", "time");
    builder.AddContent(4, ToLocal(Message.Time, tp));
    builder.CloseElement();

    builder.OpenElement(5, "span");
    builder.AddAttribute(6, "class", "author");
    builder.AddContent(7, Message?.Author ?? "Unknown");
    builder.CloseElement();

    builder.OpenElement(8, "span");
    builder.AddAttribute(9, "class", "text");
    builder.AddContent(10, Message?.Text ?? "No message content");
    builder.CloseElement();

    builder.OpenElement(11, "button");
    builder.AddAttribute(12, "onclick", EventCallback.Factory.Create(this, LikeMessage));
    builder.AddContent(13, "Like");
    builder.CloseElement();

    builder.CloseElement();

    Task LikeMessage()
    {
        var like = Message.Likes + LikeMultiplier;
        return OnLike.InvokeAsync(like);
    }

    static DateTimeOffset ToLocal(DateTimeOffset time, TimeProvider timeProvider)
    {
        return TimeZoneInfo.ConvertTime(time, timeProvider.LocalTimeZone);
    }
  }
}

And be used as usual, i.e.:

@inject MessageRepo repo
<ChatMessage Message=@message OnLike=@OnLike />
@code {
  private ChatMessage? message;

  protected override async Task OnInitializedAsync()
  {
      message = await repo.LoadMessage();
  }

  private async Task OnLike(int likes)
  {
    message.Likes += likes;
    await repo.SaveAsync(message);
  }
}
egil commented 3 months ago

@danroth27, @jaredpar, we chatted about this during the MVP summit, so pinging you here.

danroth27 commented 3 months ago

@SteveSandersonMS Do you think this would help at all with #54232?

SteveSandersonMS commented 3 months ago

In the Fortunes case it shouldn't make much difference since the app is already just 2 components. This optimization is about reducing the cost of having large numbers (say, thousands) of components.

Additionally given the constraints that @egil lists ("What components would qualify"), the Fortunes and FortunesEf components wouldn't qualify anyway.

That aside, I do think this is a good idea. Whether it's implemented in the Razor compiler, or whether we make it work as a runtime feature - it could go either way. Doing it in the Razor compiler could eliminate one allocation (the component object instance itself) but one allocation per component instance isn't likely a key bottleneck anyway, the costs of regular component instances are more to do with how we set up independent render trees and run their lifecycles and diffing independently, all of which could be skipped in the runtime if we knew that:

  1. The component didn't have an independent lifecycle
  2. The app author is happy for the component to always re-render fully whenever its parent re-renders, even if there's no change to its parameters
SteveSandersonMS commented 3 months ago

On naming, I'd call these something like "stateless functional components", as it's exactly equivalent to React's stateless function/functional components: https://www.robinwieruch.de/react-function-component/#react-stateless-function-component

Or just "stateless components".

Imagine a directive @stateless you'd put at the top to opt into these restrictions and possible perf benefit. It would cause your component to use a different default base class (e.g., StatelessComponentBase) which has no lifecycle methods to override, and perhaps triggers either compile-time or runtime errors if you try to implement IDisposable.

This would be nicer than React's model since besides that directive there's no other syntactical change, so you could later upgrade a stateless component to a stateful one by just removing the directive (whereas in React, there are big syntax differences between the two component types).

danroth27 commented 3 months ago

@SteveSandersonMS @chsienki Should we move this to the dotnet/razor repo for initial consideration? It sounds like having this be a compiler feature would be the preferred approach. Thoughts?

chsienki commented 3 months ago

@SteveSandersonMS @chsienki Yeah, if we're going to have a directive to opt-in then it's a compiler feature either way. If we're requiring users to opt-in, it makes sense to me for the compiler to explicitly emit something different rather than have the runtime have to infer it.

Seems like a nice feature though. I'll wait for @SteveSandersonMS to ok it before transferring over but feel free for either of you to move if you agree.

SteveSandersonMS commented 3 months ago

We'd need to collaborate on a design that works both for compiler and runtime, but in principle I'm definitely in support!

danroth27 commented 3 months ago

Let's land the design here first and then create a separate issue in the dotnet/razor repo once we know what the relevant compiler work is. We're also fully booked right now for .NET 9, so this is presumably a post-.NET 9 discussion.

egil commented 3 months ago

Doing it in the Razor compiler could eliminate one allocation (the component object instance itself)...

Would it not also save the allocation in the Renderer of a ComponentState object used to track each component? Probably not significant though.

On naming, I'd call these something like "stateless functional components", as it's exactly equivalent to React's stateless function/functional components: https://www.robinwieruch.de/react-function-component/#react-stateless-function-component

Or just "stateless components".

I like that name better too. I'll update the issue title.

Imagine a directive @stateless you'd put at the top to opt into these restrictions and possible perf benefit...

I think it's better if users did not have to opt-in/opt out of the feature and that the compiler does what's best (for perf) by default, depending on whether or not users have life cycle methods or not. It is a nice idea to have a directive @stateless (or @inherits StatelessComponentBase) as an explicit opt-in.

If the compiler team does not have the bandwidth to add support there, a runtime-only approach that leverages a special base component, e.g. StatelessComponentBase, is a good fallback, and it sounds as if there are additional savings possible besides reduced allocations.

Another thing I did not consider yesterday was the impact on libraries like bUnit. There, we allow users to walk the component tree (i.e. finding the instances of child components/parent components). It will be confusing in a bUnit context if some components are available in the component tree/render tree, and others are not. So from a runtime perspective, it would be great if there is a way to track stateless components in renderer implementations that want to do so. It could be a virtual method in Renderer like protected virtual ?void/? RegisterStatelessComponent<TStatelessComponent>(ComponentState parentComponentState).

SteveSandersonMS commented 3 months ago

Would it not also save the allocation in the Renderer of a ComponentState object used to track each component? Probably not significant though.

That allocation (ComponentState, and the whole chain of things created to support that) woud be avoided whether it's a compiler-only or runtime feature.

I think it's better if users did not have to opt-in/opt out of the feature and that the compiler does what's best (for perf) by default,

I'm afraid the compiler can't know the usage pattern. It doesn't know whether you'll have a large number of instances, whether the parent will re-render much more often than the child needs to, and whether the rendering output is going to be large. This (and other information) would be needed to determine whether it's better or worse to inline the component output into its parent vs let it maintain its own separate rendertree.

hakenr commented 3 months ago

Imagine a directive @stateless you'd put at the top to opt into these restrictions and possible perf benefit.

Do we need a new directive for this? The layout also derive from LayoutComponentBase and do not need a new directive for that.

egil commented 3 months ago

I'm afraid the compiler can't know the usage pattern. It doesn't know whether you'll have a large number of instances, whether the parent will re-render much more often than the child needs to, and whether the rendering output is going to be large. This (and other information) would be needed to determine whether it's better or worse to inline the component output into its parent vs let it maintain its own separate rendertree.

That makes sense. Been thinking about this from a SSR perspective where the are few/no rerenders anyway (only in streaming rendering).

SteveSandersonMS commented 3 months ago

Do we need a new directive for this? The layout also derive from LayoutComponentBase and do not need a new directive for that.

Yes, because it would be changing the semantics of how and when it renders (specifically, it would make the component unable to render independently from its parent, e.g., there would be no equivalent to StateHasChanged). That's quite different from LayoutComponentBase which behaves exactly like any other component.

We'd also have to think through what events mean for stateless components. Presumably somehow the closest non-stateless ancestor would become the component that re-renders after an event. This is awkward because the ancestor might not know anything about this, and might have been coded in a way that strictly assumes it never re-renders. An easy design solution would be saying "stateless components can't have events", but that might be too limiting.

egil commented 3 months ago

We'd also have to think through what events mean for stateless components.

This is a good point. I was thinking about this from the perspective of users getting a way to express static render fragments without the gnarly syntax. However, component libraries may create stateless components and to the user of these, it may not be obvious that they are stateless and behave differently.

That said, if stateless components cannot have async code (my assumption), isn't the only way they can handle e.g. an @onclick event by having a EventCallback parameter, which is hooked up to the event binding?

Thus, the user of a stateless component will have to pass a EventCallback callback to the stateless component for it to be bound. Then it is obvious to the user that a re-render may occur after the callback is invoked.

E.g.:

// <StatelessButton>
<button @onclick=@Click>Click</button>
@code {
  [Parameter]
  public EventCallback Click{ get; set; }
}

Usage of <StatelessButton>:

<h1>Counter</h1>
<p>
    Current count: @currentCount
</p>
<StatelessButton Click="IncrementCount" />
@code {
  int currentCount = 0;
  void IncrementCount()
  {
    currentCount++;
  }
}
SteveSandersonMS commented 3 months ago

That said, if stateless components cannot have async code (my assumption)

There's nothing to stop us allowing stateless components to include async event handlers. The asynchrony is not the problem.

The only restriction is that stateless components can't render independently of their parent, since the whole point of a stateless component is that it doesn't have an independent rendertree buffer, doesn't show up as a separate component in a renderbatch, and doesn't independently diff against prior output. Its render output is inlined into the parent's output.

In practise, this means you don't want them to have async lifecycle methods (that would be very awkward, as they would have to cause their parent to re-render when the returned Task completes - potentially devastating for perf when hundreds of siblings are all making their common parent and hence all siblings re-render, or even causing an infinite render loop).

But is it also confusing if stateless components have event handlers, and we re-render the parent on the event as if it was an event on the parent? I don't personally think that is hugely confusing, and is probably more useful than saying you can't have real events (or are limited to handlers calling delegates passed in from parents).

SteveSandersonMS commented 3 months ago

There might even be a case for having the naming be @inline rather than @stateless to better emphasize what's really going on.

egil commented 3 months ago

But is it also confusing if stateless components have event handlers, and we re-render the parent on the event as if it was an event on the parent? I don't personally think that is hugely confusing, and is probably more useful than saying you can't have real events (or are limited to handlers calling delegates passed in from parents).

I don't find that confusing at all. I would view stateless components as just another "native HTML element". If I bind an event handler to an event on an element, my components will re-render after the event handler is triggered. I think it is natural to have the same experience when binding an event to a child component.

There might even be a case for having the naming be @inline rather than @stateless to better emphasize what's really going on.

It may be an english-as-second-language barrier, but "inline" seems more ambiguous to me. I also think "stateless" makes it more clear what functionality should be inside a stateless component.