Commit Graph

117 Commits

Author SHA1 Message Date
Alex Rickabaugh 0de5d79bf6 Revert "fix(router): adjust UrlTree redirect to replace URL if in eager update (#31168)" (#31344)
This reverts commit 15e397816f.

Reason: it broke an internal g3 app.

PR Close #31344
2019-06-28 11:40:27 -07:00
Jason Aden 15e397816f fix(router): adjust UrlTree redirect to replace URL if in eager update (#31168)
Without this change when using UrlTree redirects in `urlUpdateStrategy="eager"`, the URL would get updated to the target location, then redirected. This resulted in having an additional entry in the `history` and thus the `back` button would be broken (going back would land on the URL causing a new redirect).

Additionally, there was a bug where the redirect, even without `urlUpdateStrategy="eager"`, could create a history with too many entries. This was due to kicking off a new navigation within the navigation cancelling logic. With this PR the new navigation is pushed to the next tick with a `setTimeout`, allowing the page being redirected from to be cancelled before starting a new navigation.

Related to #27148

PR Close #31168
2019-06-27 15:54:20 -07:00
Jason Aden 0fd9d086e4 fix(router): ensure navigations start with the current URL value incase redirect is skipped (#30344)
In some cases where multiple navigations happen to the same URL, the router will not process a given URL. In those cases, we fall into logic that resets state for the next navigation. One piece of this resetting is to set the `browserUrlTree` to the most recent `urlAfterRedirects`i.

However, there was bug in this logic because in some cases the `urlAfterRedirects` is a stale value. This happens any time a URL won't be processed, and the previous URL will also not be processed. This creates unpredictable behavior, not the least of which ends up being a broken `back` button.

This PR kicks off new navigations with the current value the router assumes is in the browser. All the logic around how to handle future navigations is based on this value compared to the current transition, so it's important to kick off all new navigations with the current value so in the edge case described above we don't end up with an old value being set into `browserUrlTree`.

Fixes #30340
Related to #30160

PR Close #30344
2019-05-15 10:14:47 -07:00
Jason Aden 8ced321bb6 Revert "fix(router): fix a problem with router not responding to back button (#30160)" (#30320)
This reverts commit 3327bd8eab.

PR Close #30320
2019-05-07 17:22:24 -07:00
Jason Aden b40f6f3eae fix(router): ensure `history.state` is set in `eager` update mode (#30154)
Without this change, `history.state` isn't being set when updating the browser URL in `eager` update mode.

PR Close #30154
2019-05-06 16:09:58 -07:00
Jason Aden 3327bd8eab fix(router): fix a problem with router not responding to back button (#30160)
There was a problem with a combination of the `eager` URL update, browser `back` button, and hybrid applications. Details provided in internal ticket http://b/123667227.

This fix handles the problem by setting `router.browserUrlTree` when all conditions have failed, meaning the browser doesn't do anything with the navigation other than update internal data structures. Without this change, the problem was an old value was stored in `router.broserUrlTree` causing some new navigations to be compared to an old value and breaking future navigations.

PR Close #30160
2019-05-06 16:06:58 -07:00
Jason Aden f9497bf28b fix(router): adjust setting navigationTransition when a new navigation cancels an existing one (#29636)
Prior to this change, if a navigation was ongoing and a new one came in, the router could get into a state where `router.currentNavigation` was `null` even though a navigation was executing. This change moves where we set the `currentNavigation` value so it's inside a `switchMap`. This solves the problem because the `finally` on the `switchMap` had been setting `currentNavigation` to `null` but the new `currentNavigation` value would have already been set. Essentially this was a timing problem and is resolved with this change.

Fixes #29389 #29590

PR Close #29636
2019-04-01 12:11:55 -07:00
Pawel Kozlowski fcd1f61476 refactor(router): change RouterLinkActive impl to account for upcoming ivy breaking change (#28560)
PR Close #28560
2019-02-08 16:42:45 -08:00
stoneLeaf 50732e1564 fix(router): redirect to root url when returned as UrlTree from guard (#28271)
When a UrlTree of root url was returned by a guard as a redirection, the
navigation was not processed. The issue came from the error handler which
incorrectly marked the router as already navigated.

Fixes #27845

PR Close #28271
2019-02-06 21:23:32 -08:00
kevinphelps ed0cf7e2cb fix(router): set href when routerLink is used on an 'area' element (#28441)
closes #28401

PR Close #28441
2019-02-05 23:28:40 -05:00
Jason Aden 50df897fdc fix(router): `skipLocationChange` with named outlets (#28300)
With #27680, a bug was fixed where multiple redirects using `eager` URL update could cause navigation to fail. However, that fix introduced a problem where with `skipLocationChange` enabled, the URL tree rendered was not properly stored for reference. This specifically caused an issue with named router outlets and subsequent navigations not being recognized.

This PR stores the correct `UrlTree` for reference with later navigations. It fixes the regression introdued with #27680.

Fixes #28200

PR Close #28300
2019-01-22 15:15:02 -08:00
Jason Aden 33e49c2894 Revert "revert: fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523)" (#28300)
This reverts commit eea2b0f288.

PR Close #28300
2019-01-22 15:15:02 -08:00
Kara Erickson 6fff74e576 test(ivy): re-enable passing tests and misc cleanup (#28093)
PR Close #28093
2019-01-14 10:03:57 -08:00
Alex Rickabaugh 51e716b6f2 feat(ivy): implement a Compiler for use in TestBedRender3 (#28033)
Previously when testing code injected the Compiler, it received the
top-level Compiler implementation defined in linker/compiler.ts
(and governed by the __PRE_R3__ switch). Code running under the
TestBed, however, should always use a TestBed-aware Compiler
implementation.

This commit adds such an implementation to the TestBedRender3,
which passes compiled modules through the _compileNgModule()
function.

With this change, 3 formerly disabled router integration tests
now pass.

FW-855 #resolve

PR Close #28033
2019-01-11 11:18:10 -08:00
Kara Erickson ad6569c744 fix(ivy): markForCheck() should not schedule change detection (#28048)
Previously, we had the logic to schedule a change detection tick
inside markViewDirty(). This is fine when used in markDirty(),
the user-facing API, because it should always schedule change
detection. However, this doesn't work when used in markForCheck()
because historically markForCheck() does not trigger change
detection.

To be backwards compatible, this commit moves the scheduling
logic out of markViewDirty() and into markDirty(), so
markForCheck no longer triggers a tick.

PR Close #28048
2019-01-10 13:37:40 -08:00
Kara Erickson e0e92cfef6 test(ivy): diagnose root causes of remaining failing router tests (#28035)
PR Close #28035
2019-01-10 10:54:37 -08:00
Marc Laval 5609764886 test(ivy): add root causes for router TestBed failures (#28014)
PR Close #28014
2019-01-09 11:49:45 -08:00
Miško Hevery b9c6df6da7 fix(ivy): use NgZone.onStable when bootstraped using PlatformRef (#27898)
PR Close #27898
2019-01-08 14:10:53 -08:00
Matias Niemelä eea2b0f288 revert: fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523) 2018-12-25 22:14:07 -08:00
Marc Laval 5da55d6246 test(ivy): add root cause analysis for failing router tests (#27792)
PR Close #27792
2018-12-21 15:06:56 -05:00
Jason Aden 07ada7f3d9 fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523)
Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes #27116

PR Close #27523
2018-12-20 16:18:52 -05:00
Jason Aden 12c317603a feat(router): add predicate function mode for runGuardsAndResolvers (#27682)
This option means guards and resolvers will ignore changes when a provided predicate function returns `false`. This supports use cases where an application needs to ignore some param updates but not others. For example, changing a sort param in the URL might need to be ignored, whereas changing the a `project` param might require re-run of guards and resolvers.

Related to #26861 #18253 #27464

PR Close #27682
2018-12-18 13:55:06 -08:00
Andrew Kushnir 4bf8d64c56 fix(ivy): restore `fixmeIvy` imports for Router-related tests (#27720)
It looks like `fixmeIvy` imports were accidentally removed from Router integration tests, thus causing build errors. The necessary imports are now restored and the project should build normally.

PR Close #27720
2018-12-17 20:12:15 -08:00
Kara Erickson 062c7af4f3 test(ivy): replace resolved issue in compiler-cli test with new root cause (#27709)
PR Close #27709
2018-12-17 16:46:42 -08:00
Miško Hevery e94975d109 fix(ivy): check semantics of NgModule for consistency (#27604)
`NgModule` requires that `Component`s/`Directive`s/`Pipe`s are listed in
declarations, and that each `Component`s/`Directive`s/`Pipe` is declared
in exactly one `NgModule`. This change adds runtime checks to ensure
that these sementics are true at runtime.

There will need to be seperate set of checks for the AoT path of the
codebase to verify that same set of semantics hold. Due to current
design there does not seem to be an easy way to share the two checks
because JIT deal with references where as AoT deals with AST nodes.

PR Close #27604
2018-12-17 14:49:51 -08:00
Miško Hevery 650c5a02ac Revert "fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523)"
This reverts commit ad26cd6d0c.
2018-12-14 13:18:16 -08:00
Jason Aden ad26cd6d0c fix(router): ensure URL is updated after second redirect with UrlUpdateStrategy="eager" (#27523)
Navigating to a route such as `/users`, you may get redirected to `/login`. Previously, if you go then route to `/users` again the URL will end up showing `/users` after the second redirect. This only happened in `UrlUpdateStrategy="eager"`. This is now fixed so after the second redirect, the URL shows the correct page.

Fixes #27116

PR Close #27523
2018-12-13 14:59:41 -08:00
Marc Laval f013c57186 test(router): correct a wrong test (FW-766) (#27511)
PR Close #27511
2018-12-06 13:41:20 -08:00
Marc Laval 8fa7e93c30 test(ivy): enable router tests with pending timers (FW-768) (#27509)
PR Close #27509
2018-12-06 13:40:42 -08:00
Marc Laval 8e9858fadb fix(ivy): align NgModuleRef implementation between Ivy and ViewEngine (#27482)
Solves FW-765 and FW-767

PR Close #27482
2018-12-06 13:35:08 -08:00
Jason Aden d70a7f3ac9 feat(router): add pathParamsOrQueryParamsChange mode for runGuardsAndResolvers (#27464)
This option means guards and resolvers will ignore changes to matrix parameters. Guards and resolvers will be rerun when the path changes, when path parameters change, or when query parameters change.

The primary use case for such a mode is when updating the UI and getting the URL to be in sync with local changes. For example, if displaying a sortable table, changing the sort direction is often handled by the table itself. But you would want to update the URL to be in sync with what's being displayed to the user. As long as the table sort direction is stored as a matrix parameter, you can use this option to update the URL without causing the overhead of re-running guards and resolvers.

Related to #26861 #18253

PR Close #27464
2018-12-05 10:43:29 -08:00
Misko Hevery f52600e261 test(ivy): add ability to find already passing tests (#27449)
PR Close #27449
2018-12-05 09:34:52 -08:00
Jason Aden 0d4149c736 test(router): add test verifying guards re-run when parent path segments change (#27440)
Related to #26861

PR Close #27440
2018-12-04 19:59:53 -08:00
Marc Laval bd864fb274 feat(ivy): support WrappedValue in pipes (FW-726) (#27409)
PR Close #27409
2018-12-04 19:59:01 -08:00
Marc Laval 35e02ad5e0 test(ivy): add root causes for router TestBed failures (#27325)
PR Close #27325
2018-11-30 14:58:22 -08:00
Marc Laval a566e52ca4 test(ivy): run more router tests in CI (#27325)
PR Close #27325
2018-11-30 14:58:22 -08:00
Jason Aden d40af0c137 feat(router): add a Navigation type available during navigation (#27198)
Provides target URLs, Navigation, and `NavigationExtras` data.

FW-613

PR Close #27198
2018-11-30 13:34:55 -08:00
Jason Aden 73f6ed9be1 feat(router): allow passing `state` to routerLink directives (#27198)
This value will get written to the `history.state` entry.

FW-613 (related)
Related to #24617

PR Close #27198
2018-11-30 13:34:55 -08:00
Jason Aden 67f4a5d4bd feat(router): allow passing state to `NavigationExtras` (#27198)
This value will get written to the `history.state` entry.

FW-613 (related)

PR Close #27198
2018-11-30 13:34:55 -08:00
Jason Aden 11a8bd8aca fix(router): update URL after redirects when urlHandlingStrategy='eager' (#27356)
fixes #27076

PR Close #27356
2018-11-29 14:46:32 -08:00
Misko Hevery 39e426cde3 fix(ivy): TestBed.get(Compiler) throws "Error: Runtime compiler is not loaded" (#27223)
BREAKING CHANGE:

The public API for `DebugNode` was accidentally too broad. This change removes
1. Public constructor. Since `DebugNode` is a way for Angular to communicate information
   on to the developer there is no reason why the developer should ever need to
   Instantiate the `DebugNode`
2. We are also removing `removeChild`, `addChild`, `insertBefore`, and `insertChildAfter`.
   All of these methods are used by Angular to constructor the correct `DebugNode` tree.
   There is no reason why the developer should ever be constructing a `DebugNode` tree
   And these methods should have never been made public.
3. All properties have been change to `readonly` since `DebugNode` is used by Angular
   to communicate to developer and there is no reason why these APIs should be writable.

While technically breaking change we don’t expect anyone to be effected by this change.

PR Close #27223
2018-11-27 13:42:23 -08:00
Marc Laval c2f30542e7 fix(ivy): should support components without selector (#27169)
PR Close #27169
2018-11-27 10:17:35 -08:00
Marc Laval 8ce59a583b test(ivy): run router tests with ivy on CI (#27195)
PR Close #27195
2018-11-21 09:19:40 -08:00
Jason Aden bf6ac6cef8 feat(router): add pathParamsChange mode for runGuardsAndResolvers (#26861)
This option means guards and resolvers will ignore changes to optional
parameters such as query and matrix params. When the path or any path
params change, guards and resolvers will be run

Related to discussion in #18253
FW-560 #resolve

PR Close #26861
2018-11-14 12:24:43 -08:00
Sarun Intaralawan a752971bca fix(router): add `relativeLinkResolution` to `recognize` operator (#26990)
Close #26983

PR Close #26990
2018-11-13 16:18:08 -08:00
Jason Aden 4e9f2e5895 feat(router): guard returning UrlTree cancels current navigation and redirects (#26521)
Fixes #24618
FW-153 #resolve

PR Close #26521
2018-10-31 19:51:50 -04:00
Jason Aden 1c9b06504b fix(router): fix regression where navigateByUrl promise didn't resolve on CanLoad failure (#26455)
Fixes #26284

PR Close #26455
2018-10-15 16:53:25 -07:00
Jason Aden 9acd04c192 refactor(router): update test based on router quick-cancelling ongoing navigations (#25740)
PR Close #25740
2018-09-27 14:02:58 -07:00
Jason Aden c091d40fb0 refactor(router): make sure redirect within NavigationStart event works (#25740)
PR Close #25740
2018-09-27 14:02:58 -07:00
Jason Aden b7baf632c0 refactor(router): move routing into a single Observable stream (#25740)
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
2018-09-27 14:02:57 -07:00