Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 1e1f91c

Browse files
committed
refactor($http): remove unneeded call to $apply
This `$apply` call was needed only for tests, so it's been moved to `$httpBackend.flush`. BREAKING CHANGE: `$httpProvider.useApplyAsync` has been removed
1 parent 7d50b2e commit 1e1f91c

File tree

4 files changed

+37
-107
lines changed

4 files changed

+37
-107
lines changed

src/ng/http.js

+3-49
Original file line numberDiff line numberDiff line change
@@ -343,34 +343,6 @@ function $HttpProvider() {
343343
jsonpCallbackParam: 'callback'
344344
};
345345

346-
var useApplyAsync = false;
347-
/**
348-
* @ngdoc method
349-
* @name $httpProvider#useApplyAsync
350-
* @description
351-
*
352-
* Configure $http service to combine processing of multiple http responses received at around
353-
* the same time via {@link ng.$rootScope.Scope#$applyAsync $rootScope.$applyAsync}. This can result in
354-
* significant performance improvement for bigger applications that make many HTTP requests
355-
* concurrently (common during application bootstrap).
356-
*
357-
* Defaults to false. If no value is specified, returns the current configured value.
358-
*
359-
* @param {boolean=} value If true, when requests are loaded, they will schedule a deferred
360-
* "apply" on the next tick, giving time for subsequent requests in a roughly ~10ms window
361-
* to load and share the same digest cycle.
362-
*
363-
* @returns {boolean|Object} If a value is specified, returns the $httpProvider for chaining.
364-
* otherwise, returns the current configured value.
365-
**/
366-
this.useApplyAsync = function(value) {
367-
if (isDefined(value)) {
368-
useApplyAsync = !!value;
369-
return this;
370-
}
371-
return useApplyAsync;
372-
};
373-
374346
/**
375347
* @ngdoc property
376348
* @name $httpProvider#interceptors
@@ -1342,17 +1314,9 @@ function $HttpProvider() {
13421314
var applyHandlers = {};
13431315
forEach(eventHandlers, function(eventHandler, key) {
13441316
applyHandlers[key] = function(event) {
1345-
if (useApplyAsync) {
1346-
$rootScope.$applyAsync(callEventHandler);
1347-
} else if ($rootScope.$$phase) {
1348-
callEventHandler();
1349-
} else {
1350-
$rootScope.$apply(callEventHandler);
1351-
}
1352-
1353-
function callEventHandler() {
1317+
$rootScope.$evalAsync(function callEventHandler() {
13541318
eventHandler(event);
1355-
}
1319+
});
13561320
};
13571321
});
13581322
return applyHandlers;
@@ -1364,7 +1328,6 @@ function $HttpProvider() {
13641328
* Callback registered to $httpBackend():
13651329
* - caches the response if desired
13661330
* - resolves the raw $http promise
1367-
* - calls $apply
13681331
*/
13691332
function done(status, response, headersString, statusText, xhrStatus) {
13701333
if (cache) {
@@ -1376,16 +1339,7 @@ function $HttpProvider() {
13761339
}
13771340
}
13781341

1379-
function resolveHttpPromise() {
1380-
resolvePromise(response, status, headersString, statusText, xhrStatus);
1381-
}
1382-
1383-
if (useApplyAsync) {
1384-
$rootScope.$applyAsync(resolveHttpPromise);
1385-
} else {
1386-
resolveHttpPromise();
1387-
if (!$rootScope.$$phase) $rootScope.$apply();
1388-
}
1342+
resolvePromise(response, status, headersString, statusText, xhrStatus);
13891343
}
13901344

13911345

src/ngMock/angular-mocks.js

+4
Original file line numberDiff line numberDiff line change
@@ -1849,10 +1849,14 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
18491849
var part = responses.splice(skip, 1);
18501850
if (!part.length) throw new Error('No more pending request to flush !');
18511851
part[0]();
1852+
// Calling $digest again to execute the callbacks of the promises returned from $http
1853+
// as these callbacks can add new requests to the queue to flush.
1854+
if (digest !== false) $rootScope.$digest();
18521855
}
18531856
} else {
18541857
while (responses.length > skip) {
18551858
responses.splice(skip, 1)[0]();
1859+
if (digest !== false) $rootScope.$digest();
18561860
}
18571861
}
18581862
$httpBackend.verifyNoOutstandingExpectation(digest);

test/ng/directive/ngIncludeSpec.js

+8-3
Original file line numberDiff line numberDiff line change
@@ -362,18 +362,23 @@ describe('ngInclude', function() {
362362

363363
it('should construct SVG template elements with correct namespace', function() {
364364
if (!window.SVGRectElement) return;
365-
module(function($compileProvider) {
365+
module(function($compileProvider, $provide) {
366366
$compileProvider.directive('test', valueFn({
367367
templateNamespace: 'svg',
368368
templateUrl: 'my-rect.html',
369369
replace: true
370370
}));
371+
$provide.decorator('$templateRequest', function($delegate) {
372+
return jasmine.createSpy('$templateRequest').and.callFake($delegate);
373+
});
371374
});
372-
inject(function($compile, $rootScope, $httpBackend) {
375+
inject(function($compile, $rootScope, $httpBackend, $templateRequest) {
373376
$httpBackend.expectGET('my-rect.html').respond('<g ng-include="\'include.svg\'"></g>');
374377
$httpBackend.expectGET('include.svg').respond('<rect></rect><rect></rect>');
375378
element = $compile('<svg><test></test></svg>')($rootScope);
376-
$httpBackend.flush();
379+
expect($templateRequest).toHaveBeenCalledTimes(1);
380+
$httpBackend.flush(2);
381+
expect($templateRequest).toHaveBeenCalledTimes(2);
377382
var child = element.find('rect');
378383
expect(child.length).toBe(2);
379384
// eslint-disable-next-line no-undef

test/ng/httpSpec.js

+22-55
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,6 @@ describe('$http', function() {
301301
$http = $h;
302302
$rootScope = $rs;
303303
$sce = $sc;
304-
spyOn($rootScope, '$apply').and.callThrough();
305304
}]));
306305

307306
it('should throw error if the request configuration is not an object', function() {
@@ -1073,35 +1072,7 @@ describe('$http', function() {
10731072

10741073
describe('callbacks', function() {
10751074

1076-
it('should $apply after success callback', function() {
1077-
$httpBackend.when('GET').respond(200);
1078-
$http({method: 'GET', url: '/some'});
1079-
$httpBackend.flush();
1080-
expect($rootScope.$apply).toHaveBeenCalledOnce();
1081-
});
1082-
1083-
1084-
it('should $apply after error callback', function() {
1085-
$httpBackend.when('GET').respond(404);
1086-
$http({method: 'GET', url: '/some'}).catch(noop);
1087-
$httpBackend.flush();
1088-
expect($rootScope.$apply).toHaveBeenCalledOnce();
1089-
});
1090-
1091-
1092-
it('should $apply even if exception thrown during callback', inject(function($exceptionHandler) {
1093-
$httpBackend.when('GET').respond(200);
1094-
callback.and.throwError('error in callback');
1095-
1096-
$http({method: 'GET', url: '/some'}).then(callback);
1097-
$httpBackend.flush();
1098-
expect($rootScope.$apply).toHaveBeenCalledOnce();
1099-
1100-
$exceptionHandler.errors = [];
1101-
}));
1102-
1103-
1104-
it('should pass the event handlers through to the backend', function() {
1075+
it('should pass the event handlers through to the backend', inject(function($browser) {
11051076
var progressFn = jasmine.createSpy('progressFn');
11061077
var uploadProgressFn = jasmine.createSpy('uploadProgressFn');
11071078
$httpBackend.when('GET').respond(200);
@@ -1117,16 +1088,17 @@ describe('$http', function() {
11171088
expect(mockXHR.upload.$$events.progress).toEqual(jasmine.any(Function));
11181089

11191090
var eventObj = {};
1120-
spyOn($rootScope, '$digest');
11211091

11221092
mockXHR.$$events.progress(eventObj);
1093+
// The invocation of the callback is scheduled via `$evalAsync`,
1094+
// that's why `flush` is needed here.
1095+
$browser.defer.flush();
11231096
expect(progressFn).toHaveBeenCalledOnceWith(eventObj);
1124-
expect($rootScope.$digest).toHaveBeenCalledTimes(1);
11251097

11261098
mockXHR.upload.$$events.progress(eventObj);
1099+
$browser.defer.flush();
11271100
expect(uploadProgressFn).toHaveBeenCalledOnceWith(eventObj);
1128-
expect($rootScope.$digest).toHaveBeenCalledTimes(2);
1129-
});
1101+
}));
11301102
});
11311103

11321104

@@ -2293,35 +2265,27 @@ describe('$http', function() {
22932265
});
22942266

22952267

2296-
describe('$http with $applyAsync', function() {
2297-
var $http, $httpBackend, $rootScope, $browser, log;
2298-
beforeEach(module(function($httpProvider) {
2299-
$httpProvider.useApplyAsync(true);
2300-
}, provideLog));
2301-
2268+
describe('$http interacting with digest cycle', function() {
2269+
var $http, $httpBackend, $browser, log;
2270+
beforeEach(module(provideLog));
23022271

2303-
beforeEach(inject(['$http', '$httpBackend', '$rootScope', '$browser', 'log', function(http, backend, scope, browser, logger) {
2272+
beforeEach(inject(['$http', '$httpBackend', '$browser', 'log', function(http, backend, browser, logger) {
23042273
$http = http;
23052274
$httpBackend = backend;
2306-
$rootScope = scope;
23072275
$browser = browser;
2308-
spyOn($rootScope, '$apply').and.callThrough();
2309-
spyOn($rootScope, '$applyAsync').and.callThrough();
2310-
spyOn($rootScope, '$digest').and.callThrough();
2311-
spyOn($browser.defer, 'cancel').and.callThrough();
23122276
log = logger;
23132277
}]));
23142278

23152279

2316-
it('should schedule coalesced apply on response', function() {
2280+
it('should execute callbacks asynchronously in $digest', function() {
23172281
var handler = jasmine.createSpy('handler');
23182282
$httpBackend.expect('GET', '/template1.html').respond(200, '<h1>Header!</h1>', {});
23192283
$http.get('/template1.html').then(handler);
2320-
// Ensure requests are sent
2321-
$rootScope.$digest();
2284+
// Ensure requests are sent. ($http is internally promise-based and doesn't start working until
2285+
// $digest occurs.)
2286+
$browser.defer.flush();
23222287

23232288
$httpBackend.flush(null, null, false);
2324-
expect($rootScope.$applyAsync).toHaveBeenCalledOnce();
23252289
expect(handler).not.toHaveBeenCalled();
23262290

23272291
$browser.defer.flush();
@@ -2336,11 +2300,14 @@ describe('$http with $applyAsync', function() {
23362300
$http.get('/template1.html').then(log.fn('response 1'));
23372301
$http.get('/template2.html').then(log.fn('response 2'));
23382302
// Ensure requests are sent
2339-
$rootScope.$digest();
2303+
$browser.defer.flush();
23402304

2305+
// Resolve the promises. When a $q promise is resolved it uses $rootScope.$evalAsync to schedule
2306+
// the execution of its callbacks.
23412307
$httpBackend.flush(null, null, false);
23422308
expect(log).toEqual([]);
23432309

2310+
// Execute the promises' callbacks in the $digest scheduled with $evalAsync
23442311
$browser.defer.flush();
23452312
expect(log).toEqual(['response 1', 'response 2']);
23462313
});
@@ -2354,16 +2321,16 @@ describe('$http with $applyAsync', function() {
23542321
$http.get('/template1.html').then(log.fn('response 1'));
23552322
$http.get('/template2.html').then(log.fn('response 2'));
23562323
$http.get('/template3.html').then(log.fn('response 3'));
2357-
// Ensure requests are sent
2358-
$rootScope.$digest();
23592324

23602325
// Intermediate $digest occurs before 3rd response is received, assert that pending responses
2361-
/// are handled
2326+
// are handled. Unless false is passed as the second parameter, $httpBackend.flush calls
2327+
// $rootScope.$digest at least twice (before and after doing the flush).
23622328
$httpBackend.flush(2);
23632329
expect(log).toEqual(['response 1', 'response 2']);
23642330

2365-
// Finally, third response is received, and a second coalesced $apply is started
2331+
// Finally, third response is received, and its callback is scheduled with $evalAsync
23662332
$httpBackend.flush(null, null, false);
2333+
// Execute the promises' callbacks in the $digest scheduled with $evalAsync
23672334
$browser.defer.flush();
23682335
expect(log).toEqual(['response 1', 'response 2', 'response 3']);
23692336
});

0 commit comments

Comments
 (0)