Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Correct client rewrite resolving with query #16860

Merged
merged 2 commits into from
Sep 4, 2020

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Sep 4, 2020

This makes sure we only pass the as value's pathname instead of the full value so that we don't accidentally include query values while resolving the rewrites. This also adds tests to ensure the rewrites are resolved with the correct query values when only providing href and when manually mapping them with href and as

Fixes: #16825

@ijjk
Copy link
Member Author

ijjk commented Sep 4, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
buildDuration 12.2s 12.6s ⚠️ +412ms
nodeModulesSize 56.7 MB 56.7 MB ⚠️ +97 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
/ failed reqs 0 0
/ total time (seconds) 2.175 2.216 ⚠️ +0.04
/ avg req/sec 1149.67 1128.24 ⚠️ -21.43
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.223 1.24 ⚠️ +0.02
/error-in-render avg req/sec 2043.78 2016.7 ⚠️ -27.08
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB
framework.HASH.js gzip 39 kB 39 kB
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 57.3 kB 57.3 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 52.3 kB 52.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.71 kB 7.71 kB
Client Pages Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
index.html gzip 968 B 969 B ⚠️ +1 B
link.html gzip 975 B 977 B ⚠️ +2 B
withRouter.html gzip 962 B 962 B
Overall change 2.9 kB 2.91 kB ⚠️ +3 B

Diffs

Diff for index.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for link.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -123,13 +123,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for withRouter.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
buildDuration 13.8s 13.9s ⚠️ +43ms
nodeModulesSize 56.7 MB 56.7 MB ⚠️ +97 B
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..b5de.js gzip 10.5 kB N/A N/A
framework.HASH.js gzip 39 kB 39 kB
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB
webpack-e067..f178.js gzip 751 B 751 B
677f882d2ed8..cf8c.js gzip N/A 10.5 kB N/A
Overall change 57.3 kB 57.3 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..dule.js gzip 6.4 kB N/A N/A
framework.HA..dule.js gzip 39 kB 39 kB
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB
webpack-07c5..dule.js gzip 751 B 751 B
677f882d2ed8..dule.js gzip N/A 6.4 kB N/A
Overall change 52.3 kB 52.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.71 kB 7.71 kB
Client Pages Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Serverless bundles
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_error.js 1.03 MB 1.03 MB
404.html 4.22 kB 4.22 kB
hooks.html 3.86 kB 3.86 kB
index.js 1.03 MB 1.03 MB
link.js 1.08 MB 1.08 MB
routerDirect.js 1.07 MB 1.07 MB
withRouter.js 1.07 MB 1.07 MB
Overall change 5.29 MB 5.29 MB
Commit: 2d2f34a

@ijjk
Copy link
Member Author

ijjk commented Sep 4, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
buildDuration 12.4s 12.7s ⚠️ +354ms
nodeModulesSize 56.7 MB 56.7 MB ⚠️ +97 B
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
/ failed reqs 0 0
/ total time (seconds) 2.261 2.222 -0.04
/ avg req/sec 1105.75 1125 +19.25
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.22 1.177 -0.04
/error-in-render avg req/sec 2048.96 2124.54 +75.58
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB
framework.HASH.js gzip 39 kB 39 kB
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB
webpack-e067..f178.js gzip 751 B 751 B
Overall change 57.3 kB 57.3 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB
framework.HA..dule.js gzip 39 kB 39 kB
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB
webpack-07c5..dule.js gzip 751 B 751 B
Overall change 52.3 kB 52.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.71 kB 7.71 kB
Client Pages Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
index.html gzip 968 B 969 B ⚠️ +1 B
link.html gzip 975 B 977 B ⚠️ +2 B
withRouter.html gzip 962 B 962 B
Overall change 2.9 kB 2.91 kB ⚠️ +3 B

Diffs

Diff for index.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for link.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -123,13 +123,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for withRouter.html
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.695ae9abdaa5c90f38d1.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.ecb1099d47a74ae628f6.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.50e586e3504862d725d1.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.a69c2af1b9467350a845.module.js"
       async=""
       crossorigin="anonymous"
       type="module"

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
buildDuration 14.2s 13.8s -378ms
nodeModulesSize 56.7 MB 56.7 MB ⚠️ +97 B
Client Bundles (main, webpack, commons)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..b5de.js gzip 10.5 kB N/A N/A
framework.HASH.js gzip 39 kB 39 kB
main-195c642..b6b7.js gzip 7.08 kB 7.08 kB
webpack-e067..f178.js gzip 751 B 751 B
677f882d2ed8..cf8c.js gzip N/A 10.5 kB N/A
Overall change 57.3 kB 57.3 kB
Client Bundles (main, webpack, commons) Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
677f882d2ed8..dule.js gzip 6.4 kB N/A N/A
framework.HA..dule.js gzip 39 kB 39 kB
main-ad445e5..dule.js gzip 6.14 kB 6.14 kB
webpack-07c5..dule.js gzip 751 B 751 B
677f882d2ed8..dule.js gzip N/A 6.4 kB N/A
Overall change 52.3 kB 52.3 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
polyfills-4b..e242.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-9a0b9e1..b37e.js gzip 1.28 kB 1.28 kB
_error-ed1b0..8fbd.js gzip 3.44 kB 3.44 kB
hooks-89731c..c609.js gzip 887 B 887 B
index-17468f..5d83.js gzip 227 B 227 B
link-d2344ce..8b36.js gzip 1.3 kB 1.3 kB
routerDirect..924c.js gzip 284 B 284 B
withRouter-7..c13d.js gzip 284 B 284 B
Overall change 7.71 kB 7.71 kB
Client Pages Modern
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_app-75d3a82..dule.js gzip 625 B 625 B
_error-4469a..dule.js gzip 2.29 kB 2.29 kB
hooks-cbf13f..dule.js gzip 387 B 387 B
index-b9a643..dule.js gzip 226 B 226 B
link-f8c0daf..dule.js gzip 1.26 kB 1.26 kB
routerDirect..dule.js gzip 284 B 284 B
withRouter-f..dule.js gzip 282 B 282 B
Overall change 5.36 kB 5.36 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_buildManifest.js gzip 322 B 322 B
_buildManife..dule.js gzip 329 B 329 B
Overall change 651 B 651 B
Serverless bundles
vercel/next.js canary ijjk/next.js fix/client-rewrite-resolving Change
_error.js 1.03 MB 1.03 MB
404.html 4.22 kB 4.22 kB
hooks.html 3.86 kB 3.86 kB
index.js 1.03 MB 1.03 MB
link.js 1.08 MB 1.08 MB
routerDirect.js 1.07 MB 1.07 MB
withRouter.js 1.07 MB 1.07 MB
Overall change 5.29 MB 5.29 MB
Commit: ef94479

@kodiakhq kodiakhq bot merged commit 6233ef7 into vercel:canary Sep 4, 2020
@Timer Timer deleted the fix/client-rewrite-resolving branch September 4, 2020 19:20
HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this pull request Sep 26, 2020
This makes sure we only pass the as value's `pathname` instead of the full value so that we don't accidentally include `query` values while resolving the rewrites. This also adds tests to ensure the rewrites are resolved with the correct query values when only providing `href` and when manually mapping them with `href` and `as`

Fixes: vercel#16825
@vercel vercel locked as resolved and limited conversation to collaborators Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[9.5.3 - Rewrites] Navigating route with dynamic routes no longer work properly
2 participants