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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parentheses remains in URL when child auxiliary route outlet path is set to null #24656

Closed
galtonova opened this issue Jun 25, 2018 · 15 comments
Closed
Labels
Milestone

Comments

@galtonova
Copy link

I'm submitting a...


[x] Bug report  

Current behavior

From the following URL:
/some/(primary/route//auxOutlet:someAuxRoute)

When routed like this:
this.router.navigate( ['/some', { outlets: { auxOutlet: null } }]);

Parentheses remains in URL string:
https://angular-kasves.stackblitz.io/some/(primary/route)

Expected behavior

URL string should be trimmed of parentheses like that:
https://angular-kasves.stackblitz.io/some/primary/route

Demo

https://stackblitz.com/edit/angular-kasves
https://angular-kasves.stackblitz.io/some/primary/route
First, click "Aux On" button. Then click "Aux Off" button.

Reproduction Steps

  • Route to some child auxiliary route:
    this.router.navigate( ['/some', { outlets: { auxOutlet: ['someAuxRoute'] } }]);

    URL will be something similar:
    https://angular-kasves.stackblitz.io/some/(primary/route//auxOutlet:someAuxRoute)

  • After that, set auxiliary router outlet path to null:
    this.router.navigate( ['/some', { outlets: { auxOutlet: null } }]);

    URL will still contain parentheses:
    https://angular-kasves.stackblitz.io/some/(primary/route)

@bartando
Copy link

This is exactly what happens to me too. The problem is that I cannot change the url either by this.location.replaceState(this.router.url.replace(/\(|\)/g, ''));. Hopefully this will get soon, because for me it even leaves route like https://angular-kasves.stackblitz.io/some/()

@ngbot ngbot bot added this to the needsTriage milestone Jun 25, 2018
@galtonova
Copy link
Author

galtonova commented Jun 25, 2018

@bartando In my project, I implemented a custom URL serializer and then regex replaced parentheses. It looks like it's working right now but it's still a hack, not a permanent solution. I honestly don't know if it will cause a problem in the future.

Here's my class:

// app.module.ts
import {DefaultUrlSerializer, UrlSerializer, UrlTree} from '@angular/router';

export class MyUrlSerializer extends DefaultUrlSerializer implements UrlSerializer  {
  /** Converts a `UrlTree` into a url */
  serialize(tree: UrlTree): string {
    return super.serialize(tree).replace(/\((((?!(\/\/)).)*)\)/g, '$1');
  }
}

@NgModule({
  providers: [{ provide: UrlSerializer, useClass: MyUrlSerializer }],
  /* other stuff */
})
export class AppModule { }

Regex I used /\((((?!(\/\/)).)*)\)/g looks for a substring which starts after (,ends before ) and does not contains // in between. It would also remove () in your case.

@bartando
Copy link

Thank you very much, this is by far the best solution. At least for now.

@jasonaden jasonaden added type: bug/fix help wanted An issue that is suitable for a community contributor (based on its complexity/scope). freq3: high severity3: broken labels Jul 3, 2018
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jul 3, 2018
@jasonaden
Copy link
Contributor

This looks like something with the serialization logic. It would seem you're right, where we should have a check for this scenario, to see if the only child in the UrlTree is the primary outlet and ensure we serialize with slashes in the case rather than () notation.

This would be great if any community members wanted to pick this one up. Let me know if any of you are interested in helping on this one.

@joshpcausey
Copy link

@jasonaden I'll start working on a fix for this tomorrow evening.

@ahafsi
Copy link

ahafsi commented Aug 8, 2018

with my team we are facing the same problem.
@joshpcausey have you resolve this issue?

@amhhernandez
Copy link

amhhernandez commented Aug 8, 2018

I implemented a similar solution like galtonova's with this class:

import {DefaultUrlSerializer, UrlSerializer, UrlTree} from '@angular/router';

export class MyUrlSerializer extends DefaultUrlSerializer implements UrlSerializer  {
  serialize(tree: UrlTree): string {
    return super.serialize(tree).replace(/\(|\)|\w+-\w+:/g, '');
  }
}

The key part is the regex used in serialize function.

@jamsuu
Copy link

jamsuu commented Oct 22, 2018

I believe I'm experiencing a problem with this with a combination of default routes (through an empty path ''), where a route would end up as /parent/(primaryPath) and end up redirecting me to /parent/defaultPath route instead after an imperative redirect (using router.navigate).

My code is something along the lines of this

{
    path: 'main',     
    component: MainComponent,
    children: [
      {
        path: 'summary',
        loadChildren: 'summary.module#SummaryModule',
      },
      {
        path: 'groups',
        loadChildren: 'groups.module#GroupsModule',
      },
      {
        path: 'group',
        loadChildren: 'group.module#GroupModule',
        outlet: 'view',
      },
      {path: '', redirectTo: 'summary'}, // NAVIGATE WORKS CORRECTLY IF THIS LINE IS GONE
    ],
  },
  {path: '', redirectTo: 'main'},
}

@LayZeeDK
Copy link
Contributor

LayZeeDK commented Oct 23, 2018

@jamsuu Your issue is probably that the Angular Router does not support multiple redirects when resolving a route from a URL.

If your user navigates to /, they first hit {path: '', redirectTo: 'main'} and then {path: '', redirectTo: 'summary'}. Unfortunately, this is not supported. You would have to use {path: '', redirectTo: 'main/summary' } in the last route.

@oqx
Copy link

oqx commented Nov 20, 2018

I'm experiencing this as well. I wouldn't mind so much, but if the user refreshes the page, the view fails to load. If the parentheses are removed, it works. It does work when navigated to via router.navigate, but that still leaves this edge case. I'll give the serializer a shot.

@lincolnthree
Copy link
Contributor

Still having trouble with this in 2019. The serializer helps. Is it intended that apps should generally need to override this? E.g. is it common practice? Obviously there is an API for it, but... I just wonder how often this is done.

@Mark12870
Copy link

Mark12870 commented Mar 5, 2020

It's 2020 and still no solution? :/ (except the custom serialiser)

@IssamTechs
Copy link

IssamTechs commented May 14, 2020

in May 2020.
using injected Serializer is still a hack and it may have implications in the future.
one of the implications that been confirmed up until now is when you reload the page or manually navigate to URL via address bar uncaught error "Cannot match any route"
It's almost 2 years since the issue was raised

ggradnig pushed a commit to ggradnig/angular that referenced this issue May 17, 2020
…ving auxiliary outlet segment

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

Resolves angular#24656
ggradnig pushed a commit to ggradnig/angular that referenced this issue May 17, 2020
…ving auxiliary outlet segment (angular#24656)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656
ggradnig added a commit to ggradnig/angular that referenced this issue May 17, 2020
…ving auxiliary outlet segment (angular#24656)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656
ggradnig added a commit to ggradnig/angular that referenced this issue May 17, 2020
…ving auxiliary outlet segment (angular#24656)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656
ggradnig added a commit to ggradnig/angular that referenced this issue May 17, 2020
…ving auxiliary outlet segment (angular#24656)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656
@ggradnig
Copy link
Contributor

Added a check to serialize without parentheses if there is only a primary outlet being used. Should fix the issue.

mhevery pushed a commit to ggradnig/angular that referenced this issue Jun 18, 2020
…ving auxiliary outlet segment (angular#24656)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656
ngwattcos pushed a commit to ngwattcos/angular that referenced this issue Jun 25, 2020
…ving auxiliary outlet segment (angular#24656) (angular#37163)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656

PR Close angular#37163
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 19, 2020
profanis pushed a commit to profanis/angular that referenced this issue Sep 5, 2020
…ving auxiliary outlet segment (angular#24656) (angular#37163)

For URLs that use auxiliary route outlets in the second or following path segments,
when removing the auxiliary route segment, parenthesis remain for the primary outlet segment.
This causes the following error when trying to reload an URL: "Cannot match any route".
The commit adds a check for this scenario, serializing the URL as "a/b" instead of "a/(b)".

PR Close angular#24656

PR Close angular#37163
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.