Use MessagesCommand in BigPipe to remove special casing of the messages placeholder

Messages will appear as soon as they're available on big pipe responses, instead of all being added at the end of the request.

🤯

I had not even thought of that! Tempts me to tag this too 🤓

It 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 📌 Add PHP Fibers support to BigPipe RTBC (which might be nearly RTBC?!?!), so not in a huge hurry.

That's AWESOME news! 😄

OK OK CCF failures I was in a hurry :(

The last submitted patch, 7: 3379885.patch, failed testing. View results →

The last submitted patch, 7: 3379885.patch, failed testing. View results →

This may also fix whatever is going on in 💬 Status messages block not showing while BigPipe module enabled in contrib/custom admin theme Postponed: needs info .

OK 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 a 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 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.

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.

Ready for review now.

This looks amazing! Simpler, better performance, better usability too! 🤩

Just one nit and one question.

Beautiful.

Discussed 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.

Committed and pushed 44c345dd9b to 11.x and 2172d1009c to 10.3.x. Thanks!

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.

Just 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).

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

FYI, this is the cause of 🐛 10.3 upgrade now missing status-message theme suggestions Postponed