-
Notifications
You must be signed in to change notification settings - Fork 27.8k
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
Refactor Page Paths utils and Middleware Plugin #36576
Refactor Page Paths utils and Middleware Plugin #36576
Conversation
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
buildDuration | 19.1s | 18.6s | -451ms |
buildDurationCached | 7.3s | 7.2s | -63ms |
nodeModulesSize | 463 MB | 463 MB |
Page Load Tests Overall decrease ⚠️
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 4.909 | 4.878 | -0.03 |
/ avg req/sec | 509.23 | 512.49 | +3.26 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.668 | 1.702 | |
/error-in-render avg req/sec | 1499.05 | 1468.62 |
Client Bundles (main, webpack) Overall increase ⚠️
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
925.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 28.6 kB | 28.6 kB | |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 72.3 kB | 72.3 kB |
Legacy Client Bundles (polyfills)
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 193 B | 193 B | ✓ |
amp-HASH.js gzip | 308 B | 308 B | ✓ |
css-HASH.js gzip | 327 B | 327 B | ✓ |
dynamic-HASH.js gzip | 3.08 kB | 3.08 kB | ✓ |
head-HASH.js gzip | 359 B | 359 B | ✓ |
hooks-HASH.js gzip | 920 B | 920 B | ✓ |
image-HASH.js gzip | 5.71 kB | 5.71 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.64 kB | 2.64 kB | ✓ |
routerDirect..HASH.js gzip | 320 B | 320 B | ✓ |
script-HASH.js gzip | 391 B | 391 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 16.3 kB | 16.3 kB | ✓ |
Client Build Manifests
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
index.html gzip | 533 B | 533 B | ✓ |
link.html gzip | 546 B | 547 B | |
withRouter.html gzip | 528 B | 529 B | |
Overall change | 1.61 kB | 1.61 kB |
Diffs
Diff for main-HASH.js
@@ -4223,6 +4223,45 @@
/***/
},
+ /***/ 6301: /***/ function(
+ __unused_webpack_module,
+ exports,
+ __webpack_require__
+ ) {
+ "use strict";
+
+ Object.defineProperty(exports, "__esModule", {
+ value: true
+ });
+ exports.denormalizePagePath = denormalizePagePath;
+ var _utils = __webpack_require__(8588);
+ var _normalizePathSep = __webpack_require__(9997);
+ function denormalizePagePath(page) {
+ var _page = (0, _normalizePathSep).normalizePathSep(page);
+ return _page.startsWith("/index/") && !(0, _utils).isDynamicRoute(_page)
+ ? _page.slice(6)
+ : _page !== "/index"
+ ? _page
+ : "/";
+ } //# sourceMappingURL=denormalize-page-path.js.map
+
+ /***/
+ },
+
+ /***/ 9997: /***/ function(__unused_webpack_module, exports) {
+ "use strict";
+
+ Object.defineProperty(exports, "__esModule", {
+ value: true
+ });
+ exports.normalizePathSep = normalizePathSep;
+ function normalizePathSep(path) {
+ return path.replace(/\\/g, "/");
+ } //# sourceMappingURL=normalize-path-sep.js.map
+
+ /***/
+ },
+
/***/ 2692: /***/ function(
__unused_webpack_module,
exports,
@@ -4446,7 +4485,7 @@
var _routeLoader = __webpack_require__(2497);
var _script = __webpack_require__(3573);
var _isError = _interopRequireWildcard(__webpack_require__(676));
- var _denormalizePagePath = __webpack_require__(4522);
+ var _denormalizePagePath = __webpack_require__(6301);
var _normalizeLocalePath = __webpack_require__(4769);
var _mitt = _interopRequireDefault1(__webpack_require__(8550));
var _utils = __webpack_require__(670);
@@ -9534,37 +9573,6 @@
/***/
},
- /***/ 4522: /***/ function(
- __unused_webpack_module,
- exports,
- __webpack_require__
- ) {
- "use strict";
-
- Object.defineProperty(exports, "__esModule", {
- value: true
- });
- exports.normalizePathSep = normalizePathSep;
- exports.denormalizePagePath = denormalizePagePath;
- var _utils = __webpack_require__(8588);
- function normalizePathSep(path) {
- return path.replace(/\\/g, "/");
- }
- function denormalizePagePath(page) {
- page = normalizePathSep(page);
- if (page.startsWith("/index/") && !(0, _utils).isDynamicRoute(page)) {
- page = page.slice(6);
- } else if (page === "/index") {
- page = "/";
- }
- return page;
- }
-
- //# sourceMappingURL=denormalize-page-path.js.map
-
- /***/
- },
-
/***/ 2431: /***/ function() {
/* (ignored) */
/***/
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
Default Build with SWC (Increase detected ⚠️ )
General Overall increase ⚠️
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
buildDuration | 20.6s | 20.4s | -174ms |
buildDurationCached | 7.1s | 7.1s | -69ms |
nodeModulesSize | 463 MB | 463 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 4.728 | 4.344 | -0.38 |
/ avg req/sec | 528.82 | 575.48 | +46.66 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.698 | 1.701 | 0 |
/error-in-render avg req/sec | 1472.43 | 1469.36 |
Client Bundles (main, webpack) Overall decrease ✓
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
925.HASH.js gzip | 178 B | 178 B | ✓ |
framework-HASH.js gzip | 42.2 kB | 42.2 kB | ✓ |
main-HASH.js gzip | 29.1 kB | 29 kB | -5 B |
webpack-HASH.js gzip | 1.45 kB | 1.45 kB | ✓ |
Overall change | 72.9 kB | 72.9 kB | -5 B |
Legacy Client Bundles (polyfills)
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 179 B | 179 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 324 B | 324 B | ✓ |
dynamic-HASH.js gzip | 3.08 kB | 3.08 kB | ✓ |
head-HASH.js gzip | 357 B | 357 B | ✓ |
hooks-HASH.js gzip | 921 B | 921 B | ✓ |
image-HASH.js gzip | 5.76 kB | 5.76 kB | ✓ |
index-HASH.js gzip | 261 B | 261 B | ✓ |
link-HASH.js gzip | 2.76 kB | 2.76 kB | ✓ |
routerDirect..HASH.js gzip | 322 B | 322 B | ✓ |
script-HASH.js gzip | 392 B | 392 B | ✓ |
withRouter-HASH.js gzip | 317 B | 317 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 16.5 kB | 16.5 kB | ✓ |
Client Build Manifests
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
_buildManifest.js gzip | 458 B | 458 B | ✓ |
Overall change | 458 B | 458 B | ✓ |
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary | javivelasco/next.js refactor/page-paths-n-middleware-plugin | Change | |
---|---|---|---|
index.html gzip | 529 B | 528 B | -1 B |
link.html gzip | 543 B | 541 B | -2 B |
withRouter.html gzip | 526 B | 525 B | -1 B |
Overall change | 1.6 kB | 1.59 kB | -4 B |
Diffs
Diff for main-HASH.js
@@ -4223,6 +4223,45 @@
/***/
},
+ /***/ 6301: /***/ function(
+ __unused_webpack_module,
+ exports,
+ __webpack_require__
+ ) {
+ "use strict";
+
+ Object.defineProperty(exports, "__esModule", {
+ value: true
+ });
+ exports.denormalizePagePath = denormalizePagePath;
+ var _utils = __webpack_require__(8588);
+ var _normalizePathSep = __webpack_require__(9997);
+ function denormalizePagePath(page) {
+ var _page = (0, _normalizePathSep).normalizePathSep(page);
+ return _page.startsWith("/index/") && !(0, _utils).isDynamicRoute(_page)
+ ? _page.slice(6)
+ : _page !== "/index"
+ ? _page
+ : "/";
+ } //# sourceMappingURL=denormalize-page-path.js.map
+
+ /***/
+ },
+
+ /***/ 9997: /***/ function(__unused_webpack_module, exports) {
+ "use strict";
+
+ Object.defineProperty(exports, "__esModule", {
+ value: true
+ });
+ exports.normalizePathSep = normalizePathSep;
+ function normalizePathSep(path) {
+ return path.replace(/\\/g, "/");
+ } //# sourceMappingURL=normalize-path-sep.js.map
+
+ /***/
+ },
+
/***/ 2692: /***/ function(
__unused_webpack_module,
exports,
@@ -4446,7 +4485,7 @@
var _routeLoader = __webpack_require__(2497);
var _script = __webpack_require__(3573);
var _isError = _interopRequireWildcard(__webpack_require__(676));
- var _denormalizePagePath = __webpack_require__(4522);
+ var _denormalizePagePath = __webpack_require__(6301);
var _normalizeLocalePath = __webpack_require__(4769);
var _mitt = _interopRequireDefault1(__webpack_require__(8550));
var _utils = __webpack_require__(670);
@@ -9534,37 +9573,6 @@
/***/
},
- /***/ 4522: /***/ function(
- __unused_webpack_module,
- exports,
- __webpack_require__
- ) {
- "use strict";
-
- Object.defineProperty(exports, "__esModule", {
- value: true
- });
- exports.normalizePathSep = normalizePathSep;
- exports.denormalizePagePath = denormalizePagePath;
- var _utils = __webpack_require__(8588);
- function normalizePathSep(path) {
- return path.replace(/\\/g, "/");
- }
- function denormalizePagePath(page) {
- page = normalizePathSep(page);
- if (page.startsWith("/index/") && !(0, _utils).isDynamicRoute(page)) {
- page = page.slice(6);
- } else if (page === "/index") {
- page = "/";
- }
- return page;
- }
-
- //# sourceMappingURL=denormalize-page-path.js.map
-
- /***/
- },
-
/***/ 2431: /***/ function() {
/* (ignored) */
/***/
Diff for index.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
Diff for link.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
Diff for withRouter.html
@@ -19,7 +19,7 @@
defer=""
></script>
<script
- src="/_next/static/chunks/main-60883fd45bd4ecbd.js"
+ src="/_next/static/chunks/main-69b0414e03e5e1c2.js"
defer=""
></script>
<script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this increased client-side bundles quite a bit +2.7 kB
gzipped. Could we prevent this by ensuring methods used client-side are separated out from non-client side needed ones?
Of course @ijjk! Any pointers on which one is the entry that causes it? |
This comment was marked as outdated.
This comment was marked as outdated.
d36a7d5
to
60e92c9
Compare
60e92c9
to
f6020f0
Compare
f6020f0
to
158fb00
Compare
@ijjk In the latest commit I've split the helper into multiple files and move them to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR brings some significant refactoring in preparation for upcoming middleware changes. Each commit can be reviewed independently, here is a summary of what each one does and the reasoning behind it:
pagesDir
property to the dev server which is the only place where it is needed. Having it for every server is misleading.getPagePaths
that is used to generate an array of paths to inspect when looking for a page file fromfindPageFile
. Then it refactors such function to use it parallelizing lookups. This will allow us to print every path we look at when looking for a file which can be useful for debugging. It also adds aflatten
helper.next-dev-server
that transforms an absolute path into a page name. Of course it adds comments, parameters and examples.ssrEntries
that was deprecated in favour of module metadata added to Webpack from loaders keeping less dependencies. It also adds types and makes a clear distinction between phases where we statically analyze the code, find metadata and generate the manifest file cc @shuding @huozhiEDIT:
shared/lib
in order to make explicit that those can be also imported from client.