Revert "fix(router): do not call `location.go` when skipping a navigation (#19463)"
This reverts commit 6651541230
.
This commit is contained in:
parent
79deeacbd7
commit
d53b96f2a2
|
@ -580,9 +580,10 @@ export class Router {
|
||||||
}
|
}
|
||||||
|
|
||||||
private runNavigate(
|
private runNavigate(
|
||||||
url: UrlTree, rawUrl: UrlTree, skipLocationChange: boolean, replaceUrl: boolean, id: number,
|
url: UrlTree, rawUrl: UrlTree, shouldPreventPushState: boolean, shouldReplaceUrl: boolean,
|
||||||
precreatedState: RouterStateSnapshot|null): Promise<boolean> {
|
id: number, precreatedState: RouterStateSnapshot|null): Promise<boolean> {
|
||||||
if (id !== this.navigationId) {
|
if (id !== this.navigationId) {
|
||||||
|
this.location.go(this.urlSerializer.serialize(this.currentUrlTree));
|
||||||
(this.events as Subject<Event>)
|
(this.events as Subject<Event>)
|
||||||
.next(new NavigationCancel(
|
.next(new NavigationCancel(
|
||||||
id, this.serializeUrl(url),
|
id, this.serializeUrl(url),
|
||||||
|
@ -704,9 +705,9 @@ export class Router {
|
||||||
|
|
||||||
(this as{routerState: RouterState}).routerState = state;
|
(this as{routerState: RouterState}).routerState = state;
|
||||||
|
|
||||||
if (!skipLocationChange) {
|
if (!shouldPreventPushState) {
|
||||||
const path = this.urlSerializer.serialize(this.rawUrlTree);
|
const path = this.urlSerializer.serialize(this.rawUrlTree);
|
||||||
if (this.location.isCurrentPathEqualTo(path) || replaceUrl) {
|
if (this.location.isCurrentPathEqualTo(path) || shouldReplaceUrl) {
|
||||||
this.location.replaceState(path);
|
this.location.replaceState(path);
|
||||||
} else {
|
} else {
|
||||||
this.location.go(path);
|
this.location.go(path);
|
||||||
|
@ -754,13 +755,14 @@ export class Router {
|
||||||
(this as{routerState: RouterState}).routerState = storedState;
|
(this as{routerState: RouterState}).routerState = storedState;
|
||||||
this.currentUrlTree = storedUrl;
|
this.currentUrlTree = storedUrl;
|
||||||
this.rawUrlTree = this.urlHandlingStrategy.merge(this.currentUrlTree, rawUrl);
|
this.rawUrlTree = this.urlHandlingStrategy.merge(this.currentUrlTree, rawUrl);
|
||||||
this.resetUrlToCurrentUrlTree();
|
this.location.replaceState(this.serializeUrl(this.rawUrlTree));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
private resetUrlToCurrentUrlTree(): void {
|
private resetUrlToCurrentUrlTree(): void {
|
||||||
this.location.replaceState(this.urlSerializer.serialize(this.rawUrlTree));
|
const path = this.urlSerializer.serialize(this.rawUrlTree);
|
||||||
|
this.location.replaceState(path);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -12,7 +12,6 @@ import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/
|
||||||
import {By} from '@angular/platform-browser/src/dom/debug/by';
|
import {By} from '@angular/platform-browser/src/dom/debug/by';
|
||||||
import {expect} from '@angular/platform-browser/testing/src/matchers';
|
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 {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 {Observable} from 'rxjs/Observable';
|
||||||
import {Observer} from 'rxjs/Observer';
|
import {Observer} from 'rxjs/Observer';
|
||||||
import {of } from 'rxjs/observable/of';
|
import {of } from 'rxjs/observable/of';
|
||||||
|
@ -51,87 +50,28 @@ describe('Integration', () => {
|
||||||
expect(fixture.nativeElement).toHaveText('route');
|
expect(fixture.nativeElement).toHaveText('route');
|
||||||
})));
|
})));
|
||||||
|
|
||||||
describe('navigation', function() {
|
it('should navigate to the current URL',
|
||||||
it('should navigate to the current URL', fakeAsync(inject([Router], (router: Router) => {
|
fakeAsync(inject([Router, Location], (router: Router) => {
|
||||||
router.resetConfig([
|
router.resetConfig([
|
||||||
{path: '', component: SimpleCmp},
|
{path: '', component: SimpleCmp},
|
||||||
{path: 'simple', component: SimpleCmp},
|
{path: 'simple', component: SimpleCmp},
|
||||||
]);
|
]);
|
||||||
|
|
||||||
const fixture = createRoot(router, RootCmp);
|
const fixture = createRoot(router, RootCmp);
|
||||||
const events: Event[] = [];
|
const events: Event[] = [];
|
||||||
router.events.subscribe(e => onlyNavigationStartAndEnd(e) && events.push(e));
|
router.events.subscribe(e => onlyNavigationStartAndEnd(e) && events.push(e));
|
||||||
|
|
||||||
router.navigateByUrl('/simple');
|
router.navigateByUrl('/simple');
|
||||||
tick();
|
tick();
|
||||||
|
|
||||||
router.navigateByUrl('/simple');
|
router.navigateByUrl('/simple');
|
||||||
tick();
|
tick();
|
||||||
|
|
||||||
expectEvents(events, [
|
expectEvents(events, [
|
||||||
[NavigationStart, '/simple'], [NavigationEnd, '/simple'], [NavigationStart, '/simple'],
|
[NavigationStart, '/simple'], [NavigationEnd, '/simple'], [NavigationStart, '/simple'],
|
||||||
[NavigationEnd, '/simple']
|
[NavigationEnd, '/simple']
|
||||||
]);
|
]);
|
||||||
})));
|
})));
|
||||||
|
|
||||||
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', () => {
|
describe('should execute navigations serially', () => {
|
||||||
let log: any[] = [];
|
let log: any[] = [];
|
||||||
|
|
Loading…
Reference in New Issue