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

issue #1190 : renaming jsonp callback #1213

Closed
wants to merge 5 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

riadhchtara commented Jul 4, 2012

No description provided.

@TooTallNate TooTallNate and 1 other commented on an outdated diff Jul 4, 2012

lib/response.js
@@ -188,12 +188,21 @@ res.json = function(obj){
, replacer = app.get('json replacer')
, spaces = app.get('json spaces')
, body = JSON.stringify(obj, replacer, spaces)
- , callback = this.req.query.callback;
+ , callbackname = app.get('jsonp callback name')
@TooTallNate

TooTallNate Jul 4, 2012

You could do pretty much all of this in one line:

  , callback = this.req.query[app.get('jsonp callback name') || 'callback']
@riadhchtara

riadhchtara Jul 4, 2012

Contributor

I change it

@tj tj and 1 other commented on an outdated diff Jul 6, 2012

lib/response.js
@@ -188,12 +188,12 @@ res.json = function(obj){
, replacer = app.get('json replacer')
, spaces = app.get('json spaces')
, body = JSON.stringify(obj, replacer, spaces)
- , callback = this.req.query.callback;
+ , callback = this.req.query[app.get('jsonp callback name') || 'callback'];
@tj

tj Jul 6, 2012

Owner

we could even kill the || callback operation which will be the most common use-case and just add:

app.set('jsonp callback name', 'callback');

in application.js https://github.com/visionmedia/express/blob/master/lib/application.js#L84

I dont think many people will be changing it

@riadhchtara

riadhchtara Jul 6, 2012

Contributor

I have just fix that

Member

jonathanong commented Sep 10, 2013

this or something like it has already been merged: https://github.com/visionmedia/express/blob/master/lib/response.js#L234

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