Aerogear 534 - First try at Pipeline jsonp

[lholmquist]

see dev list for examples

[kborchers]

I would not say this has a default of false since you're actually just checking if it is undefined, which is falsey. I would have no default.

[lholmquist]

wrt the jsonp = "jsonp" , using the example below, i could only get it to work when i added the jsonp='jsonp' variable

var url = "http://us.battle.net/api/wow/achievement/2144"
//a jquery version for a baseline to make sure it worked
$.ajax( {
    type:'GET',
    url:url,
    contentType: 'application/json',
    dataType: 'jsonp',
    jsonp: 'jsonp',
    jsonpCallback: 'customCallback',
    success: function( data ){
        console.log( data );
    }
});

[lholmquist]

wrt to the first comment, right, i had the code a little different when i wrote that, forgot to change it

[lholmquist]

most likely the callback won't be overriden, but if some one is designing both the client and server, they could add a little bit of security by defining the callback on the server, so in that case they would need an override

[kborchers]

@lholmquist OK, we should remove the jsonp="jsonp" ... we don't want to program our lib to work with a specific service, i.e. battle.net. some other may just use "callback" or another may use "zippitydoda"

For the callback, ok. Leave it in for now.

[lholmquist]

@kborchers very true, i'm thinking then maybe we should have a settings for jsonp or something like that

//currently using this
pipeline.add([
    {
        name: "realmStatus",
        settings: {
            baseURL: baseURL,
            endpoint: "realm/status",
            jsonp: true
        }
    }
]);

//maybe should do this
pipeline.add([
    {
        name: "realmStatus",
        settings: {
            baseURL: baseURL,
            endpoint: "realm/status",
            jsonp: {
                callback: 'something',
                jsonp: 'jsonp' //zippitydoda,
                someOtherVarIfNeeded: 'another'
            }
        }
    }
]);

[kborchers]

I could probably go along with that. Let's see what it looks like and how it works and we'll go from there.

[lholmquist]

@kborchers not sure what else needs to be added, i actually got rid of the custom callback part, not sure if we need to do anything if they want to use save, since this will change POST to GET

[kborchers]

Well, I don't like the idea of changing a POST to a GET when someone tries to save. How about this? Let's add a crossDomain setting which is an object and if included means that all read()'s should use jsonp unless specifically set to false and all save()'s should check for CORS (both client and server via header) unless set to false. That object can contain:

crossDomain: {
    jsonp: "callback", // "callback" is default or can be set to any string or false to disable
    cors: true // true is the default or can be set to false to disable
}

Appropriate errors should be thrown for both if something goes wrong and possibly handled internally to make it easier on the developer (if possible). Specifically, for CORS, throw an error if on the client side XMLHttpRequest doesn't have the withCredentials prop (non-IE) or if XDomainRequest doesn't exist (IE) and on the server side, if the server returns an error and/or the Access-Control-Allow-Origin header is missing or doesn't allow * or the current app's domain.

If you get all of that, cool. If not, we can set up some time to chat if you want.

[lholmquist]

Well the change to GET happens by default from jquery.

let me digest this book, i mean small paragraph

[kborchers]

You'll want to move xdr.js to a subfolder and include the license file https://github.com/jaubourg/ajaxHooks/blob/master/MIT-LICENSE.txt like I just did for UUID

[kborchers]

Why not assume JSONP unless cors: true is specified as described here #8 (comment)

[kborchers]

Also, we shouldn't default to JSONP if they specify CORS but it's not supported because we have no idea if their server supports JSONP.

This is how I would imagine it working best but am open to other ideas:

if(crossDomain)
    if(cors && !hasCORS)
        fail the request
    else
        assume jsonp because they added crossDomain options but not CORS and set dataType and jsonp options

[lholmquist]

closing this since it was already added in another PR