Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Discovered in #3377570: Add PHP Fibers support to BigPipe.
When BigPipe was added to core, we didn't have MessagesCommand in core, see this comment:
// The 'status messages' placeholder needs to be special cased, because it
// depends on global state that can be modified when other placeholders are
// being rendered: any code can add messages to render.
// This violates the principle that each lazy builder must be able to render
// itself in isolation, and therefore in any order. However, we cannot
// change the way \Drupal\Core\Messenger\MessengerInterface::addMessage()
// works in the Drupal 8 cycle. So we have to accommodate its special needs.
// Allowing placeholders to be rendered in a particular order (in this case:
// last) would violate this isolation principle. Thus a monopoly is granted
// to this one special case, with this hard-coded solution.
// @see \Drupal\Core\Render\Element\StatusMessages
// @see \Drupal\Core\Render\Renderer::replacePlaceholders()
// @see https://www.drupal.org/node/2712935#comment-11368923
It's not the Drupal 8.0.x release cycle any more! MessagesCommand was added in 8.8 https://www.drupal.org/node/3086403
Steps to reproduce
Proposed resolution
After rendering a placeholder in BigPipe::renderPlaceholders(), use Messenger::deleteAll() to collect whatever messages are in session, then add them to the ajax response in a MessageCommand.
If we only do this, then hopefully the tests will continue to pass (there's extensive test coverage of the current logic), or will at least pass with minimal tweaks.
Then if that bit works, we can remove all the logic that keeps messages last in the placeholder stack, since it shouldn't matter when that placeholder gets replaced - we'll be using the AJAX system to add the messages as they come in, instead of relying on collecting $_SESSION at the end of the request to replace them all at once. This will then require some reworking of the tests.
This should have two advantages:
1. It'll simplify BigPipe module.
2. Messages will appear as soon as they're available on big pipe responses, instead of all being added at the end of the request. While it's uncommon for placeholders to set a message, this could end up improving largest contentful paint when the messages area is the largest item above the fold. If there's already a message ready to be displayed at the beginning of the request, it will still be rendered as part of the placeholder (i.e. not via MessagesCommand), with the difference that the messages placeholder will be rendered in order instead of last.
Because the messages placeholder is usually in the view port, this should help at least a little bit with with cumulative layout shift and largest contentful paint for responses with messages to display.
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3379885.patch | 3.24 KB | catch |
| #7 | 3379885.patch | 3.24 KB | catch |
| #5 | 3379885.patch | 3.51 KB | catch |
Issue fork drupal-3379885
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:
compare
Comments
Comment #2
catchComment #3
wim leers🤯
I had not even thought of that! Tempts me to tag this too 🤓
Comment #4
wim leersComment #5
catchIt works - not completely transparent but it's fine. Because the messages are added via the AJAX system, the resulting markup is slightly different, but it's just a small behaviour change, because we're changing the behaviour, not a regression.
BigPipeRegressionTest now passes for me locally.
BigPipeTest will fail because it's asserting hard-coded placeholder content, that'll need updating, but stopping for now so uploading what I've got.
@todos for next steps:
1. Fix the assertions in BigPipeTest to work with the new placeholder output.
2. Remove the special casing of the messages placeholder from everywhere + inject the message service into BigPipe.
3. Remove the test coverage for the special casing of the messages placeholder (but not all the other message tests!).
Also all the above is going to massively conflict with #3377570: Add PHP Fibers support to BigPipe (which might be nearly RTBC?!?!), so not in a huge hurry.
Comment #6
wim leersThat's AWESOME news! 😄
Comment #7
catchOK OK CCF failures I was in a hurry :(
Comment #10
wim leersThis may also fix whatever is going on in #3341735: Status messages block not showing while BigPipe module enabled in contrib/custom admin theme.
Comment #12
catchOK i went to the trouble of getting green tests first while the messages placeholder was still special-cased. This required adjusting some of the placeholder test cases since messages moved to the AJAX command.
Then I removed the special casing and the tests failed again - because with the messages placeholder rendered first, it directly shows the messages again, not via the AJAX command - which is expected behaviour if there are messages ready to show when the messages placeholder itself is rendered. So just had to revert the commit to get them to pass again.
So that was a bit two steps forward one step back, but the end result is the functional js tests include the AJAX command output and the functional tests include messages rendered by the placeholder, and we only have very minimal changes to the test coverage overall. Could have saved some work, but the end result is good I think.
This also means that messages really are shown as soon as they possibly can be per Wim's comment above - either as soon as the messages placeholder is rendered, or immediately via AJAX command if one is set by a placeholder. So should be a good usability improvement, as well as reducing cumulative layout shift and potentially largest contentful paint etc. i.e. previously if there were messages at the beginning of the request, they'd be rendered after everything else (often above the fold), now they're rendered before lower elements which might be pushed below the fold by the time they get rendered anyway. If there are messages to render via AJAX, that can result in some layout shift, but it'll be smaller/earlier/more often than if we replaced all at once at the end too.
Still need to inject the messaging service but that's the last change I know of - trying to do this incrementally, even though it's a big simplification, it's removing complex logic and assumptions so could have gone wrong.
Comment #13
catchReady for review now.
Comment #14
catchComment #15
catchComment #16
wim leersThis looks amazing! Simpler, better performance, better usability too! 🤩
Just one nit and one question.
Comment #17
wim leersBeautiful.
Comment #18
alexpottDiscussed with @catch.
There's one small interesting behaviour change. The new way of immediately adding messages via ajax means that you're more likely to get duplicate messages on the screen because you don't benefit from automatic deduping by the messaging system. If this turns out to be a big issue we can look to do the deduping in the messages AJAX.
Also it is very odd that the markup is different - but themes already have to cope with this so it is not new.
Given the benefits going to commit to 11.x and 10.3.x.
We considered a change record for the above behaviour changes but decided that it would be noise as there is nothing obviously actionable or an easy way to see if you will be affected.
Comment #19
alexpottCommitted and pushed 44c345dd9b to 11.x and 2172d1009c to 10.3.x. Thanks!
Comment #22
catchJust to say a caveat to this is that if you submit a form, the messages are added to session, and the next page renders them (the most likely way to get lots of duplicate messages), then the deduping logic will still kick in, because any messages in session are still rendered via PHP with the placeholder, not via AJAX. So this would only happen when messages are rendered after the messages placeholder itself is rendered (i.e. from another placeholder).
Comment #24
solideogloria commentedFYI, this is the cause of a regression:
#3456176: 10.3 upgrade now missing status-message theme suggestions