ComurImageBundle icon indicating copy to clipboard operation
ComurImageBundle copied to clipboard

Support Symfony 5.x

Open thelem opened this issue 4 years ago • 13 comments

After upgrading Symfony I see this warning in the console:

The "Symfony\Component\Config\Definition\Builder\TreeBuilder::root()" method called for the "comur_image" configuration is deprecated since Symfony 4.3, pass the root name to the constructor instead.

This is due to the change at https://github.com/symfony/symfony/pull/27476

We need to maintain compatibility with Symfony 3 and 4.2, so suggested fix is:

$treeBuilder = new TreeBuilder("my_node"); $rootNode = method_exists($treeBuilder, "getRootNode") ? $treeBuilder->getRootNode() : $treeBuilder->root("my_node"); // BC layer for symfony/config 4.2 and older

thelem avatar May 09 '20 20:05 thelem

I am also seeing this warning (only occurs immediately after clearing cache)

The spaceless tag in "@ComurImage/Form/fields.html.twig" at line 4 is deprecated since Twig 2.7, use the "spaceless" filter with the "apply" tag instead.

Reviewing the docs I can see the apply tag was introduced in Twig 1.40 and 2.9 https://twig.symfony.com/doc/1.x/tags/apply.html https://twig.symfony.com/doc/2.x/tags/apply.html

Currently the bundle's composer.json defines:

"twig/twig": "~1.35 || ~2.5",

Which is too old for us to switch to apply. However, we depend on Symfony 3.4 or greater, and Symfony 3.4's composer.json defines:

"twig/twig": "^1.41|^2.10",

https://github.com/symfony/symfony/blob/3.4/composer.json

(looking in the symfony commit history I see they bumped to 1.40 for exactly this issue, then bumped to 1.41 for PHP 7.4 support)

Therefore I see no reason not to increase our minimum version to match Symfony's, which will allow us to use apply and remove that deprecation warning.

thelem avatar May 10 '20 12:05 thelem

Third deprecation:

User Deprecated: Using the "Twig_Extension_GlobalsInterface" class is deprecated since Twig version 2.7, use "Twig\Extension\GlobalsInterface" instead.

The namespaced version has been available since twig 1.34 and 2.4, so this is a simple fix.

https://github.com/twigphp/Twig/commit/c9a2f3db8b04d8dc92c825f1bfd05ee57a89fd7f

thelem avatar May 10 '20 12:05 thelem

Fourth deprecation:

User Deprecated: Referencing controllers with ComurImageBundle:Upload:getLibraryImages is deprecated since Symfony 4.1, use "Comur\ImageBundle\Controller\UploadController::getLibraryImagesAction" instead.

Further documentation is at https://symfony.com/blog/new-in-symfony-4-1-deprecated-the-bundle-notation

The suggested fix is supported in Symfony 3.4, so again this will be a simple change (in three places).

thelem avatar May 10 '20 12:05 thelem

Fifth deprecation:

User Deprecated: The "Comur\ImageBundle\Controller\UploadController" class extends "Symfony\Bundle\FrameworkBundle\Controller\Controller" that is deprecated since Symfony 4.2, use "Symfony\Bundle\FrameworkBundle\Controller\AbstractController" instead.

This is documented at https://symfony.com/blog/new-in-symfony-4-1-deprecated-the-bundle-notation The important difference between these two parents is that services must now be explicitly injected rather than relying on $this->get()

thelem avatar May 10 '20 13:05 thelem

That's all the deprecation messages that I can see. These changes are working well for me in Symfony 4.4 and based on the documentation should work with the older versions that are supported. I'm happy with these changes.

thelem avatar May 10 '20 13:05 thelem

Use of Symfony\Component\Translation\TranslatorInterface prevents Symfony 5 working at all when the bundle is enabled. Its replacement was only introduced in Symfony 4.2, so I've had to make that the minimum supported version.

thelem avatar Mar 08 '21 14:03 thelem

Hi @thelem

Thx for your contribution and sorry for the delay. Can you check your PR / update it so I can merge ? My recent updates (I removed JMSTranslation as you suggested) so it created a conflict.

Thx

comur avatar Mar 08 '21 15:03 comur

@comur Thanks for the response. No worries, I know what it's like to be an open source maintainer.

I've merged master into my branch which includes the removal of twig extensions. The JMSTranslation issue is still open and it's still listed as a dependency in composer.json.

I've got a separate problem with my Symfony 5 upgrade at the moment so I can't say for sure whether the current changes are enough for Symfony 5. I'll comment again once Symfony 5 is working.

thelem avatar Mar 08 '21 15:03 thelem

With that last commit this is now working for me on Symfony 5 and twig 3.

I have tested uploading, cropping and using an existing image and everything worked well.

thelem avatar Mar 08 '21 17:03 thelem

With that last commit this is now working for me on Symfony 5 and twig 3.

I have tested uploading, cropping and using an existing image and everything worked well.

What... how is that possible. First of all, the docs on this repo says to install "composer require comur/content-admin-bundle" - which has no inclusion of this repo. Second, trying to install this repo on a Symfony 5 project will fail because http foundation is not updated in composer.json.

Mecanik avatar Oct 15 '21 15:10 Mecanik

@Mecanik I think that's just a readme error - a block of text that has been copy-pasted from the admin bundle readme without being modified for the image bundle. Try composer require comur/comur-image-bundle.

This is an open PR though. At the moment the master branch does not support symfony 5.

thelem avatar Oct 16 '21 13:10 thelem

@thelem I saw... Pitty the author made a joke of this bundle. Actually, I took one of your branches from your fork and it works completely fine with Symphony 5. I had to make other changes as well to fit my needs with BS5, etc. Overall the bundle works well, just scattered everywhere.

Mecanik avatar Oct 16 '21 14:10 Mecanik

The author has provided and maintains this bundle for free. Yes, it would be good if he was able to be more responsive to pull requests and other improvements, but he's got his own life to lead and a bundle that he wrote several years ago isn't necessarily his top priority any more. You mention you've made other changes. Are these changes that other people would find useful? You don't seem to have made any effort to contribute them back to the community.

thelem avatar Oct 17 '21 18:10 thelem