-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
PSR-7 Implementation Decoupling #2529
Conversation
Thank you very much for this great feature. My question is: Will Slim 4 be delivered with a "default" PSR-7 implementation or do we have to make this decision ourselves now? |
@odan you have to make the decision yourself as to which PSR-7 implementation you choose. However Slim-Skeleton will ship with Nyholm/psr-7 and Slim-Http decorators. |
@l0gicgate I am surprised that the Slim "inhouse" PSR-7 implementation slimphp/Slim-Http is not listed. What reason is there for not using slim/http? What does this mean for the future of Slim-Http library? Is it possible to use use the slim/http library in Slim 4 or is it not recommended? |
Can you also include an example using Guzzle/psr7? |
While, I very much like the separation created by ResponseEmitter, I would like it used by default in |
It would be convenient if |
Slim/Router.php
Outdated
// Add route | ||
$route = $this->createRoute($methods, $pattern, $handler); | ||
/** @var callable $routeHandler */ | ||
$routeHandler = $handler; |
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.
What is the point of renaming the variable by assignment? Surely we should just rename it in the method parameter?
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.
Because PHPStan complains.
parameters: | ||
level: max | ||
ignoreErrors: | ||
- '^Parameter #1 $callable of callable Slim\\Interfaces\\InvocationStrategyInterface expects callable, array|callable given.^' |
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.
Can we add a comments to point out where this one happens?
tests/AppTest.php
Outdated
{ | ||
HeaderStackTestAsset::reset(); | ||
} | ||
|
||
public static function setupBeforeClass() | ||
{ | ||
// ini_set('log_errors', 0); |
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.
Don't need this commented out line or the one in tearDownAfterClass()
README.md
Outdated
$responseEmitter = new ResponseEmitter(); | ||
$responseEmitter->emit($response); | ||
$request = $serverRequestFactory->fromGlobals(); | ||
$app->run($request, $psr17Factory); |
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.
run()
no longer takes a ResponseFactory.
$app->get('/hello/{name}', function ($request, $response, $args) { | ||
return $response->getBody()->write("Hello, " . $args['name']); | ||
}); | ||
|
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.
Lose this blank line.
README.md
Outdated
* which include ResponseFactoryInterface | ||
* @param ServerRequestInterface An instantiation of a ServerRequest | ||
* @param ResponseFactoryInterface An instantiation of a ResponseFactory | ||
* The App::__constructor() Method takes 1 mandatory parameter and 2 optional parameters |
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.
Method has a lowercase m unless starting a sentence.
README.md
Outdated
* You will need to emit the response by using an emitter of your choice | ||
* We will use Slim ResponseEmitter for this example | ||
* But you could use Zend HttpHandleRunner SapiEmitter or other | ||
* The App::run() Method takes 1 parameters |
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.
s/parameters/parameter
This PR decouples the
App
and all the out-of-the-box Slim middlewares entirely from any specific PSR-7 Implementation. You are now able to use any implementation of your choice. I also extracted theApp::respond()
andApp::finalize()
methods and created aResponseEmitter
.This also removes the dependency on
Slim/Http
which has become a PSR-7 Object Decorator repository since version 0.5. You are free to use the decorators if you would like since they piggy back on top of the PSR-17 factories that are usually provided by the PSR-7 Implementation of your choice.Choose a PSR-7 Implementation
Before you can get up and running with Slim you will need to choose a PSR-7 implementation that best fits your application. A few notable ones:
Example Usage With Nyholm/psr7 and Nyholm/psr7-server
Example Usage With Zend Diactoros & Zend HttpHandleRunner Response Emitter
Example Usage With Slim-Http Decorators and Zend Diactoros
Example Usage With Guzzle PSR-7 and Guzzle HTTP Factory
Status