Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Pass status and XHR to success and complete handlers #84

Open
wants to merge 2 commits into from

5 participants

@inf0rmer

Hi,

I needed to access the XHR object on my success handlers, so I added this quick hack. Is this the proper way to do it? Also, is there any reason it wasn't included from the start?

Thanks!

@posativ

+1 for this.

@rvagg
Collaborator

@inf0rmer whydid you remove the IE JSONP hack? And.. you've edited the build file, reqwest.js and also the source file src/reqwest.js, you should only be editing the latter for your commits. Cheers.

@inf0rmer

Ah, about the IE thing, I must've screwed up the pull request. I only meant to commit the part relevant to the request title. It's unrelated to this issue, but I had to do that or otherwise JSONP requests would not work on IE10 (although that was on a beta version, but I needed it for a project at the time).

A similar excuse about pullrequesting the built file, that was a silly mistake. My first pull request, how much more wrong could it have gone? :P

@rvagg
Collaborator

Congrats on your first pull request! It's great that you decided to contribute to Reqwest.

Interesting news that JSONP is broken on IE10, I really need to sort out a Windows 8 machine now that IE10 is in the wild.

To fix the pull request, you can either make another commit to your master branch and it'll fix it up here (it's better to do a PR from a separate branch by the way so it's easier to avoid adding extraneous commits), or close this PR and open a new one with just the commit/changes you're proposing.

@adrianheine

If you set the type to something non-existing, you get the original XHR object, see this example.

@terinjokes

I also have a use case for the XHR object.

@ded Can we get something similar merged, JSONP hack for IE notwithstanding?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 23, 2012
  1. @inf0rmer
Commits on Sep 14, 2012
  1. @inf0rmer
This page is out of date. Refresh to see the latest.
Showing with 5 additions and 12 deletions.
  1. +0 −8 reqwest.js
  2. +5 −4 src/reqwest.js
View
8 reqwest.js
@@ -99,20 +99,12 @@
script.type = 'text/javascript'
script.src = url
script.async = true
- if (typeof script.onreadystatechange !== 'undefined') {
- // need this for IE due to out-of-order onreadystatechange(), binding script
- // execution to an event listener gives us control over when the script
- // is executed. See http://jaubourg.net/2010/07/loading-script-as-onclick-handler-of.html
- script.event = 'onclick'
- script.htmlFor = script.id = '_reqwest_' + reqId
- }
script.onload = script.onreadystatechange = function () {
if ((script[readyState] && script[readyState] !== 'complete' && script[readyState] !== 'loaded') || loaded) {
return false
}
script.onload = script.onreadystatechange = null
- script.onclick && script.onclick()
// Call the user callback with the last value stored and clean up values and scripts.
o.success && o.success(lastValue)
lastValue = undefined
View
9 src/reqwest.js
@@ -176,7 +176,8 @@
}
function success(resp) {
- var r = resp.responseText
+ var r = resp.responseText,
+ xhr = resp
if (r) {
switch (type) {
case 'json':
@@ -198,10 +199,10 @@
}
}
- fn(resp)
- o.success && o.success(resp)
+ fn(resp, xhr.status, xhr)
+ o.success && o.success(resp, xhr.status, xhr)
- complete(resp)
+ complete(resp, xhr.status, xhr)
}
function error(resp, msg, t) {
Something went wrong with that request. Please try again.