Make loadPage track success of script loading
#16334
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Stats from current PRDefault Server Mode (Increase detected
|
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| buildDuration | 12.3s | 12.3s | -26ms |
| nodeModulesSize | 57.7 MB | 57.7 MB |
Page Load Tests Overall increase ✓
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 2.163 | 2.229 | |
| / avg req/sec | 1155.66 | 1121.52 | |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.216 | 1.185 | -0.03 |
| /error-in-render avg req/sec | 2055.45 | 2108.88 | +53.43 |
Client Bundles (main, webpack, commons) Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..4e55.js gzip | 10.2 kB | 10.2 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-8da9cf5..55dc.js gzip | 7.18 kB | 7.21 kB | |
| webpack-e067..f178.js gzip | 751 B | 751 B | ✓ |
| Overall change | 57.1 kB | 57.2 kB |
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..dule.js gzip | 6.11 kB | 6.11 kB | ✓ |
| framework.HA..dule.js gzip | 39 kB | 39 kB | ✓ |
| main-e458620..dule.js gzip | 6.2 kB | 6.21 kB | |
| webpack-07c5..dule.js gzip | 751 B | 751 B | ✓ |
| Overall change | 52 kB | 52 kB |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| polyfills-4b..e242.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-9a0b9e1..b37e.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-1464c..a26f.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-000f151..65d4.js gzip | 1.29 kB | 1.29 kB | ✓ |
| routerDirect..924c.js gzip | 284 B | 284 B | ✓ |
| withRouter-7..c13d.js gzip | 284 B | 284 B | ✓ |
| Overall change | 7.69 kB | 7.69 kB | ✓ |
Client Pages Modern
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-75d3a82..dule.js gzip | 625 B | 625 B | ✓ |
| _error-e550f..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-4cfda7a..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.35 kB | 5.35 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | Timer/next.js refactor/page-loader | 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 | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| index.html gzip | 947 B | 949 B | |
| link.html gzip | 953 B | 953 B | ✓ |
| withRouter.html gzip | 938 B | 940 B | |
| Overall change | 2.84 kB | 2.84 kB |
Diffs
Diff for main-02c0657..76.module.js
@@ -1313,6 +1313,24 @@
return res;
}
+ function loadScript(url) {
+ return new Promise((res, rej) => {
+ var script = document.createElement("script");
+
+ if (true && hasNoModule) {
+ script.type = "module";
+ }
+
+ script.crossOrigin = "anonymous";
+ script.src = url;
+ script.onload = res;
+
+ script.onerror = () => rej(pageLoadError(url));
+
+ document.body.appendChild(script);
+ });
+ }
+
class PageLoader {
constructor(buildId, assetPrefix, initialPage, initialStyleSheets) {
this.initialPage = void 0;
@@ -1377,10 +1395,7 @@
? m[route].map(url =>
"".concat(this.assetPrefix, "/_next/").concat(encodeURI(url))
)
- : (this.pageRegisterEvents.emit(route, {
- error: pageLoadError(route)
- }),
- []);
+ : Promise.reject(pageLoadError(route));
});
}
/**
@@ -1514,57 +1529,55 @@
this.loadingRoutes[route] = true;
if (true) {
- this.getDependencies(route).then(deps => {
- deps.forEach(d => {
- if (
- d.endsWith(".js") &&
- !document.querySelector('script[src^="'.concat(d, '"]'))
- ) {
- this.loadScript(d, route);
- } // Prefetch CSS as it'll be needed when the page JavaScript
- // evaluates. This will only trigger if explicit prefetching is
- // disabled for a <Link>... prefetching in this case is desirable
- // because we *know* it's going to be used very soon (page was
- // loaded).
-
- if (
- d.endsWith(".css") &&
- !document.querySelector(
- 'link[rel="'
- .concat(relPreload, '"][href^="')
- .concat(d, '"]')
- )
- ) {
- appendLink(d, relPreload, "style").catch(() => {
- /* ignore preload error */
- });
- }
+ this.getDependencies(route)
+ .then(deps => {
+ var pending = [];
+ deps.forEach(d => {
+ if (
+ d.endsWith(".js") &&
+ !document.querySelector('script[src^="'.concat(d, '"]'))
+ ) {
+ pending.push(loadScript(d));
+ } // Prefetch CSS as it'll be needed when the page JavaScript
+ // evaluates. This will only trigger if explicit prefetching is
+ // disabled for a <Link>... prefetching in this case is desirable
+ // because we *know* it's going to be used very soon (page was
+ // loaded).
+
+ if (
+ d.endsWith(".css") &&
+ !document.querySelector(
+ 'link[rel="'
+ .concat(relPreload, '"][href^="')
+ .concat(d, '"]')
+ )
+ ) {
+ // This is not pushed into `pending` because we don't need to
+ // wait for these to resolve. To prevent an unhandled
+ // rejection, we swallow the error which is handled later in
+ // the rendering cycle (this is just a preload optimization).
+ appendLink(d, relPreload, "style").catch(() => {
+ /* ignore preload error */
+ });
+ }
+ });
+ return Promise.all(pending);
+ })
+ .catch(err => {
+ // Mark the page as failed to load if any of its required scripts
+ // fail to load:
+ this.pageCache[route] = {
+ error: err
+ };
+ fire({
+ error: err
+ });
});
- });
} else {
var url, scriptRoute;
}
}
});
- }
-
- loadScript(url, route) {
- var script = document.createElement("script");
-
- if (true && hasNoModule) {
- script.type = "module";
- }
-
- script.crossOrigin = "anonymous";
- script.src = url;
-
- script.onerror = () => {
- this.pageRegisterEvents.emit(route, {
- error: pageLoadError(url)
- });
- };
-
- document.body.appendChild(script);
} // This method if called by the route code.
registerPage(route, regFn) {Diff for main-e9105a4..cb75348e7.js
@@ -1697,6 +1697,26 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
return res;
}
+ function loadScript(url) {
+ return new Promise(function(res, rej) {
+ var script = document.createElement("script");
+
+ if (true && hasNoModule) {
+ script.type = "module";
+ }
+
+ script.crossOrigin = "anonymous";
+ script.src = url;
+ script.onload = res;
+
+ script.onerror = function() {
+ return rej(pageLoadError(url));
+ };
+
+ document.body.appendChild(script);
+ });
+ }
+
var PageLoader = /*#__PURE__*/ (function() {
function PageLoader(
buildId,
@@ -1777,10 +1797,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
.concat(_this.assetPrefix, "/_next/")
.concat(encodeURI(url));
})
- : (_this.pageRegisterEvents.emit(route, {
- error: pageLoadError(route)
- }),
- []);
+ : Promise.reject(pageLoadError(route));
});
}
/**
@@ -1944,71 +1961,66 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
_this4.loadingRoutes[route] = true;
if (true) {
- _this4.getDependencies(route).then(function(deps) {
- deps.forEach(function(d) {
- if (
- d.endsWith(".js") &&
- !document.querySelector(
- 'script[src^="'.concat(d, '"]')
- )
- ) {
- _this4.loadScript(d, route);
- } // Prefetch CSS as it'll be needed when the page JavaScript
- // evaluates. This will only trigger if explicit prefetching is
- // disabled for a <Link>... prefetching in this case is desirable
- // because we *know* it's going to be used very soon (page was
- // loaded).
-
- if (
- d.endsWith(".css") &&
- !document.querySelector(
- 'link[rel="'
- .concat(relPreload, '"][href^="')
- .concat(d, '"]')
- )
- ) {
- appendLink(d, relPreload, "style")["catch"](
- function() {
- /* ignore preload error */
- }
- );
- }
+ _this4
+ .getDependencies(route)
+ .then(function(deps) {
+ var pending = [];
+ deps.forEach(function(d) {
+ if (
+ d.endsWith(".js") &&
+ !document.querySelector(
+ 'script[src^="'.concat(d, '"]')
+ )
+ ) {
+ pending.push(loadScript(d));
+ } // Prefetch CSS as it'll be needed when the page JavaScript
+ // evaluates. This will only trigger if explicit prefetching is
+ // disabled for a <Link>... prefetching in this case is desirable
+ // because we *know* it's going to be used very soon (page was
+ // loaded).
+
+ if (
+ d.endsWith(".css") &&
+ !document.querySelector(
+ 'link[rel="'
+ .concat(relPreload, '"][href^="')
+ .concat(d, '"]')
+ )
+ ) {
+ // This is not pushed into `pending` because we don't need to
+ // wait for these to resolve. To prevent an unhandled
+ // rejection, we swallow the error which is handled later in
+ // the rendering cycle (this is just a preload optimization).
+ appendLink(d, relPreload, "style")["catch"](
+ function() {
+ /* ignore preload error */
+ }
+ );
+ }
+ });
+ return Promise.all(pending);
+ })
+ ["catch"](function(err) {
+ // Mark the page as failed to load if any of its required scripts
+ // fail to load:
+ _this4.pageCache[route] = {
+ error: err
+ };
+ fire({
+ error: err
+ });
});
- });
} else {
var url, scriptRoute;
}
}
});
- }
- },
- {
- key: "loadScript",
- value: function loadScript(url, route) {
- var _this5 = this;
-
- var script = document.createElement("script");
-
- if (true && hasNoModule) {
- script.type = "module";
- }
-
- script.crossOrigin = "anonymous";
- script.src = url;
-
- script.onerror = function() {
- _this5.pageRegisterEvents.emit(route, {
- error: pageLoadError(url)
- });
- };
-
- document.body.appendChild(script);
} // This method if called by the route code.
},
{
key: "registerPage",
value: function registerPage(route, regFn) {
- var _this6 = this;
+ var _this5 = this;
var register = function register(styleSheets) {
try {
@@ -2018,15 +2030,15 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
mod: mod,
styleSheets: styleSheets
};
- _this6.pageCache[route] = pageData;
+ _this5.pageCache[route] = pageData;
- _this6.pageRegisterEvents.emit(route, pageData);
+ _this5.pageRegisterEvents.emit(route, pageData);
} catch (error) {
- _this6.pageCache[route] = {
+ _this5.pageCache[route] = {
error: error
};
- _this6.pageRegisterEvents.emit(route, {
+ _this5.pageRegisterEvents.emit(route, {
error: error
});
}
@@ -2052,11 +2064,11 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
);
},
function(error) {
- _this6.pageCache[route] = {
+ _this5.pageCache[route] = {
error: error
};
- _this6.pageRegisterEvents.emit(route, {
+ _this5.pageRegisterEvents.emit(route, {
error: error
});
}
@@ -2070,7 +2082,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
{
key: "prefetch",
value: function prefetch(route, isDependency) {
- var _this7 = this;
+ var _this6 = this;
// https://github.com/GoogleChromeLabs/quicklink/blob/453a661fa1fa940e2d2e044452398e38c67a98fb/src/index.mjs#L115-L118
// License: Apache 2.0
@@ -2112,7 +2124,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
this.getDependencies(route).then(function(urls) {
return Promise.all(
urls.map(function(dependencyUrl) {
- return _this7.prefetch(dependencyUrl, true);
+ return _this6.prefetch(dependencyUrl, true);
})
);
})Diff for index.html
@@ -6,7 +6,7 @@
<meta name="next-head-count" content="2" />
<link
rel="preload"
- href="/_next/static/chunks/main-02c06573f47461638176.module.js"
+ href="/_next/static/chunks/main-779f39f4fe96a9ebe825.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -81,13 +81,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-e9105a4c4bfcb75348e7.js"
+ src="/_next/static/chunks/main-f96ebf7814a35905d943.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-02c06573f47461638176.module.js"
+ src="/_next/static/chunks/main-779f39f4fe96a9ebe825.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/chunks/main-02c06573f47461638176.module.js"
+ href="/_next/static/chunks/main-779f39f4fe96a9ebe825.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -86,13 +86,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-e9105a4c4bfcb75348e7.js"
+ src="/_next/static/chunks/main-f96ebf7814a35905d943.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-02c06573f47461638176.module.js"
+ src="/_next/static/chunks/main-779f39f4fe96a9ebe825.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/chunks/main-02c06573f47461638176.module.js"
+ href="/_next/static/chunks/main-779f39f4fe96a9ebe825.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -81,13 +81,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-e9105a4c4bfcb75348e7.js"
+ src="/_next/static/chunks/main-f96ebf7814a35905d943.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-02c06573f47461638176.module.js"
+ src="/_next/static/chunks/main-779f39f4fe96a9ebe825.module.js"
async=""
crossorigin="anonymous"
type="module"Serverless Mode (Increase detected ⚠️ )
General Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| buildDuration | 13.7s | 13.9s | |
| nodeModulesSize | 57.7 MB | 57.7 MB |
Client Bundles (main, webpack, commons) Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..4e55.js gzip | 10.2 kB | 10.2 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-8da9cf5..55dc.js gzip | 7.18 kB | N/A | N/A |
| webpack-e067..f178.js gzip | 751 B | 751 B | ✓ |
| main-92b2173..1cff.js gzip | N/A | 7.21 kB | N/A |
| Overall change | 57.1 kB | 57.2 kB |
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..dule.js gzip | 6.11 kB | 6.11 kB | ✓ |
| framework.HA..dule.js gzip | 39 kB | 39 kB | ✓ |
| main-e458620..dule.js gzip | 6.2 kB | N/A | N/A |
| webpack-07c5..dule.js gzip | 751 B | 751 B | ✓ |
| main-13b0492..dule.js gzip | N/A | 6.21 kB | N/A |
| Overall change | 52 kB | 52 kB |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| polyfills-4b..e242.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-9a0b9e1..b37e.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-1464c..a26f.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-000f151..65d4.js gzip | 1.29 kB | 1.29 kB | ✓ |
| routerDirect..924c.js gzip | 284 B | 284 B | ✓ |
| withRouter-7..c13d.js gzip | 284 B | 284 B | ✓ |
| Overall change | 7.69 kB | 7.69 kB | ✓ |
Client Pages Modern
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-75d3a82..dule.js gzip | 625 B | 625 B | ✓ |
| _error-e550f..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-4cfda7a..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.35 kB | 5.35 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | Timer/next.js refactor/page-loader | 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 | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _error.js | 1.03 MB | 1.03 MB | ✓ |
| 404.html | 4.18 kB | 4.18 kB | ✓ |
| hooks.html | 3.82 kB | 3.82 kB | ✓ |
| index.js | 1.03 MB | 1.03 MB | ✓ |
| link.js | 1.07 MB | 1.07 MB | ✓ |
| routerDirect.js | 1.07 MB | 1.07 MB | ✓ |
| withRouter.js | 1.07 MB | 1.07 MB | ✓ |
| Overall change | 5.27 MB | 5.27 MB | ✓ |
Timer
force-pushed
the
refactor/page-loader
branch
from
8aeb4b0 to
4a5e0ee
Compare
Stats from current PRDefault Server Mode (Increase detected
|
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| buildDuration | 10.4s | 10.2s | -123ms |
| nodeModulesSize | 57.7 MB | 57.7 MB |
Page Load Tests Overall decrease ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| / failed reqs | 0 | 0 | ✓ |
| / total time (seconds) | 1.894 | 1.861 | -0.03 |
| / avg req/sec | 1319.67 | 1343.69 | +24.02 |
| /error-in-render failed reqs | 0 | 0 | ✓ |
| /error-in-render total time (seconds) | 1.031 | 1.062 | |
| /error-in-render avg req/sec | 2423.83 | 2353.69 |
Client Bundles (main, webpack, commons) Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..4e55.js gzip | 10.2 kB | 10.2 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-ff9b23f..5a93.js gzip | 7.17 kB | 7.2 kB | |
| webpack-e067..f178.js gzip | 751 B | 751 B | ✓ |
| Overall change | 57.1 kB | 57.2 kB |
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..dule.js gzip | 6.11 kB | 6.11 kB | ✓ |
| framework.HA..dule.js gzip | 39 kB | 39 kB | ✓ |
| main-b54ffe3..dule.js gzip | 6.2 kB | 6.21 kB | |
| webpack-07c5..dule.js gzip | 751 B | 751 B | ✓ |
| Overall change | 52 kB | 52 kB |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| polyfills-4b..e242.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-9a0b9e1..b37e.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-1464c..a26f.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-000f151..65d4.js gzip | 1.29 kB | 1.29 kB | ✓ |
| routerDirect..924c.js gzip | 284 B | 284 B | ✓ |
| withRouter-7..c13d.js gzip | 284 B | 284 B | ✓ |
| Overall change | 7.69 kB | 7.69 kB | ✓ |
Client Pages Modern
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-75d3a82..dule.js gzip | 625 B | 625 B | ✓ |
| _error-e550f..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-4cfda7a..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.35 kB | 5.35 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | Timer/next.js refactor/page-loader | 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 decrease ✓
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| index.html gzip | 950 B | 947 B | -3 B |
| link.html gzip | 955 B | 953 B | -2 B |
| withRouter.html gzip | 941 B | 938 B | -3 B |
| Overall change | 2.85 kB | 2.84 kB | -8 B |
Diffs
Diff for main-795929d..68.module.js
@@ -1313,6 +1313,24 @@
return res;
}
+ function loadScript(url) {
+ return new Promise((res, rej) => {
+ var script = document.createElement("script");
+
+ if (true && hasNoModule) {
+ script.type = "module";
+ }
+
+ script.crossOrigin = "anonymous";
+ script.src = url;
+ script.onload = res;
+
+ script.onerror = () => rej(pageLoadError(url));
+
+ document.body.appendChild(script);
+ });
+ }
+
class PageLoader {
constructor(buildId, assetPrefix, initialPage, initialStyleSheets) {
this.initialPage = void 0;
@@ -1377,10 +1395,7 @@
? m[route].map(url =>
"".concat(this.assetPrefix, "/_next/").concat(encodeURI(url))
)
- : (this.pageRegisterEvents.emit(route, {
- error: pageLoadError(route)
- }),
- []);
+ : Promise.reject(pageLoadError(route));
});
}
/**
@@ -1514,57 +1529,55 @@
this.loadingRoutes[route] = true;
if (true) {
- this.getDependencies(route).then(deps => {
- deps.forEach(d => {
- if (
- d.endsWith(".js") &&
- !document.querySelector('script[src^="'.concat(d, '"]'))
- ) {
- this.loadScript(d, route);
- } // Prefetch CSS as it'll be needed when the page JavaScript
- // evaluates. This will only trigger if explicit prefetching is
- // disabled for a <Link>... prefetching in this case is desirable
- // because we *know* it's going to be used very soon (page was
- // loaded).
-
- if (
- d.endsWith(".css") &&
- !document.querySelector(
- 'link[rel="'
- .concat(relPreload, '"][href^="')
- .concat(d, '"]')
- )
- ) {
- appendLink(d, relPreload, "style").catch(() => {
- /* ignore preload error */
- });
- }
+ this.getDependencies(route)
+ .then(deps => {
+ var pending = [];
+ deps.forEach(d => {
+ if (
+ d.endsWith(".js") &&
+ !document.querySelector('script[src^="'.concat(d, '"]'))
+ ) {
+ pending.push(loadScript(d));
+ } // Prefetch CSS as it'll be needed when the page JavaScript
+ // evaluates. This will only trigger if explicit prefetching is
+ // disabled for a <Link>... prefetching in this case is desirable
+ // because we *know* it's going to be used very soon (page was
+ // loaded).
+
+ if (
+ d.endsWith(".css") &&
+ !document.querySelector(
+ 'link[rel="'
+ .concat(relPreload, '"][href^="')
+ .concat(d, '"]')
+ )
+ ) {
+ // This is not pushed into `pending` because we don't need to
+ // wait for these to resolve. To prevent an unhandled
+ // rejection, we swallow the error which is handled later in
+ // the rendering cycle (this is just a preload optimization).
+ appendLink(d, relPreload, "style").catch(() => {
+ /* ignore preload error */
+ });
+ }
+ });
+ return Promise.all(pending);
+ })
+ .catch(err => {
+ // Mark the page as failed to load if any of its required scripts
+ // fail to load:
+ this.pageCache[route] = {
+ error: err
+ };
+ fire({
+ error: err
+ });
});
- });
} else {
var url, scriptRoute;
}
}
});
- }
-
- loadScript(url, route) {
- var script = document.createElement("script");
-
- if (true && hasNoModule) {
- script.type = "module";
- }
-
- script.crossOrigin = "anonymous";
- script.src = url;
-
- script.onerror = () => {
- this.pageRegisterEvents.emit(route, {
- error: pageLoadError(url)
- });
- };
-
- document.body.appendChild(script);
} // This method if called by the route code.
registerPage(route, regFn) {Diff for main-fbef6ed..30274b71a.js
@@ -1697,6 +1697,26 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
return res;
}
+ function loadScript(url) {
+ return new Promise(function(res, rej) {
+ var script = document.createElement("script");
+
+ if (true && hasNoModule) {
+ script.type = "module";
+ }
+
+ script.crossOrigin = "anonymous";
+ script.src = url;
+ script.onload = res;
+
+ script.onerror = function() {
+ return rej(pageLoadError(url));
+ };
+
+ document.body.appendChild(script);
+ });
+ }
+
var PageLoader = /*#__PURE__*/ (function() {
function PageLoader(
buildId,
@@ -1777,10 +1797,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
.concat(_this.assetPrefix, "/_next/")
.concat(encodeURI(url));
})
- : (_this.pageRegisterEvents.emit(route, {
- error: pageLoadError(route)
- }),
- []);
+ : Promise.reject(pageLoadError(route));
});
}
/**
@@ -1944,71 +1961,66 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
_this4.loadingRoutes[route] = true;
if (true) {
- _this4.getDependencies(route).then(function(deps) {
- deps.forEach(function(d) {
- if (
- d.endsWith(".js") &&
- !document.querySelector(
- 'script[src^="'.concat(d, '"]')
- )
- ) {
- _this4.loadScript(d, route);
- } // Prefetch CSS as it'll be needed when the page JavaScript
- // evaluates. This will only trigger if explicit prefetching is
- // disabled for a <Link>... prefetching in this case is desirable
- // because we *know* it's going to be used very soon (page was
- // loaded).
-
- if (
- d.endsWith(".css") &&
- !document.querySelector(
- 'link[rel="'
- .concat(relPreload, '"][href^="')
- .concat(d, '"]')
- )
- ) {
- appendLink(d, relPreload, "style")["catch"](
- function() {
- /* ignore preload error */
- }
- );
- }
+ _this4
+ .getDependencies(route)
+ .then(function(deps) {
+ var pending = [];
+ deps.forEach(function(d) {
+ if (
+ d.endsWith(".js") &&
+ !document.querySelector(
+ 'script[src^="'.concat(d, '"]')
+ )
+ ) {
+ pending.push(loadScript(d));
+ } // Prefetch CSS as it'll be needed when the page JavaScript
+ // evaluates. This will only trigger if explicit prefetching is
+ // disabled for a <Link>... prefetching in this case is desirable
+ // because we *know* it's going to be used very soon (page was
+ // loaded).
+
+ if (
+ d.endsWith(".css") &&
+ !document.querySelector(
+ 'link[rel="'
+ .concat(relPreload, '"][href^="')
+ .concat(d, '"]')
+ )
+ ) {
+ // This is not pushed into `pending` because we don't need to
+ // wait for these to resolve. To prevent an unhandled
+ // rejection, we swallow the error which is handled later in
+ // the rendering cycle (this is just a preload optimization).
+ appendLink(d, relPreload, "style")["catch"](
+ function() {
+ /* ignore preload error */
+ }
+ );
+ }
+ });
+ return Promise.all(pending);
+ })
+ ["catch"](function(err) {
+ // Mark the page as failed to load if any of its required scripts
+ // fail to load:
+ _this4.pageCache[route] = {
+ error: err
+ };
+ fire({
+ error: err
+ });
});
- });
} else {
var url, scriptRoute;
}
}
});
- }
- },
- {
- key: "loadScript",
- value: function loadScript(url, route) {
- var _this5 = this;
-
- var script = document.createElement("script");
-
- if (true && hasNoModule) {
- script.type = "module";
- }
-
- script.crossOrigin = "anonymous";
- script.src = url;
-
- script.onerror = function() {
- _this5.pageRegisterEvents.emit(route, {
- error: pageLoadError(url)
- });
- };
-
- document.body.appendChild(script);
} // This method if called by the route code.
},
{
key: "registerPage",
value: function registerPage(route, regFn) {
- var _this6 = this;
+ var _this5 = this;
var register = function register(styleSheets) {
try {
@@ -2018,15 +2030,15 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
mod: mod,
styleSheets: styleSheets
};
- _this6.pageCache[route] = pageData;
+ _this5.pageCache[route] = pageData;
- _this6.pageRegisterEvents.emit(route, pageData);
+ _this5.pageRegisterEvents.emit(route, pageData);
} catch (error) {
- _this6.pageCache[route] = {
+ _this5.pageCache[route] = {
error: error
};
- _this6.pageRegisterEvents.emit(route, {
+ _this5.pageRegisterEvents.emit(route, {
error: error
});
}
@@ -2052,11 +2064,11 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
);
},
function(error) {
- _this6.pageCache[route] = {
+ _this5.pageCache[route] = {
error: error
};
- _this6.pageRegisterEvents.emit(route, {
+ _this5.pageRegisterEvents.emit(route, {
error: error
});
}
@@ -2070,7 +2082,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
{
key: "prefetch",
value: function prefetch(route, isDependency) {
- var _this7 = this;
+ var _this6 = this;
// https://github.com/GoogleChromeLabs/quicklink/blob/453a661fa1fa940e2d2e044452398e38c67a98fb/src/index.mjs#L115-L118
// License: Apache 2.0
@@ -2112,7 +2124,7 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([
this.getDependencies(route).then(function(urls) {
return Promise.all(
urls.map(function(dependencyUrl) {
- return _this7.prefetch(dependencyUrl, true);
+ return _this6.prefetch(dependencyUrl, true);
})
);
})Diff for index.html
@@ -6,7 +6,7 @@
<meta name="next-head-count" content="2" />
<link
rel="preload"
- href="/_next/static/chunks/main-795929d87ed798e41168.module.js"
+ href="/_next/static/chunks/main-2ff3f81fff451af57216.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -81,13 +81,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-fbef6ed026230274b71a.js"
+ src="/_next/static/chunks/main-74c305815b3a3f708e65.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-795929d87ed798e41168.module.js"
+ src="/_next/static/chunks/main-2ff3f81fff451af57216.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/chunks/main-795929d87ed798e41168.module.js"
+ href="/_next/static/chunks/main-2ff3f81fff451af57216.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -86,13 +86,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-fbef6ed026230274b71a.js"
+ src="/_next/static/chunks/main-74c305815b3a3f708e65.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-795929d87ed798e41168.module.js"
+ src="/_next/static/chunks/main-2ff3f81fff451af57216.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/chunks/main-795929d87ed798e41168.module.js"
+ href="/_next/static/chunks/main-2ff3f81fff451af57216.module.js"
as="script"
crossorigin="anonymous"
/>
@@ -81,13 +81,13 @@
src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js"
></script>
<script
- src="/_next/static/chunks/main-fbef6ed026230274b71a.js"
+ src="/_next/static/chunks/main-74c305815b3a3f708e65.js"
async=""
crossorigin="anonymous"
nomodule=""
></script>
<script
- src="/_next/static/chunks/main-795929d87ed798e41168.module.js"
+ src="/_next/static/chunks/main-2ff3f81fff451af57216.module.js"
async=""
crossorigin="anonymous"
type="module"Serverless Mode (Increase detected ⚠️ )
General Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| buildDuration | 11.6s | 11.6s | -24ms |
| nodeModulesSize | 57.7 MB | 57.7 MB |
Client Bundles (main, webpack, commons) Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..4e55.js gzip | 10.2 kB | 10.2 kB | ✓ |
| framework.HASH.js gzip | 39 kB | 39 kB | ✓ |
| main-ff9b23f..5a93.js gzip | 7.17 kB | N/A | N/A |
| webpack-e067..f178.js gzip | 751 B | 751 B | ✓ |
| main-6b32d04..be2c.js gzip | N/A | 7.2 kB | N/A |
| Overall change | 57.1 kB | 57.2 kB |
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| 677f882d2ed8..dule.js gzip | 6.11 kB | 6.11 kB | ✓ |
| framework.HA..dule.js gzip | 39 kB | 39 kB | ✓ |
| main-b54ffe3..dule.js gzip | 6.2 kB | N/A | N/A |
| webpack-07c5..dule.js gzip | 751 B | 751 B | ✓ |
| main-397a2a0..dule.js gzip | N/A | 6.21 kB | N/A |
| Overall change | 52 kB | 52 kB |
Legacy Client Bundles (polyfills)
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| polyfills-4b..e242.js gzip | 31 kB | 31 kB | ✓ |
| Overall change | 31 kB | 31 kB | ✓ |
Client Pages
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-9a0b9e1..b37e.js gzip | 1.28 kB | 1.28 kB | ✓ |
| _error-1464c..a26f.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-000f151..65d4.js gzip | 1.29 kB | 1.29 kB | ✓ |
| routerDirect..924c.js gzip | 284 B | 284 B | ✓ |
| withRouter-7..c13d.js gzip | 284 B | 284 B | ✓ |
| Overall change | 7.69 kB | 7.69 kB | ✓ |
Client Pages Modern
| vercel/next.js canary | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _app-75d3a82..dule.js gzip | 625 B | 625 B | ✓ |
| _error-e550f..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-4cfda7a..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.35 kB | 5.35 kB | ✓ |
Client Build Manifests
| vercel/next.js canary | Timer/next.js refactor/page-loader | 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 | Timer/next.js refactor/page-loader | Change | |
|---|---|---|---|
| _error.js | 1.03 MB | 1.03 MB | ✓ |
| 404.html | 4.18 kB | 4.18 kB | ✓ |
| hooks.html | 3.82 kB | 3.82 kB | ✓ |
| index.js | 1.03 MB | 1.03 MB | ✓ |
| link.js | 1.07 MB | 1.07 MB | ✓ |
| routerDirect.js | 1.07 MB | 1.07 MB | ✓ |
| withRouter.js | 1.07 MB | 1.07 MB | ✓ |
| Overall change | 5.27 MB | 5.27 MB | ✓ |
timneutkens
approved these changes
Aug 19, 2020
m-lautenbach
pushed a commit
to m-lautenbach/next.js
that referenced
this pull request
Aug 20, 2020
Prior to this PR, `loadPage` would call `loadScript` which would then report if the script failed to load. This was problematic because `loadScript` notified a failure to load via `pageRegisterEvents`, which would not set the `pageCache` value for future requests. This means a one-off promise rejection would happen, [in lieu of being] typically consumed within the client-side router, causing a server-side reload. However, when `loadPage` was used independently (i.e. to preload pages), this promise rejection would be ignored as a preload failure. When the real routing request comes in, the `loadPage` function skips its attempt to load the `<script>` because it was already in the DOM, and the router would stop functioning. --- To fix this behavior, I've removed erroneous emits on `pageRegisterEvents` to only happen during the page registration lifecycle (its intended use). The new behavior is that `loadScript` returns a `Promise` that `loadPage` can track, and if any of the page(s) scripts fail to load, we mark the entire page as errored in `pageCache`. This ensures future requests to `loadPage` will always immediately reject with a `PAGE_LOAD_ERROR`, which causes the server-side redirect at the appropriate point. --- Fixes vercel#16333
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this PR,
loadPagewould callloadScriptwhich would then report if the script failed to load.This was problematic because
loadScriptnotified a failure to load viapageRegisterEvents, which would not set thepageCachevalue for future requests.This means a one-off promise rejection would happen, [in lieu of being] typically consumed within the client-side router, causing a server-side reload.
However, when
loadPagewas used independently (i.e. to preload pages), this promise rejection would be ignored as a preload failure.When the real routing request comes in, the
loadPagefunction skips its attempt to load the<script>because it was already in the DOM, and the router would stop functioning.To fix this behavior, I've removed erroneous emits on
pageRegisterEventsto only happen during the page registration lifecycle (its intended use).The new behavior is that
loadScriptreturns aPromisethatloadPagecan track, and if any of the page(s) scripts fail to load, we mark the entire page as errored inpageCache. This ensures future requests toloadPagewill always immediately reject with aPAGE_LOAD_ERROR, which causes the server-side redirect at the appropriate point.Fixes #16333