Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Symfony 5.x #123

Open
thelem opened this issue May 9, 2020 · 13 comments
Open

Support Symfony 5.x #123

thelem opened this issue May 9, 2020 · 13 comments

Comments

@thelem
Copy link

thelem commented May 9, 2020

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 symfony/symfony#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 pushed a commit to thelem/ComurImageBundle that referenced this issue May 9, 2020
@thelem
Copy link
Author

thelem commented May 10, 2020

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
Copy link
Author

thelem commented May 10, 2020

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.

twigphp/Twig@c9a2f3d

thelem pushed a commit to thelem/ComurImageBundle that referenced this issue May 10, 2020
@thelem
Copy link
Author

thelem commented May 10, 2020

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
Copy link
Author

thelem commented May 10, 2020

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
Copy link
Author

thelem commented May 10, 2020

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 thelem changed the title Deprecation error in Symfony 4.3 Deprecation errors in Symfony 4.x May 10, 2020
thelem pushed a commit to thelem/ComurImageBundle that referenced this issue Sep 6, 2020
in case controller has not been registered with controller.service_arguments
thelem pushed a commit to thelem/ComurImageBundle that referenced this issue Sep 6, 2020
Register the controller as a service with autowiring
thelem pushed a commit to thelem/ComurImageBundle that referenced this issue Jan 10, 2021
@thelem thelem changed the title Deprecation errors in Symfony 4.x Support Symfony 5.x Mar 8, 2021
thelem pushed a commit to thelem/ComurImageBundle that referenced this issue Mar 8, 2021
…#123

As documented in UPGRADE-4.2.md. 
Minimum Symfony version raised from 3.4 to 4.2.
Explict dependency added for symfony/translation.
@thelem
Copy link
Author

thelem commented Mar 8, 2021

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
Copy link
Owner

comur commented Mar 8, 2021

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
Copy link
Author

thelem commented Mar 8, 2021

@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 pushed a commit to thelem/ComurImageBundle that referenced this issue Mar 8, 2021
Interface was introduced on twig 1.23. Required for twig 2/3.
@thelem
Copy link
Author

thelem commented Mar 8, 2021

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
Copy link

Mecanik commented Oct 15, 2021

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
Copy link
Author

thelem commented Oct 16, 2021

@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
Copy link

Mecanik commented Oct 16, 2021

@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
Copy link
Author

thelem commented Oct 17, 2021

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants