Problem/Motivation
The use of Twig_SimpleFilter in /src/Twig/Extension/FieldValueExtension.php is deprecated and should be replaced with Twig\TwigFilter.
Twig_SimpleFilter was deprecated in Twig 2, and is no longer available in Twig 3, which is used in Drupal 10. The new class is available also in Twig 2, and is a simple replacement of the class name, as the Twig_SimpleFilter in Twig 2 is an alias of TwigFilter.
Likewise, the Twig_Extension has been deprecated and replaced with Twig\Extension\AbstractExtension
| Comment | File | Size | Author |
|---|---|---|---|
| #18 | 3310839-18.patch | 3.67 KB | jcnventura |
| #18 | interdiff.txt | 569 bytes | jcnventura |
Comments
Comment #2
immaculatexavier commentedComment #4
sutharsan commentedWill the module with this patch be compatible with Drupal 8.0?
Comment #5
jcnventura@Sutharsan, unfortunately, no. Drupal 8 used Twig 1, which didn't have this new class. But it is impossible to support Twig 3 / Drupal 10, Twig 2 / Drupal 9 and Twig 1 / Drupal 8 at the same time. The only way would be through a check if the class exists or not and to that condition to change between Twig 1/2 code, and Twig 2/3 code.
However, Drupal 8 is no longer supported, and modules should focus on supporting Drupal 10 instead of ^8 || ^9 || ^10.
In any case the patch above is missing a full class path (\Twig\TwigFilter) or a use statement.
Comment #6
sutharsan commentedI’m fine with limiting the use of the
module to D9+. That requires a new branch, and this patch should include the required changes to the info file.
Comment #7
immaculatexavier commentedAddressed #5 - Fixed deprecation compatibility with D9+, and addressed #6 by limiting the module's use to D9+ against #2.
Comment #9
immaculatexavier commentedTried to fix the failed issues against #7
Comment #10
jcnventuraWell, seems like this is now the Drupal 10 update issue, as such let's change it, and add the new bug I found: the \Twig_Extension is also not part of Twig 3, and has been replaced with:
Comment #11
jcnventuraComment #12
sutharsan commentedComment #13
jcnventuraNeeds a re-roll after #3295680: Drupal Coding Standards
Comment #14
jcnventuraThis is the re-roll of #11. Can't create an interdiff easily, but the only reason the patch didn't apply is that the coding standards issue fixed a double space before an '=' sign.
The patch in #11 was maintaining that error, and fixing that and removing a blank line at the start of that function were the only changes in this patch.
Comment #15
jcnventuraFurther re-roll after https://git.drupalcode.org/project/twig_field_value/-/commit/925f0296df9...
Comment #16
b0b commentedOne more error being thrown... core: 8.x needs to be core_version_requirement: ^9 || ^10. Seems to be working after that.
Comment #17
jcnventuraRegarding #16, the proper fix should be defining that module to be part of the "Testing" package, and not specifying any core or core_version_requirement at all.
Comment #18
jcnventuraNow fixing also #16, as I described above.
Comment #19
sutharsan commentedI did not know that, but I see some respected modules to use this approach too. This combined with a 'green' test result, I think it is RTBC again.
Comment #20
jcnventuraTested now with D10 as well, all green.
As to the testing package and no "core_version_requirement" it is documented here: https://www.drupal.org/node/3070687#test
Please commit this and tag a new release, since version 2.0.1 declares itself as Drupal 10 compatible, but it really isn't without these changes.
Comment #22
sutharsan commented