-
Notifications
You must be signed in to change notification settings - Fork 10.3k
[Blazor] Prerendered state #50742
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
[Blazor] Prerendered state #50742
Conversation
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.
The approach looks great to me 👍
26212fd
to
bbb0e5d
Compare
/// </summary> | ||
/// <param name="renderMode">The <see cref="IComponentRenderMode"/> in question.</param> | ||
/// <returns><c>true</c> if the render mode is supported by the store, otherwise <c>false</c>.</returns> | ||
bool SupportsRenderMode(IComponentRenderMode renderMode) => true; |
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.
Adding an interface member, even with a default implementation, is technically considered a breaking change as per https://learn.microsoft.com/en-us/dotnet/core/compatibility/library-change-rules but that doc isn't specific about the cases where it can fail. Perhaps it's fine 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.
I think it's fine in this case, alternatively, we can do this with a callback or a separate interface.
src/Components/Components/src/Infrastructure/ComponentStatePersistenceManager.cs
Show resolved
Hide resolved
|
||
if (ssrRenderBoundary is null) | ||
{ | ||
throw new InvalidOperationException("Cannot infer render mode."); |
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.
This seems like a bug. For SSR components, there will be no enclosing boundary. How come this doesn't throw in practice? Are there tests covering what happens if purely SSR components try to persist state?
src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.PrerenderingState.cs
Outdated
Show resolved
Hide resolved
src/Components/Endpoints/src/Rendering/SSRRenderModeBoundary.cs
Outdated
Show resolved
Hide resolved
Log.InvalidComponentTypeForUpdate(_logger, message: "Component type mismatch."); | ||
} | ||
|
||
_ = Renderer.UpdateRootComponentAsync(operation.ComponentId.Value, descriptor.Parameters); |
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.
Why is it OK to discard the task 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.
Any exception that happens will be dispatched through the UnhandledExceptionHandler in the remote renderer
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.
OK, could you add a comment to that effect?
} | ||
} | ||
|
||
pendingTasks = new Task[operations.Length]; |
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.
The use of an array, rather than List<Task>
, is pretty surprising. There will often be gaps in the array then, since some of the operations don't add tasks. Why not use List<Task>
, at least to avoid forcing future devs to figure out if the gaps serve some purpose?
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.
We are on the first update, and we just validated that all operations are Add
, so we know the exact number of tasks we need to capture and there won't be any gaps.
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.
Oh right, I see. It would be more obvious if the variable name was pendingTasksForFirstRender
rather than just pendingTasks
, so it's clearer the code below behaves how it does due to the assumptions in the code above.
{ | ||
if (!_serverComponentSerializer.TryDeserializeRootComponentOperations(serializedComponentOperations, out var operations)) | ||
{ | ||
// Return error |
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.
TODO?
@@ -25,7 +26,7 @@ public DefaultAntiforgeryStateProvider(PersistentComponentState state) | |||
{ | |||
state.PersistAsJson(PersistenceKey, _currentToken); | |||
return Task.CompletedTask; | |||
}); | |||
}, new InteractiveAutoRenderMode()); |
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.
Does this mean we're going to emit prerendered state for WebAssembly even if you're not using WebAssembly? In fact, even if you're not using any interactive rendering at all?
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.
No, it's covered in a later commit, but essentially if you only do AddWebAssemblyRenderMode
or AddServerRenderMode
we only emit the comment for webassembly or server respectively.
If you have declared that you are using both, then we will emit it. I haven't done it, but we can look at the set of actual render modes that a component used during a request and narrow down to a single render mode if possible too, in that situation.
The point here, is that auto means, "wherever it's actually needed" as opposed to all locations.
src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHost.cs
Outdated
Show resolved
Hide resolved
|
||
private void BlockWebAssemblyResourceLoad() | ||
{ | ||
((IJavaScriptExecutor)Browser).ExecuteScript("sessionStorage.setItem('block-load-boot-resource', 'true')"); |
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.
What is this used for? I can't see anything that reads the value.
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 forces the app to start on Blazor Server when auto is enabled on the page. Other similar tests follow this approach.
src/Components/test/testassets/Components.TestServer/Components - Backup.TestServer.csproj
Outdated
Show resolved
Hide resolved
src/Components/test/testassets/Components.TestServer/Components.TestServer.csproj
Outdated
Show resolved
Hide resolved
src/Components/test/testassets/Components.TestServer/Properties/launchSettings.json
Outdated
Show resolved
Hide resolved
src/Components/test/testassets/Components.TestServer/RazorComponentEndpointsStartup.cs
Outdated
Show resolved
Hide resolved
...nts/test/testassets/Components.TestServer/RazorComponents/Pages/PersistStateComponents.razor
Outdated
Show resolved
Hide resolved
...nts/test/testassets/Components.TestServer/RazorComponents/Pages/PersistStateComponents.razor
Outdated
Show resolved
Hide resolved
...estassets/Components.TestServer/RazorComponents/Pages/PersistentState/EndStreamingPage.razor
Outdated
Show resolved
Hide resolved
...ssets/TestContentPackage/PersistentComponents/NonStreamingComponentWithPersistentState.razor
Outdated
Show resolved
Hide resolved
da6950c
to
26efa93
Compare
src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.Prerendering.cs
Show resolved
Hide resolved
@@ -270,6 +270,74 @@ private bool IsWellFormedServerComponent(ComponentMarker record) | |||
return (componentDescriptor, serverComponent); | |||
} | |||
|
|||
public bool TryDeserializeRootComponentOperations(string serializedComponentOperations, out (RootComponentOperation, ComponentDescriptor?)[] operations) |
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.
We should check for the uniqueness of the Update/Delete operations to ensure that there is at most one update/delete per component ID.
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.
Excellent! I understand there are still some tests to add but we've been through this in enough detail now so I'm fine with approving. Please let me know if anything meaningful still has to change about the runtime code.
…plications. Persists state both for server and webassembly as necessary. Initializes the state when a given interactive runtime is initialized and renders the first set of components. On WebAssembly, this is the first time the app starts. On Server this happens every time a circuit starts. The state is available during the first render, until the components reach quiescence. The approach we follow is different for server and webassembly: On Server, we support initializing the circuit with an empty set of descriptors and in that case, we delay initialization until the first UpdateRootComponents call is issued. This is because it's hard to deal with the security constraints imposed by starting a new circuit multiple times, and its easier to handle them within UpdateRootComponents. We might switch this approach in the future to go through StartCircuit too. On WebAssembly, we query for the initial set of webassembly components when we are starting the runtime in a Blazor Web Scenario. We do this because Blazor WebAssembly offers a programatic API to render root components at a given location defined by their selectors, so we need to make sure that those components can receive state at the same time the initial set of WebAssembly components added to the page. There are a set of tests validating different behaviors with regards to enhanced navigation and streaming rendering, as well as making sure that auto mode can access the state on Server and WebAssembly, and that Server gets new state every time a circuit is opened.
a957206
to
bc2eb3b
Compare
I'm running the test host, but I can't figure out how to navigate to the page that is testing this. All I get is |
* Revert "Remove hardcoded System.Security.Cryptography.Xml version (#48029)" (#50723) This reverts commit 42d14c4. * [Blazor] Prerendered state (#50742) [Blazor] Adds support for persting prerendered state on Blazor Web applications. * Persists state both for server and webassembly as necessary. * Initializes the state when a given interactive runtime is initialized and renders the first set of components. * On WebAssembly, this is the first time the app starts. * On Server this happens every time a circuit starts. * The state is available during the first render, until the components reach quiescence. The approach we follow is different for server and webassembly: * On Server, we support initializing the circuit with an empty set of descriptors and in that case, we delay initialization until the first `UpdateRootComponents` call is issued. * This is because it's hard to deal with the security constraints imposed by starting a new circuit multiple times, and its easier to handle them within UpdateRootComponents. We might switch this approach in the future to go through `StartCircuit` too. * On WebAssembly, we query for the initial set of webassembly components when we are starting the runtime in a Blazor Web Scenario. * We do this because Blazor WebAssembly offers a programatic API to render root components at a given location defined by their selectors, so we need to make sure that those components can receive state at the same time the initial set of WebAssembly components added to the page. There are a set of tests validating different behaviors with regards to enhanced navigation and streaming rendering, as well as making sure that auto mode can access the state on Server and WebAssembly, and that Server gets new state every time a circuit is opened. * Make IEmailSender more customizable (#50301) * Make IEmailSender more customizable * Remove unnecessary metadata * Add TUser parameter * React to API review feedback * Fix IdentitySample.DefaultUI * Update branding to RTM (#50799) --------- Co-authored-by: Igor Velikorossov <[email protected]> Co-authored-by: Javier Calvarro Nelson <[email protected]> Co-authored-by: Stephen Halter <[email protected]> Co-authored-by: William Godbe <[email protected]>
[Blazor] Adds support for persting prerendered state on Blazor Web applications.
The approach we follow is different for server and webassembly:
UpdateRootComponents
call is issued.StartCircuit
too.There are a set of tests validating different behaviors with regards to enhanced navigation and streaming rendering, as well as making sure that auto mode can access the state on Server and WebAssembly, and that Server gets new state every time a circuit is opened.