Skip to content

Commit

Permalink
fix(router-plugin): prevent router overriding valid navigation (#1907)
Browse files Browse the repository at this point in the history
* fix(router-plugin): remove initial navigation check

* chore: update `CHANGELOG.md`

* chore: update CHANGELOG.md

Co-authored-by: Mark Whitfeld <markwhitfeld@users.noreply.github.com>
  • Loading branch information
arturovt and markwhitfeld authored Aug 8, 2022
1 parent 07b1a05 commit 79cbc2a
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 68 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ $ npm install @ngxs/store@dev
- Fix: Give back control to `developmentMode` config property [#1878](https://github.com/ngxs/store/pull/1878)
- Fix: Do not use `refCount()` since it makes selectable stream cold [#1883](https://github.com/ngxs/store/pull/1883)
- Fix: Remove `?` from `ctx` parameter of lifecycle hooks since they are never undefined [#1889](https://github.com/ngxs/store/pull/1889)
- Fix: Router Plugin - Prevent router overriding valid navigation [#1907](https://github.com/ngxs/store/pull/1907)
- Fix: Storage Plugin - Provide more meaningful error message when the storage quota exceeds [#1863](https://github.com/ngxs/store/pull/1863)
- Fix: Storage Plugin - Ensure the deserialization is not skipped for master key [#1887](https://github.com/ngxs/store/pull/1887)
- Fix: Storage Plugin - Do not re-hydrate the whole state when the feature state is added [#1887](https://github.com/ngxs/store/pull/1887)
Expand Down
69 changes: 1 addition & 68 deletions packages/router-plugin/src/router.state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,11 @@ import {
RouterStateSnapshot,
RoutesRecognized,
ResolveEnd,
UrlSerializer,
NavigationStart,
NavigationEnd
} from '@angular/router';
import { LocationStrategy, Location } from '@angular/common';
import { Action, Selector, State, StateContext, Store } from '@ngxs/store';
import { isAngularInTestMode } from '@ngxs/store/internals';
import { Subscription } from 'rxjs';
import { first } from 'rxjs/operators';

import {
Navigate,
Expand All @@ -39,12 +35,6 @@ export type RouterTrigger =
// The `devtools` trigger means that the state change has been triggered by Redux DevTools (e.g. when the time-traveling is used).
| 'devtools';

/**
* @description Will be provided through Terser global definitions by Angular CLI
* during the production build. This is how Angular does tree-shaking internally.
*/
declare const ngDevMode: boolean;

@State<RouterStateModel>({
name: 'router',
defaults: {
Expand Down Expand Up @@ -89,14 +79,10 @@ export class RouterState implements OnDestroy {
private _store: Store,
private _router: Router,
private _serializer: RouterStateSerializer<RouterStateSnapshot>,
private _ngZone: NgZone,
private _urlSerializer: UrlSerializer,
private _locationStrategy: LocationStrategy,
private _location: Location
private _ngZone: NgZone
) {
this.setUpStoreListener();
this.setUpRouterEventsListener();
this.checkInitialNavigationOnce();
}

ngOnDestroy(): void {
Expand Down Expand Up @@ -254,57 +240,4 @@ export class RouterState implements OnDestroy {
this._storeState = null;
this._routerState = null;
}

/**
* No sense to mess up the `setUpRouterEventsListener` method as we have
* to perform this check only once and unsubscribe after the first event
* is triggered
*/
private checkInitialNavigationOnce(): void {
// Caretaker note: we have still left the `typeof` condition in order to avoid
// creating a breaking change for projects that still use the View Engine.
if (
(typeof ngDevMode === 'undefined' || ngDevMode) &&
// Angular is running tests in development mode thus we can be sure that this method will be
// skipped in tests.
isAngularInTestMode()
) {
return;
}

const subscription = this._router.events
.pipe(first((event): event is RoutesRecognized => event instanceof RoutesRecognized))
.subscribe(({ url }) => {
// `location.pathname` always equals manually entered URL in the address bar
// e.g. `location.pathname === '/foo'`, but the `router` state has been initialized
// with another URL (e.g. used in combination with `NgxsStoragePlugin`), thus the
// `RouterNavigation` action will be dispatched and the user will be redirected to the
// previously saved URL. We want to prevent such behavior, so we perform this check

// `url` is a recognized URL by the Angular's router, while `currentUrl` is an actual URL
// entered in the browser's address bar
// `PathLocationStrategy.prototype.path()` returns a concatenation of
// `PlatformLocation.pathname` and normalized `PlatformLocation.search`.

// `Location.prototype.normalize` strips base href from the URL,
// if `baseHref` (declared in angular.json) for example is `/en`
// and the URL is `/test#anchor` - then `_locationStrategy.path(true)` will return `/en/test#anchor`,
// but `/en/test#anchor` is not known to the Angular's router, so we have to strip `/en`
// from the URL
const currentUrl = this._location.normalize(this._locationStrategy.path(true));
const currentUrlTree = this._urlSerializer.parse(currentUrl);
// We need to serialize the URL because in that example `/test/?redirect=https://google.com/`
// Angular will recognize it as `/test?redirect=https:%2F%2Fwww.google.com%2F`
// so we have to run the `currentUrl` via the `UrlSerializer` that will encode characters
const currentSerializedUrl = this._urlSerializer.serialize(currentUrlTree);

// If URLs differ from each other - we've got to perform a redirect to the manually entered URL
// in the address bar, as it must have a priority
if (currentSerializedUrl !== url) {
this._router.navigateByUrl(currentUrl);
}
});

this._subscription.add(subscription);
}
}

0 comments on commit 79cbc2a

Please sign in to comment.