comur / ComurImageBundle

Image upload / crop bundle for Symfony2
105 stars 76 forks source link

Support Symfony 5.x #123

Open thelem opened 4 years ago

thelem commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 3 years ago

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.

comur commented 3 years ago

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

thelem commented 3 years ago

@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 commented 3 years ago

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.

Mecanik commented 2 years ago

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.

thelem commented 2 years ago

@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.

Mecanik commented 2 years ago

@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.

thelem commented 2 years ago

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.