Problem/Motivation

We have some lazybuilders supplying additional libraries and drupal settings. Using this module the first page request the settings are missing and some JS functionality breaks in the website.

Steps to reproduce

Use drupal commerce with https://www.drupal.org/project/dc_ajax_add_cart ajax cart replace functionality.
Going to a product pagina, add it to the cart through the add to cart ajax form submission.
Cart block is not updated.

Proposed resolution

The current implementation of the "doProcessPlaceholders" use NoJS placeholders. Therefor the library for big_pipe.js is not added.
Couldn't we use the createBigPipeJsPlaceholder function? Or add the library of big_pipe.js for these requests to the page call?

<?php
/**
   * Transforms placeholders to BigPipe placeholders, only no-JS.
   *
   * Only no-JS placeholders to allow BigPipe to accelerate Page Cache misses.
   *
   * @param array $placeholders
   *   The placeholders to process.
   *
   * @return array
   *   The BigPipe placeholders.
   */
  protected function doProcessPlaceholders(array $placeholders) {
    $overridden_placeholders = [];
    foreach ($placeholders as $placeholder => $placeholder_elements) {
      $overridden_placeholders[$placeholder] = static::createBigPipeNoJsPlaceholder($placeholder, $placeholder_elements, static::placeholderIsAttributeSafe($placeholder));
    }

    return $overridden_placeholders;
  }
?>

Comments

jefuri created an issue. See original summary.

jefuri’s picture

The main thing I'm trying to understand is why createBigPipeJsPlaceholder isn't being use in Big Pipe Sessionless. The doProcessPlaceholders function in this module does not really explain it. Why only no-JS?

/**
   * Transforms placeholders to BigPipe placeholders, either no-JS or JS.
   *
   * @param array $placeholders
   *   The placeholders to process.
   *
   * @return array
   *   The BigPipe placeholders.
   */
  protected function doProcessPlaceholders(array $placeholders) {
    $overridden_placeholders = [];
    foreach ($placeholders as $placeholder => $placeholder_elements) {
      // BigPipe uses JavaScript and the DOM to find the placeholder to replace.
      // This means finding the placeholder to replace must be efficient. Most
      // placeholders are HTML, which we can find efficiently thanks to the
      // querySelector API. But some placeholders are HTML attribute values or
      // parts thereof, and potentially even plain text in DOM text nodes. For
      // BigPipe's JavaScript to find those placeholders, it would need to
      // iterate over all DOM text nodes. This is highly inefficient. Therefore,
      // the BigPipe placeholder strategy only converts HTML placeholders into
      // BigPipe placeholders. The other placeholders need to be replaced on the
      // server, not via BigPipe.
      // @see \Drupal\Core\Access\RouteProcessorCsrf::renderPlaceholderCsrfToken()
      // @see \Drupal\Core\Form\FormBuilder::renderFormTokenPlaceholder()
      // @see \Drupal\Core\Form\FormBuilder::renderPlaceholderFormAction()
      if (static::placeholderIsAttributeSafe($placeholder)) {
        $overridden_placeholders[$placeholder] = static::createBigPipeNoJsPlaceholder($placeholder, $placeholder_elements, TRUE);
      }
      else {
        // If the current request/session doesn't have JavaScript, fall back to
        // no-JS BigPipe.
        if ($this->requestStack->getCurrentRequest()->cookies->has(static::NOJS_COOKIE)) {
          $overridden_placeholders[$placeholder] = static::createBigPipeNoJsPlaceholder($placeholder, $placeholder_elements, FALSE);
        }
        else {
          $overridden_placeholders[$placeholder] = static::createBigPipeJsPlaceholder($placeholder, $placeholder_elements);
        }
        $overridden_placeholders[$placeholder]['#cache']['contexts'][] = 'cookies:' . static::NOJS_COOKIE;
      }
    }

    return $overridden_placeholders;
  }

jefuri’s picture

jefuri’s picture

Status: Active » Needs review

Uploaded a patch as POC to see if it will fail some test, but this seems to work on my development environment in my use case.
But I'm scared this has some kind of security implications that I do not see haha.

Status: Needs review » Needs work
jefuri’s picture

Status: Needs work » Needs review

The tests that fail are to be expected, because it's still checking for the former no-js big pipe placeholders, and expecting a different render array.

wim leers’s picture

Status: Needs review » Postponed (maintainer needs more info)

Thanks for the detailed bug report — much appreciated!

And massive kudos for even creating a patch to fix it!

That being said … this sounds like another case of #1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS. Could you please revert the patch you uploaded, apply the latest patch of that issue and check whether that fixes it? 😊

jefuri’s picture

Already thought about that, applied the patch from https://www.drupal.org/project/drupal/issues/1988968#comment-13618155 because we are still drupal 8.9.
That patch did not work, I'll try to reroll a more recent one then.

But why would this issue be the case with big pipe sessionless? Should drupal settings still get merged in if they are added through a no-js placeholder? But that's what seems to happening.
And why would big pipe sessionless be served through a no-js situation? Can't it be served with JS enabled?

jefuri’s picture

Ok the latest patch applies fine for drupal 8.9.x: https://www.drupal.org/project/drupal/issues/1988968#comment-13739867

And darn it, it works. Updating the patch to the most recent one did it. I did thought this might be the case but thought it should work with the older version.

I thought I would have a mental breakdown, because I could not seem to figure out the issue.

But I still need to know one thing before I go to bed, because it will keep me awake if I don't. Why does this module serve it with no-js placeholders?

jefuri’s picture

So it works locally, but not on our test environment.
I have no idea how to figure out what this might be, is there something I can debug on the frontend to see what happens when big pipe serves the content through sessionless?

jefuri’s picture

Status: Postponed (maintainer needs more info) » Needs review
wim leers’s picture

Title: Big Pipe Sessionless does not execute drupal settings or JS after streaming placeholders » BigPipe Sessionless does not execute drupalSettings or JS after streaming placeholders
Category: Bug report » Support request
Related issues: +#1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS

And darn it, it works.

I only skimmed this, because I short was on time, and wanted to give you some helpful pointer. That's what I did in #7. I'm glad to see my hunch was right!

Why does this module serve it with no-js placeholders?

Yes, BigPipe Sessionless only works with BigPipe placeholders that don't require JS. This is documented in \Drupal\big_pipe_sessionless\Render\Placeholder\BigPipeSessionlessStrategy:

 * To avoid a potential no-JS redirect, no-session requests always use no-JS
 * BigPipe placeholders.
…
 * 2. ::doProcessPlaceholders() is made a lot simpler: since the goal is to
 *    accelerate Page Cache misses (and to hence cause Page Cache hits for
 *    subsequent requests), we can only use no-JS BigPipe placeholders.
 *    Otherwise we would not be able to create a HTML response to store in Page
 *    Cache.
So it works locally, but not on our test environment.

See \Drupal\big_pipe_sessionless\StackMiddleware\BigPipeSessionlessPageCache and the *.services.yml file. It sounds like perhaps the container has not been rebuilt? Or something is interfering with the page_cache module, which BigPipe Sessionless requires/builds on top of?

droplet’s picture

@jefuri,

If you don't mind to zip a clean installs with DB for the buggy things, I can help you have a look :)
(no guarantees I will give you a fast reply or workaround. Just trying to understand & analysis the problem. I'm one of the contributor of #1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS. So your information will help that patch to move forward I believe. Thanks!)

jefuri’s picture

I don't know if that would help, seeing the situation seems to differ based on the hosting environment.
Locally in a dockerized situation it works, on an online test environment it does not.
But I have a guess... Maybe something with cors?

wim leers’s picture

But I have a guess...

IIRC it's more related to connection handling: all localhost/docker connections are to the same IP/webserver. So this results in a different fetching order than an "online" environment.

wim leers’s picture

Status: Needs review » Postponed (maintainer needs more info)

FYI: #1988968: Drupal.ajax does not guarantee that "add new JS file to page" commands have finished before calling said JS has been making progress! Maybe you can try to reproduce this with that applied?

nod_’s picture

Core issue is now fixed for 9.5+ branches :)

wim leers’s picture

Status: Postponed (maintainer needs more info) » Fixed

🥳

Indeed! That means this can be marked Fixed too!

Status: Fixed » Closed (fixed)

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