Automatic Jsonp support for json responses #320

Closed
christiaan opened this Issue Nov 6, 2012 · 4 comments

Projects

None yet

2 participants

@christiaan

I've created this bundle but maybe it would be good to incorporate it into the FOSRestBundle
https://github.com/Web-Power/restbundle

The detection works as following

function onKernelResponse(FilterResponseEvent $event)
{
    $response = $event->getResponse();
    if ($this->isJsonResponse($response)) {
        $callback = $this->getCallbackFromRequest($event->getRequest());
        if ($callback) {
            $this->wrapResponseInCallback($response, $callback);
        }
    }
}
@lsmith77
Member
lsmith77 commented Nov 7, 2012

have you seen this? #266

@christiaan

No I had not. This was basically a practice for me to get to know the Kernel Events and I guess it was more then 3 months ago.

As I am relatively new to Symfony could you explain what the pros and con's are of your and mine approach?
Mine seems to have a lot less coupling.

@lsmith77
Member
lsmith77 commented Nov 8, 2012

Your approach is not bound to using the ViewHandler. the draw back is that it adds overhead to all requests, even those that do not use jsonp.

There is also a fundamental difference, since you detect the fact that its a jsonp request by looking of there is a callback in the request, while in my PR the idea would be to detect it based on the mimetype. however there is an issue with my approach (see #266 (comment)), which I never had the time to investigate ..

@christiaan

Thanks for the explanation, I guess I'll just keep it as a separate bundle then. No need to integrate when there are no dependencies anyway.

@christiaan christiaan closed this Nov 9, 2012
@lsmith77 lsmith77 referenced this issue Nov 17, 2012
Merged

added a JsonpHandler #266

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