Update to Twig 3.9

The only failure https://github.com/twigphp/Twig/pull/4019 which may affect contrib as well

Swapped AbstractNodeVisitor for NodeVisitorInterface. Don't think we need to provide BC as the class names are hardcoded in TwigExtension and aren't swappable.

Maybe CR explaining this swap makes sense

now 2 more
- https://github.com/twigphp/Twig/commit/e70777405dd3dd1496d46b626f0b8589e...
- https://github.com/twigphp/Twig/commit/14d38036477b93976b56b1ee7046e86a3...

hope this should be enough

Needs more work for yeild

longwave → credited Spokje → .

Thanks @Spokje over in 📌 Bump twig/twig to 3.9.0 Closed: duplicate for finding https://github.com/twigphp/Twig/issues/4008 - we use display_end in TwigNodeVisitorCheckDeprecations which appears to be why it's stopped working.

Postponed on the above, nothing we can do until Twig fixes this.

commented our case https://github.com/twigphp/Twig/issues/4008#issuecomment-2061086856

To be fair, we can still:

- Bump twig/twig to 3.9.1 which fixes Exception: Warning: Undefined variable $blocks.
- Bump to that version in composer.json, since lower version won't pass with the yield changes.

Did that just now.

@Spokje thank you, now all functional tests passed and only left is deprecation test which is blocked on twig

https://github.com/twigphp/Twig/pull/4028 fixes the issue for me, commented over in https://github.com/twigphp/Twig/issues/4008

3.9.2 landed and made it all better.

Good work everyone.

Looked over the Twig release notes and compared them to the individual changes. All seem correct. Couple questions though:

What are the chances of Twig's update disrupting contrib/custom code edgecases? Some of the changes we had to make to the core tests are just replacing a deprecated implementation with a better one, but one appears to be hard-breaking (although I'm not sure exactly what the consequences of not doing it are from the Twig release notes). There are two decisions that would depend on this:

1. Whether we add a brief release note with a link to Twig's update info about anything.
2. What we do with the Twig version and constraint for 10.3.

Thanks!

Ignoring the bug in Twig 3.9.0/3.9.1, as far as I can tell the changes are all backward-compatible.

If you were extending AbstractNodeVisitor you should swap to implementing NodeVisitorInterface directly instead.

If you were using echo or print in ::compile() you should swap to yield and add the #[YieldReady] attribute.

Contrib should not be affected by this because none of these are swappable parts; Twig should issue deprecations that they can fix in their own time. Therefore I think we should backport this to 10.3 so contrib can upgrade when they are ready.

I think we should add a release note to 10.3.0 and 11.0.0 linking to the Twig release note, added a snippet.

Committed 973b6ec and pushed to 11.x. Thanks!

Let's get an MR for 10.3 rolling. I thought about cherry-picking and doing the composer work on commit but I think a test run is worth it.

alexpott → closed merge request !7523

Decided to do what I would have done on commit in a branch and push it to the MR repo...

MR needs to set back to 3.9.2 or needs followup for 11.x as 1h ago https://github.com/twigphp/Twig/releases/tag/v3.9.3

https://github.com/twigphp/Twig/blob/3.x/CHANGELOG

# 3.9.3 (2024-04-18)

* Add missing `twig_escape_filter_is_safe` deprecated function
* Fix yield usage with CaptureNode
* Add missing unwrap call when using a TemplateWrapper instance internally
* Ensure Lexer is initialized early on

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

@andypost I just pushed a quick follow-up to get us to 3.9.3 as we've had a test run on 10.3.x and it did not cuase any issues. Gonna update the 10.3 MR to have the same minimum.

I hope it will be green

Committed 5a43c22 and pushed to 10.3.x. Thanks!

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

quietone → closed merge request !7581

Since twig/twig 3.9: Twig node "Drupal\Core\Template\TwigNodeTrans" is not marked as ready for using "yield" instead of "echo"; please make it ready and then flag it with the #[YieldReady] attribute.

Twig deprecations may break contrib tests. Can we backport this fix to Drupal 10.2?