Problem/Motivation

When BigPipe renders multiple placeholders, it accumulates already-loaded libraries using array_merge() without deduplication:

$cumulative_assets->setAlreadyLoadedLibraries(array_merge(
    $cumulative_assets->getAlreadyLoadedLibraries(),
    $html_response->getAttachments()['library']
));

If the same library is attached in both the main page render and a lazy builder response, it accumulates twice. On a subsequent placeholder render, LibraryDependencyResolver::getMinimalRepresentativeSubset() asserts uniqueness on the resulting list and throws an AssertionError, which escapes BigPipe's catch (\Exception) blocks uncaught and leaves the response in an undefined state.

Observed duplicate libraries on sites running Gin admin theme with Flag module: gin/core_navigation, core/components.navigation--toolbar-button, flag/flag.link, flag/flag.link_ajax.

Steps to reproduce

  1. Enable big_pipe on a site with the Gin admin theme and the Flag module.
  2. Enable PHP assertions (zend.assertions = 1, assert.exception = 1).
  3. Visit any page as an admin user that renders Flag link lazy builders alongside the Gin navigation toolbar.
  4. Observe AssertionError: $libraries can't contain duplicate items from LibraryDependencyResolver::getMinimalRepresentativeSubset().

Proposed resolution

1. Deduplicate the cumulative library list:

$cumulative_assets->setAlreadyLoadedLibraries(array_values(array_unique(array_merge(
    $cumulative_assets->getAlreadyLoadedLibraries(),
    $html_response->getAttachments()['library']
))));

2. Replace catch (\Exception $e) with catch (\Throwable $e) in the placeholder rendering catch blocks in sendNoJsPlaceholders() and sendPlaceholders(), so that \Error subclasses are caught and logged via trigger_error() rather than escaping uncaught.

Issue fork drupal-3590969

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:

Comments

ezeedub created an issue. See original summary.

ezeedub’s picture

Status: Active » Needs review

ezeedub’s picture

Issue summary: View changes
needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new2.14 KB

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

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

nitinkumar_7’s picture

Status: Needs work » Needs review

I have updated the merge request to fix all the failed commit checks from the test :

Resolved PHPStan Errors: Removed the non-callable 'callback' array structure within the no-JS placeholders' #lazy_builder key. Since the mock class in ManyPlaceholderTest overrides renderPlaceholder() anyway (making the actual callbacks unneeded), we safely simplified these dummy structures to avoid PHPStan's callable validation checks.
Resolved PHPCS Errors: Added missing docblocks for the mock anonymous classes, overridden methods (renderPlaceholder(), filterEmbeddedResponse(), sendPlaceholders()), and class properties.
Both PHPCS checks and the unit tests are now fully passing locally. Setting the status back to Needs review.