Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixes to $response->json() #43

merged 2 commits into from May 17, 2012


None yet
3 participants

nickl- commented May 11, 2012

Thank you for this awesome hardcore minimalist one size fits all page router, knap gedaan!

Here is my small contribution to the amazing effort you've already done.

  • Added the Content-Type: application/json to the $request->json() function to reflect the json content we are responding with.
  • The quotes surrounding the $callback when doing the echo and passing a closure got php's knickers in a twist but by removing them it now works perfectly when doing something like;
    $response->json($data, function ($json) {
      return "<pre>". $json ."</pre>";

...not sure if this was the intended purpose or not?

PHP's complaint otherwise:
Catchable fatal error: Object of class Closure could not be converted to string in /[snip]/klein.php on line 474

Keep up the great work...

Fixes to ->json(); added Content-Type header to reflect the applicati…
…on/json content we are sending, removed quotes surrounding function call

abackstrom commented May 11, 2012

According to the README.md, $callback is the JSONP callback rather than the PHP callback, so it's valid to output $callback as a simple string. You could try to identify if a Callable was passed as $callback, but there would still be ambiguity for simple strings.

Disambiguate reference to the jsonp padding prefix misleadingly refer…
…red to as callback. Supply correct Content-Type response header for json but fall back on Content-Type text/javascript for jsonp as the proposed Content-Type application/json-p has not been adopted as yet. Documentation elaborated to further attempt to avoid confusion while keeping the implementation minimalist and klein.

nickl- commented May 12, 2012

Ahh indeed indeed, thought it didn't make sense to be a callback and due to the marginal documentation and ambiguous reference to a $callback function it results in a PLBKAC error instead of blaming PHP for being a whiney girl I have to first remove my end-user cap and replace it with the propeller-head I keep for emergencies.

Now that I am all clever with the propeller spinning to keep my engorged brain matter safely under a 100 I start knowing better than everyone and won't stop until you at least hear some of it:

  • I completely disagree with the notion that it is a callback! It is obvious what happened here again, some bloody idiot read the proposal with their limited capacity could only deduce that uhm duh uhm function oh yes I get it uhm uhm whatshamacallit that other thingy ah callback yes thats it, its a callback. Theres no bloody callback in JSON you moron its a serialized string. Why do you think its JSONP and not... lets not even go there. They called it JSON with padding for a reason you fool. No insult to injury would you even believe me if I told you these idiots standardized the request parameter for exchanging the function name as "callback", they should friggin pass their 515s I would love to "call them back" ...morons. I guess they would also expect us to refer to a "JSON callback" on the server-side and we would all stay ignorant to the actual callbacks, ahh bliss. I'm always telling you: it should hurt when you're stupid else you'll never stop.

Sound familiar? =)

Well at least a few crazy people like ourselves who probably also went off like that for a few hours managed to preserve JSON with padding so we can refer to the function definition as a padding prefix and its perfectly ok to use jsonp as the request parameter as well, just in case, you'll never know.

The intuitive approach would probably be to separate them into 2 functions function json($object) and function jsonp($prefix, $object) but I thought we would prefer to keep things _klein_.

I am also leaning heavily towards an automated approach handling the request checking for the presence of $request->callback or $request->jsonp but you know how it goes, someone would go swim upstream and sooner or later we would have to provide them with a function $request->setJSONPCallback($callback); *shivers* which would also require a function $request->getJSONPCallback(); and before you know it we change the name to vet.php... don't even go there.

My proposal:

    //Sends an object as json or jsonp by providing the padding prefix
    public function json($object, $jsonp_prefix = null) {
        $json = json_encode($object);
        if (null !== $jsonp_prefix) {
            header('Content-Type: text/javascript'); // should ideally be application/json-p once adopted
            echo "$jsonp_prefix($json);";
        } else {
            header('Content-Type: application/json');
            echo $json;

Updating the documentation to reflect the same:

   json($object, $jsonp_prefix = null)             // Send an object as JSON or JSONP by providing padding prefix

Tested Content-Type application/json-p but Chrome throws a fit so I gave her the assumed text/javascript so she can chill. Works as expected with jQuery $.getJSON(), do you think I missed anything else?

Taking off the propeller-head now before... well you know.


chriso commented May 17, 2012

Thanks. You typically pass &callback= to a JSONP endpoint hence my original approach. I agree that it could be ambiguous in the context of the library so merging now.

chriso added a commit that referenced this pull request May 17, 2012

Merge pull request #43 from nickl-/master
Fixes to $response->json()

@chriso chriso merged commit 59b17ab into klein:master May 17, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment