When Drupal\Core\Render\Renderer::doRender()
processes #lazy_builder
callbacks the #type
defaults are already loaded, so any element with a #type
property will not be rendered properly.
A #pre_render
callback often adds new types in child elements only, when it becomes possible to have the type defaults loaded. With a #lazy_builder
callback it's not common to return a child element.
Comment | File | Size | Author |
---|---|---|---|
#38 | reroll_diff_19-38.txt | 1.66 KB | ravi.shankar |
#38 | 2609250-38.patch | 923 bytes | ravi.shankar |
#19 | lazy-builder-type-2609250-19.patch | 1.11 KB | Eric_A |
#17 | lazy-builder-type-2609250-17.patch | 1.97 KB | Eric_A |
#2 | lazy-builder-type-2609250-2.patch | 954 bytes | Eric_A |
Comments
Comment #2
Eric_A CreditAttribution: Eric_A commentedSomething like this.
Comment #3
Eric_A CreditAttribution: Eric_A commentedComment #4
Eric_A CreditAttribution: Eric_A commentedComment #5
Eric_A CreditAttribution: Eric_A commentedI got confused. The observed behavior is the same as with the old, familiar
#pre_render
: to have element type defaults applied automatically, having child elements rendered is the only option.Closing this one.
Comment #6
Eric_A CreditAttribution: Eric_A commentedBut then again, a
#pre_render
callback would normally add new types in child elements only. With a#lazy_builder
callback it's different...Comment #7
Eric_A CreditAttribution: Eric_A commentedBack to Needs review because of #6.
Comment #8
Eric_A CreditAttribution: Eric_A commentedComment #9
Eric_A CreditAttribution: Eric_A commentedComment #11
xjmIs there any example in core that can reproduce this bug, or is it possible to provide a short example snippet and steps to reproduce and test how it is not rendered properly?
Comment #12
Eric_A CreditAttribution: Eric_A commentedNot that I'm aware of.
This element will not render as a more link. It would if the #lazy_builder callback returned a child element.
I'm not too sure this is a true bug. Our default renderer implementation shows the same behavior with #pre_render for ages. It's just not documented behavior.
Comment #13
Eric_A CreditAttribution: Eric_A commentedComment #14
Eric_A CreditAttribution: Eric_A commentedFixed ordering in the snippet.
Comment #15
Eric_A CreditAttribution: Eric_A commentedAnd fixed the text that goes with it. It works if the #lazy_builder callback returns a child element.
Comment #16
Wim LeersExactly.
Indeed.
But we should make a decision here, and then help the developer by throwing an assertion.
Comment #17
Eric_A CreditAttribution: Eric_A commentedHere's a patch with the assert() approach, both for #lazy_builder and #pre_render.
If we leave the actual processing as is and confine ourselves to adding more assertions to Drupal\Core\Render, then the issue category should probably change from "Bug report" to something else and "D8 cacheability" should be removed in favor of "DX (Developer Experience)".
Comment #19
Eric_A CreditAttribution: Eric_A commentedI've removed the #pre_render part assert(). I think it's impossible in the #pre_render part to determine fully within the assert() if a #type property was returned (wether one already existed or not, with the same or a different value.)
Comment #21
lauriiiDiscussed with @alexpott, @xjm, @Cottser and @joelpittet. We agreed that this should remain as a major bug because this is very bad DX. It is very difficult to debug why render elements are not rendered properly on the lazy builder.
Comment #22
lauriiiComment #27
Upchuk CreditAttribution: Upchuk as a volunteer commentedHey there,
Last movement on this issue was quite a long time ago so it's worth looking at it again. I updated the docs on auto-placeholdering to mention this fact. I just lost 40 minutes figuring out why the heck my renderable was returning empty.
Comment #29
lukasss CreditAttribution: lukasss commentedComment #31
bbu23 CreditAttribution: bbu23 commentedThanks Upchuk for updating the documentation. I was having the same problem: wondering why my lazy builder was not working. I was checking core, but didn't notice the difference.
Comment #37
quietone CreditAttribution: quietone at PreviousNext commentedGot this as a random bug at DrupalSouth sprint. This is still valid and needs a reroll and tests.
Comment #38
ravi.shankar CreditAttribution: ravi.shankar at OpenSense Labs commentedAdded reroll of patch #19 on Drupal 10.1.x.
Comment #39
larowlanAn alternative approach here would just be to call this on the $new_element
Comment #40
larowlanComment #41
joachim CreditAttribution: joachim as a volunteer commented#39 That sounds useful but I'm concerned it's a bit hacky to call getInfo() from a new place.
Comment #42
larowlanIt's roughly 10 lines below where the renderer already calls it (off the top of my head when I hit this issue yesterday)
Comment #43
andypost+1 to #39