Skip to content

Commit e1c1bca

Browse files
fix($route): don't process route change controllers and templates for redirectTo routes
If a route defines a `redirectTo` value, the current route should stop processing the route and instead wait to process the route of the redirect. Currently, what is happening is that Angular detects a redirectTo value and updates $location, but then continues processing the route that was intended to be redirected. Templates are loaded, resolves are processed, ng-view then updates the view with a new template and instantiates the controller all for a route that should have been redirected. A common use case for assigning a function to the redirectTo is to validation authentication, permission check, or some other logic to determine if the route should continue or redirect else where. In the end, this happens, but in between, unexpected results may occur by updating the view and instantiating controllers for logic that wasn't expected to be executed. http://plnkr.co/edit/8QlA0ouuePH3p35Ntmjy?p=preview This commit checks for a url change after a potential redirect, it the url changed, and is not `null` nor `undefined`, exit out and don't process current route change. Closes angular#3332 BREAKING CHANGE The $route service no longer instantiates controllers nor gets templates for routes that have a `redirectTo` that is a defined value and different to the current route url.
1 parent ad59781 commit e1c1bca

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

src/ngRoute/route.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -588,12 +588,19 @@ function $RouteProvider() {
588588
$route.current = nextRoute;
589589
if (nextRoute) {
590590
if (nextRoute.redirectTo) {
591+
var url = $location.url();
592+
var newUrl;
591593
if (angular.isString(nextRoute.redirectTo)) {
592-
$location.path(interpolate(nextRoute.redirectTo, nextRoute.params)).search(nextRoute.params)
594+
$location.path(interpolate(nextRoute.redirectTo, nextRoute.params))
595+
.search(nextRoute.params)
593596
.replace();
597+
newUrl = $location.url();
594598
} else {
595-
$location.url(nextRoute.redirectTo(nextRoute.pathParams, $location.path(), $location.search()))
596-
.replace();
599+
newUrl = nextRoute.redirectTo(nextRoute.pathParams, $location.path(), $location.search());
600+
$location.url(newUrl).replace();
601+
}
602+
if (isDefined(newUrl) && url !== newUrl) {
603+
return; //exit out and don't process current next value, wait for next location change from redirect
597604
}
598605
}
599606
}

test/ngRoute/routeSpec.js

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
describe('$route', function() {
3+
fdescribe('$route', function() {
44
var $httpBackend,
55
element;
66

@@ -1061,6 +1061,55 @@ describe('$route', function() {
10611061
.toEqual(['http://server/#!/bar/id3?extra=eId', true, null]);
10621062
});
10631063
});
1064+
1065+
it('should not instantiate controller or process template for a redirected route', function() {
1066+
var firstController = jasmine.createSpy('first controller spy');
1067+
var firstTemplate = jasmine.createSpy('first template spy').and.returnValue('redirected view');
1068+
var secondController = jasmine.createSpy('second controller spy');
1069+
var secondTemplate = jasmine.createSpy('second template spy').and.returnValue('redirected view');
1070+
module(function($routeProvider) {
1071+
$routeProvider.when('/redirect', {
1072+
template: firstTemplate,
1073+
redirectTo: '/redirected',
1074+
controller: firstController
1075+
});
1076+
$routeProvider.when('/redirected', {
1077+
template: secondTemplate,
1078+
controller: secondController
1079+
});
1080+
});
1081+
inject(function($route, $location, $rootScope, $compile) {
1082+
var element = $compile('<div><ng-view></ng-view></div>')($rootScope);
1083+
$location.path('/redirect');
1084+
$rootScope.$digest();
1085+
expect(firstController).not.toHaveBeenCalled();
1086+
expect(firstTemplate).not.toHaveBeenCalled();
1087+
expect(secondController).toHaveBeenCalled();
1088+
expect(secondTemplate).toHaveBeenCalled();
1089+
dealoc(element);
1090+
});
1091+
});
1092+
1093+
it('should not redirect transition if `redirectTo` returns `undefined`', function() {
1094+
var controller = jasmine.createSpy('first controller spy');
1095+
var templateFn = jasmine.createSpy('first template spy').and.returnValue('redirected view');
1096+
module(function($routeProvider) {
1097+
$routeProvider.when('/redirect/to/undefined', {
1098+
template: templateFn,
1099+
redirectTo: function() {},
1100+
controller: controller
1101+
});
1102+
});
1103+
inject(function($route, $location, $rootScope, $compile) {
1104+
var element = $compile('<div><ng-view></ng-view></div>')($rootScope);
1105+
$location.path('/redirect/to/undefined');
1106+
$rootScope.$digest();
1107+
expect(controller).toHaveBeenCalled();
1108+
expect(templateFn).toHaveBeenCalled();
1109+
expect($location.path()).toEqual('/redirect/to/undefined');
1110+
dealoc(element);
1111+
});
1112+
});
10641113
});
10651114

10661115

0 commit comments

Comments
 (0)