fix(router): handle URL that does not match a route#7379
Closed
petebacondarwin wants to merge 4 commits intoangular:masterfrom
Closed
fix(router): handle URL that does not match a route#7379petebacondarwin wants to merge 4 commits intoangular:masterfrom
petebacondarwin wants to merge 4 commits intoangular:masterfrom
Conversation
petebacondarwin
added
type: bug/fix
comp: router
action: review
|
We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm. |
petebacondarwin
force-pushed
the
fix-router-location-exception
branch
from
2d99f87 to
0ad1326
Compare
|
CLAs look good, thanks! |
petebacondarwin
force-pushed
the
fix-router-location-exception
branch
from
0ad1326 to
d55652b
Compare
petebacondarwin
force-pushed
the
fix-router-location-exception
branch
3 times, most recently
from
8f5a42f to
40fe70f
Compare
btford
added
pr_state: LGTM
action: merge
Contributor
|
LGTM, let's merge this! |
Contributor
|
@petebacondarwin - Can you rebase this? Thanks |
The directiveIntrospector was a bit of a hack to allow the router to read the `$routeConfig` annocation and `$routerCanActivate` hook from directives when they were registered. It turns out that if we put these properties on the component controller's constructor function (i.e. as static class methods) then we can simply use the `$injector` to access it as required. Currently, people put the properties directly on their component definition objects. In Angular 1.5.1, we will copy these properties onto the controller constructor to maintain a simple migration path. But going forward it may be better to encourage people to add the properties directly to the controller constructor.
These tests were registering new components after the application had been bootstrapped, which is not a valid use case for synchronous routes in Angular 1. In particular it was registering the "root" component, which caused the `$rootRouter` to blow up, when it was instantiated, pointing to a root component that did not yet exist.
Until Angular 1.5.1 is released, the `$routeConfig` and `$routerCanActivate` annotations for components must live on the controller constructor. In Angular 1.5.1, it will automatically copy these annotations across from the component definition file.
petebacondarwin
force-pushed
the
fix-router-location-exception
branch
from
40fe70f to
520add7
Compare
Contributor
Author
|
@vikerman Rebased! |
Contributor
|
#7319 is in, can you rebase? |
Contributor
|
merged as 8e3e450 |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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.
Only the last commit is relevant here. Once #7319 lands I'll rebase
Closes #7349
Closes #7203