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

New YQL module for using JSONP #395

Merged
merged 2 commits into from Jan 9, 2013

Conversation

Projects
None yet
2 participants
Owner

davglass commented Jan 7, 2013

This creates a plugin module for YQL that uses JSONP so that it can be conditionally loaded. This will allow other environments (Nodejs and Win8) to not require loading JSONP when they are not needed.

Note: this requires a yql and a yui build to function properly

@ericf ericf commented on the diff Jan 8, 2013

src/yql/js/yql-jsonp.js
@@ -0,0 +1,24 @@
+/**
+* Plugin for YQL to use JSONP to make YQL requests. This is the default method,
+* when loaded in nodejs or winjs this will not load. The new module is needed
+* to make sure that JSONP is not loaded in the environments that it is not needed.
+* @module yql
+* @submodule yql-jsonp
+*/
+
+//Over writes Y.YQLRequest._send to use IO instead of JSONP
+Y.YQLRequest.prototype._send = function (url, o) {
+ if (o.allowCache !== false) {
+ o.allowCache = true;
@ericf

ericf Jan 8, 2013

Owner

Messing with other's objects here.

@ericf

ericf Jan 8, 2013

Owner

…looking at the main YQL source, that's happening in a few places. I feel that it's only okay to mess with objects in protected/private methods, if they are guaranteed to be a copy of a user's object passed into a public method.

@davglass

davglass Jan 8, 2013

Owner

It's been that way for years and not in the scope of this pull request. I'm not changing it now.. The scope of this pull request was to move code that depended on the jsonp module to another module so that the other environments don't need to require it.

@davglass davglass merged commit 2cda518 into yui:dev-3.x Jan 9, 2013

1 check failed

default The Travis build failed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment