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

Optional Routing Parameters issue #32853

Open
gowthambj opened this issue Sep 25, 2019 · 15 comments
Open

Optional Routing Parameters issue #32853

gowthambj opened this issue Sep 25, 2019 · 15 comments
Labels
area: router freq2: medium P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent router: URL parsing/generation state: confirmed type: bug/fix
Milestone

Comments

@gowthambj
Copy link

gowthambj commented Sep 25, 2019

🐞 bug report

Affected Package

@angular/router

Is this a regression?

Yes, working in angularjs 1.x

Description

Optional Routing Parameters issue

Route declaration

const routes: Routes = [
{ path: '', component: SearchpagesComponent ,
children: [
{
path: ’near/:nearSearch/:val1/:val2/:name/:filter', loadChildren: './near-search/near-search.module#NearSearchModule',
}
]
}
];

RouterLink config

<a [routerLink]="['/near/','','37’,’122','San-Francisco','All']" > Goto Near Page - San Francisco

by this we are able to achieve optional param and able to load component and URL generated by routerLink is below which is expected
http://localhost:3000/near//37/122/San-Francisco/All

but when we refresh or reload page it's not working and redirecting to http://localhost:3000/

-Two forward slash after near "//" is required and url should remain same.

-Expected fix is if we pass nearSearch params as a null component should be loaded and and blank forward slash '//' should be persist in URL'

🔥 Exception or Error

Redirecting back to home page

🌍 Your Environment

Angular Version:
Angular 5

@ngbot ngbot bot added this to the needsTriage milestone Sep 25, 2019
@gowthambj
Copy link
Author

please can it be prioritized?

@gowthambj
Copy link
Author

Similar issue - #3957 (comment)

But was closed without fixing. High priority issues and needs fix.

@ajarudingunga
Copy link

++1
Exact issue for me, have asked questions on stack-overflow but no luck yet.

https://stackoverflow.com/questions/58059143/how-to-pass-optional-router-parameter-with-preserved-url-in-angular5

@FDIM
Copy link
Contributor

FDIM commented Oct 3, 2019

Have either of you tried to use matcher option?

For cases at work, usually simple redirect with trailing slash was good enough. For few other cases custom matcher was the solution, it's a bit verbose but not too complicated.

Just make sure that you export the function! (Needed for AOT)

EDIT: here is an example to use as a matcher option

export function SearchPageMatcher(segments: UrlSegment[]):  UrlMatchResult | null {
  if (segments.length && segments[0].path === 'search' ) {
    return {
      consumed: segments,
      posParams: {
        ...segments.length > 1 ? { optionalParamName: segments[1] } : {},
        ...segments.length > 2 ? { optionalParam2Name: segments[2] } : {}
      }
    };
  }
  return null;
}

Just an FYI, you'll need to cast this function to UrlMatcher because of #29824

@ajarudingunga
Copy link

ajarudingunga commented Oct 9, 2019

@FDIM Thanks for response
I have tried with URL-rewrite which is replacing double slashes to single slash and defining multiple routes one with single and one with double so its working.

const __stripTrailingSlash = (Location as any).stripTrailingSlash;
(Location as any).stripTrailingSlash = function _stripTrailingSlash(url: string): string {
    const queryString$ = url.match(/([^?]*)?(.*)/);
    if (queryString$[2].length > 0) {
        return /[^\/]\/$/.test(queryString$[1]) ? queryString$[1] + '.' + queryString$[2] : __stripTrailingSlash(url);
    }
    if (url.indexOf('//') > -1) {
        url = url.replace('//', '/');
        return url;
    }
    return /[^\/]\/$/.test(url) ? url + '.' : __stripTrailingSlash(url);
};

But I not able to understand how to use URLmatcher. how and where to write URLmatcher code so it can prevent route breaking and consider as blank param ?

Clicking on routerLink element its working as expected, but on refresh it's considering till http://localhost:3000/near/ only, where actual url is http://localhost:3000/near//37/122/San-Francisco/All. This I have debugged with { enableTracing: true } in routes.

@FDIM
Copy link
Contributor

FDIM commented Oct 9, 2019

@ajarudingunga matcher is basically a replacement for path option in route definition where you get total control.

In regards to double slash, I'd try matcher approach first and see if it just works before replacing built on methods.

@ajarudingunga
Copy link

@FDIM I have tried below code, but same issue for double slash. if I pass some value of optional param then its working.

export function SearchPageMatcher(segments: UrlSegment[]): UrlMatchResult | null {
    if (segments.length && segments[0].path === 'near') {
    return {
      consumed: segments,
      posParams: {
        ...segments.length > 1 ? { nearSearch: segments[1] } : {},
        ...segments.length > 2 ? { val1: segments[2] } : {},
        ...segments.length > 3 ? { val2: segments[3] } : {},
        ...segments.length > 4 ? { name: segments[4] } : {},
        ...segments.length > 5 ? { filter: segments[5] } : {}
      }
    };
  }
  return null;
}

const routes: Routes = [
  { path: '', component: SearchpagesComponent ,
    children: [
      {
        matcher: SearchPageMatcher,
        loadChildren: './search/search.module#SearchModule',
      }
    ]
  }
];

@ajarudingunga
Copy link

@FDIM Here is stackbliz without URLmatcher if you want for some experiments !

@gowthambj
Copy link
Author

Any other solutions? Can we prioritize this?

@IgorMinar
Copy link
Contributor

I was able to reproduce this with 8.2.13.

Seems like a bug in the parsing or init code because if there is any value provided in between // (e..g /foo/), the route is properly matched.

@IgorMinar
Copy link
Contributor

I created a workaround for this issue using a custom UrlSerializer: https://stackblitz.com/edit/angular-routing-child-routes-vpn7lf?file=app/app.module.ts

The issue seems to be due to a bug in the url parsing logic at:

while (this.peekStartsWith('/') && !this.peekStartsWith('//') && !this.peekStartsWith('/(')) {

This code assumes that // always means auxiliary routes, which is not the case when // is present outside of parentheses in the url (()). The parsing logic needs to keep track of if we are in the aux parsing context and if we are not, we should allow // and parse it as segment with value set to ''.

@ajarudingunga
Copy link

ajarudingunga commented Nov 25, 2019

@IgorMinar Given stackblitz seems working for angulr 8. code written in app.modules.ts I have tried with angular 5.2.11 but not working with lazyload children routes, throwing error like module not found.

Error: Cannot find module './search/search.module'. Error: Cannot find module './search/search.module'. at webpackAsyncContext ($_lazy_route_resource lazy:89) at SystemJsNgModuleLoader.webpackJsonp../node_modules/@angular/core/esm5/core.js.SystemJsNgModuleLoader.loadAndCompile (core.js:6570)

@ajarudingunga
Copy link

ajarudingunga commented Nov 25, 2019

@IgorMinar Found scenarios where code working/not working.

If I run with ng serve your code working with angular5 as well.
If I run with ng build it's not wroking. Lazyload/children modules even not compiling and not generating bundle chunks of that modules. that's why getting error Error: Cannot find module './search/search.module'.

@ajarudingunga
Copy link

@IgorMinar

Anyhow, we managed in the frontend by editing prod dist files by removing && !this.peekStartsWith('//'). But the workaround is not working for angular/universal APP, It is getting broken.

@ajarudingunga
Copy link

The workaround is working for A8 and SSR Universal App.

@jelbourn jelbourn added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent and removed severity3: broken labels Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: router freq2: medium P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent router: URL parsing/generation state: confirmed type: bug/fix
Projects
None yet
Development

No branches or pull requests

7 participants