-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
SSR as library #46935
SSR as library #46935
Conversation
…ching. Allows the caller more control over fine-grained sync logic.
34b5b70
to
7a48877
Compare
Thank you for your API proposal. I'm removing the |
API proposal at #47018 |
/// <param name="store">The <see cref="IPersistentComponentStateStore"/> to restore the application state from.</param> | ||
/// <param name="dispatcher">The <see cref="Dispatcher"/> corresponding to the components' renderer.</param> | ||
/// <returns>A <see cref="Task"/> that will complete when the state has been restored.</returns> | ||
public Task PersistStateAsync(IPersistentComponentStateStore store, Dispatcher dispatcher) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think we need this API. We are the only ones that can provide a ComponentStatePersistenceManager to the system. Or am I missing something here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a layering thing. The existing caller is in Microsoft.AspNetCore.Mvc.TagHelpers
and it can't see the internals of M.A.Components
. It's same reason that you added the other PersistStateAsync
overload before.
This will cover both #38114 and be a basis for passive rendering for Blazor United. It's like the older prerendering logic but tidied up and enabling more control over asynchrony/dispatch.
New public APIs
All new:
Added:
namespace Microsoft.AspNetCore.Components.Infrastructure; public class ComponentStatePersistenceManager { public Task PersistStateAsync(IPersistentComponentStateStore store, Renderer renderer) + public Task PersistStateAsync(IPersistentComponentStateStore store, Dispatcher dispatcher) }
This addition is because the prerendering code no longer has a
Microsoft.AspNetCore.Components.RenderTree.Renderer
instance (because we don't want to expose the renderer internals), but it does have the associatedDispatcher
which is all thatComponentStatePersistenceManager
actually needs. We could mark the olderPersistStateAsync
API as obsolete but I don't know whether that would accomplish much. Opinions welcome.Performance
This PR removes one of the layers of buffering in the output. It no longer prerenders into a
ViewBuffer
, but instead supplies anIHtmlContent
object that writes the components' HTML directly from theRenderTreeFrame
buffer to the response'sTextWriter
. On the surface that sounds likely to improve perf (less work, less copying going on), but it still warrants verifying. Using this new benchmark, running in the perf infrastructure, I got the following results:Conclusion: This PR has a marginal positive effect on performance, but only in the region of 3%. That's fine since the point of this PR isn't about perf but rather is to add a new public API and prepare us for the new Blazor United SSR features.