New twig variable deprecation check should use a service and not the direct class

Created on 18 December 2023, over 1 year ago

Problem/Motivation

When the TwigExtension service is overridden, 📌 Provide a mechanism for deprecation of variables used in twig templates Fixed uses the wrong class and throws the exceptions
```
The website encountered an unexpected error. Try again later.

Twig\Error\RuntimeError: The "Drupal\Core\Template\TwigExtension" extension is not enabled in "__string_template__d7d7f1a462505651fc222178924da388" at line 1. in Twig\ExtensionSet->getExtension() (line 73 of /var/www/html/vendor/twig/twig/src/ExtensionSet.php).
Twig\Environment->getExtension('\Drupal\Core\Template\TwigExtension') (Line: 40)
```

Steps to reproduce

Override the class in a custom module service provider class. Then visit any page that uses twig.

/**
 * Modifies the date_formatter service.
 */
class MyModuleServiceProvider extends ServiceProviderBase {

  /**
   * {@inheritdoc}
   */
  public function alter(ContainerBuilder $container) {
    if ($container->hasDefinition('twig.extension')) {
      $container->getDefinition('twig.extension')
        ->setClass('Drupal\my_module\Template\TwigExtension');
    }
  }

}

The overridden class can be simply defined.

<?php

namespace Drupal\my_module\Template;

use Drupal\Core\Template\TwigExtension as CoreTwigExtension;

/**
 * class TwigExtension.
 */
class TwigExtension extends CoreTwigExtension {
}
🐛 Bug report
Status

Active

Version

10.2

Component
Theme 

Last updated about 12 hours ago

Created by

🇺🇸United States douggreen Winchester, VA

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @douggreen
  • 🇺🇸United States douggreen Winchester, VA

    Patch attached

  • 🇺🇸United States douggreen Winchester, VA
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.0 & MySQL 5.7
    last update over 1 year ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    25,727 pass, 1,781 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update over 1 year ago
    Build Successful
  • last update over 1 year ago
    25,722 pass, 1,829 fail
  • Status changed to Needs review over 1 year ago
  • 🇮🇳India SandeepSingh199

    Tried to fixed the #2 issue.

  • Status changed to Needs work over 1 year ago
  • 🇺🇸United States smustgrave

    If going to use patches an interdiff should be provided to show what you changed but one was not or an explanation.

    Issue summary is missing proposed solution

    Will need test coverage

  • 🇫🇷France eliechoufani Marseille, Beirut

    Hello, I had the error

    [error] 8924#8924: *155212268 FastCGI sent in stderr: "PHP message: Uncaught PHP Exception Twig\Error\RuntimeError: "The "Drupal\Core\Template\TwigExtension" extension is not enabled." at /home/XXXXX/www/web/themes/contrib/stable/templates/field/time.html.twig line 22"

    after I upgraded Drupal to 10.2.1

    And I fixed it with the attached patch, maybe it needs more testing.

    I am using

    "drupal/twig_extender": "5.1.0",
    "drupal/twig_extender_extras": "5.1.0",

    in my project.

    Good luck.

  • 🇵🇱Poland lamp5 Rzeszow

    I can confirm. The issue exists after update to Drupal 10.2.2 and when using "drupal/twig_extender": "5.1.0",

  • 🇺🇸United States bdanin

    Confirmed, the patch in comment #6 fixes this issue for me. I am also running twig_extender .

  • 🇮🇳India djsagar

    Updated summary with list of review tasks.

  • 🇺🇸United States neclimdul Houston, TX

    Replacing a extension sounds pretty dangerous.

    The current approach of calling \Drupal::service is functional but seems like a _huge_ performance hit going to the container ever template render.

    Does anyone know why the deprecation compiler code was written into the extension? It seems like it might have just been convenient.

    If it was important, maybe we can move it to a dedicated "extension" that's not really an extension that just provides this helper?

    If the approach isn't important, maybe we can just inline the check. Something like this maybe:

    
        $compiler->write("isset(\$context['deprecations']) && array_map(");
        $compiler->indent();
        $compiler->write("fn (\$name) =>");
        $compiler->indent();
        $compiler->write(
          "isset(\$context['deprecations'][\$name])",
          "&& \array_key_exists(\$name, \$context)",
          "&& @trigger_error(\$context['deprecations'][\$name], E_USER_DEPRECATED),"
        );
        $compiler->outdent();
        $compiler->subcompile($usedNamesNode);
        $compiler->outdent();
        $compiler->raw(");");
    
  • last update about 1 year ago
    Patch Failed to Apply
  • 🇺🇸United States momow

    momow changed the visibility of the branch 3409549-new-twig-variable to hidden.

  • 🇮🇳India santhosh@21

    This issue exists after drupal upgrade to 10.3.1 from 10.2.7
    Patch 6 works for me

  • 🇵🇭Philippines mjgruta

    We upgraded from Drupal 10.1 to 10.2.7 and had a WSOD

    Twig\Error\RuntimeError: The "Drupal\Core\Template\TwigExtension" extension is not enabled. in Twig\ExtensionSet->getExtension() (line 35 of docroot/core/themes/claro/templates/admin/admin-block-content.html.twig).

    We also had an old version of twig_extender.

    Applying the patch #6 and using twig_extender 5.1.0 works fine.

  • First commit to issue fork.
  • Status changed to Needs review 8 months ago
  • 🇮🇳India arunkumark Coimbatore

    I created an MR with the #6 patch. I ran the PHPUnit test locally. It is working fine. I have attached a screenshot for your

    @smustgrave hope issue summary is fine

  • Pipeline finished with Failed
    8 months ago
    Total: 922s
    #275632
  • Status changed to Needs work 8 months ago
  • 🇺🇸United States smustgrave

    Was also tagged for tests which are still needed.

  • 🇺🇸United States scampbell1 New York

    I just tried upgrading to 10.3.6 and am now getting a WSOD. After some googling, I came across this thread (please let me know if it's best if I create a separate issue).

    Currently, the error says Twig\Error\RuntimeError: The "Drupal\Core\Template\TwigExtension" extension is not enabled in "core/themes/claro/templates/classy/layout/html.html.twig". in /code/vendor/twig/twig/src/ExtensionSet.php on line 80

    However, #6 would not apply for me because my TwigNodeCheckDeprecations.php already written that way.

    I, too, am running Twig Extender.

  • The issue is that the extension is called by class name statically, For the twig_extender module users the extension called is different than the core one, So this patch get the class name from the service itself.

  • Pipeline finished with Failed
    7 months ago
    #302875
  • Pipeline finished with Failed
    7 months ago
    #302869
  • Pipeline finished with Failed
    7 months ago
    #302879
  • 🇺🇸United States scampbell1 New York

    I haven't done heavy testing (but am no longer getting a WSOD!) but I used the second part of what alshami wrote in their patch to create one for 10.3.6.

    There are two versions of the patch: one for the first patch alshami wrote and another for the second. This is the first.

  • 🇺🇸United States scampbell1 New York

    Patch version #2 attached

  • The patch in comment #21 fixes this issue for me after upgrading from 10.3.5 to 10.3.6. I replaced patch at comment #6 with the new patch 3409549-21.patch
    I am also running twig_extender.

  • 🇺🇸United States bdanin

    Once again hit the error Twig\Error\RuntimeError: The "Drupal\Core\Template\TwigExtension" extension is not enabled (see #8) and I replaced the patch I was originally using in #6 with the patch provided in #21, which is working for me in 10.3.6 using twig_extender.

  • 🇬🇧United Kingdom oily Greater London

    MR !9765 seems to be the closest to a fix of the two merge requests. The other MR breaks several tests. This MR only breaks one PHPUnit Unit test.

    However it does not include a failing test, in fact a test is still required. Also the existing code needs fixed so it does not break any existing tests.

  • Pipeline finished with Failed
    5 months ago
    Total: 5787s
    #345550
  • 🇮🇳India arunkumark Coimbatore

    I tried to fix the TestCase issues but was unable to solve them. I am facing a problem with the test cases for the TWIG render function with arguments replaced.

    Inside testFormatDate() $result = $twig->render('{{ time|format_date("html_date") }}', ['time' => $timestamp]);
    and testFileUrl()$result = $twig->render('{{ file_url(file) }}', ['file' => 'public://picture.jpg']);.

    But I checked by replacing the $compiler->write("\$this->env->getExtension(\\Drupal::service('twig.extension')::class)\n"); with $compiler->write("\$this->env->getExtension('\Drupal\Core\Template\TwigExtension')\n");, then the testcases are working fine.

    Trying to solve by below ways on TwigNodeCheckDeprecations.php. But no luck,

    1. $compiler->write("\$this->env->getExtension(\Drupal::service('twig.extension')::class)\n");
    2. $compiler->write("\$this->env->getExtension(\\Drupal::service('twig.extension')::class)\n");
    3. $compiler->write("\$this->env->getExtension(\\" . \Drupal::service('twig.extension')::class) . "\n");
    3. $compiler->write("\$this->env->getExtension('\\" . \Drupal::service('twig.extension')::class) . "'\n");

  • 🇬🇧United Kingdom oily Greater London

    oily changed the visibility of the branch 3409549-twig-deprecation-check to hidden.

  • 🇬🇧United Kingdom oily Greater London

    Hi @arunkumark, Which branch are you using for your code changes/ trials? I cannot see any commits?

    We probably should hide the other (non-active) branch(es).

  • 🇺🇸United States ericpugh

    I'm using Drupal 10.3.10, and while patch #21(and others) fixes the fatal error on the site, the patch does not seem to fix the error when using Single-Directory Components module. (which will supposedly be a stable core module in D11)

  • 🇮🇳India arunkumark Coimbatore

    @oily am using the 3409549-New-twig-variable-deprecation-check-should-use-a-service-and-not-the-direct-class branch and my MR 9765

  • 🇺🇸United States douggreen Winchester, VA

    Attached is #23 rerolled for core 10.4.x

  • 🇯🇴Jordan oways23

    Re-roll patch #23 for drupal 10.4.x.

  • 🇱🇧Lebanon christina_abboud

    Re-rolled the patch for Drupal Core 10.3.12

  • 🇬🇧United Kingdom oily Greater London
  • 🇬🇧United Kingdom oily Greater London

    We are now using MR's instead of patches. The MR's are made against the 11.x branch and can then be back-ported to other versions.

  • 🇮🇳India rohitsharma401

    rohitsharma401 changed the visibility of the branch 3409549-New-twig-variable-deprecation-check-should-use-a-service-and-not-the-direct-class to hidden.

  • First commit to issue fork.
  • 🇹🇳Tunisia adel-by

    Patch for Drupal Core 10.4.x (added same changes to ComponentNodeVisitor)

  • Pipeline finished with Failed
    about 2 months ago
    Total: 585s
    #439829
  • 🇹🇳Tunisia adel-by

    Please ignore patches from #41
    here's a patch that's against 10.4.x (added same changes to ComponentNodeVisitor)

  • Pipeline finished with Failed
    about 2 months ago
    Total: 392s
    #439901
  • 🇺🇦Ukraine Foxy-vikvik

    SCD components not working after applying the patch

    Twig\Error\RuntimeError: The "Drupal\Core\Template\TwigExtension" extension is not enabled in "theme:footer". in Twig\ExtensionSet->getExtension() (line 80 of /var/www/html/vendor/twig/twig/src/ExtensionSet.php).
    
  • 🇨🇦Canada efrainh

    #43 works on my setup. I'm now on 10.4.5

    The error messages disappeared.

    Thanks!

Production build 0.71.5 2024