Problem/Motivation

After upgrading to Drupal 11.3.1 from 11.2.10, I noticed that if there are multiple BigPipe placeholders with identical IDs, only one seems to get replaced. The insert() command in big_pipe.commands.js currently only performs its action for a single target matching the selector, which seems to be a change from what happened before.

Steps to reproduce

1. Create two identical render arrays, both of which use #create_placeholder to make a placeholder.
2. Ensure that BigPipe is being used to render the content for the placeholders (log in, clear caches, etc.).
3. Load the page, and notice that only one of the placeholders has been replaced, instead of all of them being replaced.

Proposed resolution

Modify the insert() command function in core/modules/big_pipe/js/big_pipe.commands.js to use htmx.findAll() to locate every target matching the selector, and then loop over all the targets and apply the action to each.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#4 bigpipe.patch1.78 KBartem_kondra

Issue fork drupal-3564880

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

drumlin44 created an issue. See original summary.

artem_kondra made their first commit to this issue’s fork.

artem_kondra changed the visibility of the branch 3564880-bigpipe-placeholders-with to hidden.

artem_kondra’s picture

artem_kondra commented
StatusFileSize
new bigpipe.patch1.78 KB

avpaderno changed the visibility of the branch 3564880-bigpipe-placeholders-with to active.

drumlin44’s picture

drumlin44 commented

The patch in #4 works for me -- thanks!

quietone’s picture

quietone commented
Version: 11.3.x-dev » 11.x-dev

Changes are made on 11.x first and then backported.

scott_euser’s picture

scott_euser commented
Status: Active » Needs work

This seemed to sort the issue for us as well. Moving to NW because it needs an MR and I imagine some test coverage to demonstrate.
Priority-wise perhaps this is 'major' unless we consider disabling big pipe a valid alternative?

catch’s picture

catch
he/him
Primary language English
commented
Priority: Normal » Major

Definitely major.

godotislate made their first commit to this issue’s fork.

godotislate opened merge request !14298

godotislate’s picture

godotislate
he/him
commented
Status: Needs work » Needs review

Converted patch to MR and added test: https://git.drupalcode.org/project/drupal/-/merge_requests/14298

Test-only failure: https://git.drupalcode.org/issue/drupal-3564880/-/jobs/7945791

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Added a question, otherwise the solution looks correct to me.

fathershawn’s picture

fathershawn
Primary language English
Location New York
commented
Status: Needs review » Reviewed & tested by the community

The tests are clearly passing. I also manually compared the behavior of big_pipe_test.big_pipe_test_multi_occurrence manually with both 11.3.2 and the MR branch. The problem and the fix are double verified!

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

voleger’s picture

voleger
he/his
Primary language Ukrainian
Location Ukraine, Rivne
commented

Can confirm, with the MR patch, I can finally observe the replacement of the bigpipe placeholder with the user.toolbar_link_builder:renderToolbarLinks lazy builder markup on Drupal 11.3.2 project and Gin theme.

catch’s picture

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

Committed/pushed to main and cherry-picked to 11.x and 11.3.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 committed fe299100 on 11.3.x 
    fix: #3564880 BigPipe placeholders with identical IDs are not all...

  • catch committed 7bc15056 on 11.x 
    fix: #3564880 BigPipe placeholders with identical IDs are not all...

catch closed merge request !14298

  • catch committed 9091188c on main 
    fix: #3564880 BigPipe placeholders with identical IDs are not all...

Status: Fixed » Closed (fixed)

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