-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Use #[Autowire]
attribute instead of Yaml config
#1461
Conversation
afa8b8d
to
00ce9fb
Compare
bind: # defines the scalar arguments once and apply them to any service defined/created in this file | ||
string $locales: '%app_locales%' | ||
string $defaultLocale: '%locale%' | ||
string $emailSender: '%app.notifications.email_sender%' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would do it only for this one as ultra specific param
other are more generic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I reverted the 2 first.
c028cb0
to
bf229ce
Compare
@@ -62,6 +63,7 @@ public function changePassword( | |||
#[CurrentUser] User $user, | |||
Request $request, | |||
EntityManagerInterface $entityManager, | |||
#[Autowire('@security.logout_url_generator')] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this? We're using the default logout URL generator service, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The question is whether Symfony makes it auto-wirable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's needed.
Cannot autowire argument $logoutUrlGenerator of "App\Controller\UserController::changePassword()": it references class "Symfony\Component\Security\Http\Logout\LogoutUrlGenerator" but no such service exists. You should maybe alias this class to the existing "security.logout_url_generator" service.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking this 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened a PR symfony/symfony#52833
bf229ce
to
9f906e4
Compare
Thanks Jérôme. I like this a lot. However, while merging I reverted the change related to autowiring the LogoutUrlGenerator service. It looked a bit weird to me. Also, after seeing this pull request from yours: symfony/symfony#52833 this might be a thing of the past in the next Symfony version. Thanks! |
I'm not sure this is something you want to do: replacing Yaml config for attribute for class alias, to use the
#[Autowire]
attribute available since Symfony 6.1.I did the change, so let you decide if that's good or not for the demo.