[routing-utils] Allow passing internal params to convertRewrites (ver…

…cel#6742) This adds an argument to allow passing internal param names that should be ignored when considering whether params should be auto-added to a rewrite's destination query. After adding this we should be able to resolve vercel/next.js#27563 in the runtime where `convertRewrites` is called. This matches Next.js' handling for internal params which can be seen [here](https://github.com/vercel/next.js/blob/e90825ad88c6f8191bed1dcc43e03465afba32ed/packages/next/shared/lib/router/utils/prepare-destination.ts#L203) ### Related Issues x-ref: vercel/next.js#27563 ### 📋 Checklist  #### Tests - [x] The code changed/added as part of this PR has been covered with tests - [x] All tests pass locally with `yarn test-unit` #### Code Review - [ ] This PR has a concise title and thorough description useful to a reviewer - [ ] Issue from task tracker has a link to this PR
honuuk · Sep 21, 2021 · 2fd3fc7 · 2fd3fc7
1 parent de0b13a
commit 2fd3fc7
Show file tree

Hide file tree

Showing 2 changed files with 143 additions and 113 deletions.
diff --git a/packages/routing-utils/src/superstatic.ts b/packages/routing-utils/src/superstatic.ts
@@ -77,12 +77,21 @@ export function convertRedirects(
   });
 }
 
-export function convertRewrites(rewrites: Rewrite[]): Route[] {
+export function convertRewrites(
+  rewrites: Rewrite[],
+  internalParamNames?: string[]
+): Route[] {
   return rewrites.map(r => {
     const { src, segments } = sourceToRegex(r.source);
     const hasSegments = collectHasSegments(r.has);
     try {
-      const dest = replaceSegments(segments, hasSegments, r.destination);
+      const dest = replaceSegments(
+        segments,
+        hasSegments,
+        r.destination,
+        false,
+        internalParamNames
+      );
       const route: Route = { src, dest, check: true };
 
       if (r.has) {
@@ -220,7 +229,8 @@ function replaceSegments(
   segments: string[],
   hasItemSegments: string[],
   destination: string,
-  isRedirect?: boolean
+  isRedirect?: boolean,
+  internalParamNames?: string[]
 ): string {
   const namedSegments = segments.filter(name => name !== UN_NAMED_SEGMENT);
   const canNeedReplacing =
@@ -296,7 +306,13 @@ function replaceSegments(
   // or more params aren't already used in the destination's path
   const paramKeys = Object.keys(indexes);
   const needsQueryUpdating =
-    !isRedirect && !paramKeys.some(param => destParams.has(param));
+    // we do not consider an internal param since it is added automatically
+    !isRedirect &&
+    !paramKeys.some(
+      param =>
+        !(internalParamNames && internalParamNames.includes(param)) &&
+        destParams.has(param)
+    );
 
   if (needsQueryUpdating) {
     for (const param of paramKeys) {

diff --git a/packages/routing-utils/test/superstatic.spec.js b/packages/routing-utils/test/superstatic.spec.js
@@ -471,115 +471,122 @@ test('convertRedirects', () => {
 });
 
 test('convertRewrites', () => {
-  const actual = convertRewrites([
-    { source: '/some/old/path', destination: '/some/new/path' },
-    { source: '/proxy/(.*)', destination: 'https://www.firebase.com' },
-    { source: '/proxy/(.*)', destination: 'https://www.firebase.com/' },
-    {
-      source: '/proxy-regex/([a-zA-Z]{1,})',
-      destination: 'https://firebase.com/$1',
-    },
-    {
-      source: '/proxy-port/([a-zA-Z]{1,})',
-      destination: 'https://firebase.com:8080/$1',
-    },
-    { 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: '/dont-override-qs/:name/:age',
-      destination: '/final?name=bob&age=',
-    },
-    { source: '/catchall/:hello*/', destination: '/catchall/:hello*' },
-    {
-      source: '/another-catch/:hello+/',
-      destination: '/another-catch/:hello+',
-    },
-    { source: '/catchme/:id*', destination: '/api/user' },
-    { source: '/:path', destination: '/test?path=:path' },
-    { source: '/:path/:two', destination: '/test?path=:path' },
-    { source: '/(.*)-:id(\\d+).html', destination: '/blog/:id' },
-    {
-      source: '/feature-{:slug}',
-      destination: '/blog-{:slug}',
-    },
-    {
-      source: '/hello/:world',
-      destination: '/somewhere?else={:world}',
-    },
-    {
-      source: '/hello/:first',
-      destination: '/another/:a/:b',
-      has: [
-        {
-          type: 'host',
-          value: '(?<a>.*)\\.(?<b>.*)',
-        },
-      ],
-    },
-    {
-      source: '/hello/:first',
-      destination:
-        '/another/:first/:username/:pathname/:another/:host/:a/:b/:c/:d',
-      has: [
-        {
-          type: 'header',
-          key: 'x-rewrite',
-        },
-        {
-          type: 'cookie',
-          key: 'loggedIn',
-          value: '1',
-        },
-        {
-          type: 'host',
-          value: 'vercel.com',
-        },
-        {
-          type: 'host',
-          value: '(?<a>.*)\\.(?<b>.*)',
-        },
-        {
-          type: 'header',
-          key: 'host',
-          value: '(?<c>.*)\\.(?<d>.*)',
-        },
-        {
-          type: 'query',
-          key: 'username',
-        },
-        {
-          type: 'header',
-          key: 'x-pathname',
-          value: '(?<pathname>.*)',
-        },
-        {
-          type: 'header',
-          key: 'X-Pathname',
-          value: '(?<another>hello|world)',
-        },
-      ],
-    },
-    {
-      source: '/array-query-string/:id/:name',
-      destination: 'https://example.com/?tag=1&tag=2',
-    },
-  ]);
+  const actual = convertRewrites(
+    [
+      { source: '/some/old/path', destination: '/some/new/path' },
+      { source: '/proxy/(.*)', destination: 'https://www.firebase.com' },
+      { source: '/proxy/(.*)', destination: 'https://www.firebase.com/' },
+      {
+        source: '/proxy-regex/([a-zA-Z]{1,})',
+        destination: 'https://firebase.com/$1',
+      },
+      {
+        source: '/proxy-port/([a-zA-Z]{1,})',
+        destination: 'https://firebase.com:8080/$1',
+      },
+      { 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: '/dont-override-qs/:name/:age',
+        destination: '/final?name=bob&age=',
+      },
+      { source: '/catchall/:hello*/', destination: '/catchall/:hello*' },
+      {
+        source: '/another-catch/:hello+/',
+        destination: '/another-catch/:hello+',
+      },
+      { source: '/catchme/:id*', destination: '/api/user' },
+      { source: '/:path', destination: '/test?path=:path' },
+      { source: '/:path/:two', destination: '/test?path=:path' },
+      { source: '/(.*)-:id(\\d+).html', destination: '/blog/:id' },
+      {
+        source: '/feature-{:slug}',
+        destination: '/blog-{:slug}',
+      },
+      {
+        source: '/hello/:world',
+        destination: '/somewhere?else={:world}',
+      },
+      {
+        source: '/hello/:first',
+        destination: '/another/:a/:b',
+        has: [
+          {
+            type: 'host',
+            value: '(?<a>.*)\\.(?<b>.*)',
+          },
+        ],
+      },
+      {
+        source: '/hello/:first',
+        destination:
+          '/another/:first/:username/:pathname/:another/:host/:a/:b/:c/:d',
+        has: [
+          {
+            type: 'header',
+            key: 'x-rewrite',
+          },
+          {
+            type: 'cookie',
+            key: 'loggedIn',
+            value: '1',
+          },
+          {
+            type: 'host',
+            value: 'vercel.com',
+          },
+          {
+            type: 'host',
+            value: '(?<a>.*)\\.(?<b>.*)',
+          },
+          {
+            type: 'header',
+            key: 'host',
+            value: '(?<c>.*)\\.(?<d>.*)',
+          },
+          {
+            type: 'query',
+            key: 'username',
+          },
+          {
+            type: 'header',
+            key: 'x-pathname',
+            value: '(?<pathname>.*)',
+          },
+          {
+            type: 'header',
+            key: 'X-Pathname',
+            value: '(?<another>hello|world)',
+          },
+        ],
+      },
+      {
+        source: '/array-query-string/:id/:name',
+        destination: 'https://example.com/?tag=1&tag=2',
+      },
+      {
+        source: '/:nextInternalLocale/:path',
+        destination: '/api/hello',
+      },
+    ],
+    ['nextInternalLocale']
+  );
 
   const expected = [
     { src: '^\\/some\\/old\\/path$', dest: '/some/new/path', check: true },
@@ -728,6 +735,11 @@ test('convertRewrites', () => {
       dest: 'https://example.com/?tag=1&tag=2&id=$1&name=$2',
       check: true,
     },
+    {
+      check: true,
+      dest: '/api/hello?nextInternalLocale=$1&path=$2',
+      src: '^(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$',
+    },
   ];
 
   deepEqual(actual, expected);
@@ -755,6 +767,7 @@ test('convertRewrites', () => {
     ['/hello/world'],
     ['/hello/world'],
     ['/array-query-string/10/email'],
+    ['/en/hello'],
   ];
 
   const mustNotMatch = [
@@ -780,6 +793,7 @@ test('convertRewrites', () => {
     ['/hllooo'],
     ['/hllooo'],
     ['/array-query-string/10'],
+    ['/en/hello/world', '/en/hello/'],
   ];
 
   assertRegexMatches(actual, mustMatch, mustNotMatch);