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

refactor($http): remove unneeded call to $apply #13128

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 3 additions & 49 deletions src/ng/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -343,34 +343,6 @@ function $HttpProvider() {
jsonpCallbackParam: 'callback'
};

var useApplyAsync = false;
/**
* @ngdoc method
* @name $httpProvider#useApplyAsync
* @description
*
* Configure $http service to combine processing of multiple http responses received at around
* the same time via {@link ng.$rootScope.Scope#$applyAsync $rootScope.$applyAsync}. This can result in
* significant performance improvement for bigger applications that make many HTTP requests
* concurrently (common during application bootstrap).
*
* Defaults to false. If no value is specified, returns the current configured value.
*
* @param {boolean=} value If true, when requests are loaded, they will schedule a deferred
* "apply" on the next tick, giving time for subsequent requests in a roughly ~10ms window
* to load and share the same digest cycle.
*
* @returns {boolean|Object} If a value is specified, returns the $httpProvider for chaining.
* otherwise, returns the current configured value.
**/
this.useApplyAsync = function(value) {
if (isDefined(value)) {
useApplyAsync = !!value;
return this;
}
return useApplyAsync;
};

/**
* @ngdoc property
* @name $httpProvider#interceptors
Expand Down Expand Up @@ -1342,17 +1314,9 @@ function $HttpProvider() {
var applyHandlers = {};
forEach(eventHandlers, function(eventHandler, key) {
applyHandlers[key] = function(event) {
if (useApplyAsync) {
$rootScope.$applyAsync(callEventHandler);
} else if ($rootScope.$$phase) {
callEventHandler();
} else {
$rootScope.$apply(callEventHandler);
}

function callEventHandler() {
$rootScope.$evalAsync(function callEventHandler() {
eventHandler(event);
}
});
};
});
return applyHandlers;
Expand All @@ -1364,7 +1328,6 @@ function $HttpProvider() {
* Callback registered to $httpBackend():
* - caches the response if desired
* - resolves the raw $http promise
* - calls $apply
*/
function done(status, response, headersString, statusText, xhrStatus) {
if (cache) {
Expand All @@ -1376,16 +1339,7 @@ function $HttpProvider() {
}
}

function resolveHttpPromise() {
resolvePromise(response, status, headersString, statusText, xhrStatus);
}

if (useApplyAsync) {
$rootScope.$applyAsync(resolveHttpPromise);
} else {
resolveHttpPromise();
if (!$rootScope.$$phase) $rootScope.$apply();
}
resolvePromise(response, status, headersString, statusText, xhrStatus);
}


Expand Down
4 changes: 4 additions & 0 deletions src/ngMock/angular-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -1849,10 +1849,14 @@ function createHttpBackendMock($rootScope, $timeout, $delegate, $browser) {
var part = responses.splice(skip, 1);
if (!part.length) throw new Error('No more pending request to flush !');
part[0]();
// Calling $digest again to execute the callbacks of the promises returned from $http
// as these callbacks can add new requests to the queue to flush.
if (digest !== false) $rootScope.$digest();
}
} else {
while (responses.length > skip) {
responses.splice(skip, 1)[0]();
if (digest !== false) $rootScope.$digest();
}
}
$httpBackend.verifyNoOutstandingExpectation(digest);
Expand Down
11 changes: 8 additions & 3 deletions test/ng/directive/ngIncludeSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -362,18 +362,23 @@ describe('ngInclude', function() {

it('should construct SVG template elements with correct namespace', function() {
if (!window.SVGRectElement) return;
module(function($compileProvider) {
module(function($compileProvider, $provide) {
$compileProvider.directive('test', valueFn({
templateNamespace: 'svg',
templateUrl: 'my-rect.html',
replace: true
}));
$provide.decorator('$templateRequest', function($delegate) {
return jasmine.createSpy('$templateRequest').and.callFake($delegate);
});
});
inject(function($compile, $rootScope, $httpBackend) {
inject(function($compile, $rootScope, $httpBackend, $templateRequest) {
$httpBackend.expectGET('my-rect.html').respond('<g ng-include="\'include.svg\'"></g>');
$httpBackend.expectGET('include.svg').respond('<rect></rect><rect></rect>');
element = $compile('<svg><test></test></svg>')($rootScope);
$httpBackend.flush();
expect($templateRequest).toHaveBeenCalledTimes(1);
$httpBackend.flush(2);
expect($templateRequest).toHaveBeenCalledTimes(2);
var child = element.find('rect');
expect(child.length).toBe(2);
// eslint-disable-next-line no-undef
Expand Down
77 changes: 22 additions & 55 deletions test/ng/httpSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,6 @@ describe('$http', function() {
$http = $h;
$rootScope = $rs;
$sce = $sc;
spyOn($rootScope, '$apply').and.callThrough();
}]));

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

describe('callbacks', function() {

it('should $apply after success callback', function() {
$httpBackend.when('GET').respond(200);
$http({method: 'GET', url: '/some'});
$httpBackend.flush();
expect($rootScope.$apply).toHaveBeenCalledOnce();
});


it('should $apply after error callback', function() {
$httpBackend.when('GET').respond(404);
$http({method: 'GET', url: '/some'}).catch(noop);
$httpBackend.flush();
expect($rootScope.$apply).toHaveBeenCalledOnce();
});


it('should $apply even if exception thrown during callback', inject(function($exceptionHandler) {
$httpBackend.when('GET').respond(200);
callback.and.throwError('error in callback');

$http({method: 'GET', url: '/some'}).then(callback);
$httpBackend.flush();
expect($rootScope.$apply).toHaveBeenCalledOnce();

$exceptionHandler.errors = [];
}));


it('should pass the event handlers through to the backend', function() {
it('should pass the event handlers through to the backend', inject(function($browser) {
var progressFn = jasmine.createSpy('progressFn');
var uploadProgressFn = jasmine.createSpy('uploadProgressFn');
$httpBackend.when('GET').respond(200);
Expand All @@ -1117,16 +1088,17 @@ describe('$http', function() {
expect(mockXHR.upload.$$events.progress).toEqual(jasmine.any(Function));

var eventObj = {};
spyOn($rootScope, '$digest');

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

mockXHR.upload.$$events.progress(eventObj);
$browser.defer.flush();
expect(uploadProgressFn).toHaveBeenCalledOnceWith(eventObj);
expect($rootScope.$digest).toHaveBeenCalledTimes(2);
});
}));
});


Expand Down Expand Up @@ -2293,35 +2265,27 @@ describe('$http', function() {
});


describe('$http with $applyAsync', function() {
var $http, $httpBackend, $rootScope, $browser, log;
beforeEach(module(function($httpProvider) {
$httpProvider.useApplyAsync(true);
}, provideLog));

describe('$http interacting with digest cycle', function() {
var $http, $httpBackend, $browser, log;
beforeEach(module(provideLog));

beforeEach(inject(['$http', '$httpBackend', '$rootScope', '$browser', 'log', function(http, backend, scope, browser, logger) {
beforeEach(inject(['$http', '$httpBackend', '$browser', 'log', function(http, backend, browser, logger) {
$http = http;
$httpBackend = backend;
$rootScope = scope;
$browser = browser;
spyOn($rootScope, '$apply').and.callThrough();
spyOn($rootScope, '$applyAsync').and.callThrough();
spyOn($rootScope, '$digest').and.callThrough();
spyOn($browser.defer, 'cancel').and.callThrough();
log = logger;
}]));


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

$httpBackend.flush(null, null, false);
expect($rootScope.$applyAsync).toHaveBeenCalledOnce();
expect(handler).not.toHaveBeenCalled();

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

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

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

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

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