I noticed a regression in the 11.x branch between 11.2 and 11.3: If a placeholder is in a noscript tag, the bigpipe javascript (and htmx) will throw an uncaught TypeError.
On the 11.x branch, enable BigPipe module, and in a custom module, add a placeholder to a noscript tag. For example,
$page_bottom['my_noscript_tag'] = [
'#type' => 'html_tag',
'#tag' => 'noscript',
'child' => [
'#create_placeholder' => TRUE,
'#lazy_builder' => [MyTrustedCallback::class . '::getImage', []],
],
];
[Note, For easier debugging, I am using htmx.js rather than htmx.min.js]
When loading the page with a logged-in user, Firefox (or Chromium) will throw
Uncaught (in promise) TypeError: can't access property "dispatchEvent", elt is null
with stack trace:
triggerEvent /core/assets/vendor/htmx/htmx.js?v=2.0.4:3043
insert /core/modules/big_pipe/js/big_pipe.commands.js?v=11.3-dev:60
commandExecutionQueue /core/modules/big_pipe/js/big_pipe.commands.js?v=11.3-dev:31
(Async: promise callback)
commandExecutionQueue /core/modules/big_pipe/js/big_pipe.commands.js?v=11.3-dev:24
reduce self-hosted:217
commandExecutionQueue /core/modules/big_pipe/js/big_pipe.commands.js?v=11.3-dev:21
processReplacement /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:69
checkMutationAndProcess /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:94
forEach self-hosted:157
processMutations /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:118
forEach self-hosted:157
processMutations /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:117
(Async: MutationCallback)
<anonymous> /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:134
<anonymous> /core/modules/big_pipe/js/big_pipe.js?v=11.3-dev:166In this case, in Drupal 11.2 and earlier, bigpipe did not throw an error.
For obvious reasons we do not require that javascript replaces a placeholder in a noscript tag. We only need it to be replaced - by other means - when javascript is disabled.
So, in cases where the element can't be found and target === null, I would say bigpipe could simply stop processing this placeholder?
Show commandsStart 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:
changes, plain diff MR !13415
Comments
Comment #3
mfbComment #4
catchI think this would mean that no other placeholder strategy would find the placeholder either.
Wondering if bigpipe's placeholder creation should try to special case no script placeholders somehow.
Comment #5
mfbComment #6
mfbNot sure what you mean by this - the non-javascript placeholdering has always worked fine - the placeholders are replaced on server side.
I did find a work-around I could use on Drupal 11.3 and later: I can instead have the noscript tag be part of the replacement markup, instead of wrapping the placeholder. Then, of course the javascript can find the placeholder, because it's not in a noscript tag.
However, rather than having to work around the issue, I think it would be best to avoid throwing here - just don't try to process the target if it can't be found.
Comment #7
mfbMarkup in a noscript tag is by definition already special-cased, because it's not interpreted as being part of the dom. And since it's only relevant when javascript is disabled, handling replacement on server side is fine.
I don't think there anything special that bigpipe should do, except just not error in any rare cases where the target can't be found.
Comment #8
mfbActually I realized my attempted work-around I mentioned in #6 doesn't work, because the noscript tag is added to the dom, and whatever is inside actually loads. So, I think my MR is the best solution: bigpipe javascript should just ignore any situation where target is null.
Comment #9
catchWhat I mean is - I would assume that if something has already been replaced server side, then it would not be sent to the browser, so would not be a target at all.
Comment #10
mfbYes that would be correct. The exception is only thrown in the browser when it was not done server side.
There is no way to tell big pipe "only do this replacement server side because it's in a noscript tag", but such complexity isn't needed. We can just keep things working like they did historically: don't throw if the browser can't find a placeholder.
Comment #11
catch@mfb ah OK - but doesn't that mean nothing would replace that placeholder then?
Comment #12
mfbI just double-checked this and there is no change between 11.2 and the 11.x branch except that the TypeError is thrown. BigPipe JS also can't find a placeholder in a <noscript> tag in 11.2 - I had never actually noticed this fact before, since it wasn't a problem.. :)
Comment #13
nod_Tried it out and that is the appropriate fix. Even if it's not in a noscript element it just makes sense to check if the element exists before trying to run things off it.
Comment #14
catch#12 makes sense and explains some of my confusion.
Committed/pushed to 11.x, thanks!