This repository was archived by the owner on Apr 12, 2024. It is now read-only.
Jardilio route redirect fix #14658
Closed
Jardilio route redirect fix #14658
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… `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.
|
I accidentally changed the authorship of the first commit. I will change back before merging... |
| @@ -1,6 +1,6 @@ | |||
| 'use strict'; | |||
|
|
|||
| describe('$route', function() { | |||
| fdescribe('$route', function() { | |||
|
Where are the docs ? 😛 |
|
I would also mention |
| @@ -1061,6 +1061,55 @@ describe('$route', function() { | |||
| .toEqual(['http://server/#!/bar/id3?extra=eId', true, null]); | |||
| }); | |||
| }); | |||
|
|
|||
| it('should not instantiate controller or process template for a redirected route', function() { | |||
|
So at the moment null works differently so this pr does not change that. We should consider making null work like undefined in a follow up pr |
|
I'll add checks commit message and docs for resolve |
… `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 calls resolves or template functions for routes that have a `redirectTo` unless the `redirectTo` is a function that returns `undefined`.
|
LGTM |
|
Don't forget to change back the authorship though! |
| * 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 their was no |
|
The commit message mentions that LGTM EDIT: Too slow 😞 |
|
Oops. Well I guess I have to fix the |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See #3332