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
35.5k stars 10.04k forks source link

Support persistent component state across enhanced page navigations #51584

Open javiercn opened 1 year ago

javiercn commented 1 year ago

This issue tracks adding a new mechanism to deliver state updates from enhanced navigations coming from the server to running runtimes.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

Markz878 commented 1 year ago

This could lead to developers (at least me) making some mistakes about assuming which branch of the TryTakeFromJson condition is run at which point. In my app I initialized a SignalR connection in the true branch (cause I thought it would always run on the WASM client side), and was confused when the connection was sometimes initialized and sometimes not. This also led to double database queries when navigating inside the app to the WASM page. Hopefully this is fixed ASAP.

oliverw commented 1 year ago

This could lead to developers (at least me) making some mistakes about assuming which branch of the TryTakeFromJson condition is run at which point. In my app I initialized a SignalR connection in the true branch (cause I thought it would always run on the WASM client side), and was confused when the connection was sometimes initialized and sometimes not. This also lead to double database queries when navigating inside the app to the WASM page. Hopefully this is fixed ASAP.

I agree. Not fixing this for a LTS release is a mistake IMO.

oliverw commented 1 year ago

Has anyone come up with a workaround for this yet? The intense flickering on page load when navigating is a truly awful UX.

oliverw commented 1 year ago

In case anyone is interested. I have created a repo both demonstrating the problem and implementing a workaround. If anyone improves on that please let me know: https://github.com/oliverw/BlazorCustomApplicationStateApp

I should add that this is a Band-Aid at best and not a particularly good one:

Markz878 commented 1 year ago

Here is a GIF of a demo site I made, that shows the behavior on slow connections (I used fast 3G browser throttle). The first main page showing the topics is server rendered, and the pages showing the messages use WASM interactivity. Even though the WASM pages use the PersistComponentState mechanism, the mechanism is only used randomly. First and third navigation in this GIF utilize the state persistence, and second and fourth navigation fetch the data from the API, causing page flickering and user having to wait for the API call, which leads to bad UX. Please make the state persist mechanism consistent. StatePersistBug

oliverw commented 1 year ago

Here is a GIF of a demo site I made, that shows the behavior on slow connections (I used fast 3G browser throttle). The first main page showing the topics is server rendered, and the pages showing the messages use WASM interactivity. Even though the WASM pages use the PersistComponentState mechanism, the mechanism is only used randomly. Every other navigation in this GIF uses the state persistence, and every other navigation has to fetch the data from the API, causing page flickering and user having to wait for the API call, which leads to bad UX. Please make the state persist mechanism consistent. StatePersistBug StatePersistBug

Might be worth giving my workaround a shot.

Markz878 commented 1 year ago

I managed to solve my issue by moving from full WASM interactivity page to using a server rendered page, and having interactive elements as WASM components, to which I pass the required state as parameters. It removes any client side data fetching needs, usage of PersistComponentState and therefore the page flickering.

Don't know why I hadn't tried passing state from a server rendered component to a WASM component previously (too stuck on previous hosted WASM model I guess), now I'm just wondering how on earth is the state actually persisted when passing parameters to WASM from server... (Edit: I see that the parameter state is deserialized into the prerendered html, makes sense. Still I hope that the PersistComponentState would work more consistently, having all sorts of random bugs with it when using it in some places)

ghost commented 11 months ago

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

guardrex commented 11 months ago

@javiercn ... Due to the importance of this subject, I'm going to place a cross-link to this issue for further dev discussion, particularly so devs can find a workaround approach. If you have a workaround approach that the topic can describe and will be supported, then we can add it to the topic and drop the cross-link to this issue. I'm 👂 if you want to sanction something. If not, we can leave the cross-link in place until .NET 9 lands.

https://github.com/dotnet/AspNetCore.Docs/pull/31276

oliverw commented 11 months ago

@javiercn ... Due to the importance of this subject, I'm going to place a cross-link to this issue for further dev discussion, particularly so devs can find a workaround approach. If you have a workaround approach that the topic can describe, then perhaps we could add it to the topic, saying that it's a workaround, and drop the cross-link to this issue. I'm 👂 if you want to sanction something. If not, we can leave the cross-link in place until .NET 9 lands.

@Markz878's workaround is working pretty well.

ShawnTheBeachy commented 6 months ago
  • State is rendered twice on initial page load. One time by PersistentComponentState and another copy by the workaround.

Is there any reason to keep the PersistentComponentState with this workaround, though? It seems to me as if your workaround does what PersistComponentState does -- but better.

I did build off of your example to try to reduce the boilerplate required. Here's what I ended up with:

app.js

function getInnerText(id) {
    return document.getElementById(id).innerText;
}

PersistentComponent.cs

public abstract class PersistentComponentBase<TState> : ComponentBase
    where TState : new()
{
    [Inject]
    private IJSRuntime Js { get; set; } = default!;
    protected TState State { get; } = new();
    protected abstract string StateKey { get; }

    protected override void BuildRenderTree(RenderTreeBuilder builder) =>
        builder.AddMarkupContent(1, Serialize());

    protected virtual Task InitializeStateAsync() => Task.CompletedTask;

    protected override async Task OnInitializedAsync()
    {
        if (!OperatingSystem.IsBrowser())
        {
            await InitializeStateAsync();
            return;
        }

        // When using InvokeAsync there can still be a flash. Use Invoke if possible.
        var stateJson = Js is IJSInProcessRuntime inProcJs
            ? inProcJs.Invoke<string?>("getInnerText", [StateKey])
            : await Js.InvokeAsync<string?>("getInnerText", [StateKey]);

        if (string.IsNullOrWhiteSpace(stateJson))
        {
            await InitializeStateAsync();
            return;
        }

        try
        {
            var buffer = Convert.FromBase64String(stateJson);
            var json = Encoding.UTF8.GetString(buffer);
            RestoreState(JsonSerializer.Deserialize<TState>(json)!);
        }
        catch
        {
            await InitializeStateAsync();
        }
    }

    protected abstract void RestoreState(TState state);

    private string Serialize()
    {
        if (State is null || OperatingSystem.IsBrowser())
            return "";

        var json = JsonSerializer.SerializeToUtf8Bytes(State);
        var base64 = Convert.ToBase64String(json);
        return $"<script id=\"{StateKey}\" type=\"text/template\">{base64}</script>";
    }
}

MyComponent.razor

@inherits PersistentComponentBase<State>

...

@{ base.BuildRenderTree(__builder); }

MyComponent.razor.cs

protected override string StateKey => "my.state";

protected override async Task InitializeStateAsync()
{
    // Load from database or whatever.
}

protected override void RestoreState(State state)
{
    // Rehydrate state.
}
oliverw commented 6 months ago
  • State is rendered twice on initial page load. One time by PersistentComponentState and another copy by the workaround.

Is there any reason to keep the PersistentComponentState with this workaround, though? It seems to me as if your workaround does what PersistComponentState does -- but better.

I did build off of your example to try to reduce the boilerplate required. Here's what I ended up with:

app.js

function getInnerText(id) {
    return document.getElementById(id).innerText;
}

PersistentComponent.cs

public abstract class PersistentComponentBase<TState> : ComponentBase
    where TState : new()
{
    [Inject]
    private IJSRuntime Js { get; set; } = default!;
    protected TState State { get; } = new();
    protected abstract string StateKey { get; }

    protected override void BuildRenderTree(RenderTreeBuilder builder) =>
        builder.AddMarkupContent(1, Serialize());

    protected virtual Task InitializeStateAsync() => Task.CompletedTask;

    protected override async Task OnInitializedAsync()
    {
        if (!OperatingSystem.IsBrowser())
        {
            await InitializeStateAsync();
            return;
        }

        var stateJson = await Js.InvokeAsync<string?>("getInnerText", [StateKey]);

        if (string.IsNullOrWhiteSpace(stateJson))
        {
            await InitializeStateAsync();
            return;
        }

        try
        {
            var buffer = Convert.FromBase64String(stateJson);
            var json = Encoding.UTF8.GetString(buffer);
            RestoreState(JsonSerializer.Deserialize<TState>(json)!);
        }
        catch
        {
            await InitializeStateAsync();
        }
    }

    protected abstract void RestoreState(TState state);

    private string Serialize()
    {
        if (State is null || OperatingSystem.IsBrowser())
            return "";

        var json = JsonSerializer.SerializeToUtf8Bytes(State);
        var base64 = Convert.ToBase64String(json);
        return $"<script id=\"{StateKey}\" type=\"text/template\">{base64}</script>";
    }
}

MyComponent.razor

@inherits PersistentComponentBase<State>

...

@{ base.BuildRenderTree(__builder); }

MyComponent.razor.cs

protected override string StateKey => "my.state";

protected override async Task InitializeStateAsync()
{
    // Load from database or whatever.
}

protected override void RestoreState(State state)
{
    // Rehydrate state.
}

Don't use that. @Markz878's workaround works well with none of the downsides.

ShawnTheBeachy commented 6 months ago

Don't use that. @Markz878's workaround works well with none of the downsides.

Unless I'm missing something, that only works until you have a nested component loading data. Then you're back to square one. It's the approach I took at first, but it doesn't work for very complex applications.

jamescarter-le commented 2 months ago

What is the recommended navigation mode for Blazor if this bug (or limitation) is not high priority?

It seems like Enhanced Navigation is the equivalent of SPA like behavior for Blazor Server apps, PersistentComponentState although cumbersome and boilerplate, does work to prevent double invocations of expensive calls, but having to hand-code invidual workarounds for different types of data for Enhanced navigation feels like a large hole currently.

Should we not be using Enhanced Navigation if I assume that MS are not building apps this way if they feel like this is not important?

YuriyDurov commented 1 week ago

Just wanted to do a quick drive-by - for anyone struggling with this, we have built a nuget package that allows persisting component state between prerender and the subsequent interactive render. It uses the approaches suggested and discussed in this issue. It may not be an ideal solution, but may help if anyone needs this functionality before it is implemented in the framework.