Problem/Motivation
Following #3467293: twig/twig 3.11.0 introduces (for Drupal) breaking changes and #3473195: twig/twig has a possible sandbox bypass <v3.14.0 there are several Twig deprecations that are skipped for now, we need to resolve these so when Twig 4 is released we can upgrade cleanly:
%Since twig/twig 3.11: Changing the value of a "filter" node in a NodeVisitor class is not supported anymore.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "attach_library" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "add_component_context" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "render_var" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "validate_component_props" function of type "Twig\\Node\\Expression\\FunctionExpression" is deprecated.%
%Since twig/twig 3.12: Getting node "filter" on a "Twig\\Node\\Expression\\FilterExpression" class is deprecated.%
%Since twig/twig 3.12: Getting node "filter" on a "Twig\\Node\\Expression\\Filter\\DefaultFilter" class is deprecated.%
%Since twig/twig 3.12: Getting node "filter" on a "Twig\\Node\\Expression\\Filter\\RawFilter" class is deprecated.%
%Since twig/twig 3.12: The "tag" constructor argument of the "Drupal\\Core\\Template\\TwigNodeTrans" class is deprecated and ignored%
%Since twig/twig 3.12: Twig Filter "spaceless" is deprecated%
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3473440
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #5
andypostComment #6
finnsky commentedSpaceless. Seems easier one from this bundle rewritten
We can remove it without any problems and get better perfomance
https://github.com/twigphp/Twig/pull/4236
https://github.com/twigphp/Twig/pull/4308
Probably every deprecation thing requires own subticket. Not sure. I gonna continue here in new branch for every deprecation to have full picture.
Sending for review to understand if approach is correct.
Comment #8
finnsky commentedComment #9
finnsky commentedComment #10
finnsky commentedComment #11
finnsky commentedComment #12
finnsky commentedComment #14
finnsky commentedComment #15
finnsky commentedSo we have 4(5) types of deprecations
1. "Not passing an instance of "TwigFunction" when creating a "XXX" function"
Fixed here in the test-passing-function branch. Can be moved to a separate task.
2. "The "tag" constructor argument"
Also fixed here.
3. "spaceless is deprecated"
Fixed for tests here. Also fixed tests.
4 and 5 are related to modifications of the default escape filter using drupal_escape
https://twig.symfony.com/doc/3.x/filters/escape.html#custom-escapers
it says here that
"Built-in escapers cannot be overridden mainly because they should be considered as the final implementation and also for better performance."
The last point definitely deserves a separate task. All the rest can be merged here or in their subtasks
Comment #16
bbralaJust so you know.
This complely broke upgrade status and the update bot. Since we now ship deprecations with core. The d11 readiness dashboard showed like +200k errors suddenly and we eventually tracked it back here.
Comment #17
finnsky commentedGonna check that last deprecations type here.
Comment #18
gábor hojtsyShould #3474692: Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.12.0 be directly solved in this issue or remain in its own?
Comment #19
finnsky commented@gábor hojtsy
It's hard to say now. In the current task (which should later become meta) I consider all deprecations at once and globally. Several are already ready to receive their subtask and be merged.
As for the task that you linked, I think it has the wrong name. We can't just rewrite the functions there. But consider how drupal_escape and trans work in general. And add it as required by the modern version of Twig. This is currently in progress.
Comment #20
longwaveThere are four child issues now, one with its own MR, the three MRs from here need moving to their individual issues - but I think that covers all the newly added deprecations.
Comment #21
finnsky commentedI would say 5 issues but not sure yet ;) As i said better to review also how `trans` and `drupal_escape` works
Comment #22
longwave@finnsky #3474692: Fix "Twig\Node\Expression\FilterExpression" deprecation introduced in twig/twig 3.12.0 is RTBC and should fix both drupal_escape and trans.
edit: oh I did not see #3457168: Since twig/twig 3.9: error with "twig_escape_filter" function usage in /core/lib/Drupal/Core/Template/TwigExtension.php !
Comment #23
finnsky commentedImo we shouldn't just `fix deprecation message` here. But fix deprecations deeper, with research of new twig extending style. This issue is RTBC of course. But - can we do it better? Idk yet.
Comment #27
bbralaWhile reviewing the child issues i concluded we might need a change record here (or on the twig update issue?).
Contrib will want to know how to fix these errors, having a guideline on where to find the core fixes will help a lot.
Comment #28
gábor hojtsyI think it would be great to have a "good enough" solution ASAP, and then spend any time to fix it even better later. Because these deprecated API uses produce errors with most runs of Upgrade Status on all contrib projects and people can't do anything about it. When we ran Project Status on all contrib modules, these produced 190 thousand twig deprecacted API calls detected across all of Drupal 10 contrib :) This also means that none of those are getting project update bot patches/MRs because they have these unsolvable issues. So a quick solution would be important to unblock the rest of the ecosystem / tooling, then a better solution is not out of question :)
Comment #29
gábor hojtsyThese were the top errors found in contrib by Project Analysis (data in https://git.drupalcode.org/project/deprecation_status/-/blob/d3d2a182959... including lists of projects for each error if someone wants to validate that Upgrade Status is not reporting the errors anymore):
Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\FilterExpression" class is deprecated.Since twig/twig 3.12: Not passing an instance of "TwigFunction" when creating a "render_var" function of type "Twig\Node\Expression\FunctionExpression" is deprecated.Since twig/twig 3.12: The "tag" constructor argument of the "Drupal\Core\Template\TwigNodeTrans" class is deprecated and ignored (check which TokenParser class set it to "trans"), the tag is now automatically set by the Parser when needed.Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\Filter\RawFilter" class is deprecated.Since twig/twig 3.12: Getting node "filter" on a "Twig\Node\Expression\Filter\DefaultFilter" class is deprecated.Since twig/twig 3.12: Twig Filter "spaceless" is deprecated. See https://drupal.org/node/3071078.Are these all being resolved in the children? :)
Also these two were found in one project each, not a priority IMHO:
Since twig/twig 3.12: Character """ at position 2 should not be escaped; the "\ character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character."in formatage_models 4.x-devSince twig/twig 3.12: Character "T" at position 7 should not be escaped; the "\ character is ignored in Twig v3 but will not be in v4. Please remove the extra "\" character."in festeva 1.0.x-devComment #30
bbralaYeah all being handled in the children.
Comment #31
finnsky commentedyep. now all deprecations covered
not sure is this
but at least
:)
Comment #32
bbralaOnly the spaceless deprecation fix needs work, 2 are merged, 1 is RTBC. Almost there ^^
Comment #34
longwaveThe last issue was solved in #3486170: Remove use of deprecated "spaceless" filter in core templates, closing this one out.