Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
\Drupal\webform\WebformEntityViewBuilder::view() force-disables the render cache by setting max-age to 0.
This class was there since the very first commit of the 8.x-5.x branch.
I don't think it's necessary, if there were caching problems then someone would have reported them when using the form directly and I can't see a reason why those two cases should be kept separate.
I guess we can start with removing it and seeing if any tests break :)
Comment | File | Size | Author |
---|---|---|---|
#11 | webform.webform.issue_2975902.yml | 3.69 KB | jrockowitz |
#10 | 2975902-10.patch | 1.46 KB | jrockowitz |
| |||
#7 | 2975902-7.patch | 2.28 KB | jrockowitz |
| |||
#7 | interdiff-2975902-2-7.txt | 1.33 KB | jrockowitz |
#2 | webform-entity-view-builder-cache-2975902-2.patch | 655 bytes | Berdir |
|
Comments
Comment #2
BerdirAmong other things, this actually disables the render cache of any entity this is embedded into as it bubbles up all the way up to the node and then dynamic page cache. (It's worth pointing out that forms in core actually do the same right now for authenticated users, so this will only make a difference for anonymous users).
Comment #3
BerdirIt's also interesting that \Drupal\webform\Plugin\Field\FieldFormatter\WebformEntityReferenceEntityFormatter::viewElements() calls $entity->getSubmissionForm() directly, so skips the view builder, which means this is *only* used for standard entity reference fields (Which we actually use in a paragraph type as the additional settings the webform field exposes isn't something we want to have there).
Comment #4
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented@Berdir Caching in D8 is tricky. I absolutely appreciate you help.
I can confirm that the max-age is set to 0 for authenticated users via \Drupal\Core\Form\FormBuilder::prepareForm
As you noted, my only concern would be the caching of a webform for an anonymous user when the webform is displayed in a node and block.
I did some manual testing and webform nodes and blocks are working as expected for anonymous users.
Comment #5
BerdirWhat field did you use for testing, a normal entity reference field or a webform field? As mentioned, the webform field formatter actually doesn't even use this and therefore doesn't have caching disabled.
Considering that, it seems pretty pretty safe to change?
For consistency, we should then also remove \Drupal\webform\Plugin\Block\WebformBlock::getCacheMaxAge().
Comment #6
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedWe should also remove the max-age=0 from \Drupal\webform\Element\Webform
I will write out a full (manual) test script to help to check that caching is working correctly for webform nodes and blocks.
Comment #7
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedAttached is a patch with more caching disabled.
Things to check
Notes
Comment #8
Berdir> Page cache should be enabled
max-age does not affect the anonymous page cache: #2352009: [pp-3] Bubbling of elements' max-age to the page's headers and the page cache. So you will not be able to see a difference with or without that actually.
The only thing you can see to confirm that it is working would be that you can see cache entries for the affected block/node in the cache_render table for anonymous users (but not for authenticated ones, because the form itself is still not cacheable until #2578855: Form tokens are now rendered lazily, allow forms to opt in to be cacheable is fixed).
One possible option for that would be to make the form a lazy-builder component, which should actually already happen for blocks. I did that for the poll module because that is highly-personalized based on user/cookie. The result would be that the rest of the rendered node would still be cached and just the form would be then inserted later. With big pipe, you should be able to visually see that the form is inserted after the initial page load then. See \Drupal\poll\PollViewBuilder::view().
But I think this can be done in a separate issue.
Comment #9
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedI am not sure having Big Pipe lazy load a webform is the best UX because generally the webform is the main piece of content on a page. I do agree any experimentation with lazy loading can be handled in a new ticket.
I need to do another review and round of manual testing and then I will commit this patch.
Comment #10
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedRe-rolling the patch.
Comment #11
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedManual Testing Steps…
Notes
Webform Block
Webform Node
Comment #13
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented