[now-routing-utils] Fix segments in query string (#3640)

This PR a regression when path segments are used in the query string.

Take a look at the following ASCII Table for why I had to delete certain parts of the parsed url before formatting again.

https://nodejs.org/api/url.html#url_url_strings_and_url_objects

Related to #3539

packages/now-routing-utils/src/superstatic.ts

@@ -130,32 +130,50 @@ export function sourceToRegex(
 
 function replaceSegments(segments: string[], destination: string): string {
   const parsedDestination = parseUrl(destination, true);
-  let { pathname, hash } = parsedDestination;
+  delete parsedDestination.href;
+  delete parsedDestination.path;
+  delete parsedDestination.search;
+  // eslint-disable-next-line prefer-const
+  let { pathname, hash, query, ...rest } = parsedDestination;
   pathname = pathname || '';
   hash = hash || '';
-
-  if ((pathname + hash).includes(':') && segments.length > 0) {
-    const pathnameCompiler = compile(pathname);
-    const hashCompiler = compile(hash);
+  if (segments.length > 0) {
     const indexes: { [k: string]: string } = {};
-
     segments.forEach((name, index) => {
       indexes[name] = toSegmentDest(index);
     });
-    pathname = pathnameCompiler(indexes);
-    hash = hash ? `${hashCompiler(indexes)}` : null;
+
+    if (destination.includes(':')) {
+      const pathnameCompiler = compile(pathname);
+      const hashCompiler = compile(hash);
+      pathname = pathnameCompiler(indexes);
+      hash = hash ? `${hashCompiler(indexes)}` : null;
+
+      for (const [key, strOrArray] of Object.entries(query)) {
+        let value = Array.isArray(strOrArray) ? strOrArray[0] : strOrArray;
+        if (value) {
+          const queryCompiler = compile(value);
+          value = queryCompiler(indexes);
+        }
+        query[key] = value;
+      }
+    } else {
+      for (const [name, value] of Object.entries(indexes)) {
+        query[name] = value;
+      }
+    }
+
     destination = formatUrl({
-      ...parsedDestination,
+      ...rest,
       pathname,
+      query,
       hash,
     });
-  } else if (segments.length > 0) {
-    let prefix = '?';
-    segments.forEach((name, index) => {
-      destination += `${prefix}${name}=${toSegmentDest(index)}`;
-      prefix = '&';
-    });
+
+    // url.format() escapes the query string but we must preserve dollar signs
+    destination = destination.replace(/=%24/g, '=$');
   }
+
   return destination;
 }
 

packages/now-routing-utils/test/superstatic.spec.js

@@ -301,6 +301,23 @@ test('convertRewrites', () => {
     { source: '/some/old/path', destination: '/some/new/path' },
     { source: '/firebase/(.*)', destination: 'https://www.firebase.com' },
     { source: '/projects/:id/edit', destination: '/projects.html' },
+    {
+      source: '/users/:id',
+      destination: '/api/user?identifier=:id&version=v2',
+    },
+    {
+      source: '/:file/:id',
+      destination: '/:file/get?identifier=:id',
+    },
+    {
+      source: '/qs-and-hash/:id/:hash',
+      destination: '/api/get?identifier=:id#:hash',
+    },
+    {
+      source: '/fullurl',
+      destination:
+        'https://user:pass@sub.example.com:8080/path/goes/here?v=1&id=2#hash',
+    },
     { source: '/catchall/:hello*/', destination: '/catchall/:hello*' },
     {
       source: '/another-catch/:hello+/',
@@ -329,6 +346,27 @@ test('convertRewrites', () => {
       dest: '/projects.html?id=$1',
       check: true,
     },
+    {
+      src: '^\\/users(?:\\/([^\\/#\\?]+?))$',
+      dest: '/api/user?identifier=$1&version=v2',
+      check: true,
+    },
+    {
+      src: '^(?:\\/([^\\/#\\?]+?))(?:\\/([^\\/#\\?]+?))$',
+      dest: '/$1/get?identifier=$2',
+      check: true,
+    },
+    {
+      src: '^\\/qs-and-hash(?:\\/([^\\/#\\?]+?))(?:\\/([^\\/#\\?]+?))$',
+      dest: '/api/get?identifier=$1#$2',
+      check: true,
+    },
+    {
+      src: '^\\/fullurl$',
+      dest:
+        'https://user:pass@sub.example.com:8080/path/goes/here?v=1&id=2#hash',
+      check: true,
+    },
     {
       src: '^\\/catchall(?:\\/((?:[^\\/#\\?]+?)(?:\\/(?:[^\\/#\\?]+?))*))?\\/$',
       dest: '/catchall/$1',
@@ -363,6 +401,10 @@ test('convertRewrites', () => {
     ['/some/old/path'],
     ['/firebase/one', '/firebase/two'],
     ['/projects/one/edit', '/projects/two/edit'],
+    ['/users/four', '/users/five'],
+    ['/file1/yep', '/file2/nope'],
+    ['/qs-and-hash/test/first', '/qs-and-hash/test/second'],
+    ['/fullurl'],
     ['/catchall/first/', '/catchall/first/second/'],
     ['/another-catch/first/', '/another-catch/first/second/'],
     ['/firebase/admin', '/firebase/anotherAdmin'],
@@ -374,6 +416,10 @@ test('convertRewrites', () => {
     ['/nope'],
     ['/fire', '/firebasejumper/two'],
     ['/projects/edit', '/projects/two/delete', '/projects'],
+    ['/users/edit/four', '/users/five/delete', '/users'],
+    ['/'],
+    ['/qs-and-hash', '/qs-and-hash/onlyone'],
+    ['/full'],
     ['/random-catch/'],
     ['/another-catch/'],
     ['/firebase/user/1', '/firebase/another/1'],