Problem/Motivation

When \Drupal\Core\Template\TwigExtension::renderVar() results in a NULL this can cause deprecations in PHP 8.1 because NULL is not something that has been rendered.

Steps to reproduce

See tests on #3220021: [meta] Ensure compatibility of Drupal 9 with PHP 8.1 (as it evolves). This change fixed Fix \Drupal\Tests\demo_umami\Functional\DemoUmamiProfileTest for example.

Proposed resolution

Return an empty string.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 3239860-4.patch1.99 KBalexpott
#5 3-4-interdiff.txt2.21 KBalexpott
#3 3239860-3.patch812 bytesalexpott
#2 3239860-2.patch512 bytesalexpott

Comments

alexpott created an issue. See original summary.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new 3239860-2.patch512 bytes
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3239860-3.patch812 bytes

Hmmm... I wonder what happens if we do this... the use of == over === is interesting. Here the == matches FALSE, [], NULL and ""

alexpott credited lauriii.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
StatusFileSize
new 3-4-interdiff.txt2.21 KB
new 3239860-4.patch1.99 KB

Discussed this issue with @lauriii. AS a result opened the follow up #3240093: \Drupal\Core\Template\TwigExtension::renderVar() should always return a stringable.

The only reason #3failed is because Umami and Olivero do things like:

  {% if page.banner_top|render|striptags|trim is not empty %}
    <div class="layout-banner-top">
      {{ page.banner_top }}
    </div>
  {% endif %}

to determine whether to render something. Bartik, for example, does:

    {% if page.highlighted %}
      <div class="highlighted">
        <aside class="layout-container section clearfix" role="complementary">
          {{ page.highlighted }}
        </aside>
      </div>
    {% endif %}

For my the Umami and Olivero approach of doing |render|striptags|trim is wrong see #2937640: Umami theme ignores placeholders and HTML replaced elements when checking for empty regions.

However the upshot is that we shouldn't really check the behaviour to much here. We have to change it to return an empty string in the $arg == NULL case because other wise we have deprecations in PHP 8.1 but the rest of the discussion can happen on the followup.

I've added a test so that the current behaviour of the method when it comes to scalars and early returns is unit tested.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

All expectations are covered by test, gtg

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -525,9 +525,11 @@ public function renderVar($arg) {
+    //   this behavior is correct or should be deprecated.
     if ($arg == NULL) {
-      return NULL;
+      return '';

++ to keep it in follow-up

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed 7c88f64 and pushed to 9.3.x. Thanks!

  • catch committed 9005e1b on 9.3.x 
    Issue #3239860 by alexpott, lauriii: \Drupal\Core\Template\TwigExtension...

Status: Fixed » Closed (fixed)

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