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

callback&&callback({data:'here'}) JSONP format support #1374

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

yozik04 commented Oct 16, 2012

Currently JSONP only supports callback({data:'here'}) format.

This pull request adds callback&&callback({data:'here'}) format support.

Activate this format using
app.enable('jsonp callback long');

Jevgeni Kiski Long JSONP 5095604
Owner

tj commented Oct 16, 2012

this wouldn't be correct anyway because it could be undefined, in which case you'd need typeof. Also the client requesting the JSONP response is in charge of ensuring the callback exists

@tj tj closed this Oct 16, 2012

yozik04 commented Oct 16, 2012

If it will be undefined then it will silently skip callback. Try in browser
console. As JSONP request can not be properly aborted then it needs
something that will check if code is still waiting for the response. The
method I proposed is widely used.

Jevgeni

On 16.10.2012, at 18:33, TJ Holowaychuk notifications@github.com wrote:

this wouldn't be correct anyway because it could be undefined, in which
case you'd need typeof. Also the client requesting the JSONP response is in
charge of ensuring the callback exists


Reply to this email directly or view it on
GitHubhttps://github.com/visionmedia/express/pull/1374#issuecomment-9494587.

Owner

tj commented Oct 16, 2012

that will only work if it's declared but undefined, if you want to abort the client can easily replace that function with a noop

yozik04 commented Oct 16, 2012

Try with undefined in console or make a test page. It always works for me.

I do not want to override jQuery logic that removes callback function.

Jevgeni

On 16.10.2012, at 19:51, TJ Holowaychuk notifications@github.com wrote:

that will only work if it's declared but undefined, if you want to abort
the client can easily replace that function with a noop


Reply to this email directly or view it on
GitHubhttps://github.com/visionmedia/express/pull/1374#issuecomment-9497656.

Owner

tj commented Oct 16, 2012

that's not how javascript works.. if it's not declared then it will be a ReferenceError:

bar || 'something'
ReferenceError: bar is not defined

yozik04 commented Oct 16, 2012

Google maps uses callback&&callback()

Jevgeni

On 16.10.2012, at 20:16, TJ Holowaychuk notifications@github.com wrote:

that's not how javascript works.. if it's not declared then it will be a
ReferenceError:

bar || 'something'
ReferenceError: bar is not defined


Reply to this email directly or view it on
GitHubhttps://github.com/visionmedia/express/pull/1374#issuecomment-9498502.

Owner

tj commented Oct 16, 2012

if ('undefined' != typeof callback) callback() is what we really should be doing, but i dont think we would need to make it a new express option, the check wont really hurt to be on by default if we were to include it. Personally I think this is just sloppy behaviour on the client end but oh well I guess

yozik04 commented Oct 16, 2012

Open https://maps.google.com/
Open Developer Tools and Networking tab (Chrome)
Find "sentodata" request (https://maps.google.com/maps/sendtodata?&callback=_xdc_._6h8dc663t)

You will see that they are using something like that:
xdc._6h8dc663t && xdc._6h8dc663t(data)

Another option will be an ability to override an wrapper function

@tj tj added a commit that referenced this pull request Oct 16, 2012

@tj tj add cb && cb(payload) to `res.jsonp()`. Closes #1374 b3936b9
Owner

tj commented Oct 16, 2012

I've added the check though this will obviously fail if the var is not declared

yozik04 commented Oct 16, 2012

Thank you. That resolves my problem.
Uncaught TypeError: Property 'request_1234567' of object [object Window] is not a function

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