Commit Graph

710 Commits

Author SHA1 Message Date
unknown f44cb57c12 fix(router): prevent componentless routes from being detached (#44240)
Currently, when deactivating a route, the detach method of the RouteReuseStrategy is always called
even when we are dealing with a componentless route. If when using a custom strategy we attempt to
detach a componentless route, an exception is thrown causing the navigation to break.
This change prevents this from happening, by not triggering the detach of the route reuse strategy.

Fixes #44239

PR Close #44240
2021-11-23 17:12:27 +00:00
Martin von Gagern aac579d976 refactor(router): add type annotation for UrlSegment.parameterMap (#44175)
Having the type specified explicitly makes the API reference more readable.
PR Close #44175
2021-11-16 09:36:17 -08:00
Andrew Scott d015ac214f Revert "refactor(router): restore history in a consistent way on canceled navigations (#43651)" (#44006)
This reverts commit a268c4471f.

Causes failures in internal team's code. The failures likely relate to
attempting to manually update private Router state tracking properties,
which this change would have affected.

PR Close #44006
2021-11-01 09:25:36 -07:00
krzysztof-grzybek 45e4e60fd6 fix(router): reuse route strategy fix (#43791)
Currently, it's impossible to cache (detach/attach) parent route without caching child routes.
This produces a bug, when navigating from a/b to c, then to a/d, where a route is cached.
On the last navigation, we incorrectly restore a/b route instead of a/d.
This change introduces new behavior: if the route should be detached/attached,
we do so, but we check also child routes recursively.

Fixes #17333

PR Close #43791
2021-10-21 16:28:03 +00:00
Andrew Scott 32f368af2d fix(router): Do not clear currentNavigation if already set to next one (#43852)
Experimentation with the Router URL management exposed a situation where
the `currentNavigation` was being cleared in the `finalize` after the
`currentNavigation` was already set to the next one.

This change ensures that the `currentNavigation` is only cleared if the
id of the finalized transition matches the one on the
`currentNavigation` object.

PR Close #43852
2021-10-18 10:30:50 -07:00
Andrew Scott a3ee474897 refactor(router): Remove use of browserUrlTree in scheduleNavigation (#43632)
There are no added tests here because the change is generally equivalent
to what was there before. The goal of that piece of code is to prevent
duplicate navigations due to the location synchronization code between
AngularJS and Angular. That is, the Angular router listens to the
`popstate` event and triggers navigations, but so does the AngularJS
router and triggers navigations through `router.navigateByUrl`. The
`setTimeout` in the Angular Router's `setUpLocationChangeListener` is
there to make this bit of code work: the 'popstate'/'hashchange'
navigation will necessarily come after the imperative nav triggered by
AngularJS's location sync.

Anyhow, in the long run, I would like to get rid of this bit of code
altogether. We should not have special handling for these cases. The
AngularJS/Angular location sync should either:

1. Not trigger a duplicate navigation in the first place
or
2. Be tolerant to processing the duplicate navigation.

In the short term, this change benefits the router while we investigate
removing this logic completely because now browserUrlTree has a single
responsiblity: to support `onSameUrlNavigation==='reload'`. This means
that we can work towards removing the `browserUrlTree` tracking from the
Router code at some point as well and have the `onSameUrlNavigation`
logic be part of a pre-navigation hook that can be handled outside the
`Router`. This may or may not be possible, but the change here opens up
that change for investigation.

PR Close #43632
2021-10-18 10:29:22 -07:00
Andrew Scott e79e3cafec fix(router): fix Router's public API for canceledNavigationResolution (#43842)
The commit which made the `canceledNavigationResolution` property on the `Router`
public did not add the corresponding configuration in the `ExtraOptions`.
https://github.com/angular/angular/commit/3c6b653089837459809a370ebcaf8911c3bab9ed
This was a mistake and is being corrected in this commit. We should not
encourage changing the properties post-setup (i.e.
`inject(Router).canceledNavigationResolution = 'computed'`). This manner
of configuration makes the options non-tree shakeable because we have to
keep both implementations in case the value changes at runtime.

PR Close #43842
2021-10-15 11:35:01 -07:00
Andrew Scott 892158f769 Revert "refactor(router): clean up internal hooks (#43804)" (#43845)
This reverts commit 5cc51880db.

PR Close #43845
2021-10-14 14:43:41 -07:00
Andrew Scott 5cc51880db refactor(router): clean up internal hooks (#43804)
* beforePreactivation hook is unused
* The only place that uses afterPreactivation does not use the arguments

Not to say we won't want to provide hooks similar to this in the future,
but the current state is over-engineered for what it's being used for.

PR Close #43804
2021-10-14 10:04:01 -07:00
Wey-Han Liaw 2ab2a080b6 fix(router): unset attachRef when router-outlet is destroyed to avoid mounting a destroyed component (#43697)
Previously, when a router-outlet is conditionally shown with an ngIf, and a sub-route was re-attached
via a custom RouteReuseStrategy, router-outlet would try to mount a destroyed component into the view
if the router-outlet is destroyed and re-initialized.

This commit fixes it by unsetting context.attachRef when router-outlet is destroyed, so when the
router-outlet is being initialized again, it no longer sees an attachRef that it needs to mount to the
view.

Fixes #43696

PR Close #43697
2021-10-05 09:48:39 -07:00
Andrew Scott a268c4471f refactor(router): restore history in a consistent way on canceled navigations (#43651)
The Router code currently has special-case handling around when and how
the internal state is reset. Specifically, it only resets the internal
tracking of the state when an error is thrown, which does not happen
when guards reject or resolvers return `EMPTY`. Other than the
navigation URL not matching a config, guards rejecting would be the main
cause of a navigation being turned down.

This change updates the router code to always reset the internal state
in the same way, regardless of the reason for navigation cancellation.

In the end, this will only affect _very_ specific use-cases with
`UrlHandlingStrategy`. Because the internal state is not updated until
the end of the transition pipe, the state reset generally doesn't do
anything at all. However, because the `rawUrlTree` is reset by calling
`urlHandlingStrategy.merge` with the _attempted_ `rawUrl` that failed,
the resulting browser URL reset could be different than before (but will
now be consistent with how the URL is reset in other scenarios, like a
URL not matching a `Route` config).

PR Close #43651
2021-10-04 10:55:49 -07:00
JoostK 361273fad5 refactor(router): remove support for loadChildren string syntax (#43591)
This commit removes the ability to configure lazy routes using a string
for `loadChildren`, together with the supporting classes to load an
`NgModuleFactory` at runtime.

BREAKING CHANGE:

It is no longer possible to use `Route.loadChildren` using a string
value. The following supporting classes were removed from
`@angular/core`:

- `NgModuleFactoryLoader`
- `SystemJsNgModuleFactoryLoader`

The `@angular/router` package no longer exports these symbols:

- `SpyNgModuleFactoryLoader`
- `DeprecatedLoadChildren`

The signature of the `setupTestingRouter` function from
`@angular/core/testing` has been changed to drop its `NgModuleFactoryLoader`
parameter, as an argument for that parameter can no longer be created.

PR Close #43591
2021-09-29 14:45:18 -07:00
JoostK 05c08cac9a docs: refactor example apps to use dynamic imports for loadChildren (#43591)
The string syntax for `loadChildren` has been deprecated and is being
removed, so this commit refactors the examples to the new syntax.

PR Close #43591
2021-09-29 14:45:17 -07:00
Dmitrij Kuba 4f3beffdbf feat(router): emit activate/deactivate events when an outlet gets attached/detached (#43333)
Previously the events of `RouterOutlet` (activate/deactivate) were not fired
when an outlet got attached/detached with `RouteReuseStrategy`. The changes configure
`RouterOutlet` to emit events when an outlet gets attached/detached.

Fixes #25521, #20501

PR Close #43333
2021-09-28 14:16:39 -07:00
Daniel Díaz cc427d18c4 docs(router): fix typo in router event API documentation (#43555)
PR Close #43555
2021-09-23 14:45:27 -07:00
Andrew Scott c5d0bd4966 fix(router): Prevent URL flicker when new navigations cancel ongoing ones (#43496)
This bit of code is problematic for several reasons:
1. AngularJS-specific handling should not exist in core router code if it can be avoided.
It's not something that should complicated the code long-term
2. It causes URL flickering (the `replace: /` really does happen and can be observed)
3. The original intent was likely for the URL to not actually change;
since the Router only initially supported 'deferred' navigations, this would _usually_ be the case.
4. It actually causes observers of URL changes more problems in handling changes
because the router is already about to process another navigation request.
Notice that when using `'computed'` restoration logic, we do not reset the URL
because it would cause another location change event and conflict with the new navigation.
5. It only resets the browser URL but does nothing to sync the rest of the internal state of the router.
6. It makes guards which call `router.navigate()` behave _slightly_ differently
than guard which return `UrlTree`. Guards which return `UrlTree` do not reset the
URL, but instead just trigger a new navigation.
7. It resets the entire URL rather than just the portion that the
   `UrlHandlingStrategy` is configured to handle. Theoretically, the
   Router could be configured to only handle a part of the URL so failed
   navigations should not reset parts it is not configured to touch.
   Note that this is actually a problem in other places in the router as
   well where `resetState` is not called before
   `resetUrlToCurrentUrlTree`.

As a nice benefit, when `urlUpdateStrategy` is set to `'eager'`, this
makes #17004 possible.

BREAKING CHANGE:
The router will no longer replace the browser URL when a new navigation
cancels an ongoing navigation. This often causes URL flicker and was
only in place to support some AngularJS hybrid applications. Hybrid
applications which rely on the `navigationId` being present on initial
navigations that were handled by the Angular router should instead
subscribe to `NavigationCancel` events and perform the
`location.replaceState` themselves to add `navigationId` to the Router
state.
In addition, tests which assert `urlChanges` on the `SpyLocation` may
need to be adjusted to account for the `replaceState` which is no longer
triggered.

PR Close #43496
2021-09-20 20:57:57 +00:00
Andrew Scott 79700822fd refactor(router): remove unnecessary getTransition function (#43436)
The getTransition helper function ensures that urlAfterRedirects is assigned for when
the router does not process the previous or current URL. In this case, we would set
the browserUrlTree to be the urlAfterRedirects, which was initialized to be the browserUrlTree.
This is a no-op, so there's no need for the getTransition function at all.

PR Close #43436
2021-09-16 19:20:57 +00:00
Andrew Scott 3ec9ee65b9 refactor(router): Remove TODO and slightly adjust eager browserUrlTree update (#43426)
No test was added for this case because I can't think of a test case to
write that would work.

This change updates the code for a consistent mental model of setting
the `browserUrlTree`. It's only meant to track the `UrlTree` that the
`UrlHandlingStrategy` is set to extract, not the full `rawUrl`. Notice
that everywhere else, the `browserUrlTree` is set to
`urlAfterRedirects`, which is computed based on the extracted URL, _not_
the `rawUrl`.

PR Close #43426
2021-09-14 11:11:43 -07:00
Andrew Scott 796da641f0 fix(router): Do not modify parts of URL excluded from with 'eager' updates (#43421)
The previous code would set the browser URL to be only the part that's
extracted by the `UrlHandlingStrategy`. However, there may be parts of
the URL which are _should not_ be handled by the Angular Router. This
change updates the code to set the browser URL in the same way that's
done with `'deferred'`: Merging the extracted URL after redirects with
the whole raw URL of the navigation, which includes parts not handled by
the `UrlHandlingStrategy`.

PR Close #43421
2021-09-14 08:35:15 -07:00
Andrew Scott 0e8548f667 fix(router): Allow renavigating to failed URLs (#43424)
There are situations where the Router does not currently clean up failed navigations
correctly. While this is problematic on its own, we can mitigate some of
the damage by processing any URL when we get a navigation request when
the internal router state is out of sync.

Each of the added tests would fail without this change.

fixes #34795

PR Close #43424
2021-09-13 14:48:00 -07:00
Andrew Scott 79eee5509f docs(router): Add documentation for the router reset rawUrlTree (#43420)
Add documentation to the router and an additional test based on more
complete understanding of how things work.

PR Close #43420
2021-09-13 14:41:23 -07:00
Andrew Scott cbac0c3514 docs: Add documentation and TODOs for cleanup (#43391)
* Document `currentUrlTree`, `browserUrlTree`, and `rawUrlTree`
* Add a couple `TODO` comments to investigate changes based on understanding

PR Close #43391
2021-09-09 14:53:27 -07:00
Andrew Kushnir 012209f55f Revert "refactor(router): clean up unnecessary flag in restoreHistory function" (#43409)
This reverts commit 061a456e39.

PR Close #43409
2021-09-09 14:23:30 -07:00
Andrew Scott f57f0edd23 refactor(router): clean up unnecessary flag in restoreHistory function (#43382)
This restores the `finalize` function to directly call
`resetUrlToCurrentUrlTree`, as it was before efb440eb2f.
This allows us to simplify the `restoreHistory` function because it no
longer needs to handle the call from `finalize` where it should not
reset the internal router state.

PR Close #43382
2021-09-09 09:22:05 -07:00
Andrew Scott 9e039ca68b fix(router): Only trigger router navigation on popstate events from Location subscription (#43328)
With the merging of #41730, our tests now more closely emulate real
browser behavior. This means that we can make changes to the `Router` which depend
on actual browser behavior rather than incorrectly mocked test behavior.

This change updates the logic in the `Router` to only trigger
navigations on `popstate` events. Since our `SpyLocation` now triggers
the `popstate` event correctly on `simulateHashChange`, `back`,
`forward`, and `simulatePopState`, we are able to rely on this being
true for all of these navigations in tests when using `SpyLocation` (the
default for `RouterTestingModule`).

In addition, this behavior relies on documented browser behavior: The
`popstate` event happens due to browser transitions as a result of user
triggered "back" button or otherwise.
https://developer.mozilla.org/en-US/docs/Web/API/Window/popstate_event#when_popstate_is_sent

In addition, this change chooses `popstate` over `hashchange` because it
is the event that is already being used for navigations since the
documented behavior of browsers is that `popstate` happens before
`hashchange`.

fixes #41782

PR Close #43328
2021-09-08 16:53:49 +00:00
Andrew Scott 3c6b653089 feat(router): Option to correctly restore history on failed navigation (#43289)
Exposes implementation from #38884 as a public opt-in option.

From that commit:
> We can’t determine whether the user actually meant the back or
> the forward using the popstate event (triggered by a browser
> back/forward)
> so we instead need to store information on the state and compute the
> distance the user is traveling withing the browser history.
> So by using the History#go method,
> we can bring the user back to the page where he is supposed to be after
> performing the action.

Resolves #13586

PR Close #43289
2021-09-02 17:46:35 +00:00
Alan Agius 95f3aecc29 fix(router): add more context to Unhandled Navigation Error (#43291)
Previously, the error message was not added to `Unhandled Navigation Error`

Before
```
main.js:1 Unhandled Navigation Errors:
```

After
```
main.js:1 Unhandled Navigation Errors: SecurityError: Failed to execute 'replaceState' on 'History': A history state object with URL 'http://localhost:4200/' cannot be created in a document with origin 'http://127.0.0.1:8080' and URL 'http://127.0.0.1:8080/'.
```

PR Close #43291
2021-09-01 21:15:00 +00:00
anandtiwary faf9f5a3bc feat(router): new output that would notify when link is activated (#43280)
This commit adds a new output to `routerLinkActive` directive.
Whenever, the associated link becomes active or inactive, an
event will be fired on this out with the correct status

PR Close #37284

PR Close #43280
2021-09-01 08:23:15 -07:00
Andrew Scott 78690ee354 docs(router): Fix incorrect name for url match options (#43268)
fixes #43123

PR Close #43268
2021-08-26 10:07:23 -07:00
Andrew Scott ccb09b4558 fix(router): null/undefined routerLink should disable navigation (#43087)
The current behavior of `routerLink` for `null` and `undefined` inputs is to treat
the input the same as `[]`. This creates several unresolvable issues with
correctly disabling the links because `commands = []` does _not_ behave the same
as disabling a link. Instead, it navigates to the current page, but will also
clear any fragment and/or query params.

The new behavior of the `routerLink` input will be to completely disable navigation
for `null` and `undefined` inputs. For HTML Anchor elements, this will also mean
removing the `href` attribute.

Fixes #21457
Fixes #13980
Fixes #31154

BREAKING CHANGE:
Previously `null` and `undefined` inputs for `routerLink` were
equaivalent to empty string and there was no way to disable the link's
navigation.
In addition, the `href` is changed from a property `HostBinding()` to an
attribute binding (`HostBinding('attr.href')`). The effect of this
change is that `DebugElement.properties['href']` will now return the
`href` value returned by the native element which will be the full URL
rather than the internal value of the `RouterLink` `href` property.

PR Close #43087
2021-08-20 12:51:09 -07:00
Andrew Scott 286b2807de fix(router): eagerly update internal state on browser-triggered navigations (#43102)
The management of `browserUrlTree` currently has several problems with
correctly tracking the actual state of the browser.

This change makes the Router eagerly update the `browserUrlTree` when
handling navigations triggered by browser events (i.e., not 'imperative'). This
is because with those types of navigations, the browser URL bar is
_already_ updated. If we do not update the internal tracking of the
`browserUrlTree`, we will be out of sync with the real URL if the
navigation is rejected.

It would be best if we could remove `browserUrlTree` completely, but doing that
would require a lot more investigation and is blocked by #27059 because
the SpyLocation used in tests does not emulate real browser behavior.

fixes #43101

PR Close #43102
2021-08-16 09:33:23 -07:00
Andrew Scott bbad42310b fix(router): ensure check for match options is compatible with property renaming (#43086)
After renaming, 'paths' is not in routerLinkOptions since it's been renamed.

PR Close #43086
2021-08-09 10:26:07 -07:00
Jon Rimmer 784671597e fix(router): Allow question marks in query param values (#31187)
According to the URI spec, question mark characters are valid in query data,
so these should accepted by the param parsing.

See https://tools.ietf.org/html/rfc3986#section-3.4

Fixes #31116

BREAKING CHANGE: The default url serializer would previously drop
everything after and including a question mark in query parameters. That
is, for a navigation to `/path?q=hello?&other=123`, the query
params would be parsed to just `{q: 'hello'}`. This is
incorrect because the URI spec allows for question mark characers in
query data. This change will now correctly parse the params for the
above example to be `{v: 'hello?', other: '123'}`.

PR Close #31187
2021-08-06 10:49:37 -07:00
Steven Masala 0d81b007e4 fix(router): add missing outlet events to RouterOutletContract (#42431)
Exposes both activateEvents and deactivateEvents as the original outlet interface did.

PR Close #42431
2021-07-27 16:34:24 -07:00
Andrew Scott 4b49824236 test(router): move computed state restoration tests to own file (#42933)
To reduce the enormouse size of the integration.spec.ts file, move tests related to the computed
state restoration to their own file.

PR Close #42933
2021-07-23 14:25:06 -07:00
Andrew Scott 9119e1f7bd refactor(router): Ensure computed state restoration works for thrown errors (#42933)
When `canceledNavigationResolution='computed'`, the `Router` needs to
handle the cases where errors are thrown. Previously, the logic was not
updated and would simply do a `replaceState` rather than determining
where in the history we should move to restore the page/url from before
the failed navigation.

PR Close #42933
2021-07-23 14:25:06 -07:00
Andrew Scott a6c256fcd6 docs: Add import line to router event example for clarification (#42935)
Without the `import {Event} from '@angular/router';`, the filter will
not work because the type is understood as the native `Event`.

Fixes #42920

PR Close #42935
2021-07-23 12:06:34 -07:00
Paul Gschwendtner abc77a6a39 refactor(router): ensure compatibility with noImplicitOverride (#42512)
Adds the `override` keyword to the `router` sources to ensure
compatibility with `noImplicitOverride`.

PR Close #42512
2021-07-12 13:11:15 -07:00
Andrew Scott 3791ae0c95 refactor(router): Adjust behavior for computed navigation restoration (#42751)
When another navigation is triggered during an in-process navigation and
the `canceledNavigationResolution` is `'computed'`, we should not
attempt to restore the browser history using `history.go`. Doing that
would trigger a third navigation through the router which would conflict
with the new navigation that we were trying to process. Instead, we
treat this as a redirect and skip the history restoration attempt. This
acts similarly to returning `UrlTree` from a guard.

Fixes issue described in https://github.com/angular/angular/pull/38884#issuecomment-863767152

PR Close #42751
2021-07-09 10:19:09 -07:00
Kristiyan Kostadinov 07c1ddc487 fix(router): error if module is destroyed before location is initialized (#42560)
This is something I ran into while working on a fix for the `TestBed` module teardown behavior for #18831. In the `RouterInitializer.appInitializer` we have a callback to the `LOCATION_INITIALIZED` which has to do some DI lookups. The problem is that if the module is destroyed before the location promise resolves, the `Injector.get` calls will fail. This is unlikely to happen in a real app, but it'll show up in unit tests once the test module teardown behavior is fixed.

PR Close #42560
2021-06-17 18:11:53 +00:00
Ahmed Ayed efb440eb2f refactor(router): compute correct history restoration when navigation is cancelled (#38884)
We can’t determine whether the user actually meant the `back` or
the `forward` using the popstate event (triggered by a browser
back/forward)
so we instead need to store information on the state and compute the
distance the user is traveling withing the browser history.
So by using the `History#go` method,
we can bring the user back to the page where he is supposed to be after
performing the action.

implementation for #13586

PR Close #38884
2021-06-10 14:09:21 -07:00
Andrew Scott 536c3738ba docs: clarify onSameUrlNavigation behavior (#42275)
`onSameUrlNavigation` only affects whether the Angular Router
processes the URL and runs it through the navigation pipeline,
retriggering redirects, guards, and resolvers. The name `reload` is a
little confusing because it does _not_ reload the component. Developers
_also_ need to implement a custom `RouteReuseStrategy` to trigger a
component reload on same URL navigation.

Fixes #21115

PR Close #42275
2021-06-09 09:45:25 -07:00
Sam Severance c44ab4f6da fix(router): fix serializeQueryParams logic (#42481)
corrects a bug that resulted in query params such as
`[queryParams]={a: 1, b:[]}` being serialized as 'a=1&'
instead of 'a=1'

resolves #42445

PR Close #42481
2021-06-08 10:43:00 -07:00
Paul Gschwendtner 2d0ff0a5d3 ci: add lint error for files with missing trailing new-line (#42478)
For quite a while it is an unspoken convention to add a trailing
new-line files within the Angular repository. This was never enforced
automatically, but has been frequently raised in pull requests through
manual review. This commit sets up a lint rule so that this is
"officially" enforced and doesn't require manual review.

PR Close #42478
2021-06-04 13:31:03 -07:00
Andrew Scott 6a2e3e2b2c docs: change Navigation from type to interface (#42281)
Navigation type should be interface so it can be documented on aio site.

fixes #37384

PR Close #42281
2021-06-01 14:13:20 -07:00
Igor Minar 184de5048a docs: expand documentation for ROUTES (#42398)
Previously the docs were very minimalistic. The most important thing missing from the docs
was that people should primarily use higher level APIs instead of using ROUTES directly.

It would be nice to holistically overhaul more of the router API docs, but that's out of
scope of this change.

Fixes #39350

PR Close #42398
2021-05-27 16:47:39 -07:00
Andrew Scott 8228170919 docs: document shallow equality emit for ActivatedRoute (#42280)
fixes #40769

PR Close #42280
2021-05-25 22:15:39 +00:00
Andrew Scott 6da0c97423 docs: add note about redirects after absolute redirect (#42277)
This commit addresses the documentation portion of #19069.

PR Close #42277
2021-05-25 22:13:07 +00:00
Andrew Scott ef33806480 docs: add small clarity around navigationCancel (#42282)
Add small clarity to sentence in documentation for navigation cancel event
to indicate that router guards returning false or urlTree is only one of
several reasons a NavigationCancel event happens.

fixes #26613

PR Close #42282
2021-05-25 17:55:22 +00:00
Andrew Scott a39169851e docs: clarify pathMatch strategy prefix (#42287)
This commit clarifies the description of the default `pathMatch`
strategy (prefix) to indicate that the path segments must each match to
a config.

fixes #39737

PR Close #42287
2021-05-24 15:43:31 -07:00