Split from #3587806: Add placeholdering to help and branding blocks.
The system does not get placeholdered, so its cache items are looked up one by one instead of via multiple get and set. Originally placeholdering was enabled for the messages block in #3587806: Add placeholdering to help and branding blocks, but that resulted in the messages being added to the bottom of the page when rendered with big pipe. Messages block placeholdering was reverted in that issue to be addressed here separately.
Capturing conversation on Slack between @godotislate and @catch about possible root cause and fix:
After some investigation the StatusMessages render element already does the placeholdering and lazy building and adds big pipe to the deny list. In which case, the MessageBlock probably can just implement CacheOptionalInterface, so that a render cache item will not be stored for the block. Since status messages are placeholdered already, rendering the block should be cheap, and the cache tags should still bubble to response as expected.
Need manual testing to confirm that after the change, status messages still render as expected on the page and not at the bottom. Confirmed, see #6.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | messages_block.png | 45.26 KB | dcam |
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
Comments
Comment #3
godotislateMR https://git.drupalcode.org/project/drupal/-/merge_requests/16035
Comment #4
godotislateComment #5
godotislateComment #6
dcam commentedI tested this by first reproducing the bug that occurred in the other issue. I manually re-added
createPlaceholder()to the message block and reinstalled the site from the standard profile. The confirmation message after usingdrush uliappeared at the bottom of the page.I reverted that function addition, applied MR 16035, and followed the same test procedure above. The block was not placeholdered and appeared at the top of the page as shown in the following screenshot which is a split-screen image of the rendered page and its HTML.
This looks like it's behaving correctly to me.
I went a step further and checked the number of entries in the
cache_rendertable: once on the main branch and once on the MR branch. The CID counts were 11 and 10, respectively. So I did have one less cache entry with this MR, confirming the results of the automated tests.Comment #7
dcam commentedComment #8
catchHad a vague memory we'd tried to do this before and discovered twig template rendering was potentially worse than the cache get, and then found #3516051: Add CacheOptionalInterface to more blocks where @berdir was exploring the same thing and ran some numbers.
I guess the slight difference here is now this is the only block that's not placeholdered we're also saving the cache tag lookup (at least in core, but potentially also on real sites especially if we make placeholdering the default soon). Moving back to needs review for now.