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
Comment #2
jefuri commentedThe 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?
Comment #3
jefuri commentedComment #4
jefuri commentedUploaded 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.
Comment #6
jefuri commentedThe 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.
Comment #7
wim leersThanks 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? 😊
Comment #8
jefuri commentedAlready 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?
Comment #9
jefuri commentedOk 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?
Comment #10
jefuri commentedSo 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?
Comment #11
jefuri commentedComment #12
wim leersI 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!
Yes, BigPipe Sessionless only works with BigPipe placeholders that don't require JS. This is documented in
\Drupal\big_pipe_sessionless\Render\Placeholder\BigPipeSessionlessStrategy:See
\Drupal\big_pipe_sessionless\StackMiddleware\BigPipeSessionlessPageCacheand the*.services.ymlfile. It sounds like perhaps the container has not been rebuilt? Or something is interfering with thepage_cachemodule, which BigPipe Sessionless requires/builds on top of?Comment #13
droplet commented@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!)
Comment #14
jefuri commentedI 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?
Comment #15
wim leersIIRC 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.
Comment #16
wim leersFYI: #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?
Comment #17
nod_Core issue is now fixed for 9.5+ branches :)
Comment #18
wim leers🥳
Indeed! That means this can be marked too!