Repository navigation
Conversation
eab89ba to
df21572
Compare
|
You can preview df21572 at https://pr25740-df21572.ngbuilds.io/. |
|
You can preview 0b85cc5 at https://pr25740-0b85cc5.ngbuilds.io/. |
|
You can preview a15e683 at https://pr25740-a15e683.ngbuilds.io/. |
|
You can preview e5e9939 at https://pr25740-e5e9939.ngbuilds.io/. |
e5e9939 to
99a63a5
Compare
|
You can preview 99a63a5 at https://pr25740-99a63a5.ngbuilds.io/. |
|
You can preview 78885c4 at https://pr25740-78885c4.ngbuilds.io/. |
|
You can preview ce4e81d at https://pr25740-ce4e81d.ngbuilds.io/. |
|
You can preview 8c24876 at https://pr25740-8c24876.ngbuilds.io/. |
8c24876 to
92c7b90
Compare
|
You can preview 92c7b90 at https://pr25740-92c7b90.ngbuilds.io/. |
3eae0c5 to
5417fb8
Compare
|
You can preview 3eae0c5 at https://pr25740-3eae0c5.ngbuilds.io/. |
5417fb8 to
35905e2
Compare
|
You can preview 5417fb8 at https://pr25740-5417fb8.ngbuilds.io/. |
There was a problem hiding this comment.
use mergeMap... reasoning: "flattening" is too general.
There was a problem hiding this comment.
If order matters, this should be a concatMap.
There was a problem hiding this comment.
nit: name this urlAfterRedirects
There was a problem hiding this comment.
extract the "async tap" into it's own operator and reuse... that's the bit that is mergeMap(t => whatever().pipe(map(() => t)).
There was a problem hiding this comment.
Same as above with async tap comment.
There was a problem hiding this comment.
Add some comments through this error handling describing why/what is being done
There was a problem hiding this comment.
could just be:
const activate = (rootContexts: ChildrenOutletContexts, routeReuseStrategy: RouteReuseStrategy,
forwardEvent: (evt: Event) => void) => map(t => {
new ActivateRoutes(
routeReuseStrategy, t.targetRouterState !, t.currentRouterState, forwardEvent)
.activate(rootContexts);
return t;
})Also, maybe make the name more explicit, activate what?
35905e2 to
2bc405a
Compare
|
You can preview 2bc405a at https://pr25740-2bc405a.ngbuilds.io/. |
2bc405a to
9ea5928
Compare
|
You can preview 9ea5928 at https://pr25740-9ea5928.ngbuilds.io/. |
9ea5928 to
b42836c
Compare
|
You can preview b42836c at https://pr25740-b42836c.ngbuilds.io/. |
|
You can preview 698f80e at https://pr25740-698f80e.ngbuilds.io/. |
|
You can preview 5ac8d64 at https://pr25740-5ac8d64.ngbuilds.io/. |
|
You can preview 0ce3cb5 at https://pr25740-0ce3cb5.ngbuilds.io/. |
…cumentation * Pull out `activateRoutes` into new operator * Add `asyncTap` operator * Use `asyncTap` operator for router hooks and remove corresponding abstracted operators * Clean up formatting * Minor performance improvements
0ce3cb5 to
b2e5580
Compare
|
You can preview b2e5580 at https://pr25740-b2e5580.ngbuilds.io/. |
This is a major refactor of how the router previously worked. There are a couple major advantages of this refactor, and future work will be built on top of it. First, we will no longer have multiple navigations running at the same time. Previously, a new navigation wouldn't cause the old navigation to be cancelled and cleaned up. Instead, multiple navigations could be going at once, and we imperatively checked that we were operating on the most current `router.navigationId` as we progressed through the Observable streams. This had some major faults, the biggest of which was async races where an ongoing async action could result in a redirect once the async action completed, but there was no way to guarantee there weren't also other redirects that would be queued up by other async actions. After this refactor, there's a single Observable stream that will get cleaned up each time a new navigation is requested. Additionally, the individual pieces of routing have been pulled out into their own operators. While this was needed in order to create one continuous stream, it also will allow future improvements to the testing APIs as things such as Guards or Resolvers should now be able to be tested in much more isolation. * Add the new `router.transitions` observable of the new `NavigationTransition` type to contain the transition information * Update `router.navigations` to pipe off of `router.transitions` * Re-write navigation Observable flow to a single configured stream * Refactor `switchMap` instead of the previous `mergeMap` to ensure new navigations cause a cancellation and cleanup of already running navigations * Wire in existing error and cancellation logic so cancellation matches previous behavior PR Close #25740
|
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. |
6 participants
This is a major refactor of how the router previously worked. There are a couple major advantages of this refactor, and future work will be built on top of it.
First, we will no longer have multiple navigations running at the same time. Previously, a new navigation wouldn't cause the old navigation to be cancelled and cleaned up. Instead, multiple navigations could be going at once, and we imperatively checked that we were operating on the most current
router.navigationIdas we progressed through the Observable streams. This had some major faults, the biggest of which was async races where an ongoing async action could result in a redirect once the async action completed, but there was no way to guarantee there weren't also other redirects that would be queued up by other async actions. After this refactor, there's a single Observable stream that will get cleaned up each time a new navigation is requested.Additionally, the individual pieces of routing have been pulled out into their own operators. While this was needed in order to create one continuous stream, it also will allow future improvements to the testing APIs as things such as Guards or Resolvers should now be able to be tested in much more isolation.
router.transitionsobservable of the newNavigationTransitiontype to contain the transition informationrouter.navigationsto pipe off ofrouter.transitionsswitchMapinstead of the previousmergeMapto ensure new navigations cause a cancellation and cleanup of already running navigations