-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Removing Controller::getRequest() deprecation note #12121
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
Conversation
It was also deprecated as you can inject it into your actions if needed. Having two ways of doing the same thing is never a good idea (and injecting the request works with any controllers.) That said I do understand the argument of the extra use statement. |
I know programmers are supposed to be lazy, but adding a use statement is not a problem unless you write your code in notepad.exe. In my opinion, it should not be a reason to remove this deprecation. |
If you are lazy you can create your own base controller class and add |
Hi guys! I do occasionally see Notepad++ actually - especially with beginners ;). I also don't expect a beginner to add their own base controller method - if it's useful, it should just be there. The only disadvantage I see is what Fabien mention: that there would be 2 ways to get the request. In the Framework, I would actually be ok with always showing getRequest() in the docs - it's more consistent with how we get any other object. A small mention would remain (as well as plenty of docs on the web) about using it as an argument if you're registering your controller as a service. So, I'd like to un-deprecate this and actually recommend it in the docs. Thanks! |
@weaverryan If anything, I like Symfony to recommend nice solutions over the easy ones. Yet it doesn't mean that Symfony shouldn't provide the easy options nor ignore them in the documentation. CoaS vs Parent Controller I really, really don't want to see this un-deprecated because of the argument that a use-statement is too much work. I think it's fine the way it is and the current solution works for both Controller as a Service and with a parent controller. // DIC/service wise
use ...;
//...
public function someAction(Request $request)
{
return [
'address' => $this->addressService->getFromRequest($request),
'contact' => $this->contactService->getFromRequest($request)
];
}
// Parent Controller wise
public function someAction()
{
$request = $this->getRequest();
return [
'address' => $this->get('address_finder')->getFromRequest($request),
'contact' => $this->get('contact_finder')->getFromRequest($request)
];
} PS: if anything, Symfony2 should recommend Controller as a Service in the docs because it enforces 3 important things:
|
I agree with Ryan here. I know that experienced Symfony developers won't use this ... but unfortunately, for the regular PHP developer, adding a |
And actually, I would use this - it saves me from going to the top of my method or file to add other information. That's a small convenience, but I would prefer this way :). |
|
I'm 👎 for removing the deprecation. I think it's important to force newcomers to understand the big picture of Symfony: converting a request into a response. And that's what a controller does in Symfony. There is no magic! It makes also more sense to remove this In my opinion, if the developer forgets to put the
We can easily deduct the use statement is missing if the typehint of the argument is |
@hhamon AFAIK, we already suggest that you forgot the use statement and suggest alternatives when using the DebugClassLoader |
I do agree with Hugo's point on how having the Request as an argument adds clarity to the "your job is to transform a request into a response" mantra. So, I'm going to close this. BUT, if generators add the use statement and beginner docs that talk about controllers includes the use statement, then we probably won't have this problem anyways. I'll follow up in that direction. Thanks for the conversation |
Hi guys!
When the
request_stack
was introduced, we deprecatedController::getRequest()
- #9553.I would argue that there is value to this method, as it saves me from needing to add a
use
statement in my controller to get the request. This is similar to why we have theredirect
shortcut method.Since the method does use the
request_stack
, I don't see the disadvantage to having this.Thanks!