Skip to content

Commit

Permalink
Stop processing original response streaming content if user has start…
Browse files Browse the repository at this point in the history
…ed navigating away (#50814)

* Reproduce #50733 as a failing E2E test

* Don't process original request blazor-ssr content if the user has already navigated away

* Quarantine (actually comment out) one of the CanRenderComponentWithPersistedState cases

See #50810

* Update EventTest.cs

* Disable another flaky test
  • Loading branch information
SteveSandersonMS authored Sep 20, 2023
1 parent acdb4fd commit 79b23f4
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ private void SendBatchAsStreamingUpdate(in RenderBatch renderBatch, TextWriter w
}

// Now process the list, skipping any we've already visited in an earlier iteration
var isEnhancedNavigation = IsProgressivelyEnhancedNavigation(_httpContext.Request);
for (var i = 0; i < componentIdsInDepthOrder.Length; i++)
{
var componentId = componentIdsInDepthOrder[i].ComponentId;
Expand All @@ -132,7 +133,7 @@ private void SendBatchAsStreamingUpdate(in RenderBatch renderBatch, TextWriter w
// as it is being written out.
writer.Write($"<template blazor-component-id=\"");
writer.Write(componentId);
writer.Write("\">");
writer.Write(isEnhancedNavigation ? "\" enhanced-nav=\"true\">" : "\">");

// We don't need boundary markers at the top-level since the info is on the <template> anyway.
WriteComponentHtml(componentId, writer, allowBoundaryMarkers: false);
Expand Down
2 changes: 1 addition & 1 deletion src/Components/Web.JS/dist/Release/blazor.web.js

Large diffs are not rendered by default.

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

import { SsrStartOptions } from '../Platform/SsrStartOptions';
import { NavigationEnhancementCallbacks, performEnhancedPageLoad, replaceDocumentWithPlainText } from '../Services/NavigationEnhancement';
import { NavigationEnhancementCallbacks, hasNeverStartedAnyEnhancedPageLoad, performEnhancedPageLoad, replaceDocumentWithPlainText } from '../Services/NavigationEnhancement';
import { isWithinBaseUriSpace, toAbsoluteUri } from '../Services/NavigationUtils';
import { synchronizeDomContent } from './DomMerging/DomSync';

Expand Down Expand Up @@ -36,7 +36,14 @@ class BlazorStreamingUpdate extends HTMLElement {
if (node instanceof HTMLTemplateElement) {
const componentId = node.getAttribute('blazor-component-id');
if (componentId) {
insertStreamingContentIntoDocument(componentId, node.content);
// For enhanced nav page loads, we automatically cancel the response stream if another enhanced nav supersedes it. But there's
// no way to cancel the original page load. So, to avoid continuing to process <blazor-ssr> blocks from the original page load
// if an enhanced nav supersedes it, we must explicitly check whether this content is from the original page load, and if so,
// ignore it if any enhanced nav has started yet. Fixes https://github.com/dotnet/aspnetcore/issues/50733
const isFromEnhancedNav = node.getAttribute('enhanced-nav') === 'true';
if (isFromEnhancedNav || hasNeverStartedAnyEnhancedPageLoad()) {
insertStreamingContentIntoDocument(componentId, node.content);
}
} else {
switch (node.getAttribute('type')) {
case 'redirection':
Expand Down
4 changes: 4 additions & 0 deletions src/Components/Web.JS/src/Services/NavigationEnhancement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ export function isPageLoading() {
return performingEnhancedPageLoad || document.readyState === 'loading';
}

export function hasNeverStartedAnyEnhancedPageLoad() {
return !currentEnhancedNavigationAbortController;
}

export function attachProgressivelyEnhancedNavigationListener(callbacks: NavigationEnhancementCallbacks) {
navigationEnhancementCallbacks = callbacks;
document.addEventListener('click', onDocumentClick);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,4 +222,50 @@ static async Task<string> BandwidthThrottledGet(string url, int chunkLength, int
}
}
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public async void StopsProcessingStreamingOutputFromPreviousRequestAfterEnhancedNav(bool duringEnhancedNavigation)
{
IWebElement originalH1Elem;

if (duringEnhancedNavigation)
{
Navigate($"{ServerPathBase}/nav");
originalH1Elem = Browser.Exists(By.TagName("h1"));
Browser.Equal("Hello", () => originalH1Elem.Text);
Browser.Exists(By.TagName("nav")).FindElement(By.LinkText("Streaming")).Click();
}
else
{
Navigate($"{ServerPathBase}/streaming");
originalH1Elem = Browser.Exists(By.TagName("h1"));
}

// Initial "waiting" state
Browser.Equal("Streaming Rendering", () => originalH1Elem.Text);
var getStatusText = () => Browser.Exists(By.Id("status"));
var getDisplayedItems = () => Browser.FindElements(By.TagName("li"));
var addItemsUrl = Browser.FindElement(By.Id("add-item-link")).GetDomProperty("href");
var endResponseUrl = Browser.FindElement(By.Id("end-response-link")).GetDomProperty("href");
Assert.Equal("Waiting for more...", getStatusText().Text);
Assert.Empty(getDisplayedItems());
Assert.StartsWith("http", addItemsUrl);

// Navigate away using enhanced nav, before the response is completed
Browser.Exists(By.TagName("nav")).FindElement(By.LinkText("Streaming with interactivity")).Click();
Browser.Equal("Streaming Rendering with Interactivity", () => originalH1Elem.Text);
var statusElem = Browser.FindElement(By.Id("status"));
Assert.Equal("Not streaming", statusElem.Text);

// Now if the earlier navigation produces more output, we do *not* add it to the page
var addItemsOutput = await new HttpClient().GetStringAsync(addItemsUrl);
Assert.Equal("Added item", addItemsOutput);
await Task.Delay(1000); // Make sure we would really have seen the UI change by now if it was going to
Browser.Equal("Not streaming", () => statusElem.Text); // It didn't get removed from the doc, nor was its text updated

// Tidy up
await new HttpClient().GetAsync(endResponseUrl);
}
}
4 changes: 2 additions & 2 deletions src/Components/test/E2ETest/Tests/EventTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,8 @@ public void DragDrop_CanTrigger()
var actions = new Actions(Browser).DragAndDrop(input, target);

actions.Perform();
// drop doesn't seem to trigger in Selenium. But it's sufficient to determine "any" drag event works
Browser.Equal("dragstart,", () => output.Text);
// drop doesn't reliably trigger in Selenium. But it's sufficient to determine "any" drag event works
Browser.True(() => output.Text.StartsWith("dragstart,", StringComparison.Ordinal));
}

[Fact]
Expand Down
2 changes: 1 addition & 1 deletion src/Components/test/E2ETest/Tests/StatePersistenceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public override Task InitializeAsync()
[InlineData(false, typeof(InteractiveServerRenderMode), "ServerStreaming")]
[InlineData(false, typeof(InteractiveWebAssemblyRenderMode), (string)null)]
[InlineData(false, typeof(InteractiveWebAssemblyRenderMode), "WebAssemblyStreaming")]
[InlineData(false, typeof(InteractiveAutoRenderMode), (string)null)]
// [InlineData(false, typeof(InteractiveAutoRenderMode), (string)null)] https://github.com/dotnet/aspnetcore/issues/50810
// [InlineData(false, typeof(InteractiveAutoRenderMode), "AutoStreaming")] https://github.com/dotnet/aspnetcore/issues/50810
public void CanRenderComponentWithPersistedState(bool suppressEnhancedNavigation, Type renderMode, string streaming)
{
Expand Down

0 comments on commit 79b23f4

Please sign in to comment.