modxcms / fred

The friendly front-end editor for visual, drag-and-drop content building in MODX CMS
https://fred.modx.com
MIT License
57 stars 25 forks source link

Update Twig to version 3.x to avoid conflicts with other packages #476

Closed muzzwood closed 1 year ago

muzzwood commented 1 year ago

Hi! :)

Currently Fred is using Twig 1.x which is now rather ancient. Other MODX packages are using the current 3.x version, and it's possible for the incorrect version to be loaded for that PHP namespace, causing errors, if both packages are installed on the same MODX deployment.

It would be great to see Fred updating Twig to 3.x to avoid needing to use workarounds similar to this: https://github.com/modmore/guzzle7/blob/main/core/components/guzzle7/bootstrap.php

Mark-H commented 1 year ago

For context, Commerce updated from 1.x to 3.x in its 1.3 release, and we're now seeing conflicts there.

PHP Fatal error: Uncaught Error: Class 'Twig_Environment' not found in /www/core/components/fred/model/fred/src/Endpoint/Ajax/RenderElement.php:49
Stack trace:
#0 /www/core/components/fred/model/fred/src/Endpoint/Ajax/Endpoint.php(75): Fred\Endpoint\Ajax\RenderElement->process()
#1 /www/core/components/fred/model/fred/src/Endpoint/Ajax.php(61): Fred\Endpoint\Ajax\Endpoint->run()
#2 /www/assets/components/fred/web/endpoints/ajax.php(14): Fred\Endpoint\Ajax->run()
#3 {main}
thrown in /www/core/components/fred/model/fred/src/Endpoint/Ajax/RenderElement.php on line 49

[18-Apr-2023 06:22:08 UTC] PHP Fatal error:
Declaration of
Twig\NodeVisitor\MacroAutoImportNodeVisitor::enterNode(Twig\Node\Node $node, Twig\Environment $env):
Twig\Node\Node must be compatible with
Twig\NodeVisitor\NodeVisitorInterface::enterNode(Twig_NodeInterface $node, Twig\Environment $env) in
/www/core/components/commerce/vendor/twig/twig/src/NodeVisitor/MacroAutoImportNodeVisitor.php on line 34

These errors happen due to essentially an undefined loading order when multiple auto loaders have the same(-ish) dependency. In the RenderElement, Commerce with v3 may've been loaded sooner (or it could be an actual error just in Fred?). In the declaration error, the v3 implementation doesn't match the v1 interface.

Pre-loading classes like we do in the guzzle7 package would help with the interface error and is something we may add to Commerce as a short-term fix, however ideally we would like to coordinate and offer help updating Twig to 3. The update in Commerce 1.3 was pretty straightforward - the primary challenge for users was the removal of the {% spaceless %} filter, which we largely automated in a resolver which we're also happy to share.

theboxer commented 1 year ago

Updating twig is not a huge issue, the problem is we're using twig.js for the client side parsing of elements, but it doesn't seem it's up to date with twig 3. I'll have to check it out more what options we have.

whatafunc commented 1 year ago

Updating twig is not a huge issue, the problem is we're using twig.js for the client side parsing of elements, but it doesn't seem it's up to date with twig 3. I'll have to check it out more what options we have.

Thank you for looking into this @theboxer , can you do us a huge favour and first just release the Fred with updated twig asap, please? We just got really stuck 3rd week with the error as Mark has kindly indicated

matdave commented 1 year ago

This is resolved in version 2.1.0

muzzwood commented 1 year ago

Thanks heaps @matdave and @theboxer !