-
Notifications
You must be signed in to change notification settings - Fork 56
Use response facttories in ErrorHandler and NotFoundHandler #153
Use response facttories in ErrorHandler and NotFoundHandler #153
Conversation
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.
Changes look good, and are as we discussed. I marked a couple additional changes for type safety, and will push those changes to your branch momentarily.
Additionally, I'll provide migration notes.
src/Middleware/ErrorHandler.php
Outdated
{ | ||
$this->responsePrototype = $responsePrototype; | ||
$this->responseFactory = function () use ($responseFactory) { |
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.
Needs a return typehint if decorating it is to be useful.
src/Middleware/NotFoundHandler.php
Outdated
{ | ||
$this->responsePrototype = $responsePrototype; | ||
$this->responseFactory = $responseFactory; |
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.
This one should be decorated with a closure as well.
composer.json
Outdated
@@ -20,7 +20,7 @@ | |||
}, | |||
"require": { | |||
"php": "^7.1", | |||
"psr/http-message": "^1.0", | |||
"fig/http-message-util": "^1.1.2", |
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 don't understand these particular changes.
- We depend directly on psr/http-message; removing it doesn't make sense.
- I'm unclear what the update to http-message-util 1.1.2 has to do with the changes in this patch.
Reinstates the `psr/http-message` requirement, as this is a direct dependency. Modifies the `fig/http-message-util` requirement to be `^1.1`, as that is the release that adds the `StatusCodeInterface`; we don't use any specifics from the later releases to make pinning to a later version necessary.
Updates the `$responseFactory` property of the `ErrorHandler` to add a return type hint, ensuring return type safety of the composed callable. Wraps the `$responseFactory` property of the `NotFoundHandler` in a callable, to ensure return type safety of the composed callable.
Thanks, @webimpress. |
No description provided.