slimphp / Slim

Slim is a PHP micro framework that helps you quickly write simple yet powerful web applications and APIs.
http://slimframework.com
MIT License
11.94k stars 1.95k forks source link

(Suggestion) change Response::withJson() to be in line with PSR-7 methods #1776

Closed npx closed 8 years ago

npx commented 8 years ago
public function withJson($data, $status = 200, $encodingOptions = 0)
{
    $body = $this->getBody();
    $body->rewind();
    $body->write(json_encode($data, $encodingOptions));

    return $this->withStatus($status)->withHeader('Content-Type', 'application/json;charset=utf-8');
}

Yes, withJson() is not part of PSR-7, but the naming suggests that it works like the other PSR-7 compliant methods.

I expected $response->withStatus(500)->withJson([ ... ]) to not override my status code.

Granted, I could use it as $response->withJson([...], 500) which to be honest I did not know about until I stumbled across this.

I feel like the naming strongly suggests the usual PSR-7 "cloning and keep your properties"-behaviour and suggest keeping the status code:

public function withJson($data, $status = null, $encodingOptions = 0)
{
    $body = $this->getBody();
    $body->rewind();
    $body->write(json_encode($data, $encodingOptions));

    $status = is_null($status) ? $this->getStatusCode() : null;

    return $this->withStatus($status)->withHeader('Content-Type', 'application/json;charset=utf-8');
}

What do you think?

TL;DR withJson is not part of PSR-7 but is named like a PSR-7 compliant method and therefore should not override a response's status code

akrabat commented 8 years ago

This has already been fixed in https://github.com/slimphp/Slim/pull/1737 and will be available in version 3.2.