laminas / laminas-diactoros

PSR HTTP Message implementations
https://docs.laminas.dev/laminas-diactoros/
BSD 3-Clause "New" or "Revised" License
478 stars 62 forks source link

Add ImageResponse? #28

Closed weierophinney closed 2 years ago

weierophinney commented 4 years ago

How about adding an Zend\Diactoros\Response\ImageResponse that supports the most common file types like JPG, GIF and PNG? How could this be archieved?


Originally posted by @RalfEggert at https://github.com/zendframework/zend-diactoros/issues/141

weierophinney commented 4 years ago

I would say let Diactoros be safe from all these and only rely on the psr-7 interfaces.

If we need, we can add to Diactoros\Extras or something.

You may ask, what is the advantage ?

If we do add more helper function people will use those, and the purpose of switching between psr-7 packages will get into trouble. Lets not bring it due to the same.


Originally posted by @harikt at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-176289646

weierophinney commented 4 years ago

@harikt

Thanks for your feedback. For the same reason then Zend\Diactoros\Response\HtmlResponse or Zend\Diactoros\Response\JsonResponse should be deleted.


Originally posted by @RalfEggert at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-176366428

weierophinney commented 4 years ago

I don't know that will happen due to BC break. But I think yes.


Originally posted by @harikt at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-176567188

weierophinney commented 4 years ago

But these Reponse classes are really useful in the context of a Zend\Expressive application. If you want to remove them from Zend\Diactoros where should they be placed? Moving them to a different Zend\Diactoros package won't really solve the problem. People will still use them.


Originally posted by @RalfEggert at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-176581143

weierophinney commented 4 years ago

@RalfEggert I don't know the whole edge cases, but this is what I was thinking.

Zend\Diactoros\Extras can be type hinted to psr-7 response interfaces. So that means not just diactors can use these, but also any psr-7 responses.

But I understand that is not so simple, as the current ones are doing.

return new [JsonResponse](https://github.com/zendframework/zend-diactoros/blob/master/src/Response/JsonResponse.php)();

So I don't have a real answer for the current time. But I will think about the same.


Originally posted by @harikt at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-176582492

weierophinney commented 4 years ago

I want to keep them. I don't see how they reduce compatibility with PSR7. Those helper methods still create a PSR7 ResponseInterface. We typically only use those helper classes on controllers (which are tied to a framework like Expressive, so it definitely makes sense to use them), but on other layer we still typehint against ResponseInterface.

Those are just shortcuts that are super convenient to use. -1 for removing them.


Originally posted by @bakura10 at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-181282263

weierophinney commented 4 years ago

@bakura10

I have the same opinion that they are very useful and should be kept.

But what about the Zend\Diactoros\Response\ImageResponse I suggested?


Originally posted by @RalfEggert at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-181507439

weierophinney commented 4 years ago

I feel we should support anything that is often required. I'm not sure about ImageResponse. What is the use case? You return an image after processing in the back-end?


Originally posted by @bakura10 at https://github.com/zendframework/zend-diactoros/issues/141#issuecomment-181545741

sattes-faction commented 3 years ago

A possible use case for that might be image processing like generating and returning a thumbnail upon request.

RalfEggert commented 3 years ago

Or you return an image after doing some user right checks. If you are allowed to see the image, you get it delivered. If not you get a placeholder image.

alamagus commented 2 years ago

Or you return an image after doing some user right checks. If you are allowed to see the image, you get it delivered. If not you get a placeholder image.

It's better to delegate transfer of static files to web server. For example, via XSendfile in case of nginx

gsteel commented 2 years ago

I'm not in favour of this addition - If you're in a position where you need to stream image responses via PHP, then it's not much more bother to either implement your own responses, or manipulate a vanilla PSR-7 response.

Most of the existing concrete response classes cover the usual suspects such as html/xml/json - if we start adding image responses, do we stop at jpg/gif/png or start dragging in mime detection libs so we can send the right headers for tiff/webp/etc/etc.

This would also open the door for video response, Microsoft excel response et al.

Closing this as wont-fix for now, but please do open an RFC for further discussion if you feel this is an oversight.