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

Created on 18 December 2023, 11 months 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 16 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 11 months ago
    Custom Commands Failed
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    25,727 pass, 1,781 fail
  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 11 months ago
    Build Successful
  • last update 11 months ago
    25,722 pass, 1,829 fail
  • Status changed to Needs review 11 months ago
  • 🇮🇳India SandeepSingh199

    Tried to fixed the #2 issue.

  • Status changed to Needs work 10 months 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 7 months 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 3 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
    3 months ago
    Total: 922s
    #275632
  • Status changed to Needs work 3 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
    about 2 months ago
    #302875
  • Pipeline finished with Failed
    about 2 months ago
    #302869
  • Pipeline finished with Failed
    about 2 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 andrew.farquharson

    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
    about 3 hours ago
    #345550
Production build 0.71.5 2024