JSONP async fix and tests #396

Merged
merged 7 commits into from Jan 9, 2013

Conversation

Projects
None yet
4 participants
Owner

davglass commented Jan 7, 2013

This is the proper fix for #371

This adds Y.Get.js().execute() support.

I also added proper tests that work with echoecho under yogi test too.

Note: this should be tested with yogi@0.0.63 to make sure you get the latest echoecho

Ping: @lsmith @rgrove @ericf

@rgrove rgrove and 2 others commented on an outdated diff Jan 7, 2013

src/jsonp/js/jsonp.js
onFailure : wrap(config.on.failure),
onTimeout : wrap(config.on.timeout, true),
timeout : config.timeout,
charset : config.charset,
attributes: config.attributes
- });
+ }).execute();
@rgrove

rgrove Jan 7, 2013

Contributor

Might want to add a comment here explaining why execute() is called, just to avoid potential confusion.

@lsmith

lsmith Jan 7, 2013

Contributor

This makes all calls non-serial, but doesn't pass through the async config from jsonp() to Get.script().

I'm having trouble finding my pull request for this...

@lsmith

lsmith Jan 7, 2013

Contributor

Passing through the async config is just adding async: config.async at line 202, fwiw.

Contributor

rgrove commented Jan 7, 2013

👍

Owner

ericf commented Jan 7, 2013

👍

Contributor

lsmith commented Jan 7, 2013

👍

Contributor

lsmith commented Jan 7, 2013

#371 also added tests for the async attribute.

Owner

davglass commented Jan 7, 2013

I'll merge that in by hand since the file structure has been all changed up.

Owner

davglass commented Jan 7, 2013

Test added by hand..

Contributor

lsmith commented Jan 7, 2013

Cool. 👍 for realz

davglass merged commit 78a4c18 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