fix(router): redirects now account for query string (#23337)

resolves #23136

core/src/components/router/router.tsx

@@ -6,7 +6,7 @@ import { debounce } from '../../utils/helpers';
 import { ROUTER_INTENT_BACK, ROUTER_INTENT_FORWARD, ROUTER_INTENT_NONE } from './utils/constants';
 import { printRedirects, printRoutes } from './utils/debug';
 import { readNavState, waitUntilNavNode, writeNavState } from './utils/dom';
-import { routeRedirect, routerIDsToChain, routerPathToChain } from './utils/matching';
+import { findRouteRedirect, routerIDsToChain, routerPathToChain } from './utils/matching';
 import { readRedirects, readRoutes } from './utils/parser';
 import { chainToPath, generatePath, parsePath, readPath, writePath } from './utils/path';
 
@@ -188,7 +188,7 @@ export class Router implements ComponentInterface {
 
   private onRedirectChanged() {
     const path = this.getPath();
-    if (path && routeRedirect(path, readRedirects(this.el))) {
+    if (path && findRouteRedirect(path, readRedirects(this.el))) {
       this.writeNavStateRoot(path, ROUTER_INTENT_NONE);
     }
   }
@@ -226,13 +226,15 @@ export class Router implements ComponentInterface {
 
     // lookup redirect rule
     const redirects = readRedirects(this.el);
-    const redirect = routeRedirect(path, redirects);
+    const redirect = findRouteRedirect(path, redirects);
 
     let redirectFrom: string[] | null = null;
+
     if (redirect) {
-      this.setPath(redirect.to!, direction);
+      const { segments, queryString } = redirect.to!;
+      this.setPath(segments, direction, queryString);
       redirectFrom = redirect.from;
-      path = redirect.to!;
+      path = segments;
     }
 
     // lookup route chain

core/src/components/router/test/basic/index.html

@@ -70,7 +70,7 @@
           </ion-toolbar>
         </ion-header>
         <ion-content>
-          this is the first pahe
+          this is the first page
         </ion-content>`;
       }
     }
@@ -211,6 +211,7 @@
         <ion-route url="/four" component="tab-four"> </ion-route>
       </ion-route>
 
+      <ion-route-redirect from="/redirect-to-three" to="/three?has_query_string=true"></ion-route-redirect>
     </ion-router>
 
     <ion-nav></ion-nav>

core/src/components/router/test/basic/redirect.e2e.ts

@@ -0,0 +1,12 @@
+import { newE2EPage } from '@stencil/core/testing';
+
+test('redirect should support query string', async () => {
+  const page = await newE2EPage({
+    url: '/src/components/router/test/basic#/redirect-to-three?ionic:_testing=true'
+  });
+
+  await page.waitForChanges();
+
+  const url = await page.url();
+  expect(url).toContain('#/three?has_query_string=true');  
+});

core/src/components/router/test/parser.spec.tsx

@@ -47,20 +47,22 @@ describe('parser', () => {
       const r3 = mockRedirectElement(win, '*', null);
       const r4 = mockRedirectElement(win, '/workout/*', '');
       const r5 = mockRedirectElement(win, 'path/hey', '/path/to//login');
+      const r6 = mockRedirectElement(win, 'path/qs', '/path?qs=true');
 
       root.appendChild(r1);
       root.appendChild(r2);
       root.appendChild(r3);
       root.appendChild(r4);
       root.appendChild(r5);
+      root.appendChild(r6);
 
       const expected: RouteRedirect[] = [
         { from: [''], to: undefined },
-        { from: [''], to: ['workout'] },
+        { from: [''], to: { segments: ['workout'] }},
         { from: ['*'], to: undefined },
-        { from: ['workout', '*'], to: [''] },
-        { from: ['path', 'hey'], to: ['path', 'to', 'login'] }
-
+        { from: ['workout', '*'], to: { segments: [''] } },
+        { from: ['path', 'hey'], to: { segments: ['path', 'to', 'login'] } },
+        { from: ['path', 'qs'], to: { segments: ['path'], queryString: 'qs=true' } },
       ];
       expect(readRedirects(root)).toEqual(expected);
     });

core/src/components/router/utils/debug.ts

@@ -16,7 +16,7 @@ export const printRedirects = (redirects: RouteRedirect[]) => {
   console.group(`[ion-core] REDIRECTS[${redirects.length}]`);
   for (const redirect of redirects) {
     if (redirect.to) {
-      console.debug('FROM: ', `$c ${generatePath(redirect.from)}`, 'font-weight: bold', ' TO: ', `$c ${generatePath(redirect.to)}`, 'font-weight: bold');
+      console.debug('FROM: ', `$c ${generatePath(redirect.from)}`, 'font-weight: bold', ' TO: ', `$c ${generatePath(redirect.to.segments)}`, 'font-weight: bold');
     }
   }
   console.groupEnd();

core/src/components/router/utils/interface.ts

@@ -18,7 +18,7 @@ export interface RouterEventDetail {
 
 export interface RouteRedirect {
   from: string[];
-  to?: string[];
+  to?: ParsedRoute;
 }
 
 export interface RouteWrite {
@@ -45,6 +45,13 @@ export interface RouteNode extends RouteEntry {
   children: RouteTree;
 }
 
+export interface ParsedRoute {
+  // Parts of the route (non empty "/" separated parts of an URL).
+  segments: string[];
+  // Unparsed query string.
+  queryString?: string;
+}
+
 export type RouterDirection = 'forward' | 'back' | 'root';
 export type NavOutletElement = NavOutlet & HTMLStencilElement;
 export type RouteChain = RouteEntry[];

core/src/components/router/utils/matching.ts

@@ -1,12 +1,17 @@
 import { RouteChain, RouteID, RouteRedirect } from './interface';
 
-export const matchesRedirect = (input: string[], route: RouteRedirect): route is RouteRedirect => {
-  const { from, to } = route;
+// Returns whether the given redirect matches the given path segments.
+//
+// A redirect matches when the segments of the path and redirect.from are equal.
+// Note that segments are only checked until redirect.from contains a '*' which matches any path segment.
+// The path ['some', 'path', 'to', 'page'] matches both ['some', 'path', 'to', 'page'] and ['some', 'path', '*'].
+export const matchesRedirect = (path: string[], redirect: RouteRedirect): boolean => {
+  const { from, to } = redirect;
   if (to === undefined) {
     return false;
   }
 
-  if (from.length > input.length) {
+  if (from.length > path.length) {
     return false;
   }
 
@@ -15,15 +20,16 @@ export const matchesRedirect = (input: string[], route: RouteRedirect): route is
     if (expected === '*') {
       return true;
     }
-    if (expected !== input[i]) {
+    if (expected !== path[i]) {
       return false;
     }
   }
-  return from.length === input.length;
+  return from.length === path.length;
 };
 
-export const routeRedirect = (path: string[], routes: RouteRedirect[]) => {
-  return routes.find(route => matchesRedirect(path, route)) as RouteRedirect | undefined;
+// Returns the first redirect matching the path segments or undefined when no match found.
+export const findRouteRedirect = (path: string[], redirects: RouteRedirect[]) => {
+  return redirects.find(redirect => matchesRedirect(path, redirect));
 };
 
 export const matchesIDs = (ids: string[], chain: RouteChain): number => {

core/src/components/router/utils/parser.ts

@@ -8,7 +8,7 @@ export const readRedirects = (root: Element): RouteRedirect[] => {
       const to = readProp(el, 'to');
       return {
         from: parsePath(readProp(el, 'from')).segments,
-        to: to == null ? undefined : parsePath(to).segments,
+        to: to == null ? undefined : parsePath(to),
       };
     });
 };

core/src/components/router/utils/path.ts

@@ -1,6 +1,5 @@
-import { RouteChain, RouterDirection } from '../../../interface';
-
 import { ROUTER_INTENT_FORWARD } from './constants';
+import { ParsedRoute, RouteChain, RouterDirection } from './interface';
 
 export const generatePath = (segments: string[]): string => {
   const path = segments
@@ -81,7 +80,7 @@ export const readPath = (loc: Location, root: string, useHash: boolean): string[
 // Parses the path to:
 // - segments an array of '/' separated parts,
 // - queryString (undefined when no query string).
-export const parsePath = (path: string | undefined | null): {segments: string[], queryString?: string} => {
+export const parsePath = (path: string | undefined | null): ParsedRoute => {
   let segments = [''];
   let queryString;