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

Add support for PSR-17 Http factory #99

Open
SamMousa opened this issue Oct 13, 2020 · 2 comments
Open

Add support for PSR-17 Http factory #99

SamMousa opened this issue Oct 13, 2020 · 2 comments

Comments

@SamMousa
Copy link

Instead of passing in a response object the JsonApi class should use a HttpFactoryInterface to generate the response if and when it is needed.

This will break backwards compatibility and will require minor code changes from library consumers.

@kocsismate
Copy link
Member

I'm not convinced this change would bring much benefit, while adding a new dependency to the project, and taking away the choice from users whether they want to use PSR-17 or not.

If I'm not mistaken, one could just use HttpFactoryInterface to instantiate the response before passing it to JsonApi. Or do you see any use-case when the above solution would work suboptimally?

@SamMousa
Copy link
Author

SamMousa commented Dec 7, 2020

The downsides to this use-case are the generic downsides of passing in an object to library code and hoping to get it back. So nothing is specific to this library.
This article lists some downsides for the middleware using PSR-7 Response objects: https://blog.ircmaxell.com/2016/05/all-about-middleware.html. Some of them will apply to this case as well.

For example, currently if I pass a response with some headers, what will happen to them? Will this library always remove them, always ignore them?
All of that is essentially unspecified.

while adding a new dependency to the project

The dependency contains 6 interfaces, no implementation.

A trivial implementation would change the sample code to something like this:

// Before
$jsonApi = new JsonApi($request, new Response(), $exceptionFactory);

// After if you don't want to use an implementing dependency
$responseFactory = new class implements ResponseFactoryInterface {
    public function createResponse(int $code = 200, string $reasonPhrase = ''): ResponseInterface {
        return new Response();
    }
}

$jsonApi = new JsonApi($request, responseFactory, $exceptionFactory);

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

2 participants