Problem/Motivation

If there are slow placeholdered blocks then it can take a significant amount of time until the top bar shows up. In my project, there are 8 or more blocks (menus, local tasks and actions, breadcrumb, content blocks) that are in the bigpipe pipeline before that.

I can't yet fully reproduce the behavior I'm seeing on one projects where the main navigation blocks is really slow, way slower than I'd expect, but it could be for whatever reason.

Steps to reproduce

Proposed resolution

As a quickfix, use the denylist feature to ensure that the top bar is always rendered with the initial response but still use a lazy builder.

As an alternative option, we could use a bigpipe placeholder to reserve the top space , this could be a follow-up. One challenge with that is that the top bar isn't displayed on every page.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3570693

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

berdir created an issue. See original summary.

berdir opened merge request !14571

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs review
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue summary: View changes
berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Issue summary: View changes
smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Change LGTM but needs a rebase for the performance.

Funny enough I noticed today the drastic shift.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

Rebased.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

LGTM

  • catch committed 6f004fe1 on 11.x 
    task: #3570693 Do not render navigation top bar with bigpipe to avoid...

  • catch committed b97e4d6d on main 
    task: #3570693 Do not render navigation top bar with bigpipe to avoid...
catch’s picture

catch
he/him
Primary language English
commented
Version: main » 11.x-dev
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and 11.x, thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !14571

  • catch committed 690131f0 on 11.x 
    Revert "task: #3570693 Do not render navigation top bar with bigpipe to...

  • catch committed 090abaf4 on main 
    Revert "task: #3570693 Do not render navigation top bar with bigpipe to...
catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

At least one more test needs to be updated.

berdir opened merge request !14831

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

Updated the test and reopened the MR.

godotislate’s picture

godotislate
he/him
commented
Status: Needs review » Reviewed & tested by the community

Changes are the same as before, except with the additional changes to core/profiles/demo_umami/tests/src/FunctionalJavascript/AssetAggregationAcrossPagesTest.php, and tests are all green. lgtm

  • catch committed f8997066 on main 
    task: #3570693 Do not render navigation top bar with bigpipe to avoid...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to main and 11.x, thanks!

Ran into merge conflicts on 11.x, probably due to deprecation removals - went ahead and updated the test numbers locally prior to to commit on 11.x

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • catch committed 3ec3efe8 on 11.x 
    task: #3570693 Do not render navigation top bar with bigpipe to avoid...

catch closed merge request !14831

  • catch committed 2bdaff9d on 11.x 
    Revert "task: #3570693 Do not render navigation top bar with bigpipe to...
catch’s picture

catch
he/him
Primary language English
commented
Status: Fixed » Needs work

Seeing fails in CI in 11.x that I can't reproduce locally, rather than keeping hotfixing let's do an 11.x backport MR here. Left the main commit though since that seems fine.

godotislate opened merge request !14851

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Reviewed & tested by the community

11.x MR https://git.drupalcode.org/project/drupal/-/merge_requests/14851

I think there was a merge conflict previously bc there's a missing commit from #3443571: Mobile version of Navigation should have focus trap to 11.x.

Since the MR diff is essentially identical to this commit on main https://git.drupalcode.org/project/drupal/-/commit/f89970669b83547059cea..., putting this into RTBC.

  • catch committed 434ae341 on 11.x 
    task: #3570693 Do not render navigation top bar with bigpipe to avoid...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

Also opened #3574944: Allow placeholders to only be served by non-js big pipe

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

catch closed merge request !14851

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.