Problem/Motivation

render() calls drupal_render() which calls \Drupal::service('renderer')->render(). That's three layers. drupal_render() is deprecated. It won't make a world of difference, but it does reduce the number of unnecessary steps.

Proposed resolution

Let render() not call drupal_render(), but call the Renderer service directly.

Remaining tasks

Do it.

User interface changes

None.

API changes

None.

CommentFileSizeAuthor
#11 interdiff-11.txt274 bytesjoshi.rohit100
#11 render_should_call-2471216-11.patch1.25 KBjoshi.rohit100
#5 render_should_call-2471216-5.patch1.2 KBpieterjanput
#2 2471216-renderer-service-place-of-drupal_render-2.patch1.2 KBjoshi.rohit100

Comments

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +D8 Accelerate Dev Days
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Active » Needs review
StatusFileSize
new 2471216-renderer-service-place-of-drupal_render-2.patch1.2 KB

Done.

Status: Needs review » Needs work

The last submitted patch, 2: 2471216-renderer-service-place-of-drupal_render-2.patch, failed testing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented 
+++ b/core/includes/common.inc
@@ -1209,7 +1208,7 @@ function render(&$element) {
-    return drupal_render($element);
+    return \Drupal::service('renderer')->render($elements);

It used to be $element, now it is $elements.

:)

pieterjanput’s picture

pieterjanput commented
Status: Needs work » Needs review
StatusFileSize
new render_should_call-2471216-5.patch1.2 KB
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented

Thats what happen when you copy paste ):

pieterjanput’s picture

pieterjanput commented
Issue tags: -D8 Accelerate Dev Days +drupaldevdays
dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/includes/common.inc
@@ -1195,7 +1195,6 @@ function drupal_render_children(&$element, $children_keys = NULL) {
- * @see drupal_render()

Does it make sense to still point to the renderer?

googletorp’s picture

googletorp commented
Status: Needs review » Reviewed & tested by the community

This looks good.

#8 There's not any reason for reffering a deprecated function, so the @see reference should be deleted as in the patch.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +D8 Accelerate Dev Days

#9: @dawehner means you should add an @see \Drupal\Core\Render\RendererInterface.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
commented
Status: Needs work » Needs review
StatusFileSize
new render_should_call-2471216-11.patch1.25 KB
new interdiff-11.txt274 bytes

Done.

amateescu’s picture

amateescu commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

googletorp’s picture

googletorp commented

Also looks good to me.

RTBC imo.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented

Any reason why we don't inline the one-line show() too ?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

#14: we could, but that is less justified, since drupal_render() is deprecated, and show() isn't.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Seems like a straight-forward fix.

Committed and pushed to 8.0.x. Thanks!

  • webchick committed e02a165 on 8.0.x 
    Issue #2471216 by joshi.rohit100, PieterJanPut, Wim Leers: render()...

Status: Fixed » Closed (fixed)

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