Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Getting base url from route give different results #38972

Open
UrbanNuke opened this issue Sep 24, 2020 · 3 comments
Open

Getting base url from route give different results #38972

UrbanNuke opened this issue Sep 24, 2020 · 3 comments
Labels
area: router freq1: low P4 A relatively minor issue that is not relevant to core functions state: confirmed type: confusing
Milestone

Comments

@UrbanNuke
Copy link

UrbanNuke commented Sep 24, 2020

馃悶 bug report

Affected Package

The issue is caused by package @angular/router

Is this a regression?

I don't know

Description

Hello everyone.
I found strange behaviour in package @angular/router.
When angular app is starting, router has a emptyUrlTree in field currentUrlTree.
Then router creates field routerState based on currentUrlTree.
When I get url from (router.url) I have "/", but when I get url from (router.routerState.snapshot.url) I have empty string

I guess this behaviour is unexpected

馃敩 Minimal Reproduction

Just look at console
https://codesandbox.io/s/lingering-waterfall-bfnx4

馃實 Your Environment

Angular Version:


Angular CLI: 9.1.6
Node: 10.19.0
OS: win32 x64

Angular: 9.1.7
... animations, common, compiler, compiler-cli, core, forms
... language-service, localize, platform-browser
... platform-browser-dynamic, router
Ivy Workspace: Yes

Package                            Version
------------------------------------------------------------
@angular-devkit/architect          0.901.6
@angular-devkit/build-angular      0.901.6
@angular-devkit/build-optimizer    0.901.6
@angular-devkit/build-webpack      0.901.6
@angular-devkit/core               9.1.6
@angular-devkit/schematics         9.1.6
@angular/cdk                       9.2.3
@angular/cli                       9.1.6
@angular/material                  9.2.3
@angular/material-moment-adapter   9.2.3
@ngtools/webpack                   9.1.6
@schematics/angular                9.1.6
@schematics/update                 0.901.6
rxjs                               6.5.5
typescript                         3.8.3
webpack                            4.42.0
@sonukapoor sonukapoor added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews area: router labels Sep 24, 2020
@ngbot ngbot bot added this to the needsTriage milestone Sep 24, 2020
@sonukapoor sonukapoor removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Sep 24, 2020
@atscott
Copy link
Contributor

atscott commented Sep 24, 2020

This is simply a difference in the implementations of how the url is returned. They are equivalent, but router.url returns the serialized url, which always starts with a slash. The snapshot.url returns the string the snapshot was created from, as is.

@UrbanNuke
Copy link
Author

UrbanNuke commented Sep 24, 2020

@atscott thanks for answer.
But, is it necessary has different implementations? Are they both needed?

It doesn't matter to this issue, but ngrx router-store compares these fields.
And "" not equals "/", so sometimes it has strange navigation to "/"

@jelbourn jelbourn added P4 A relatively minor issue that is not relevant to core functions and removed severity1: confusing labels Oct 1, 2020
@kyleryanbanks
Copy link

kyleryanbanks commented Dec 4, 2020

Hey everyone,

We got bit by @ngrx/router-store causing the extra redirect to '/' because it think's it's out of sync with the router after upgrading from v7 to v10.

As a temporary fix, you can modify the DefaultUrlSerializer to strip out that leading '/' to keep the router-store happy.

I haven't had a time to spin up a repro app, but I think it's initial activations that are redirected.

The router-store gets initially set to url: '' (I think this is the routers currentUrlTree initial value that get passed over) and then after a cancelled navigation (due to UrlTree redirect), the router will serialize the empty url and return '/'. The router-store compares those two values and fires a 2nd navigation that starts racing against your intended redirect.

Here's the snippet we're using as a temporary local fix. There's a LOT of extra logic in the guts of the serializer, so this was a cheap/clean way to manually strip out the leading '/' after the default serializer has run.

/**
 * This UrlSerializer removes the leading '/' from the default serializer.
 * If you have the NgRx router store enabled the '/' will not match the empty url state ''
 * and triggers an extra navigation to '/' when redirecting with a UrlTree
 *
 * @export
 * @class UrlSerializerWithNoLeadingSlash
 * @extends {DefaultUrlSerializer}
 */
export class UrlSerializerWithNoLeadingSlash extends DefaultUrlSerializer {
  serialize(url: UrlTree): string {
    return super.serialize(url).substring(1);
  }
}

@NgModule({
  imports: [
    RouterModule.forRoot(ROUTES)
  ],
  // Router Store will trigger a navigation if an empty url does not === ''
  providers: [{ provide: UrlSerializer, useClass: UrlSerializerWithNoLeadingSlash }]
})  
export class AppModule {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: router freq1: low P4 A relatively minor issue that is not relevant to core functions state: confirmed type: confusing
Projects
None yet
Development

No branches or pull requests

5 participants