fix(router): do not call `location.go` when skipping a navigation (#19463)
Closes #18036 PR Close #19463
This commit is contained in:
parent
82fed62af2
commit
adab4f3e49
|
@ -580,10 +580,9 @@ export class Router {
|
|||
}
|
||||
|
||||
private runNavigate(
|
||||
url: UrlTree, rawUrl: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean,
|
||||
id: number, precreatedState: RouterStateSnapshot|null): Promise<boolean> {
|
||||
url: UrlTree, rawUrl: UrlTree, skipLocationChange: boolean, replaceUrl: boolean, id: number,
|
||||
precreatedState: RouterStateSnapshot|null): Promise<boolean> {
|
||||
if (id !== this.navigationId) {
|
||||
this.location.go(this.urlSerializer.serialize(this.currentUrlTree));
|
||||
(this.events as Subject<Event>)
|
||||
.next(new NavigationCancel(
|
||||
id, this.serializeUrl(url),
|
||||
|
@ -705,9 +704,9 @@ export class Router {
|
|||
|
||||
(this as{routerState: RouterState}).routerState = state;
|
||||
|
||||
if (!shouldPreventPushState) {
|
||||
if (!skipLocationChange) {
|
||||
const path = this.urlSerializer.serialize(this.rawUrlTree);
|
||||
if (this.location.isCurrentPathEqualTo(path) || shouldReplaceUrl) {
|
||||
if (this.location.isCurrentPathEqualTo(path) || replaceUrl) {
|
||||
this.location.replaceState(path);
|
||||
} else {
|
||||
this.location.go(path);
|
||||
|
@ -755,14 +754,13 @@ export class Router {
|
|||
(this as{routerState: RouterState}).routerState = storedState;
|
||||
this.currentUrlTree = storedUrl;
|
||||
this.rawUrlTree = this.urlHandlingStrategy.merge(this.currentUrlTree, rawUrl);
|
||||
this.location.replaceState(this.serializeUrl(this.rawUrlTree));
|
||||
this.resetUrlToCurrentUrlTree();
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
private resetUrlToCurrentUrlTree(): void {
|
||||
const path = this.urlSerializer.serialize(this.rawUrlTree);
|
||||
this.location.replaceState(path);
|
||||
this.location.replaceState(this.urlSerializer.serialize(this.rawUrlTree));
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -12,6 +12,7 @@ import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/
|
|||
import {By} from '@angular/platform-browser/src/dom/debug/by';
|
||||
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
||||
import {ActivatedRoute, ActivatedRouteSnapshot, ActivationEnd, ActivationStart, CanActivate, CanDeactivate, ChildActivationEnd, ChildActivationStart, DetachedRouteHandle, Event, GuardsCheckEnd, GuardsCheckStart, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, ResolveEnd, ResolveStart, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterEvent, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlTree} from '@angular/router';
|
||||
import {SpyLocation} from 'common/testing';
|
||||
import {Observable} from 'rxjs/Observable';
|
||||
import {Observer} from 'rxjs/Observer';
|
||||
import {of } from 'rxjs/observable/of';
|
||||
|
@ -50,8 +51,8 @@ describe('Integration', () => {
|
|||
expect(fixture.nativeElement).toHaveText('route');
|
||||
})));
|
||||
|
||||
it('should navigate to the current URL',
|
||||
fakeAsync(inject([Router, Location], (router: Router) => {
|
||||
describe('navigation', function() {
|
||||
it('should navigate to the current URL', fakeAsync(inject([Router], (router: Router) => {
|
||||
router.resetConfig([
|
||||
{path: '', component: SimpleCmp},
|
||||
{path: 'simple', component: SimpleCmp},
|
||||
|
@ -73,6 +74,65 @@ describe('Integration', () => {
|
|||
]);
|
||||
})));
|
||||
|
||||
it('should not pollute browser history when replaceUrl is set to true',
|
||||
fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => {
|
||||
router.resetConfig([
|
||||
{path: '', component: SimpleCmp}, {path: 'a', component: SimpleCmp},
|
||||
{path: 'b', component: SimpleCmp}
|
||||
]);
|
||||
|
||||
const fixture = createRoot(router, RootCmp);
|
||||
|
||||
router.navigateByUrl('/a', {replaceUrl: true});
|
||||
router.navigateByUrl('/b', {replaceUrl: true});
|
||||
tick();
|
||||
|
||||
expect(location.urlChanges).toEqual(['replace: /', 'replace: /b']);
|
||||
})));
|
||||
|
||||
it('should skip navigation if another navigation is already scheduled',
|
||||
fakeAsync(inject([Router, Location], (router: Router, location: SpyLocation) => {
|
||||
router.resetConfig([
|
||||
{path: '', component: SimpleCmp}, {path: 'a', component: SimpleCmp},
|
||||
{path: 'b', component: SimpleCmp}
|
||||
]);
|
||||
|
||||
const fixture = createRoot(router, RootCmp);
|
||||
|
||||
router.navigate(
|
||||
['/a'], {queryParams: {a: true}, queryParamsHandling: 'merge', replaceUrl: true});
|
||||
router.navigate(
|
||||
['/b'], {queryParams: {b: true}, queryParamsHandling: 'merge', replaceUrl: true});
|
||||
tick();
|
||||
|
||||
/**
|
||||
* Why do we have '/b?b=true' and not '/b?a=true&b=true'?
|
||||
*
|
||||
* This is because the router has the right to stop a navigation mid-flight if another
|
||||
* navigation has been already scheduled. This is why we can use a top-level guard
|
||||
* to perform redirects. Calling `navigate` in such a guard will stop the navigation, and
|
||||
* the components won't be instantiated.
|
||||
*
|
||||
* This is a fundamental property of the router: it only cares about its latest state.
|
||||
*
|
||||
* This means that components should only map params to something else, not reduce them.
|
||||
* In other words, the following component is asking for trouble:
|
||||
*
|
||||
* ```
|
||||
* class MyComponent {
|
||||
* constructor(a: ActivatedRoute) {
|
||||
* a.params.scan(...)
|
||||
* }
|
||||
* }
|
||||
* ```
|
||||
*
|
||||
* This also means "queryParamsHandling: 'merge'" should only be used to merge with
|
||||
* long-living query parameters (e.g., debug).
|
||||
*/
|
||||
expect(router.url).toEqual('/b?b=true');
|
||||
})));
|
||||
});
|
||||
|
||||
describe('should execute navigations serially', () => {
|
||||
let log: any[] = [];
|
||||
|
||||
|
|
Loading…
Reference in New Issue