aurelia / router

A powerful client-side router.
MIT License
121 stars 115 forks source link

Configured route redirect does not propagate query string #667

Open rmja opened 3 years ago

rmja commented 3 years ago

I'm submitting a bug report

Please tell us about your environment:

Current behavior: The following route configuration does not propagate the query string to the redirected route:

const routes: RouteConfig[] = [
    { route: "does-not-work", redirect: "/dashboard?autoselect=true"; },
    { route: "does-work", navigationStrategy: instruction => { instruction.config.redirect = "/dashboard"; instruction.queryString = "autoselect=true"; } },
    { route: "dashboard", name: "dashboard", moduleId: PLATFORM.moduleName("./dashboard")  }
];

Expected/desired behavior:

https://github.com/aurelia/router/blob/9ba6ef338e4e4f5e21758b245d7ebf82fb364982/src/navigation-plan.ts#L81-L84

I believe the behavior should be more like instruction.queryString || redirectInstruction.queryString

rmja commented 2 years ago

Is there an update on this? The fix seems really easy.

The error is shown specifically when using aurelia-open-id-connect, where the redirect route is assigned here: https://github.com/aurelia-contrib/aurelia-open-id-connect/blob/master/src/open-id-connect-navigation-strategies.ts#L86. For example:

instruction.config.redirect = "/dashboard?some-parameter=123";

The redirect route may contain query parameters, which are then later stripped by the router during the redirect because of this issue.

To make this work, we either need an update to the router here so that it retains the query parameters (preferred), or an update to aurelia-open-id-connect so that it assigns the query parameters to the instruction in addition to the redirect - this seems wrong.

rmja commented 2 years ago

It seems that there is a PR for a fix here https://github.com/aurelia/router/pull/643 cc @bigopon

rmja commented 2 years ago

Related to https://github.com/aurelia/router/issues/639