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

Update to use getDataHref in fetchNextData #14667

Merged
merged 5 commits into from
Jun 29, 2020

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented Jun 29, 2020

This updates fetchNextData to re-use the getDataHref function from page-loader which has more verbose handling to ensure the correct /_next/data URL is built. Re-using this logic ensures the /_next/data URL can still be built even when a mismatching href and as value is provided to next/link.

This also fixes a case in getDataHref where optional values that weren't provided would fail to build the data href since the check requiring the param be present while interpolating the route values hasn't been updated to allow missing params for optional values.

An additional test case has been added to the prerender suite to ensure the /_next/data URL is built correctly when mismatching href and as values are provided

x-ref: #14536
x-ref: #9081 (reply in thread)
Closes: #14668

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@ijjk
Copy link
Member Author

ijjk commented Jun 29, 2020

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
buildDuration 15.6s 15.8s ⚠️ +208ms
nodeModulesSize 67.1 MB 67.1 MB -210 B
Page Load Tests Overall increase ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
/ failed reqs 0 0
/ total time (seconds) 2.689 2.581 -0.11
/ avg req/sec 929.7 968.52 +38.82
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.692 1.654 -0.04
/error-in-render avg req/sec 1477.47 1511.39 +33.92
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
main-HASH.js gzip 6.51 kB 6.59 kB ⚠️ +82 B
webpack-HASH.js gzip 751 B 751 B
19b7e98f51cc..2bd7.js gzip 10.7 kB 10.6 kB -101 B
framework.HASH.js gzip 39.1 kB 39.1 kB
Overall change 57.1 kB 57.1 kB -19 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
main-HASH.module.js gzip 5.59 kB 5.68 kB ⚠️ +83 B
webpack-HASH..dule.js gzip 751 B 751 B
19b7e98f51cc..dule.js gzip 7.1 kB 7.03 kB -70 B
framework.HA..dule.js gzip 39.1 kB 39.1 kB
Overall change 52.6 kB 52.6 kB ⚠️ +13 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB
Overall change 26.3 kB 26.3 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
_buildManifest.js gzip 268 B 268 B
_buildManife..dule.js gzip 272 B 272 B
Overall change 540 B 540 B
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
index.html gzip 952 B 954 B ⚠️ +2 B
link.html gzip 958 B 960 B ⚠️ +2 B
withRouter.html gzip 946 B 946 B
Overall change 2.86 kB 2.86 kB ⚠️ +4 B

Diffs

Diff for 19b7e98f51cc..e6.module.js
@@ -216,30 +216,6 @@
       /***/
     },
 
-    /***/ Lab5: /***/ function(module, exports, __webpack_require__) {
-      "use strict";
-
-      exports.__esModule = true;
-      exports.default = getAssetPathFromRoute; // Translates a logical route into its pages asset path (relative from a common prefix)
-      // "asset path" being its javascript file, data file, prerendered html,...
-
-      function getAssetPathFromRoute(route) {
-        var ext =
-          arguments.length > 1 && arguments[1] !== undefined
-            ? arguments[1]
-            : "";
-        var path =
-          route === "/"
-            ? "/index"
-            : /^\/index(\/|$)/.test(route)
-            ? "/index".concat(route)
-            : "".concat(route);
-        return path + ext;
-      }
-
-      /***/
-    },
-
     /***/ NkFK: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
@@ -656,10 +632,6 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
       var _normalizeTrailingSlash = __webpack_require__("NkFK");
 
-      var _getAssetPathFromRoute = _interopRequireDefault(
-        __webpack_require__("Lab5")
-      );
-
       function _interopRequireDefault(obj) {
         return obj && obj.__esModule
           ? obj
@@ -718,37 +690,24 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         };
       }
 
-      function fetchNextData(pathname, query, isServerRender, cb) {
+      function fetchNextData(dataHref, isServerRender, cb) {
         var attempts = isServerRender ? 3 : 1;
 
         function getResponse() {
-          return fetch(
-            (0, _utils.formatWithValidation)({
-              pathname: addBasePath(
-                // @ts-ignore __NEXT_DATA__
-                "/_next/data/"
-                  .concat(__NEXT_DATA__.buildId)
-                  .concat(
-                    (0, _getAssetPathFromRoute.default)(pathname, ".json")
-                  )
-              ),
-              query
-            }),
-            {
-              // Cookies are required to be present for Next.js' SSG "Preview Mode".
-              // Cookies may also be required for `getServerSideProps`.
-              //
-              // > `fetch` won’t send cookies, unless you set the credentials init
-              // > option.
-              // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
-              //
-              // > For maximum browser compatibility when it comes to sending &
-              // > receiving cookies, always supply the `credentials: 'same-origin'`
-              // > option instead of relying on the default.
-              // https://github.com/github/fetch#caveats
-              credentials: "same-origin"
-            }
-          ).then(res => {
+          return fetch(dataHref, {
+            // Cookies are required to be present for Next.js' SSG "Preview Mode".
+            // Cookies may also be required for `getServerSideProps`.
+            //
+            // > `fetch` won’t send cookies, unless you set the credentials init
+            // > option.
+            // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
+            //
+            // > For maximum browser compatibility when it comes to sending &
+            // > receiving cookies, always supply the `credentials: 'same-origin'`
+            // > option instead of relying on the default.
+            // https://github.com/github/fetch#caveats
+            credentials: "same-origin"
+          }).then(res => {
             if (!res.ok) {
               if (--attempts > 0 && res.status >= 500) {
                 return getResponse();
@@ -855,22 +814,19 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
             this.change("replaceState", url, as, options);
           };
 
-          this._getStaticData = asPath => {
-            var pathname = prepareRoute((0, _url.parse)(asPath).pathname);
+          this._getStaticData = dataHref => {
+            var pathname = prepareRoute((0, _url.parse)(dataHref).pathname);
             return true && this.sdc[pathname]
-              ? Promise.resolve(this.sdc[pathname])
+              ? Promise.resolve(this.sdc[dataHref])
               : fetchNextData(
-                  pathname,
-                  null,
+                  dataHref,
                   this.isSsr,
                   data => (this.sdc[pathname] = data)
                 );
           };
 
-          this._getServerData = asPath => {
-            var { pathname, query } = (0, _url.parse)(asPath, true);
-            pathname = prepareRoute(pathname);
-            return fetchNextData(pathname, query, this.isSsr);
+          this._getServerData = dataHref => {
+            return fetchNextData(dataHref, this.isSsr);
           }; // represents the current component key
 
           this.route = toRoute(_pathname); // set up the component cache (by route keys)
@@ -1263,11 +1219,24 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                 var isValidElementType;
               }
 
+              var dataHref;
+
+              if (__N_SSG || __N_SSP) {
+                dataHref = this.pageLoader.getDataHref(
+                  (0, _utils.formatWithValidation)({
+                    pathname,
+                    query
+                  }),
+                  as,
+                  __N_SSG
+                );
+              }
+
               return this._getData(() =>
                 __N_SSG
-                  ? this._getStaticData(as)
+                  ? this._getStaticData(dataHref)
                   : __N_SSP
-                  ? this._getServerData(as)
+                  ? this._getServerData(dataHref)
                   : this.getInitialProps(
                       Component, // we provide AppTree later so this needs to be `any`
                       {
Diff for 19b7e98f51cc..a34d85065.js
@@ -239,30 +239,6 @@
       /***/
     },
 
-    /***/ Lab5: /***/ function(module, exports, __webpack_require__) {
-      "use strict";
-
-      exports.__esModule = true;
-      exports["default"] = getAssetPathFromRoute; // Translates a logical route into its pages asset path (relative from a common prefix)
-      // "asset path" being its javascript file, data file, prerendered html,...
-
-      function getAssetPathFromRoute(route) {
-        var ext =
-          arguments.length > 1 && arguments[1] !== undefined
-            ? arguments[1]
-            : "";
-        var path =
-          route === "/"
-            ? "/index"
-            : /^\/index(\/|$)/.test(route)
-            ? "/index".concat(route)
-            : "".concat(route);
-        return path + ext;
-      }
-
-      /***/
-    },
-
     /***/ NkFK: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
@@ -786,10 +762,6 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
 
       var _normalizeTrailingSlash = __webpack_require__("NkFK");
 
-      var _getAssetPathFromRoute = _interopRequireDefault(
-        __webpack_require__("Lab5")
-      );
-
       function _interopRequireDefault(obj) {
         return obj && obj.__esModule
           ? obj
@@ -848,37 +820,24 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
         };
       }
 
-      function fetchNextData(pathname, query, isServerRender, cb) {
+      function fetchNextData(dataHref, isServerRender, cb) {
         var attempts = isServerRender ? 3 : 1;
 
         function getResponse() {
-          return fetch(
-            (0, _utils.formatWithValidation)({
-              pathname: addBasePath(
-                // @ts-ignore __NEXT_DATA__
-                "/_next/data/"
-                  .concat(__NEXT_DATA__.buildId)
-                  .concat(
-                    (0, _getAssetPathFromRoute["default"])(pathname, ".json")
-                  )
-              ),
-              query: query
-            }),
-            {
-              // Cookies are required to be present for Next.js' SSG "Preview Mode".
-              // Cookies may also be required for `getServerSideProps`.
-              //
-              // > `fetch` won’t send cookies, unless you set the credentials init
-              // > option.
-              // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
-              //
-              // > For maximum browser compatibility when it comes to sending &
-              // > receiving cookies, always supply the `credentials: 'same-origin'`
-              // > option instead of relying on the default.
-              // https://github.com/github/fetch#caveats
-              credentials: "same-origin"
-            }
-          ).then(function(res) {
+          return fetch(dataHref, {
+            // Cookies are required to be present for Next.js' SSG "Preview Mode".
+            // Cookies may also be required for `getServerSideProps`.
+            //
+            // > `fetch` won’t send cookies, unless you set the credentials init
+            // > option.
+            // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
+            //
+            // > For maximum browser compatibility when it comes to sending &
+            // > receiving cookies, always supply the `credentials: 'same-origin'`
+            // > option instead of relying on the default.
+            // https://github.com/github/fetch#caveats
+            credentials: "same-origin"
+          }).then(function(res) {
             if (!res.ok) {
               if (--attempts > 0 && res.status >= 500) {
                 return getResponse();
@@ -994,22 +953,17 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
             _this.change("replaceState", url, as, options);
           };
 
-          this._getStaticData = function(asPath) {
-            var pathname = prepareRoute((0, _url.parse)(asPath).pathname);
+          this._getStaticData = function(dataHref) {
+            var pathname = prepareRoute((0, _url.parse)(dataHref).pathname);
             return true && _this.sdc[pathname]
-              ? Promise.resolve(_this.sdc[pathname])
-              : fetchNextData(pathname, null, _this.isSsr, function(data) {
+              ? Promise.resolve(_this.sdc[dataHref])
+              : fetchNextData(dataHref, _this.isSsr, function(data) {
                   return (_this.sdc[pathname] = data);
                 });
           };
 
-          this._getServerData = function(asPath) {
-            var _ref2 = (0, _url.parse)(asPath, true),
-              pathname = _ref2.pathname,
-              query = _ref2.query;
-
-            pathname = prepareRoute(pathname);
-            return fetchNextData(pathname, query, _this.isSsr);
+          this._getServerData = function(dataHref) {
+            return fetchNextData(dataHref, _this.isSsr);
           }; // represents the current component key
 
           this.route = toRoute(_pathname); // set up the component cache (by route keys)
@@ -1205,10 +1159,10 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                     return resolve(true);
                   }
 
-                  var _ref3 = (0, _url.parse)(url, true),
-                    pathname = _ref3.pathname,
-                    query = _ref3.query,
-                    protocol = _ref3.protocol; // url and as should always be prefixed with basePath by this
+                  var _ref2 = (0, _url.parse)(url, true),
+                    pathname = _ref2.pathname,
+                    query = _ref2.query,
+                    protocol = _ref2.protocol; // url and as should always be prefixed with basePath by this
                   // point by either next/link or router.push/replace so strip the
                   // basePath from the pathname to match the pages dir 1-to-1
 
@@ -1241,8 +1195,8 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                   var cleanedAs = delBasePath(as);
 
                   if ((0, _isDynamic.isDynamicRoute)(route)) {
-                    var _ref4 = (0, _url.parse)(cleanedAs),
-                      asPathname = _ref4.pathname;
+                    var _ref3 = (0, _url.parse)(cleanedAs),
+                      asPathname = _ref3.pathname;
 
                     var routeRegex = (0, _routeRegex.getRouteRegex)(route);
                     var routeMatch = (0, _routeMatcher.getRouteMatcher)(
@@ -1442,12 +1396,25 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                       var _require, isValidElementType;
                     }
 
+                    var dataHref;
+
+                    if (__N_SSG || __N_SSP) {
+                      dataHref = _this3.pageLoader.getDataHref(
+                        (0, _utils.formatWithValidation)({
+                          pathname: pathname,
+                          query: query
+                        }),
+                        as,
+                        __N_SSG
+                      );
+                    }
+
                     return _this3
                       ._getData(function() {
                         return __N_SSG
-                          ? _this3._getStaticData(as)
+                          ? _this3._getStaticData(dataHref)
                           : __N_SSP
-                          ? _this3._getServerData(as)
+                          ? _this3._getServerData(dataHref)
                           : _this3.getInitialProps(
                               Component, // we provide AppTree later so this needs to be `any`
                               {
@@ -1573,9 +1540,9 @@ THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLI
                     ? arguments[2]
                     : {};
                 return new Promise(function(resolve, reject) {
-                  var _ref5 = (0, _url.parse)(url),
-                    pathname = _ref5.pathname,
-                    protocol = _ref5.protocol;
+                  var _ref4 = (0, _url.parse)(url),
+                    pathname = _ref4.pathname,
+                    protocol = _ref4.protocol;
 
                   if (!pathname || protocol) {
                     if (false) {
Diff for main-HASH.js
@@ -1031,6 +1031,30 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
       /***/
     },
 
+    /***/ Lab5: /***/ function(module, exports, __webpack_require__) {
+      "use strict";
+
+      exports.__esModule = true;
+      exports["default"] = getAssetPathFromRoute; // Translates a logical route into its pages asset path (relative from a common prefix)
+      // "asset path" being its javascript file, data file, prerendered html,...
+
+      function getAssetPathFromRoute(route) {
+        var ext =
+          arguments.length > 1 && arguments[1] !== undefined
+            ? arguments[1]
+            : "";
+        var path =
+          route === "/"
+            ? "/index"
+            : /^\/index(\/|$)/.test(route)
+            ? "/index".concat(route)
+            : "".concat(route);
+        return path + ext;
+      }
+
+      /***/
+    },
+
     /***/ Nsbk: /***/ function(module, exports) {
       function _getPrototypeOf(o) {
         module.exports = _getPrototypeOf = Object.setPrototypeOf
@@ -1564,9 +1588,19 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
           },
           {
             key: "getDataHref",
-            value: function getDataHref(href, asPath) {
+            value: function getDataHref(href, asPath, ssg) {
               var _this2 = this;
 
+              var _ref = (0, _url.parse)(href, true),
+                hrefPathname = _ref.pathname,
+                query = _ref.query,
+                search = _ref.search;
+
+              var _ref2 = (0, _url.parse)(asPath),
+                asPathname = _ref2.pathname;
+
+              var route = normalizeRoute(hrefPathname);
+
               var getHrefForSlug =
                 /** @type string */
                 function getHrefForSlug(path) {
@@ -1577,17 +1611,10 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
                   return ""
                     .concat(_this2.assetPrefix, "/_next/data/")
                     .concat(_this2.buildId)
-                    .concat(dataRoute);
+                    .concat(dataRoute)
+                    .concat(ssg ? "" : search || "");
                 };
 
-              var _ref = (0, _url.parse)(href, true),
-                hrefPathname = _ref.pathname,
-                query = _ref.query;
-
-              var _ref2 = (0, _url.parse)(asPath),
-                asPathname = _ref2.pathname;
-
-              var route = normalizeRoute(hrefPathname);
               var isDynamic = (0, _isDynamic.isDynamicRoute)(route),
                 interpolatedRoute;
 
@@ -1604,23 +1631,25 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
 
                 if (
                   !Object.keys(dynamicGroups).every(function(param) {
-                    var value = dynamicMatches[param];
+                    var value = dynamicMatches[param] || "";
                     var _dynamicGroups$param = dynamicGroups[param],
                       repeat = _dynamicGroups$param.repeat,
                       optional = _dynamicGroups$param.optional; // support single-level catch-all
                     // TODO: more robust handling for user-error (passing `/`)
 
-                    if (repeat && !Array.isArray(value)) value = [value];
                     var replaced = "["
                       .concat(repeat ? "..." : "")
                       .concat(param, "]");
 
                     if (optional) {
-                      replaced = "[".concat(replaced, "]");
+                      replaced = ""
+                        .concat(!value ? "/" : "", "[")
+                        .concat(replaced, "]");
                     }
 
+                    if (repeat && !Array.isArray(value)) value = [value];
                     return (
-                      param in dynamicMatches && // Interpolate group into data URL if present
+                      (optional || param in dynamicMatches) && // Interpolate group into data URL if present
                       (interpolatedRoute = interpolatedRoute.replace(
                         replaced,
                         repeat
@@ -1658,7 +1687,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
                 return (
                   // Check if the route requires a data file
                   s.has(route) && // Try to generate data href, noop when falsy
-                  (_dataHref = _this3.getDataHref(href, asPath)) && // noop when data has already been prefetched (dedupe)
+                  (_dataHref = _this3.getDataHref(href, asPath, true)) && // noop when data has already been prefetched (dedupe)
                   !document.querySelector(
                     'link[rel="'
                       .concat(relPrefetch, '"][href^="')
Diff for main-HASH.module.js
@@ -728,6 +728,30 @@
       /***/
     },
 
+    /***/ Lab5: /***/ function(module, exports, __webpack_require__) {
+      "use strict";
+
+      exports.__esModule = true;
+      exports.default = getAssetPathFromRoute; // Translates a logical route into its pages asset path (relative from a common prefix)
+      // "asset path" being its javascript file, data file, prerendered html,...
+
+      function getAssetPathFromRoute(route) {
+        var ext =
+          arguments.length > 1 && arguments[1] !== undefined
+            ? arguments[1]
+            : "";
+        var path =
+          route === "/"
+            ? "/index"
+            : /^\/index(\/|$)/.test(route)
+            ? "/index".concat(route)
+            : "".concat(route);
+        return path + ext;
+      }
+
+      /***/
+    },
+
     /***/ Z577: /***/ function(module, exports) {
       Promise.prototype.finally = function(n) {
         if ("function" != typeof n) return this.then(n, n);
@@ -1191,7 +1215,14 @@
          * @param {string} asPath the URL as shown in browser (virtual path); used for dynamic routes
          */
 
-        getDataHref(href, asPath) {
+        getDataHref(href, asPath, ssg) {
+          var { pathname: hrefPathname, query, search } = (0, _url.parse)(
+            href,
+            true
+          );
+          var { pathname: asPathname } = (0, _url.parse)(asPath);
+          var route = normalizeRoute(hrefPathname);
+
           var getHrefForSlug =
             /** @type string */
             path => {
@@ -1202,12 +1233,10 @@
               return ""
                 .concat(this.assetPrefix, "/_next/data/")
                 .concat(this.buildId)
-                .concat(dataRoute);
+                .concat(dataRoute)
+                .concat(ssg ? "" : search || "");
             };
 
-          var { pathname: hrefPathname, query } = (0, _url.parse)(href, true);
-          var { pathname: asPathname } = (0, _url.parse)(asPath);
-          var route = normalizeRoute(hrefPathname);
           var isDynamic = (0, _isDynamic.isDynamicRoute)(route),
             interpolatedRoute;
 
@@ -1222,21 +1251,23 @@
 
             if (
               !Object.keys(dynamicGroups).every(param => {
-                var value = dynamicMatches[param];
+                var value = dynamicMatches[param] || "";
                 var { repeat, optional } = dynamicGroups[param]; // support single-level catch-all
                 // TODO: more robust handling for user-error (passing `/`)
 
-                if (repeat && !Array.isArray(value)) value = [value];
                 var replaced = "["
                   .concat(repeat ? "..." : "")
                   .concat(param, "]");
 
                 if (optional) {
-                  replaced = "[".concat(replaced, "]");
+                  replaced = ""
+                    .concat(!value ? "/" : "", "[")
+                    .concat(replaced, "]");
                 }
 
+                if (repeat && !Array.isArray(value)) value = [value];
                 return (
-                  param in dynamicMatches && // Interpolate group into data URL if present
+                  (optional || param in dynamicMatches) && // Interpolate group into data URL if present
                   (interpolatedRoute = interpolatedRoute.replace(
                     replaced,
                     repeat
@@ -1270,7 +1301,7 @@
               _dataHref // Check if the route requires a data file
             ) =>
               s.has(route) && // Try to generate data href, noop when falsy
-              (_dataHref = this.getDataHref(href, asPath)) && // noop when data has already been prefetched (dedupe)
+              (_dataHref = this.getDataHref(href, asPath, true)) && // noop when data has already been prefetched (dedupe)
               !document.querySelector(
                 'link[rel="'
                   .concat(relPrefetch, '"][href^="')
Diff for index.html
@@ -6,7 +6,7 @@
     <meta name="next-head-count" content="2" />
     <link
       rel="preload"
-      href="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      href="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -24,7 +24,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -81,13 +81,13 @@
       src="/_next/static/runtime/polyfills-e77205bd3f87781c0279.js"
     ></script>
     <script
-      src="/_next/static/runtime/main-e535c7ca56db95621a7e.js"
+      src="/_next/static/runtime/main-d1572344454d1b682527.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      src="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
@@ -117,13 +117,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.8efce0a4522a34d85065.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.284ee6bd3fa975a5c6b4.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for link.html
@@ -6,7 +6,7 @@
     <meta name="next-head-count" content="2" />
     <link
       rel="preload"
-      href="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      href="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -24,7 +24,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -86,13 +86,13 @@
       src="/_next/static/runtime/polyfills-e77205bd3f87781c0279.js"
     ></script>
     <script
-      src="/_next/static/runtime/main-e535c7ca56db95621a7e.js"
+      src="/_next/static/runtime/main-d1572344454d1b682527.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      src="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
@@ -122,13 +122,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.8efce0a4522a34d85065.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.284ee6bd3fa975a5c6b4.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
Diff for withRouter.html
@@ -6,7 +6,7 @@
     <meta name="next-head-count" content="2" />
     <link
       rel="preload"
-      href="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      href="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -24,7 +24,7 @@
     />
     <link
       rel="preload"
-      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      href="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       as="script"
       crossorigin="anonymous"
     />
@@ -81,13 +81,13 @@
       src="/_next/static/runtime/polyfills-e77205bd3f87781c0279.js"
     ></script>
     <script
-      src="/_next/static/runtime/main-e535c7ca56db95621a7e.js"
+      src="/_next/static/runtime/main-d1572344454d1b682527.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/runtime/main-9ec4f16ab966aac3b436.module.js"
+      src="/_next/static/runtime/main-bccb40bbe71a0ba25fb8.module.js"
       async=""
       crossorigin="anonymous"
       type="module"
@@ -117,13 +117,13 @@
       type="module"
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.8efce0a4522a34d85065.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.284ee6bd3fa975a5c6b4.js"
       async=""
       crossorigin="anonymous"
       nomodule=""
     ></script>
     <script
-      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.53b806ad9119662edae6.module.js"
+      src="/_next/static/chunks/19b7e98f51cc0d86c45d01159bbbfb942bfe49b8.a6c8b8b5bb69078be31b.module.js"
       async=""
       crossorigin="anonymous"
       type="module"

Serverless Mode (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
buildDuration 15s 15.1s ⚠️ +165ms
nodeModulesSize 67.1 MB 67.1 MB -210 B
Client Bundles (main, webpack, commons) Overall decrease ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
main-HASH.js gzip 6.51 kB 6.59 kB ⚠️ +82 B
webpack-HASH.js gzip 751 B 751 B
19b7e98f51cc..2bd7.js gzip 10.7 kB N/A N/A
framework.HASH.js gzip 39.1 kB 39.1 kB
19b7e98f51cc..2b0d.js gzip N/A 10.6 kB N/A
Overall change 57.1 kB 57.1 kB -19 B
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
main-HASH.module.js gzip 5.59 kB 5.68 kB ⚠️ +83 B
webpack-HASH..dule.js gzip 751 B 751 B
19b7e98f51cc..dule.js gzip 7.1 kB N/A N/A
framework.HA..dule.js gzip 39.1 kB 39.1 kB
19b7e98f51cc..dule.js gzip N/A 7.03 kB N/A
Overall change 52.6 kB 52.6 kB ⚠️ +13 B
Legacy Client Bundles (polyfills)
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
polyfills-HASH.js gzip 26.3 kB 26.3 kB
Overall change 26.3 kB 26.3 kB
Client Build Manifests
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
_buildManifest.js gzip 268 B 268 B
_buildManife..dule.js gzip 272 B 272 B
Overall change 540 B 540 B
Serverless bundles Overall decrease ✓
vercel/next.js canary ijjk/next.js update/next-data-url-handling Change
_error.js 875 kB 875 kB
404.html 4.17 kB 4.17 kB
hooks.html 3.79 kB 3.79 kB
index.js 876 kB 876 kB
link.js 916 kB 915 kB -719 B
routerDirect.js 908 kB 907 kB -719 B
withRouter.js 908 kB 907 kB -719 B
Overall change 4.49 MB 4.49 MB -2.16 kB
Commit: 4e6d869

Copy link
Member

@Timer Timer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@kodiakhq kodiakhq bot merged commit 89ca0d1 into vercel:canary Jun 29, 2020
@Timer Timer deleted the update/next-data-url-handling branch June 29, 2020 15:15
rokinsky pushed a commit to rokinsky/next.js that referenced this pull request Jul 11, 2020
This updates `fetchNextData` to re-use the `getDataHref` function from `page-loader` which has more verbose handling to ensure the correct `/_next/data` URL is built. Re-using this logic ensures the `/_next/data` URL can still be built even when a mismatching `href` and `as` value is provided to `next/link`.

This also fixes a case in `getDataHref` where optional values that weren't provided would fail to build the data href since the check requiring the param be present while interpolating the route values hasn't been updated to allow missing params for optional values.

An additional test case has been added to the prerender suite to ensure the `/_next/data` URL is built correctly when mismatching `href` and `as` values are provided

x-ref: vercel#14536
x-ref: vercel#9081 (reply in thread)
Closes: vercel#14668
@vercel vercel locked as resolved and limited conversation to collaborators Jan 30, 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.

fetchNextData does not support dynamic SSG routes with mismatching href and as
2 participants