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

fix: [Demo apps] Infinite rendering loop in TableOfContents #424

Closed
andreisaperski opened this issue Jun 2, 2023 · 6 comments
Closed

fix: [Demo apps] Infinite rendering loop in TableOfContents #424

andreisaperski opened this issue Jun 2, 2023 · 6 comments

Comments

@andreisaperski
Copy link
Contributor

🐛 Bug Report

OnAfterRenderAsync (in TableOfContents.razor.cs) calls QueryDom() which in its turn loads anchors and calls StateHasChanged() - rendering starts again.

💻 Repro or Code Sample

FluentUI.Demo.Client/FluentUI.Demo.Server in examples.
Browser's tab with https://www.fluentui-blazor.net/ consumes ~20% of CPU on my PC even if there's no any interaction (clicks/scroll)

🤔 Expected Behavior

Re-rendering doesn't happen all the time

😯 Current Behavior

TableOfContents keeps re-rendering without any reason.

💁 Possible Solution

Looks like QueryDom() should call StateHasChanged() only if anchors have changed e.g.:

    private async Task QueryDom()
    {
        var foundAnchors = await _jsModule.InvokeAsync<Anchor[]?>("queryDomForTocEntries");

        if (AnchorsEqual(_anchors, foundAnchors))
        {
            return;
        }

        _anchors = foundAnchors;
        StateHasChanged();
    }

    private bool AnchorsEqual(Anchor[]? firstSet, Anchor[]? secondSet)
    {
        return (firstSet ?? Array.Empty<Anchor>())
            .SequenceEqual(secondSet ?? Array.Empty<Anchor>());
    }

Anchor record needs Equal implemented because of child anchors:

        public virtual bool Equals(Anchor? other)
        {
            if (other is null) return false;

            if (Level != other.Level ||
                Text != other.Text ||
                Href != other.Href ||
                (Anchors?.Length ?? 0) != (other.Anchors?.Length ?? 0))
            {
                return false;
            }

            if (Anchors is not null &&
                Anchors.Length > 0)
            {
                for (var i = 0; i < Anchors.Length; i++)
                {
                    if (!Anchors[i].Equals(other.Anchors![i]))
                    {
                        return false;
                    }
                }
            }

            return true;
        }

🔦 Context

🌍 Your Environment

  • OS & Device: [e.g. MacOS, iOS, Windows, Linux] on [iPhone 7, PC]
  • Browser [e.g. Microsoft Edge, Google Chrome, Apple Safari, Mozilla FireFox]
  • .NET and FAST Version [e.g. 1.8.0]
@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 2, 2023

Oops, this got introduced with one of my latest changes.
Your solution looks good. I'll implement that

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 3, 2023

@andreisaperski locally your solution works on Server but not on Client. Have you tested on Client? Is it working for you there?

@andreisaperski
Copy link
Contributor Author

Yeah, i check on Client (FluentUI.Demo.Client), Server (FluentUI.Demo.Server) and Hybrid (MAUI).
I've just added logging to QueryDom() and retested Client:

        Logger.LogInformation($"QueryDom [{NavigationManager.Uri} - {DateTime.Now:HH:mm:ss}]");

With commented out check - effectively without fix

        //if (AnchorsEqual(_anchors, newAnchors))
        //{
        //    return;
        //}

the log reflects ~130 executions of QueryDom() (re-renders) per second:

image

With uncommented check, the log shows that QueryDom() executes a few times after navigation

image

When i uncommented check, i hit F5 and FluentUI.Demo.Client started with previously built version of FluentUI.Demo.Shared. I had to do Rebuild of FluentUI.Demo.Client to have it running with latest changes in FluentUI.Demo.Shared. Maybe you have the same situation - FluentUI.Demo.Client starts with FluentUI.Demo.Shared (dll) built before fix

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 3, 2023

Yes, I see that it works in a sense that it is not running continuously with your fix. But it is also not showing the actual table of contents after reading the markdown file. And that is what we want it to do.

@andreisaperski
Copy link
Contributor Author

Could you please point me to a page where it doesn't work. i checked markdown-based pages What's new?/Project setup/Code setup/Design tokens and ToC there matched with pages' structure. Also check that these pages raise OnRefreshTableOfContents, DemoMainLayout handles it and calls TableOfContents.Refresh() which in its turn calls QueryDom()

image

@vnbaaij
Copy link
Collaborator

vnbaaij commented Jun 3, 2023

For me it goes wrong on every page that loads Markdown to display contents, ie DesignTokens and CodeSetup

@vnbaaij vnbaaij mentioned this issue Jun 4, 2023
6 tasks
@vnbaaij vnbaaij closed this as completed in 9d21e4c Jun 4, 2023
@vnbaaij vnbaaij mentioned this issue Jun 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants