Raw XHR to replace jQuery AJAX #9
Conversation
Removed old jQuery AHAX based _request & _raw_request functions New single function using raw XMLHttpRequest call to handle both Works in latest Chrome, Firefox & IE Means you no longer need remote jQuery dependency at all (Saves extra ~32k and using remote resource)
|
Looks great. Using it in the dev branch of Prose for a while, to check if there are any issues. Thanks a lot. |
|
Will also review your gist stuff, as they should definitely be supported. Just gimme some time. :) |
|
Okay... actually there are some errors with. Two options: You start testing your patched github.js with the latest Prose (gh-pages branch). Or I'll look into at the end of the week... I need to get some features ready, which has priority atm. Installation instructions are here: http://prose.io/help/internals.html let me know if you have any troublez. |
|
Oh damn, well, didn't test it with Prose, Node etc, just simple browser testing. Will have a look at tweaking to work with Prose in the next couple of days (wanted to play with that anyway) :) Once reduced in size, I'd like to end up integrating into my own web IDE https://github.com/mattpass/ICEcoder as it would make a very useful addition. Anyway, will let you know of fixes. |
|
Yeah. That'd be awesome. Take your time there's no need to hurry. But that stuff would be very useful. 0.7.0 could add Gists support. Thanks for helping out! |
|
@michael i have a gists branch which added some basic features. How do you plan to test it, since prose does not include gists? |
Old headers function simplified and now part of _request function Previously wasn't sending headers and so not able to authenticate Tested with basic & oAuth and works fine
|
Doh!! Noticed I hadn't included the headers with the XHR. I guess the errors you saw are the result of not passing thru basic or oAuth details and it was causing a 401/no auth error... so, I simplified the headers function and tucked that into the _request function too! :D (Now you have a single function which encompasses _request, _request_raw and headers - one function to do the lot). I've tested this with basic auth and oAuth, in Chrome & Firefox and works great. I've not tested with Node or a dev copy of Prose as it seemed a hassle to setup :/ Any chance you could drop this github.js into your dev Prose setup and give it a whirl? |
Raw XHR to replace jQuery AJAX
|
Works after some smaller tweaks. See: 8405f05 |
|
Was just a quick fix.. you may wanna refactor it a bit. |
|
Good catch on conditionally returning raw or JSON responseText. Not quite sure why you'd want to work positively with status codes >200 and <300 tho. 200 is the only good one? |
|
Updating the tree returns 201 Created, which is valid. Is it just the two guys? |
|
Ah OK. Maybe also 204 (no content) for empty files? |
|
Possibly also 202 (accepted) might happen with some git commands. Concerned that blanket covering 200-206 includes 206 (partial content) 200 || 201 || 202 || 204 sounds good to me. |
|
Haha. You ask the wrong guy. ;-) We should just check what jQuery is doing. |
|
Had a look in the jQuery lib and they have just what you have plus 304 (Not Modified)... I adjusted my version to cover this plus also added the Content-Type header as that you had in the previous jQuery based version of github.js too. Pull forward a pull request for this. |
Extensive Mocha tests for Node and Browser versions plus code coverage using Blanket and Coveralls
So this is an initial effort at replacing the jQuery based AJAX calls in _request & _request_raw functions.
Tested in latest Chrome, Firefox & IE and all seems to work well. Might need more testing though and perhaps someone to check it works with Node (haven't done extensive checking).
Looking at it, it doesn't seem like much code but works great as far as I can tell?? I also combined into a single function to handle both JSON & raw text callbacks, saves repeating code.
What do you think?