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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
* Document `currentUrlTree`, `browserUrlTree`, and `rawUrlTree`
* Add a couple `TODO` comments to investigate changes based on understanding
PR Close#43391
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
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
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
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
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
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#21457Fixes#13980Fixes#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
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
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.4Fixes#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
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
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
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
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
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
`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
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
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
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
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
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