Skip to content
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

Progressively enhanced navigation #48899

Merged
merged 30 commits into from
Jun 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
b40c713
Factor out parts of NavigationManager.ts that will be reused
SteveSandersonMS Jun 16, 2023
b4d0d0a
Update DomSync logic to handle entire documents including doctype nodes
SteveSandersonMS Jun 16, 2023
f1ec76f
Fix an obvious bug in DomSync.ts
SteveSandersonMS Jun 16, 2023
87ca0fa
Most basic case of progressively-enhanced nav
SteveSandersonMS Jun 16, 2023
2bce561
Cancel enhanced nav if a second navigation starts
SteveSandersonMS Jun 16, 2023
c082517
Error handling
SteveSandersonMS Jun 16, 2023
9a25a40
Scroll to hash
SteveSandersonMS Jun 16, 2023
d3c8284
Refactor towards streaming SSR support
SteveSandersonMS Jun 16, 2023
8aa3698
Integrate with streaming SSR
SteveSandersonMS Jun 16, 2023
464ae89
Clean up hash handling
SteveSandersonMS Jun 16, 2023
2b09179
Minor cleanup
SteveSandersonMS Jun 16, 2023
1654593
Auto-bypass PE nav when there's an interactive router
SteveSandersonMS Jun 19, 2023
2e91fff
Clean up notes about interaction
SteveSandersonMS Jun 19, 2023
c78e62b
Start up interactive components after enhanced nav. This is not a com…
SteveSandersonMS Jun 19, 2023
a1da413
Initial E2E test
SteveSandersonMS Jun 19, 2023
500af89
E2E tests for streaming SSR + enhanced nav
SteveSandersonMS Jun 19, 2023
683e870
More E2E cases
SteveSandersonMS Jun 19, 2023
1b070e7
E2E test for scrolling to hash
SteveSandersonMS Jun 19, 2023
b1b1b2a
Fix typo
SteveSandersonMS Jun 19, 2023
8b039c6
Another comment fix
SteveSandersonMS Jun 19, 2023
a9791ed
Respect the disableDomPreservation flag
SteveSandersonMS Jun 20, 2023
22a772a
Fix build
SteveSandersonMS Jun 20, 2023
bd15759
Fix E2E test disposal issue
SteveSandersonMS Jun 20, 2023
8191949
Support navigations, plus further test case for errors
SteveSandersonMS Jun 20, 2023
75323b1
Update .js
SteveSandersonMS Jun 20, 2023
faf48ba
Fix error handling E2E test
SteveSandersonMS Jun 20, 2023
d447d5a
Support external redirections too
SteveSandersonMS Jun 20, 2023
e9fe08e
Update .js
SteveSandersonMS Jun 20, 2023
0c93607
Trigger CI
SteveSandersonMS Jun 20, 2023
af827ac
Update unit tests
SteveSandersonMS Jun 20, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public Task RenderComponent()
private async Task RenderComponentCore()
{
_context.Response.ContentType = RazorComponentResultExecutor.DefaultContentType;
_renderer.InitializeStreamingRenderingFraming(_context);

if (!await TryValidateRequestAsync(out var isPost, out var handler))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,33 @@ private static ValueTask<PrerenderedComponentHtmlContent> HandleNavigationExcept
"Navigation commands can not be issued during server-side prerendering after the response from the server has started. Applications must buffer the" +
"response and avoid using features like FlushAsync() before all components on the page have been rendered to prevent failed navigation commands.");
}
else if (IsPossibleExternalDestination(httpContext.Request, navigationException.Location) && httpContext.Request.Headers.ContainsKey("blazor-enhanced-nav"))
{
// It's unsafe to do a 301/302/etc to an external destination when this was requested via fetch, because
// assuming it doesn't expose CORS headers, we won't be allowed to follow the redirection nor will
// we even find out what the destination URL would have been. But since it's our own JS code making this
// fetch request, we can have a custom protocol for describing the URL we wanted to redirect to.
httpContext.Response.Headers.Add("blazor-enhanced-nav-redirect-location", navigationException.Location);
return new ValueTask<PrerenderedComponentHtmlContent>(PrerenderedComponentHtmlContent.Empty);
}
else
{
httpContext.Response.Redirect(navigationException.Location);
return new ValueTask<PrerenderedComponentHtmlContent>(PrerenderedComponentHtmlContent.Empty);
}
}

private static bool IsPossibleExternalDestination(HttpRequest request, string destinationUrl)
{
if (!Uri.TryCreate(destinationUrl, UriKind.Absolute, out var absoluteUri))
{
return false;
}

return absoluteUri.Scheme != request.Scheme
|| absoluteUri.Authority != request.Host.Value;
}

internal static ServerComponentInvocationSequence GetOrCreateInvocationId(HttpContext httpContext)
{
if (!httpContext.Items.TryGetValue(ComponentSequenceKey, out var result))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Runtime.InteropServices;
using System.Text.Encodings.Web;
using Microsoft.AspNetCore.Components.RenderTree;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
Expand All @@ -13,8 +14,25 @@ namespace Microsoft.AspNetCore.Components.Endpoints;

internal partial class EndpointHtmlRenderer
{
private const string _progressivelyEnhancedNavRequestHeaderName = "blazor-enhanced-nav";
private const string _streamingRenderingFramingHeaderName = "ssr-framing";
private TextWriter? _streamingUpdatesWriter;
private HashSet<int>? _visitedComponentIdsInCurrentStreamingBatch;
private string? _ssrFramingCommentMarkup;

public void InitializeStreamingRenderingFraming(HttpContext httpContext)
{
if (httpContext.Request.Headers.ContainsKey(_progressivelyEnhancedNavRequestHeaderName))
{
var id = Guid.NewGuid().ToString();
httpContext.Response.Headers.Add(_streamingRenderingFramingHeaderName, id);
_ssrFramingCommentMarkup = $"<!--{id}-->";
}
else
{
_ssrFramingCommentMarkup = string.Empty;
}
}

public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilTaskCompleted, TextWriter writer)
{
Expand All @@ -26,10 +44,16 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
throw new InvalidOperationException($"{nameof(SendStreamingUpdatesAsync)} can only be called once.");
}

if (_ssrFramingCommentMarkup is null)
{
throw new InvalidOperationException("Cannot begin streaming rendering because no framing header was set.");
}

_streamingUpdatesWriter = writer;

try
{
await writer.WriteAsync(_ssrFramingCommentMarkup);
await writer.FlushAsync(); // Make sure the initial HTML was sent
await untilTaskCompleted;
}
Expand All @@ -39,10 +63,12 @@ public async Task SendStreamingUpdatesAsync(HttpContext httpContext, Task untilT
}
catch (Exception ex)
{
// Theoretically it might be possible to let the error middleware run, capture the output,
// then emit it in a special format so the JS code can display the error page. However
// for now we're not going to support that and will simply emit a message.
HandleExceptionAfterResponseStarted(_httpContext, writer, ex);

// The rest of the pipeline can treat this as a regular unhandled exception
// TODO: Is this really right? I think we'll terminate the response in an invalid way.
await writer.FlushAsync(); // Important otherwise the client won't receive the error message, as we're about to fail the pipeline
await _httpContext.Response.CompleteAsync();
throw;
}
}
Expand Down Expand Up @@ -115,6 +141,7 @@ private void SendBatchAsStreamingUpdate(in RenderBatch renderBatch, TextWriter w
}

writer.Write("</blazor-ssr>");
writer.Write(_ssrFramingCommentMarkup);
}
}

Expand Down Expand Up @@ -143,16 +170,16 @@ private static void HandleExceptionAfterResponseStarted(HttpContext httpContext,
? exception.ToString()
: "There was an unhandled exception on the current request. For more details turn on detailed exceptions by setting 'DetailedErrors: true' in 'appSettings.Development.json'";

writer.Write("<template blazor-type=\"exception\">");
writer.Write(message);
writer.Write("</template>");
writer.Write("<blazor-ssr><template type=\"error\">");
writer.Write(HtmlEncoder.Default.Encode(message));
writer.Write("</template></blazor-ssr>");
}

private static void HandleNavigationAfterResponseStarted(TextWriter writer, string destinationUrl)
{
writer.Write("<template blazor-type=\"redirection\">");
writer.Write(destinationUrl);
writer.Write("</template>");
writer.Write("<blazor-ssr><template type=\"redirection\">");
writer.Write(HtmlEncoder.Default.Encode(destinationUrl));
writer.Write("</template></blazor-ssr>");
}

protected override void WriteComponentHtml(int componentId, TextWriter output)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ internal static Task RenderComponentToResponse(
var endpointHtmlRenderer = httpContext.RequestServices.GetRequiredService<EndpointHtmlRenderer>();
return endpointHtmlRenderer.Dispatcher.InvokeAsync(async () =>
{
endpointHtmlRenderer.InitializeStreamingRenderingFraming(httpContext);

// We could pool these dictionary instances if we wanted, and possibly even the ParameterView
// backing buffers could come from a pool like they do during rendering.
var hostParameters = ParameterView.FromDictionary(new Dictionary<string, object?>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ await RazorComponentResultExecutor.RenderComponentToResponse(

// Assert
Assert.Equal(
$"<!--bl:X-->Some output\n<!--/bl:X--><template blazor-type=\"redirection\">https://test/somewhere/else</template>",
$"<!--bl:X-->Some output\n<!--/bl:X--><blazor-ssr><template type=\"redirection\">https://test/somewhere/else</template></blazor-ssr>",
MaskComponentIds(GetStringContent(responseBody)));
}

Expand Down Expand Up @@ -269,18 +269,18 @@ public async Task OnUnhandledExceptionAfterResponseStarted_WithStreamingOn_Emits
httpContext.Response.Body = responseBody;

var expectedResponseExceptionInfo = isDevelopmentEnvironment
? "System.InvalidTimeZoneException: Test message"
: "There was an unhandled exception on the current request. For more details turn on detailed exceptions by setting 'DetailedErrors: true' in 'appSettings.Development.json'";
? "System.InvalidTimeZoneException: Test message with &lt;b&gt;markup&lt;/b&gt;"
: "There was an unhandled exception on the current request. For more details turn on detailed exceptions by setting &#x27;DetailedErrors: true&#x27; in &#x27;appSettings.Development.json&#x27;";

// Act
var ex = await Assert.ThrowsAsync<InvalidTimeZoneException>(() => RazorComponentResultExecutor.RenderComponentToResponse(
httpContext, typeof(StreamingComponentThatThrowsAsynchronously),
null, preventStreamingRendering: false));

// Assert
Assert.Contains("Test message", ex.Message);
Assert.Contains("Test message with <b>markup</b>", ex.Message);
Assert.Contains(
$"<!--bl:X-->Some output\n<!--/bl:X--><template blazor-type=\"exception\">{expectedResponseExceptionInfo}",
$"<!--bl:X-->Some output\n<!--/bl:X--><blazor-ssr><template type=\"error\">{expectedResponseExceptionInfo}",
MaskComponentIds(GetStringContent(responseBody)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ Some output
protected override async Task OnInitializedAsync()
{
await Task.Yield();
throw new InvalidTimeZoneException("Test message");
throw new InvalidTimeZoneException("Test message with <b>markup</b>");
}
}
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.server.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.web.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.webview.js

Large diffs are not rendered by default.

26 changes: 22 additions & 4 deletions src/Components/Web.JS/src/Boot.Web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,31 +15,49 @@ import { shouldAutoStart } from './BootCommon';
import { Blazor } from './GlobalExports';
import { WebStartOptions } from './Platform/WebStartOptions';
import { attachStreamingRenderingListener } from './Rendering/StreamingRendering';
import { attachProgressivelyEnhancedNavigationListener, detachProgressivelyEnhancedNavigationListener } from './Services/NavigationEnhancement';
import { WebAssemblyComponentDescriptor } from './Services/ComponentDescriptorDiscovery';
import { ServerComponentDescriptor, discoverComponents } from './Services/ComponentDescriptorDiscovery';

let started = false;
let webStartOptions: Partial<WebStartOptions> | undefined;

async function boot(options?: Partial<WebStartOptions>): Promise<void> {
if (started) {
throw new Error('Blazor has already started.');
}
started = true;
await activateInteractiveComponents(options);
webStartOptions = options;

attachStreamingRenderingListener(options?.ssr);

if (!options?.ssr?.disableDomPreservation) {
attachProgressivelyEnhancedNavigationListener(activateInteractiveComponents);
}

await activateInteractiveComponents();
}

async function activateInteractiveComponents(options?: Partial<WebStartOptions>) {
async function activateInteractiveComponents() {
const serverComponents = discoverComponents(document, 'server') as ServerComponentDescriptor[];
const webAssemblyComponents = discoverComponents(document, 'webassembly') as WebAssemblyComponentDescriptor[];

if (serverComponents.length) {
await startCircuit(options?.circuit, serverComponents);
// TEMPORARY until https://github.com/dotnet/aspnetcore/issues/48763 is implemented
// As soon we we see you have interactive components, we'll stop doing enhanced nav even if you don't have an interactive router
// This is because, otherwise, we would need a way to add new interactive root components to an existing circuit and that's #48763
detachProgressivelyEnhancedNavigationListener();

await startCircuit(webStartOptions?.circuit, serverComponents);
}

if (webAssemblyComponents.length) {
await startWebAssembly(options?.webAssembly, webAssemblyComponents);
// TEMPORARY until https://github.com/dotnet/aspnetcore/issues/48763 is implemented
// As soon we we see you have interactive components, we'll stop doing enhanced nav even if you don't have an interactive router
// This is because, otherwise, we would need a way to add new interactive root components to an existing WebAssembly runtime and that's #48763
detachProgressivelyEnhancedNavigationListener();

await startWebAssembly(webStartOptions?.webAssembly, webAssemblyComponents);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function synchronizeAttributes(destination: Element, source: Element) {

// Optimize for the common case where all attributes are unchanged and are even still in the same order
const destAttrsLength = destAttrs.length;
if (destAttrsLength === destAttrs.length) {
if (destAttrsLength === sourceAttrs.length) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was there a bug here?

Copy link
Member Author

@SteveSandersonMS SteveSandersonMS Jun 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I could backport the fix and add another test case to the earlier PR, but it didn't seem worthwhile when both of those things are going in on the same day with this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's ok, as long as we have a test to cover it, that's good enough

let hasDifference = false;
for (let i = 0; i < destAttrsLength; i++) {
const sourceAttr = sourceAttrs.item(i)!;
Expand Down
14 changes: 10 additions & 4 deletions src/Components/Web.JS/src/Rendering/DomMerging/DomSync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { applyAnyDeferredValue } from '../DomSpecialPropertyUtil';
import { synchronizeAttributes } from './AttributeSync';
import { UpdateCost, ItemList, Operation, computeEditScript } from './EditScript';

export function synchronizeDomContent(destination: CommentBoundedRange | Element, newContent: DocumentFragment | Element) {
export function synchronizeDomContent(destination: CommentBoundedRange | Node, newContent: Node) {
let destinationParent: Node;
let nextDestinationNode: Node | null;
let originalNodesForDiff: ItemList<Node>;

// Figure out how to interpret the 'destination' parameter, since it can come in two very different forms
if (destination instanceof Element) {
if (destination instanceof Node) {
destinationParent = destination;
nextDestinationNode = destination.firstChild;
originalNodesForDiff = destination.childNodes;
Expand Down Expand Up @@ -70,7 +70,7 @@ export function synchronizeDomContent(destination: CommentBoundedRange | Element

// Handle any common trailing items
// These can only exist if there were some edits, otherwise everything would be in the set of common leading items
const endAtNodeExclOrNull = destination instanceof Element ? null : destination.endExclusive;
const endAtNodeExclOrNull = destination instanceof Node ? null : destination.endExclusive;
while (nextDestinationNode !== endAtNodeExclOrNull) {
treatAsMatch(nextDestinationNode!, nextNewContentNode!);
nextDestinationNode = nextDestinationNode!.nextSibling;
Expand All @@ -94,6 +94,9 @@ function treatAsMatch(destination: Node, source: Node) {
applyAnyDeferredValue(destination as Element);
synchronizeDomContent(destination as Element, source as Element);
break;
case Node.DOCUMENT_TYPE_NODE:
// See comment below about doctype nodes. We leave them alone.
break;
default:
throw new Error(`Not implemented: matching nodes of type ${destination.nodeType}`);
}
Expand Down Expand Up @@ -130,6 +133,10 @@ function domNodeComparer(a: Node, b: Node): UpdateCost {
// For the converse (forcing retention, even if that means reordering), we could post-process the list of
// inserts/deletes to find matches based on key to treat those pairs as 'move' operations.
return (a as Element).tagName === (b as Element).tagName ? UpdateCost.None : UpdateCost.Infinite;
case Node.DOCUMENT_TYPE_NODE:
// It's invalid to insert or delete doctype, and we have no use case for doing that. So just skip such
// nodes by saying they are always unchanged.
return UpdateCost.None;
default:
// For anything else we know nothing, so the risk-averse choice is to say we can't retain or update the old value
return UpdateCost.Infinite;
Expand Down Expand Up @@ -169,4 +176,3 @@ class SiblingSubsetNodeList implements ItemList<Node> {
this.length = this.endIndexExcl - this.startIndex;
}
}

24 changes: 24 additions & 0 deletions src/Components/Web.JS/src/Rendering/StreamingRendering.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.

import { SsrStartOptions } from "../Platform/SsrStartOptions";
import { performEnhancedPageLoad } from "../Services/NavigationEnhancement";
import { isWithinBaseUriSpace } from "../Services/NavigationUtils";
import { synchronizeDomContent } from "./DomMerging/DomSync";

let enableDomPreservation = true;
Expand Down Expand Up @@ -34,6 +36,28 @@ class BlazorStreamingUpdate extends HTMLElement {
const componentId = node.getAttribute('blazor-component-id');
if (componentId) {
insertStreamingContentIntoDocument(componentId, node.content);
} else {
switch (node.getAttribute('type')) {
case 'redirection':
// We use 'replace' here because it's closest to the non-progressively-enhanced behavior, and will make the most sense
// if the async delay was very short, as the user would not perceive having been on the intermediate page.
const destinationUrl = node.content.textContent!;
if (isWithinBaseUriSpace(destinationUrl)) {
history.replaceState(null, '', destinationUrl);
performEnhancedPageLoad(destinationUrl);
} else {
location.replace(destinationUrl);
}
break;
case 'error':
// This is kind of brutal but matches what happens without progressive enhancement
document.documentElement.textContent = node.content.textContent;
const docStyle = document.documentElement.style;
docStyle.fontFamily = 'consolas, monospace';
docStyle.whiteSpace = 'pre-wrap';
docStyle.padding = '1rem';
break;
}
}
}
});
Expand Down
Loading