Permalink
Comparing changes
Open a pull request
- 4 commits
- 2 files changed
- 0 commit comments
- 2 contributors
Commits on May 24, 2016
…andler
… `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 #3332 Closes #14658 BREAKING CHANGE The $route service no longer instantiates controllers nor calls resolves or template functions for routes that have a `redirectTo` unless the `redirectTo` is a function that returns `undefined`.
Unified
Split
Showing
with
110 additions
and 32 deletions.
- +52 −32 src/ngRoute/route.js
- +58 −0 test/ngRoute/routeSpec.js
| @@ -136,6 +136,12 @@ function $RouteProvider() { | ||
| * The custom `redirectTo` function is expected to return a string which will be used | ||
| * to update `$location.path()` and `$location.search()`. | ||
| * | ||
| * Routes that specify `redirectTo` will not have their controllers, template functions | ||
| * or resolves called, the `$location` will be changed to the redirect url and route | ||
| * processing will stop. The exception to this is if the `redirectTo` is a function that | ||
| * returns `undefined`. In this case the route transition occurs as though there was no | ||
| * redirection. | ||
| * | ||
| * - `[reloadOnSearch=true]` - `{boolean=}` - reload route when only `$location.search()` | ||
| * or `$location.hash()` changes. | ||
| * | ||
| @@ -588,46 +594,25 @@ function $RouteProvider() { | ||
| $route.current = nextRoute; | ||
| if (nextRoute) { | ||
| if (nextRoute.redirectTo) { | ||
| var url = $location.url(); | ||
| var newUrl; | ||
| if (angular.isString(nextRoute.redirectTo)) { | ||
| $location.path(interpolate(nextRoute.redirectTo, nextRoute.params)).search(nextRoute.params) | ||
| $location.path(interpolate(nextRoute.redirectTo, nextRoute.params)) | ||
| .search(nextRoute.params) | ||
| .replace(); | ||
| newUrl = $location.url(); | ||
| } else { | ||
| $location.url(nextRoute.redirectTo(nextRoute.pathParams, $location.path(), $location.search())) | ||
| .replace(); | ||
| newUrl = nextRoute.redirectTo(nextRoute.pathParams, $location.path(), $location.search()); | ||
| $location.url(newUrl).replace(); | ||
| } | ||
| if (angular.isDefined(newUrl) && url !== newUrl) { | ||
| return; //exit out and don't process current next value, wait for next location change from redirect | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $q.when(nextRoute). | ||
| then(function() { | ||
| if (nextRoute) { | ||
| var locals = angular.extend({}, nextRoute.resolve), | ||
| template, templateUrl; | ||
|
|
||
| angular.forEach(locals, function(value, key) { | ||
| locals[key] = angular.isString(value) ? | ||
| $injector.get(value) : $injector.invoke(value, null, null, key); | ||
| }); | ||
|
|
||
| if (angular.isDefined(template = nextRoute.template)) { | ||
| if (angular.isFunction(template)) { | ||
| template = template(nextRoute.params); | ||
| } | ||
| } else if (angular.isDefined(templateUrl = nextRoute.templateUrl)) { | ||
| if (angular.isFunction(templateUrl)) { | ||
| templateUrl = templateUrl(nextRoute.params); | ||
| } | ||
| if (angular.isDefined(templateUrl)) { | ||
| nextRoute.loadedTemplateUrl = $sce.valueOf(templateUrl); | ||
| template = $templateRequest(templateUrl); | ||
| } | ||
| } | ||
| if (angular.isDefined(template)) { | ||
| locals['$template'] = template; | ||
| } | ||
| return $q.all(locals); | ||
| } | ||
| }). | ||
| then(resolveLocals). | ||
| then(function(locals) { | ||
| // after route change | ||
| if (nextRoute === $route.current) { | ||
| @@ -645,6 +630,41 @@ function $RouteProvider() { | ||
| } | ||
| } | ||
|
|
||
| function resolveLocals(route) { | ||
| if (route) { | ||
| var locals = angular.extend({}, route.resolve); | ||
| angular.forEach(locals, function(value, key) { | ||
| locals[key] = angular.isString(value) ? | ||
| $injector.get(value) : | ||
| $injector.invoke(value, null, null, key); | ||
| }); | ||
| var template = getTemplateFor(route); | ||
| if (angular.isDefined(template)) { | ||
| locals['$template'] = template; | ||
| } | ||
| return $q.all(locals); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| function getTemplateFor(route) { | ||
| var template, templateUrl; | ||
| if (angular.isDefined(template = route.template)) { | ||
| if (angular.isFunction(template)) { | ||
| template = template(route.params); | ||
| } | ||
| } else if (angular.isDefined(templateUrl = route.templateUrl)) { | ||
| if (angular.isFunction(templateUrl)) { | ||
| templateUrl = templateUrl(route.params); | ||
| } | ||
| if (angular.isDefined(templateUrl)) { | ||
| route.loadedTemplateUrl = $sce.valueOf(templateUrl); | ||
| template = $templateRequest(templateUrl); | ||
| } | ||
| } | ||
| return template; | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * @returns {Object} the current active route, by matching it against the URL | ||
| @@ -1061,6 +1061,64 @@ describe('$route', function() { | ||
| .toEqual(['http://server/#!/bar/id3?extra=eId', true, null]); | ||
| }); | ||
| }); | ||
|
|
||
| it('should not process route bits', function() { | ||
| var firstController = jasmine.createSpy('first controller spy'); | ||
| var firstTemplate = jasmine.createSpy('first template spy').and.returnValue('redirected view'); | ||
| var firstResolve = jasmine.createSpy('first resolve spy'); | ||
| var secondController = jasmine.createSpy('second controller spy'); | ||
| var secondTemplate = jasmine.createSpy('second template spy').and.returnValue('redirected view'); | ||
| var secondResolve = jasmine.createSpy('second resolve spy'); | ||
| module(function($routeProvider) { | ||
| $routeProvider.when('/redirect', { | ||
| template: firstTemplate, | ||
| redirectTo: '/redirected', | ||
| resolve: { value: firstResolve }, | ||
| controller: firstController | ||
| }); | ||
| $routeProvider.when('/redirected', { | ||
| template: secondTemplate, | ||
| resolve: { value: secondResolve }, | ||
| controller: secondController | ||
| }); | ||
| }); | ||
| inject(function($route, $location, $rootScope, $compile) { | ||
| var element = $compile('<div><ng-view></ng-view></div>')($rootScope); | ||
| $location.path('/redirect'); | ||
| $rootScope.$digest(); | ||
|
|
||
| expect(firstController).not.toHaveBeenCalled(); | ||
| expect(firstTemplate).not.toHaveBeenCalled(); | ||
| expect(firstResolve).not.toHaveBeenCalled(); | ||
|
|
||
| expect(secondController).toHaveBeenCalled(); | ||
| expect(secondTemplate).toHaveBeenCalled(); | ||
| expect(secondResolve).toHaveBeenCalled(); | ||
|
|
||
| dealoc(element); | ||
| }); | ||
| }); | ||
|
|
||
| it('should not redirect transition if `redirectTo` returns `undefined`', function() { | ||
| var controller = jasmine.createSpy('first controller spy'); | ||
| var templateFn = jasmine.createSpy('first template spy').and.returnValue('redirected view'); | ||
| module(function($routeProvider) { | ||
| $routeProvider.when('/redirect/to/undefined', { | ||
| template: templateFn, | ||
| redirectTo: function() {}, | ||
| controller: controller | ||
| }); | ||
| }); | ||
| inject(function($route, $location, $rootScope, $compile) { | ||
| var element = $compile('<div><ng-view></ng-view></div>')($rootScope); | ||
| $location.path('/redirect/to/undefined'); | ||
| $rootScope.$digest(); | ||
| expect(controller).toHaveBeenCalled(); | ||
| expect(templateFn).toHaveBeenCalled(); | ||
| expect($location.path()).toEqual('/redirect/to/undefined'); | ||
| dealoc(element); | ||
| }); | ||
| }); | ||
| }); | ||
|
|
||
|
|
||