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

Ensure providing only query on dynamic route works as expected #25469

Merged
merged 1 commit into from May 28, 2021

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented May 25, 2021

This ensures the previous behavior of only providing a query for a dynamic-route with no pathname works as expected still which regressed in #24199 since no existing tests covered this behavior. Additional tests have been added to the dynamic-routing suite to ensure providing only the query works + only query with hash values works as well.

Bug

  • Related issues linked using fixes #number
  • Integration tests added

Fixes: #25353

@ijjk
Copy link
Member Author

ijjk commented May 25, 2021

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
buildDuration 15.2s 14.5s -671ms
buildDurationCached 3.9s 3.8s -78ms
nodeModulesSize 46.7 MB 46.7 MB -22 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
/ failed reqs 0 0
/ total time (seconds) 2.701 2.642 -0.06
/ avg req/sec 925.7 946.38 +20.68
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.541 1.582 ⚠️ +0.04
/error-in-render avg req/sec 1622.32 1580.01 ⚠️ -42.31
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
framework-HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 19.4 kB 19.4 kB ⚠️ +7 B
webpack-HASH.js gzip 994 B 994 B
Overall change 59.7 kB 59.7 kB ⚠️ +7 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_app-HASH.js gzip 1.02 kB 1.02 kB
_error-HASH.js gzip 3.06 kB 3.06 kB
amp-HASH.js gzip 526 B 526 B
css-HASH.js gzip 334 B 334 B
hooks-HASH.js gzip 890 B 890 B
index-HASH.js gzip 262 B 262 B
link-HASH.js gzip 1.65 kB 1.64 kB -12 B
routerDirect..HASH.js gzip 331 B 331 B
withRouter-HASH.js gzip 329 B 329 B
bb14e60e810b..30f.css gzip 125 B 125 B
Overall change 8.54 kB 8.52 kB -12 B
Client Build Manifests Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_buildManifest.js gzip 390 B 392 B ⚠️ +2 B
Overall change 390 B 392 B ⚠️ +2 B
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
index.html gzip 561 B 561 B
link.html gzip 569 B 569 B
withRouter.html gzip 556 B 557 B ⚠️ +1 B
Overall change 1.69 kB 1.69 kB ⚠️ +1 B

Diffs

Diff for _buildManifest.js
@@ -12,7 +12,7 @@ self.__BUILD_MANIFEST = {
   "/hooks": [
     "static\u002Fchunks\u002Fpages\u002Fhooks-ac434c51eeb0f395a39d.js"
   ],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-6a1f3646a4b6c450a8d9.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-47f6aa1a34adbbacdbbf.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-2ffbe1bc5bc0c7d0c63f.js"
   ],
Diff for link-HASH.js
@@ -102,11 +102,10 @@
 
         var p = props.prefetch !== false;
         var router = (0, _router2.useRouter)();
-        var pathname = (router && router.asPath) || "/";
 
         var _react$default$useMem = _react["default"].useMemo(
             function() {
-              var _ref = (0, _router.resolveHref)(pathname, props.href, true),
+              var _ref = (0, _router.resolveHref)(router, props.href, true),
                 _ref2 = _slicedToArray(_ref, 2),
                 resolvedHref = _ref2[0],
                 resolvedAs = _ref2[1];
@@ -114,11 +113,11 @@
               return {
                 href: resolvedHref,
                 as: props.as
-                  ? (0, _router.resolveHref)(pathname, props.as)
+                  ? (0, _router.resolveHref)(router, props.as)
                   : resolvedAs || resolvedHref
               };
             },
-            [pathname, props.href, props.as]
+            [router, props.href, props.as]
           ),
           href = _react$default$useMem.href,
           as = _react$default$useMem.as;
Diff for main-HASH.js
@@ -3202,7 +3202,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
       function isLocalURL(url) {
         // prevent a hydration mismatch on href for url with anchor refs
-        if (url.startsWith("/") || url.startsWith("#")) return true;
+        if (url.startsWith("/") || url.startsWith("#") || url.startsWith("?"))
+          return true;
 
         try {
           // absolute urls can be local if they are on the same origin
@@ -3293,21 +3294,23 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
        * Preserves absolute urls.
        */
 
-      function resolveHref(currentPath, href, resolveAs) {
+      function resolveHref(router, href, resolveAs) {
         // we use a dummy base url for relative urls
         var base;
+        var urlAsString =
+          typeof href === "string"
+            ? href
+            : (0, _utils.formatWithValidation)(href);
 
         try {
-          base = new URL(currentPath, "http://n");
+          base = new URL(
+            urlAsString.startsWith("#") ? router.asPath : router.pathname,
+            "http://n"
+          );
         } catch (_) {
           // fallback to / for invalid asPath values e.g. //
           base = new URL("/", "http://n");
-        }
-
-        var urlAsString =
-          typeof href === "string"
-            ? href
-            : (0, _utils.formatWithValidation)(href); // Return because it cannot be routed by the Next.js router
+        } // Return because it cannot be routed by the Next.js router
 
         if (!isLocalURL(urlAsString)) {
           return resolveAs ? [urlAsString] : urlAsString;
@@ -3367,7 +3370,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
       function prepareUrlAs(router, url, as) {
         // If url and as provided as an object representation,
         // we'll format them into the string version here.
-        var _resolveHref = resolveHref(router.asPath, url, true),
+        var _resolveHref = resolveHref(router, url, true),
           _resolveHref2 = _slicedToArray(_resolveHref, 2),
           resolvedHref = _resolveHref2[0],
           resolvedAs = _resolveHref2[1];
@@ -3381,7 +3384,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
           ? resolvedHref
           : addBasePath(resolvedHref);
         var preparedAs = as
-          ? stripOrigin(resolveHref(router.asPath, as))
+          ? stripOrigin(resolveHref(router, as))
           : resolvedAs || resolvedHref;
         return {
           url: preparedUrl,
Diff for index.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       async=""
     ></script>
     <script
Diff for link.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       as="script"
     />
     <link
@@ -27,7 +27,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/pages/link-6a1f3646a4b6c450a8d9.js"
+      href="/_next/static/chunks/pages/link-47f6aa1a34adbbacdbbf.js"
       as="script"
     />
   </head>
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       async=""
     ></script>
     <script
@@ -69,7 +69,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-6a1f3646a4b6c450a8d9.js"
+      src="/_next/static/chunks/pages/link-47f6aa1a34adbbacdbbf.js"
       async=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" async=""></script>
Diff for withRouter.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      href="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       as="script"
     />
     <link
@@ -56,7 +56,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/main-aa7354814c85fc15b2e2.js"
+      src="/_next/static/chunks/main-dd8ebd5054e8f8ff5679.js"
       async=""
     ></script>
     <script

Serverless Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
buildDuration 16.5s 16.5s ⚠️ +44ms
buildDurationCached 4.8s 4.9s ⚠️ +48ms
nodeModulesSize 46.7 MB 46.7 MB -22 B
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
framework-HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 19.4 kB 19.4 kB ⚠️ +7 B
webpack-HASH.js gzip 994 B 994 B
Overall change 59.7 kB 59.7 kB ⚠️ +7 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_app-HASH.js gzip 1.02 kB 1.02 kB
_error-HASH.js gzip 3.06 kB 3.06 kB
amp-HASH.js gzip 526 B 526 B
css-HASH.js gzip 334 B 334 B
hooks-HASH.js gzip 890 B 890 B
index-HASH.js gzip 262 B 262 B
link-HASH.js gzip 1.65 kB 1.64 kB -12 B
routerDirect..HASH.js gzip 331 B 331 B
withRouter-HASH.js gzip 329 B 329 B
bb14e60e810b..30f.css gzip 125 B 125 B
Overall change 8.54 kB 8.52 kB -12 B
Client Build Manifests Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_buildManifest.js gzip 390 B 392 B ⚠️ +2 B
Overall change 390 B 392 B ⚠️ +2 B
Serverless bundles
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_error.js 16.9 kB 16.9 kB
404.html 2.42 kB 2.42 kB
500.html 2.41 kB 2.41 kB
amp.amp.html 10.8 kB 10.8 kB
amp.html 1.61 kB 1.61 kB
css.html 1.79 kB 1.79 kB
hooks.html 1.67 kB 1.67 kB
index.js 17.2 kB 17.2 kB
link.js 17.4 kB 17.4 kB
routerDirect.js 17.4 kB 17.4 kB
withRouter.js 17.4 kB 17.4 kB
Overall change 107 kB 107 kB

Webpack 4 Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
buildDuration 12.8s 12.9s ⚠️ +78ms
buildDurationCached 5.4s 5.5s ⚠️ +102ms
nodeModulesSize 46.7 MB 46.7 MB -22 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
/ failed reqs 0 0
/ total time (seconds) 2.69 2.787 ⚠️ +0.1
/ avg req/sec 929.26 896.97 ⚠️ -32.29
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.521 1.568 ⚠️ +0.05
/error-in-render avg req/sec 1643.87 1593.91 ⚠️ -49.96
Client Bundles (main, webpack, commons) Overall increase ⚠️
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
677f882d2ed8..HASH.js gzip 13.3 kB 13.3 kB ⚠️ +7 B
framework.HASH.js gzip 39 kB 39 kB
main-HASH.js gzip 7.25 kB 7.25 kB
webpack-HASH.js gzip 751 B 751 B
Overall change 60.3 kB 60.3 kB ⚠️ +7 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
polyfills-HASH.js gzip 31.3 kB 31.3 kB
Overall change 31.3 kB 31.3 kB
Client Pages Overall decrease ✓
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_app-HASH.js gzip 1.28 kB 1.28 kB
_error-HASH.js gzip 3.74 kB 3.74 kB
amp-HASH.js gzip 536 B 536 B
css-HASH.js gzip 339 B 339 B
hooks-HASH.js gzip 887 B 887 B
index-HASH.js gzip 227 B 227 B
link-HASH.js gzip 1.64 kB 1.63 kB -12 B
routerDirect..HASH.js gzip 303 B 303 B
withRouter-HASH.js gzip 302 B 302 B
e025d2764813..52f.css gzip 125 B 125 B
Overall change 9.38 kB 9.37 kB -12 B
Client Build Manifests
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
_buildManifest.js gzip 420 B 420 B
Overall change 420 B 420 B
Rendered Page Sizes
vercel/next.js canary ijjk/next.js fix/dynamic-only-query Change
index.html gzip 614 B 613 B -1 B
link.html gzip 620 B 620 B
withRouter.html gzip 606 B 607 B ⚠️ +1 B
Overall change 1.84 kB 1.84 kB

Diffs

Diff for _buildManifest.js
@@ -12,7 +12,7 @@ self.__BUILD_MANIFEST = {
   "/hooks": [
     "static\u002Fchunks\u002Fpages\u002Fhooks-f1cc32851f5e866a7e78.js"
   ],
-  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-121d9306ed12436945d4.js"],
+  "/link": ["static\u002Fchunks\u002Fpages\u002Flink-4b26731d43b4caf43737.js"],
   "/routerDirect": [
     "static\u002Fchunks\u002Fpages\u002FrouterDirect-e9bdf4dc22ca5dd5d885.js"
   ],
Diff for link-HASH.js
@@ -160,11 +160,10 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
 
         var p = props.prefetch !== false;
         var router = (0, _router2.useRouter)();
-        var pathname = (router && router.asPath) || "/";
 
         var _react$default$useMem = _react["default"].useMemo(
             function() {
-              var _ref = (0, _router.resolveHref)(pathname, props.href, true),
+              var _ref = (0, _router.resolveHref)(router, props.href, true),
                 _ref2 = _slicedToArray(_ref, 2),
                 resolvedHref = _ref2[0],
                 resolvedAs = _ref2[1];
@@ -172,11 +171,11 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
               return {
                 href: resolvedHref,
                 as: props.as
-                  ? (0, _router.resolveHref)(pathname, props.as)
+                  ? (0, _router.resolveHref)(router, props.as)
                   : resolvedAs || resolvedHref
               };
             },
-            [pathname, props.href, props.as]
+            [router, props.href, props.as]
           ),
           href = _react$default$useMem.href,
           as = _react$default$useMem.as;
Diff for 677f882d2ed8..c4df.HASH.js
@@ -1273,7 +1273,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
       function isLocalURL(url) {
         // prevent a hydration mismatch on href for url with anchor refs
-        if (url.startsWith("/") || url.startsWith("#")) return true;
+        if (url.startsWith("/") || url.startsWith("#") || url.startsWith("?"))
+          return true;
 
         try {
           // absolute urls can be local if they are on the same origin
@@ -1364,21 +1365,23 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
        * Preserves absolute urls.
        */
 
-      function resolveHref(currentPath, href, resolveAs) {
+      function resolveHref(router, href, resolveAs) {
         // we use a dummy base url for relative urls
         var base;
+        var urlAsString =
+          typeof href === "string"
+            ? href
+            : (0, _utils.formatWithValidation)(href);
 
         try {
-          base = new URL(currentPath, "http://n");
+          base = new URL(
+            urlAsString.startsWith("#") ? router.asPath : router.pathname,
+            "http://n"
+          );
         } catch (_) {
           // fallback to / for invalid asPath values e.g. //
           base = new URL("/", "http://n");
-        }
-
-        var urlAsString =
-          typeof href === "string"
-            ? href
-            : (0, _utils.formatWithValidation)(href); // Return because it cannot be routed by the Next.js router
+        } // Return because it cannot be routed by the Next.js router
 
         if (!isLocalURL(urlAsString)) {
           return resolveAs ? [urlAsString] : urlAsString;
@@ -1438,7 +1441,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
       function prepareUrlAs(router, url, as) {
         // If url and as provided as an object representation,
         // we'll format them into the string version here.
-        var _resolveHref = resolveHref(router.asPath, url, true),
+        var _resolveHref = resolveHref(router, url, true),
           _resolveHref2 = _slicedToArray(_resolveHref, 2),
           resolvedHref = _resolveHref2[0],
           resolvedAs = _resolveHref2[1];
@@ -1452,7 +1455,7 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
           ? resolvedHref
           : addBasePath(resolvedHref);
         var preparedAs = as
-          ? stripOrigin(resolveHref(router.asPath, as))
+          ? stripOrigin(resolveHref(router, as))
           : resolvedAs || resolvedHref;
         return {
           url: preparedUrl,
Diff for index.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       async=""
     ></script>
     <script
Diff for link.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       as="script"
     />
     <link
@@ -32,7 +32,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/pages/link-121d9306ed12436945d4.js"
+      href="/_next/static/chunks/pages/link-4b26731d43b4caf43737.js"
       as="script"
     />
   </head>
@@ -66,7 +66,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       async=""
     ></script>
     <script
@@ -78,7 +78,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/pages/link-121d9306ed12436945d4.js"
+      src="/_next/static/chunks/pages/link-4b26731d43b4caf43737.js"
       async=""
     ></script>
     <script src="/_next/static/BUILD_ID/_buildManifest.js" async=""></script>
Diff for withRouter.html
@@ -17,7 +17,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      href="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       as="script"
     />
     <link
@@ -61,7 +61,7 @@
       async=""
     ></script>
     <script
-      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.26e6c0f15d34a1b1b821.js"
+      src="/_next/static/chunks/677f882d2ed86fa3467b8979053c1a4c3f8bc4df.2e9dabfd40187d2d6ef4.js"
       async=""
     ></script>
     <script
Commit: 415ab29

@ijjk ijjk marked this pull request as ready for review May 25, 2021 19:49
@timneutkens timneutkens merged commit 8a06780 into vercel:canary May 28, 2021
@timneutkens timneutkens deleted the fix/dynamic-only-query branch May 28, 2021 12:51
flybayer pushed a commit to blitz-js/next.js that referenced this pull request Jun 16, 2021
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 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.

Router path query regression in next 10.2.2
2 participants