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.

Sign up for GitHub

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

Jump to bottom

Add tests for searchParams dev warning #16798

Merged
merged 4 commits into from
Sep 3, 2020
Merged

Add tests for searchParams dev warning #16798

merged 4 commits into from
Sep 3, 2020

Conversation

ztanner
Copy link
Member

@ztanner ztanner commented Sep 2, 2020

Fixes a warning being logged to the console when using href with a dynamic path. Fixes #16794

@ijjk ijjk added the type: next label Sep 2, 2020
@ijjk
Copy link
Member

ijjk commented Sep 2, 2020

Stats from current PR

Default Server Mode
General Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 13.7s 14.3s ⚠️ +590ms
nodeModulesSize 55.9 MB 55.9 MB ⚠️ +47 B
Page Load Tests Overall increase ✓
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.537 2.59 ⚠️ +0.05
/ avg req/sec 985.35 965.41 ⚠️ -19.94
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.576 1.556 -0.02
/error-in-render avg req/sec 1586.29 1606.69 +20.4
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB 10.5 kB ⚠️ +4 B
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ⚠️ +4 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB 6.4 kB ⚠️ +4 B
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ⚠️ +4 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
index.html gzip 972 B 971 B -1 B
link.html gzip 978 B 978 B ✓
withRouter.html gzip 964 B 965 B ⚠️ +1 B
Overall change 2.91 kB 2.91 kB ✓

Diffs

Diff for 677f882d2ed8..9f2822af2.js 
@@ -2330,6 +2330,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         "protocol",
         "query",
         "search",
+        "searchParams",
         "slashes"
       ];
       exports.urlObjectKeys = urlObjectKeys;
Diff for 677f882d2ed8..da.module.js 
@@ -1636,6 +1636,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         "protocol",
         "query",
         "search",
+        "searchParams",
         "slashes"
       ];
       exports.urlObjectKeys = urlObjectKeys;
Diff for index.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for link.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -123,13 +123,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for withRouter.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 16.1s 15.8s -298ms
nodeModulesSize 55.9 MB 55.9 MB ⚠️ +47 B
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB N/A N/A
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
677f882d2ed8..8ccd.js gzip N/A 10.5 kB N/A
Overall change 57.3 kB 57.3 kB ⚠️ +4 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB N/A N/A
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 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 ⚠️ +4 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Serverless bundles Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
_error.js 1.03 MB 1.03 MB ⚠️ +16 B
404.html 4.22 kB 4.22 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 1.03 MB 1.03 MB ⚠️ +16 B
link.js 1.08 MB 1.08 MB ⚠️ +16 B
routerDirect.js 1.07 MB 1.07 MB ⚠️ +16 B
withRouter.js 1.07 MB 1.07 MB ⚠️ +16 B
Overall change 5.29 MB 5.29 MB ⚠️ +80 B
Commit: 9bf37bf

@Janpot
Copy link
Contributor

Janpot commented Sep 2, 2020

But isn't searchParams a property that is not recognized by the formatUrl function? Wouldn't this PR allow urls to be passed to formatUrl that it can't handle?

@ztanner
Copy link
Member Author

ztanner commented Sep 2, 2020 •
edited
Loading

But isn't searchParams a property that is not recognized by the formatUrl function? Wouldn't this PR allow urls to be passed to formatUrl that it can't handle?

It's never explicitly consumed in formatUrl, so it seems like a safe no-op. But you're right that it doesn't match the URLObject interface. I could alternatively prevent it from making it into formatWithValidation here: https://github.com/vercel/next.js/blob/canary/packages/next/next-server/lib/router/router.ts#L572 if that's preferred?

@ijjk
Copy link
Member

ijjk commented Sep 2, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 12.3s 12.2s -113ms
nodeModulesSize 55.9 MB 55.9 MB ⚠️ +47 B
Page Load Tests Overall increase ✓
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.176 2.151 -0.03
/ avg req/sec 1149.15 1162.25 +13.1
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.235 1.211 -0.02
/error-in-render avg req/sec 2024.07 2065.07 +41
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB 10.5 kB ⚠️ +4 B
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
Overall change 57.3 kB 57.3 kB ⚠️ +4 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB 6.4 kB ⚠️ +4 B
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 kB ✓
webpack-07c5..dule.js gzip 751 B 751 B ✓
Overall change 52.3 kB 52.3 kB ⚠️ +4 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
index.html gzip 972 B 971 B -1 B
link.html gzip 978 B 978 B ✓
withRouter.html gzip 964 B 965 B ⚠️ +1 B
Overall change 2.91 kB 2.91 kB ✓

Diffs

Diff for 677f882d2ed8..9f2822af2.js 
@@ -2330,6 +2330,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         "protocol",
         "query",
         "search",
+        "searchParams",
         "slashes"
       ];
       exports.urlObjectKeys = urlObjectKeys;
Diff for 677f882d2ed8..da.module.js 
@@ -1636,6 +1636,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         "protocol",
         "query",
         "search",
+        "searchParams",
         "slashes"
       ];
       exports.urlObjectKeys = urlObjectKeys;
Diff for index.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for link.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -123,13 +123,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for withRouter.html 
@@ -25,7 +25,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -118,13 +118,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5b5e017fd5e9f2822af2.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.c2e8e638aebf9211530f.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.f4cf18108dce66b669da.module.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.5174f3eae7fe5d47f785.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Serverless Mode (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 13.8s 14s ⚠️ +180ms
nodeModulesSize 55.9 MB 55.9 MB ⚠️ +47 B
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB N/A N/A
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 kB ✓
webpack-e067..f178.js gzip 751 B 751 B ✓
677f882d2ed8..8ccd.js gzip N/A 10.5 kB N/A
Overall change 57.3 kB 57.3 kB ⚠️ +4 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB N/A N/A
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 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 ⚠️ +4 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning Change
_buildManifest.js gzip 322 B 322 B ✓
_buildManife..dule.js gzip 329 B 329 B ✓
Overall change 651 B 651 B ✓
Serverless bundles Overall increase ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
_error.js 1.03 MB 1.03 MB ⚠️ +16 B
404.html 4.22 kB 4.22 kB ✓
hooks.html 3.86 kB 3.86 kB ✓
index.js 1.03 MB 1.03 MB ⚠️ +16 B
link.js 1.08 MB 1.08 MB ⚠️ +16 B
routerDirect.js 1.07 MB 1.07 MB ⚠️ +16 B
withRouter.js 1.07 MB 1.07 MB ⚠️ +16 B
Overall change 5.29 MB 5.29 MB ⚠️ +80 B
Commit: 8e9a613

@Janpot
Copy link
Contributor

Janpot commented Sep 3, 2020 •
edited
Loading

I think the root of the problem is that the output of parseRelativeUrl is incompatible with formatWithValidation. The code currently seems to treat them as compatible and that's eventually going to lead to problems, like query strings being ignored. I think the right conversion from one to the other would be:

const { searchParams, ...parsed } = parseRelativeUrl(someUrl)
const asUrlObject = { ...parsed, query: searchParamsToUrlQuery(searchParams) }
url = formatWithValidation(asUrlObject)

This should pass without validation errors and without dropping the query.

Alternatively we can make parseRelativeUrl directly return a UrlObject (and update all usages to expect query instead of searchParams), I coded that up in #16809

@Janpot Janpot mentioned this pull request Sep 3, 2020
Merged
@ztanner
Copy link
Member Author

ztanner commented Sep 3, 2020

Makes sense @Janpot - I removed my change in favor of yours, so we can just merge the tests. Thanks for the help, everyone!

@ijjk
Copy link
Member

ijjk commented Sep 3, 2020

Stats from current PR

Default Server Mode (Decrease detected ✓)
General
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 13.1s 13.6s ⚠️ +484ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.384 2.418 ⚠️ +0.03
/ avg req/sec 1048.56 1033.84 ⚠️ -14.72
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.343 1.351 ⚠️ +0.01
/error-in-render avg req/sec 1861.21 1850.29 ⚠️ -10.92
Client Bundles (main, webpack, commons)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 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 ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB 6.39 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 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 ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
index.html gzip 972 B 972 B ✓
link.html gzip 978 B 978 B ✓
withRouter.html gzip 964 B 964 B ✓
Overall change 2.91 kB 2.91 kB ✓
Serverless Mode
General
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 15s 14.9s -108ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..01b7.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-f69b135..640b.js gzip 7.1 kB 7.1 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 ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.39 kB 6.39 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-ed2fed0..dule.js gzip 6.16 kB 6.16 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 ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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: e93de42

@ijjk
Copy link
Member

ijjk commented Sep 3, 2020

Failing test suites

Commit: e93de42

test/integration/dynamic-routing/test/index.test.js

  • Dynamic Routing > dev mode > should not have any console warnings on initial load
  • Dynamic Routing > dev mode > should not have any console warnings when navigating to dynamic route
Expand output

● Dynamic Routing › dev mode › should not have any console warnings on initial load

expect(received).toEqual(expected) // deep equality

- Expected  -  1
+ Received  + 20

- Array []
+ Array [
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+ ]

  88 |     it('should not have any console warnings on initial load', async () => {
  89 |       const browser = await webdriver(appPort, '/')
> 90 |       expect(await browser.eval('window.caughtWarns')).toEqual([])
     |                                                        ^
  91 |     })
  92 | 
  93 |     it('should not have any console warnings when navigating to dynamic route', async () => {

  at Object.<anonymous> (integration/dynamic-routing/test/index.test.js:90:56)

● Dynamic Routing › dev mode › should not have any console warnings when navigating to dynamic route

expect(received).toEqual(expected) // deep equality

- Expected  -  1
+ Received  + 26

- Array []
+ Array [
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+   Array [
+     "Unknown key passed via urlObject into url.format: searchParams",
+   ],
+ ]

  103 |         const text = await browser.elementByCss('#asdf').text()
  104 |         expect(text).toMatch(/this is.*?dynamic-1/i)
> 105 |         expect(await browser.eval('window.caughtWarns')).toEqual([])
      |                                                          ^
  106 |       } finally {
  107 |         if (browser) await browser.close()
  108 |       }

  at Object.<anonymous> (integration/dynamic-routing/test/index.test.js:105:58)

kodiakhq bot pushed a commit that referenced this pull request Sep 3, 2020
@Janpot
make parseRelativeUrl return a UrlObject (#16809)
764b194 
Alternative to #16798
@ijjk ijjk changed the title Fix searchParams dev warning Add tests for searchParams dev warning Sep 3, 2020
@ijjk
Copy link
Member

ijjk commented Sep 3, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 12.3s 12.5s ⚠️ +285ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Page Load Tests Overall increase ✓
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
/ failed reqs 0 0 ✓
/ total time (seconds) 2.287 2.171 -0.12
/ avg req/sec 1093.34 1151.74 +58.4
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.221 1.244 ⚠️ +0.02
/error-in-render avg req/sec 2047.43 2009.75 ⚠️ -37.68
Client Bundles (main, webpack, commons)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-ec49a3f..d437.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 ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-b3aecf0..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 ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
index.html gzip 971 B 971 B ✓
link.html gzip 976 B 976 B ✓
withRouter.html gzip 963 B 963 B ✓
Overall change 2.91 kB 2.91 kB ✓
Serverless Mode
General
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
buildDuration 13.5s 14.1s ⚠️ +592ms
nodeModulesSize 56.7 MB 56.7 MB ✓
Client Bundles (main, webpack, commons)
vercel/next.js canary ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..b5de.js gzip 10.5 kB 10.5 kB ✓
framework.HASH.js gzip 39 kB 39 kB ✓
main-ec49a3f..d437.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 ztanner/next.js zt/fix-dev-warning Change
677f882d2ed8..dule.js gzip 6.4 kB 6.4 kB ✓
framework.HA..dule.js gzip 39 kB 39 kB ✓
main-b3aecf0..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 ztanner/next.js zt/fix-dev-warning Change
polyfills-4b..e242.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages
vercel/next.js canary ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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 ztanner/next.js zt/fix-dev-warning 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: cd5c470

@kodiakhq kodiakhq bot merged commit 0885927 into vercel:canary Sep 3, 2020
HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this pull request Sep 26, 2020
@Janpot
make parseRelativeUrl return a UrlObject (vercel#16809)
64fb738 
Alternative to vercel#16798
HitoriSensei pushed a commit to HitoriSensei/next.js that referenced this pull request Sep 26, 2020
@ztanner
Add tests for searchParams dev warning (vercel#16798)
0b75d8f 
Fixes a warning being logged to the console when using `href` with a dynamic path. Fixes vercel#16794
@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.
Reviewers

@ijjk ijjk ijjk approved these changes

@lfades lfades Awaiting requested review from lfades

@Timer Timer Awaiting requested review from Timer

@timneutkens timneutkens Awaiting requested review from timneutkens

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dynamic href prints console warning in development mode
3 participants
@ztanner @ijjk @Janpot