Change storefrontRedirect to ignore query parameters (#1900)

* Change `storefrontRedirect` to ignore query parameters

.changeset/hip-rocks-hammer.md

@@ -2,4 +2,4 @@
 '@shopify/hydrogen': patch
 ---
 
-Fix bug where `storefrontRedirect` would return an error on soft page navigations. Also change the redirect status code from 301 to 302.
+Fix bug where `storefrontRedirect` would return an error on soft page navigations.

.changeset/red-geese-work.md

@@ -0,0 +1,14 @@
+---
+'@shopify/hydrogen': patch
+---
+
+Change `storefrontRedirect` to ignore query parameters when matching redirects. For example, a redirect in the admin from `/snowboards` to `/collections/snowboards` will now match on the URL `/snowboards?utm_campaign=buffer` and redirect the user to `/collections/snowboards?utm_campaign=buffer`. This is a breaking change. If you want to retain the legacy functionality that is query parameter sensitive, pass `matchQueryParams` to `storefrontRedirect()`:
+
+```ts
+storefrontRedirect({
+  request,
+  response,
+  storefront,
+  matchQueryParams: true,
+});
+```

packages/hydrogen/src/routing/redirect.test.ts

@@ -18,7 +18,7 @@ describe('storefrontRedirect', () => {
       }),
     ).resolves.toEqual(
       new Response(null, {
-        status: 302,
+        status: 301,
         headers: {location: shopifyDomain + '/admin'},
       }),
     );
@@ -36,7 +36,7 @@ describe('storefrontRedirect', () => {
       }),
     ).resolves.toEqual(
       new Response(null, {
-        status: 302,
+        status: 301,
         headers: {location: shopifyDomain + '/some-page'},
       }),
     );
@@ -63,7 +63,7 @@ describe('storefrontRedirect', () => {
         status: 200,
         headers: {
           'X-Remix-Redirect': shopifyDomain + '/some-page',
-          'X-Remix-Status': '302',
+          'X-Remix-Status': '301',
         },
       }),
     );
@@ -73,7 +73,7 @@ describe('storefrontRedirect', () => {
     });
   });
 
-  it('retains custom query parameters on soft navigations', async () => {
+  it('matches query parameters', async () => {
     queryMock.mockResolvedValueOnce({
       urlRedirects: {edges: [{node: {target: shopifyDomain + '/some-page'}}]},
     });
@@ -84,13 +84,14 @@ describe('storefrontRedirect', () => {
         request: new Request(
           'https://domain.com/some-page?test=true&_data=%2Fcollections%2Fbackcountry',
         ),
+        matchQueryParams: true,
       }),
     ).resolves.toEqual(
       new Response(null, {
         status: 200,
         headers: {
           'X-Remix-Redirect': shopifyDomain + '/some-page',
-          'X-Remix-Status': '302',
+          'X-Remix-Status': '301',
         },
       }),
     );
@@ -100,6 +101,69 @@ describe('storefrontRedirect', () => {
     });
   });
 
+  it('propogates query parameters to the final redirect', async () => {
+    queryMock.mockResolvedValueOnce({
+      urlRedirects: {
+        edges: [
+          {node: {target: shopifyDomain + '/some-redirect?redirectParam=true'}},
+        ],
+      },
+    });
+
+    await expect(
+      storefrontRedirect({
+        storefront: storefrontMock,
+        request: new Request(
+          'https://domain.com/some-page?requestParam=true&_data=%2Fcollections%2Fbackcountry',
+        ),
+      }),
+    ).resolves.toEqual(
+      new Response(null, {
+        status: 200,
+        headers: {
+          'X-Remix-Redirect':
+            shopifyDomain +
+            '/some-redirect?redirectParam=true&requestParam=true',
+          'X-Remix-Status': '301',
+        },
+      }),
+    );
+
+    expect(queryMock).toHaveBeenCalledWith(expect.anything(), {
+      variables: {query: 'path:/some-page'},
+    });
+  });
+
+  it('propogates query parameters to the final redirect for relative URLs', async () => {
+    queryMock.mockResolvedValueOnce({
+      urlRedirects: {
+        edges: [{node: {target: '/some-redirect?redirectParam=true'}}],
+      },
+    });
+
+    await expect(
+      storefrontRedirect({
+        storefront: storefrontMock,
+        request: new Request(
+          'https://domain.com/some-page?requestParam=true&_data=%2Fcollections%2Fbackcountry',
+        ),
+      }),
+    ).resolves.toEqual(
+      new Response(null, {
+        status: 200,
+        headers: {
+          'X-Remix-Redirect':
+            '/some-redirect?redirectParam=true&requestParam=true',
+          'X-Remix-Status': '301',
+        },
+      }),
+    );
+
+    expect(queryMock).toHaveBeenCalledWith(expect.anything(), {
+      variables: {query: 'path:/some-page'},
+    });
+  });
+
   it('queries the SFAPI and returns 404 on mismatch', async () => {
     queryMock.mockResolvedValueOnce({
       urlRedirects: {edges: []},
@@ -151,7 +215,7 @@ describe('storefrontRedirect', () => {
           ),
         }),
       ).resolves.toEqual(
-        new Response(null, {status: 302, headers: {location: '/some-page'}}),
+        new Response(null, {status: 301, headers: {location: '/some-page'}}),
       );
     });
 
@@ -168,7 +232,7 @@ describe('storefrontRedirect', () => {
           ),
         }),
       ).resolves.toEqual(
-        new Response(null, {status: 302, headers: {location: '/some-page'}}),
+        new Response(null, {status: 301, headers: {location: '/some-page'}}),
       );
     });
 
@@ -216,7 +280,7 @@ describe('storefrontRedirect', () => {
         }),
       ).resolves.toEqual(
         new Response(null, {
-          status: 302,
+          status: 301,
           headers: {location: '/some-page?param=https://another.com'},
         }),
       );

packages/hydrogen/src/routing/redirect.ts

@@ -11,6 +11,8 @@ type StorefrontRedirect = {
   response?: Response;
   /** By default the `/admin` route is redirected to the Shopify Admin page for the current storefront. Disable this redirect by passing `true`. */
   noAdminRedirect?: boolean;
+  /** By default, query parameters are not used to match redirects. Set this to `true` if you'd like redirects to be query parameter sensitive */
+  matchQueryParams?: boolean;
 };
 
 /**
@@ -28,20 +30,28 @@ export async function storefrontRedirect(
     storefront,
     request,
     noAdminRedirect,
+    matchQueryParams,
     response = new Response('Not Found', {status: 404}),
   } = options;
 
   const url = new URL(request.url);
-  const isSoftNavigation = url.searchParams.has('_data');
+  const {pathname, searchParams} = url;
+  const isSoftNavigation = searchParams.has('_data');
 
-  url.searchParams.delete('_data');
+  searchParams.delete('redirect');
+  searchParams.delete('return_to');
+  searchParams.delete('_data');
 
-  const redirectFrom = url.toString().replace(url.origin, '');
+  const redirectFrom = matchQueryParams
+    ? url.toString().replace(url.origin, '')
+    : pathname;
 
   if (url.pathname === '/admin' && !noAdminRedirect) {
     return createRedirectResponse(
       `${storefront.getShopifyDomain()}/admin`,
       isSoftNavigation,
+      searchParams,
+      matchQueryParams,
     );
   }
 
@@ -55,13 +65,23 @@ export async function storefrontRedirect(
     const location = urlRedirects?.edges?.[0]?.node?.target;
 
     if (location) {
-      return createRedirectResponse(location, isSoftNavigation);
+      return createRedirectResponse(
+        location,
+        isSoftNavigation,
+        searchParams,
+        matchQueryParams,
+      );
     }
 
     const redirectTo = getRedirectUrl(request.url);
 
     if (redirectTo) {
-      return createRedirectResponse(redirectTo, isSoftNavigation);
+      return createRedirectResponse(
+        redirectTo,
+        isSoftNavigation,
+        searchParams,
+        matchQueryParams,
+      );
     }
   } catch (error) {
     console.error(
@@ -73,16 +93,36 @@ export async function storefrontRedirect(
   return response;
 }
 
-function createRedirectResponse(location: string, isSoftNavigation: boolean) {
+const TEMP_DOMAIN = 'https://example.com';
+
+function createRedirectResponse(
+  location: string,
+  isSoftNavigation: boolean,
+  searchParams: URLSearchParams,
+  matchQueryParams?: boolean,
+) {
+  const url = new URL(location, TEMP_DOMAIN);
+
+  if (!matchQueryParams) {
+    for (const [key, value] of searchParams) {
+      // The redirect destination might include query params, so merge the
+      // original query params with the redirect destination query params
+      url.searchParams.append(key, value);
+    }
+  }
+
   if (isSoftNavigation) {
     return new Response(null, {
       status: 200,
-      headers: {'X-Remix-Redirect': location, 'X-Remix-Status': '302'},
+      headers: {
+        'X-Remix-Redirect': url.toString().replace(TEMP_DOMAIN, ''),
+        'X-Remix-Status': '301',
+      },
     });
   } else {
     return new Response(null, {
-      status: 302,
-      headers: {location: location},
+      status: 301,
+      headers: {location: url.toString().replace(TEMP_DOMAIN, '')},
     });
   }
 }