Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass status and XHR to success and complete handlers #84

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

inf0rmer
Copy link

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
Copy link

posativ commented Oct 25, 2012

+1 for this.

@rvagg
Copy link
Collaborator

rvagg commented Oct 25, 2012

@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
Copy link
Author

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
Copy link
Collaborator

rvagg commented Oct 29, 2012

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
Copy link

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

@terinjokes
Copy link

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants