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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Issue tags: +D8 Accelerate Dev Days
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Active » Needs review
FileSize
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 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia 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 CreditAttribution: PieterJanPut at iO commented
Status: Needs work » Needs review
FileSize
render_should_call-2471216-5.patch1.2 KB
joshi.rohit100’s picture

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

Thats what happen when you copy paste ):

PieterJanPut’s picture

PieterJanPut CreditAttribution: PieterJanPut at iO commented
Issue tags: -D8 Accelerate Dev Days +drupaldevdays
dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner 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 CreditAttribution: 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 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia 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
CreditAttribution: joshi.rohit100 commented
Status: Needs work » Needs review
FileSize
render_should_call-2471216-11.patch1.25 KB
interdiff-11.txt274 bytes

Done.

amateescu’s picture

amateescu CreditAttribution: amateescu for Drupal Association commented
Status: Needs review » Reviewed & tested by the community

Looks good to me.

googletorp’s picture

googletorp CreditAttribution: googletorp at Reveal IT commented

Also looks good to me.

RTBC imo.

fgm’s picture

fgm
Primary language French
Location Paris, France
CreditAttribution: fgm commented

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

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia 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 🇨🇦
CreditAttribution: webchick at Acquia 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.