feat ($http): Add custom createXhr factory to config object #9319

sjurba · 2014-09-28T18:54:49Z

To be able to construct non-standard or customised XHR objects a factory an optional factory function can be added to the config object. This is required for use of the mozAnon and mozSystem options for FirefoxOS.
See https://developer.mozilla.org/en-US/docs/DOM/XMLHttpRequest#XMLHttpRequest() for details.

This is an alternative solution to the one suggested in PR #7903. Closes #2318

sjurba · 2014-09-28T19:04:00Z

I added to commits to this PR. This first is just a simple implementation of the new config field. In the second I did a bit of refactoring to shorten the list of parameters in a few core functions in $httpBackend.
It would be good to get some feedback on whether this is required/desirable.

Secondly with the introduction of this config parameter I see the potential of greatly simplifying the process of mocking out $http for ngMock. But this will require a bit more work and it will likely have some wide spreading implications. So I won't do anything about that until I get some feedback on this idea from the angular team.

caitp · 2014-09-29T00:38:02Z

can you get the ngMockE2E test passing again? thanks

sjurba · 2014-09-29T00:55:17Z

Yeah, sry bout that.. Should be working again now.

caitp · 2014-09-29T01:10:55Z

so it's more stuff added to the public API surface, which means I can't just say 'lgtm' and merge it, people need to agree that it's what we want to do.

I'm not totally sure about letting this be configured per-request, that's not the approach jQuery takes with it. But in general it doesn't look bad to me so far.

sjurba · 2014-09-29T06:59:16Z

src/ng/http.js

@@ -684,7 +686,8 @@ function $HttpProvider() {
      var config = {
        method: 'get',
        transformRequest: defaults.transformRequest,
-        transformResponse: defaults.transformResponse
+        transformResponse: defaults.transformResponse,
+        createXhr: defaults.createXhr


You can set it on the globals object as well to configure it globally. I think that's what jquery does as well..

xhr (default: ActiveXObject when available (IE), the XMLHttpRequest otherwise) Type: Function() Callback for creating the XMLHttpRequest object. Defaults to the ActiveXObject when available (IE), the XMLHttpRequest otherwise. Override to provide your own implementation for XMLHttpRequest or enhancements to the factory.

And,

settings Type: PlainObject A set of key/value pairs that configure the Ajax request. All settings are optional. A default can be set for any option with $.ajaxSetup(). See jQuery.ajax( settings ) below for a complete list of all settings.

sjurba · 2014-09-29T07:09:08Z

You have any comments regarding using this for mocking?

caitp · 2014-09-29T14:21:18Z

src/ng/http.js

@@ -990,8 +993,7 @@ function $HttpProvider() {
          reqHeaders[(config.xsrfHeaderName || defaults.xsrfHeaderName)] = xsrfValue;
        }

-        $httpBackend(config.method, url, reqData, done, reqHeaders, config.timeout,
-            config.withCredentials, config.responseType);


even though this is smaller and saner, i'm not sure if we can actually make this change. @jeffbcross wdyt?

sjurba · 2014-09-29T14:37:55Z

The alternative is in the first commit sjurba@978e8b1

Adding yet another parameter..

pkozlowski-opensource · 2014-11-24T18:19:11Z

I was recently looking at the issues around $http and I agree that we need some flex point so people can provide they own factory for XHR objects. Having said this I agree with @caitp that having per-request attribute in a config object is kind of... I don't know... simply doesn't sound good to me.

Why not have a setting / function on the $httpBackend level that people could override / customise? Such a function would be passed a config object and return the XHR object. Basically moving createXhr to be "public" / overridable.

coolaj86 · 2015-04-30T06:26:00Z

Bump. #fxos still needs this. See http://stackoverflow.com/a/29960399/151312

It's an easy enough workaround, but there are a decent number of newbies for whom it adds significant barrier to entry for using angular on Firefox OS.

Coding isn't just for grown-ups anymore: https://www.youtube.com/watch?v=aH9Plt77cjM

DaAwesomeP · 2015-05-02T17:18:09Z

+1 I install Angular via bower and modifying the file after bower installs it is inconvenient and bower_components shouldn't be saved to a git repository anyways.

frekele · 2015-06-08T03:40:27Z

+++1 !!!!!

caitp · 2015-06-08T03:44:13Z

yeah, sorry this seems to have fell off the radar, I'll try to bring this up in the meeting tomorrow and put this back on track for landing

pkozlowski-opensource · 2015-06-08T19:51:03Z

OK, let's review this one! I will send an alternative proposal and then we will compare / decide.

Closes angular#2318

sjurba · 2015-06-13T09:28:51Z

I rebased this one onto master. I also removed the refactoring of the httpbackend arguments since from the feedback it didn't seem like something we could do.

So this is straight up attempt at adding an extra parameter to the config object.

If the new parameter should only be available configurable on the global it's a simple fix to move it out of the extend function and always overwrite it. However if this is the way to go, there is maybe a simpler solution. Do we have/can we get access to the defaults object in the httpBackend somehow? If so it might simplify the implementation a bit.

I'm more than happy to contribute, but I'll need your guidance.

pkozlowski-opensource · 2015-06-15T11:29:40Z

@sjurba we've discussed this PR during out last team meeting and I would like to run an alternative idea through you / the community.

So, I understand what you are trying to do and your approach is simple and effective. Still there is one thing I'm not too keen on - adding more properties to the $http's config object. This makes $http's API surface larger - we've got a config object with gazillion of properties. This starts to resemble the directive definition object and we all know that this is not the nicest API.

The alternative idea would be to abstract XHR object creation in a dedicated service (provider). Give me sec, I'm going to open a sketch of the PR with this idea.

gkalpak · 2015-06-15T11:37:14Z

👍 for not overloading the config object. $http's config IS the new DDO.

(I mean, I don't have to look up DDO properties any more (since several months now), but I still have to look up $http's config every time 😄)

pkozlowski-opensource · 2015-06-15T11:44:44Z

@sjurba @gkalpak OK, here is an alternative approach: pkozlowski-opensource@249efc5

Didn't test, missing unit-tests and docs etc. but this is just to get the idea across. Please let me know what do you think.

//cc: @petebacondarwin

gkalpak · 2015-06-15T11:56:17Z

@pkozlowski-opensource, I like the general idea (although your implementation has some bugs 😛).

pkozlowski-opensource · 2015-06-15T11:58:57Z

@gkalpak this is "pseudo implementation" :-) Want to confirm the approach before investing more than 5 minutes into it :-)

sjurba · 2015-06-15T11:59:03Z

@pkozlowski-opensource I like it too. Unless someone else jumps to it, I'll be happy to give it a go when I have some time later this week.

pkozlowski-opensource · 2015-06-15T12:00:00Z

@sjurba OK, I would be more than happy to let you continue along those lines! Feel free to ping me if you need any info / help!

…hr objects Closes #2318 Closes #9319 Closes #12159

DaAwesomeP · 2015-09-19T00:29:20Z

Thanks!

sjurba mentioned this pull request Sep 28, 2014

feat($http): add option for unsafe xhr requests in FirefoxOS #7903

Closed

mary-poppins added cla: yes and removed cla: no labels Sep 28, 2014

sjurba force-pushed the createXhr branch from 315872b to a1f8682 Compare September 29, 2014 00:53

sjurba reviewed Sep 29, 2014
View reviewed changes

caitp reviewed Sep 29, 2014
View reviewed changes

jeffbcross force-pushed the master branch from cad9560 to f294244 Compare October 2, 2014 22:09

btford force-pushed the master branch from 9011a65 to 46db47b Compare October 6, 2014 22:54

btford added the type: feature label Oct 6, 2014

btford added this to the Backlog milestone Oct 6, 2014

btford added the component: $http label Oct 6, 2014

jeffbcross force-pushed the master branch 4 times, most recently from e8dc429 to e83fab9 Compare October 10, 2014 17:38

petebacondarwin force-pushed the master branch from 4dd5a20 to 998c61c Compare October 19, 2014 10:58

pkozlowski-opensource self-assigned this Jan 21, 2015

caitp modified the milestones: 1.4.1, Backlog Jun 8, 2015

feat ($http): Add custom createXhr factory to config object

e0d0292

Closes angular#2318

sjurba force-pushed the createXhr branch from a1f8682 to e0d0292 Compare June 13, 2015 09:15

petebacondarwin modified the milestones: 1.4.1, 1.4.2 Jun 16, 2015

sjurba mentioned this pull request Jun 18, 2015

feat($http): add $xhrFactory service to enable creation of custom xhr objects #12159

Closed

petebacondarwin modified the milestones: 1.4.2, 1.4.3 Jul 6, 2015

petebacondarwin modified the milestones: 1.4.3, 1.4.4 Jul 16, 2015

Narretz modified the milestone: 1.4.4 Aug 14, 2015

petebacondarwin closed this in 106f90a Sep 18, 2015

petebacondarwin pushed a commit that referenced this pull request Sep 18, 2015

feat($http): add $xhrFactory service to enable creation of custom x…

7a413df

…hr objects Closes #2318 Closes #9319 Closes #12159

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat ($http): Add custom createXhr factory to config object #9319

feat ($http): Add custom createXhr factory to config object #9319

sjurba commented Sep 28, 2014

sjurba commented Sep 28, 2014

caitp commented Sep 29, 2014

sjurba commented Sep 29, 2014

caitp commented Sep 29, 2014

sjurba Sep 29, 2014

sjurba commented Sep 29, 2014

caitp Sep 29, 2014

sjurba commented Sep 29, 2014

pkozlowski-opensource commented Nov 24, 2014

coolaj86 commented Apr 30, 2015

DaAwesomeP commented May 2, 2015

frekele commented Jun 8, 2015

caitp commented Jun 8, 2015

pkozlowski-opensource commented Jun 8, 2015

sjurba commented Jun 13, 2015

pkozlowski-opensource commented Jun 15, 2015

gkalpak commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

gkalpak commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

sjurba commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

DaAwesomeP commented Sep 19, 2015

feat ($http): Add custom createXhr factory to config object #9319

feat ($http): Add custom createXhr factory to config object #9319

Conversation

sjurba commented Sep 28, 2014

sjurba commented Sep 28, 2014

caitp commented Sep 29, 2014

sjurba commented Sep 29, 2014

caitp commented Sep 29, 2014

sjurba Sep 29, 2014

Choose a reason for hiding this comment

sjurba commented Sep 29, 2014

caitp Sep 29, 2014

Choose a reason for hiding this comment

sjurba commented Sep 29, 2014

pkozlowski-opensource commented Nov 24, 2014

coolaj86 commented Apr 30, 2015

DaAwesomeP commented May 2, 2015

frekele commented Jun 8, 2015

caitp commented Jun 8, 2015

pkozlowski-opensource commented Jun 8, 2015

sjurba commented Jun 13, 2015

pkozlowski-opensource commented Jun 15, 2015

gkalpak commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

gkalpak commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

sjurba commented Jun 15, 2015

pkozlowski-opensource commented Jun 15, 2015

DaAwesomeP commented Sep 19, 2015