Problem/Motivation
From #2551989-7: Move replacing of placeholders from HtmlRenderer to HtmlResponseAttachmentsProcessor:
I've spent some time on IRC discussing this change on IRC with @Wim Leers I'm that happy that we're injecting and using
renderer.configinto the HtmlRenderer and using it. We discussed several work around including:
- Relying on
HtmlResponseAttachmentsProcessorto add them, This does not work because HtmlRenderer needs to return a properly cacheable response with all cache contexts- Adding a new method on the Renderer that does renderRoot without the placeholdering. This seems suboptimal ... it is alreadt hard to work out when to use
renderRoot(),renderPlain()and just plain ol'render().- Maybe we should rename
renderer.configto something that does not seem so specific.I think only the last one is actually worth pursuing and we can do that as a followup.
This is that follow-up.
Proposed resolution
TBD
Remaining tasks
TBD
User interface changes
None.
API changes
TBD
Data model changes
None.
Comments
Comment #2
wim leersComment #3
wim leersHaving thought about this, I can't think of a better name. :(
The easiest solution would be to have
Renderer::render()always add the required cache contexts, but that'd result in more cache context merging work. That's why the current code looks like this:It was Fabian who proposed this at #2453059-25: Set default render cache contexts: 'theme' + 'languages:' . LanguageInterface::TYPE_INTERFACE.2. This is definitely going to be faster, but the downside is that we now have this issue. If we didn't have that perf optimization, then we could make
HtmlRenderersimpler, and the naming ofrenderer.configdoes not need to be rethought (which is difficult).So, perhaps what we should do is profile it, to see what the perf delta is between HEAD (with that if-statement) vs without that if-statement.
Comment #4
wim leersComment #5
dawehnerAlso renaming config requires a container rebuild. We; should be aware of that before just going with the rename.
Comment #19
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #20
smustgrave commentedWanted to bump 1 more time.