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

Improve the transform and re-enable optimizeServerReact #70101

Merged
merged 5 commits into from
Sep 16, 2024

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Sep 14, 2024

What

Revert the temporary disabling of #69788, use a different transform that using an process env variable as condition to && with react hooks in SSR layer, and remove drop the call by DCE during minification phase.

Introduce a new internal env process.env.__NEXT_PRIVATE_MINIMIZE_MARCO_FALSE as a condition in the SWC transform of optimizeServerReact and then it will be replaced as false in minification, minifier can identify it as a condition that able to be dropped. The react hook call after it will also get dropped. This is similar to the swc transform erasing but just a bit late : )

Why

We originally disable the config in #69788 because react actions are dropped too early along with the react hooks useEffect on server side transform. Since we still need to go through the module graph to collect action modules and imported exports, they still need to be present but we can still remove them later.

Solution Evaluation Phases

During compilation phase, most simple env replacement and literals can be optimized by SWC (compiler) and webpack (bundler). Using anything like numbers (1, 0), booleans or process.env.NODE_ENV will get early optimized.

process.env.NODE_ENV !== 'production' && useEffect(() => { ... })

Will be instantly transpiled to false during bundling as the optimization can be applied before the module graph is created.

Hence I switched to a new transform below, ensure that useEffect and its callback body are still presented while gathering modules.

process.env.__NEXT_PRIVATE_MINIMIZE_MARCO_FALSE && useEffect(() => { ... })

Then minifier will replace the env var as false, also drop the following hook call with DCE.

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. Turbopack Related to Turbopack with Next.js. type: next labels Sep 14, 2024
@huozhi huozhi changed the title cond react transform wip test Sep 14, 2024
Copy link
Member Author

huozhi commented Sep 14, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @huozhi and the rest of your teammates on Graphite Graphite

@ijjk
Copy link
Member

ijjk commented Sep 14, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Sep 14, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
buildDuration 17.8s 15.7s N/A
buildDurationCached 8.7s 7.2s N/A
nodeModulesSize 358 MB 358 MB ⚠️ +1.61 kB
nextStartRea..uration (ms) 434ms 434ms
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
6784.HASH.js gzip 169 B 168 B N/A
69c16c59-HASH.js gzip 52.8 kB 52.8 kB N/A
8954-HASH.js gzip 42.5 kB 42.5 kB N/A
8966-HASH.js gzip 5.26 kB 5.25 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 224 B 223 B N/A
main-HASH.js gzip 32.6 kB 32.6 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 192 B 192 B
amp-HASH.js gzip 512 B 512 B
css-HASH.js gzip 343 B 343 B
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 266 B N/A
head-HASH.js gzip 365 B 365 B
hooks-HASH.js gzip 391 B 391 B
image-HASH.js gzip 4.4 kB 4.4 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.81 kB 2.81 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 396 B N/A
withRouter-HASH.js gzip 322 B 324 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 4.54 kB 4.54 kB
Client Build Manifests
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
_buildManifest.js gzip 749 B 750 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
index.html gzip 522 B 523 B N/A
link.html gzip 537 B 536 B N/A
withRouter.html gzip 519 B 518 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 180 kB 179 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
middleware-b..fest.js gzip 669 B 668 B N/A
middleware-r..fest.js gzip 156 B 157 B N/A
middleware.js gzip 29.8 kB 29.8 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
973-experime...dev.js gzip 322 B 322 B
973.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 317 kB 317 kB
app-page-exp..prod.js gzip 125 kB 125 kB
app-page-tur..prod.js gzip 139 kB 139 kB
app-page-tur..prod.js gzip 134 kB 134 kB
app-page.run...dev.js gzip 308 kB 308 kB
app-page.run..prod.js gzip 121 kB 121 kB
app-route-ex...dev.js gzip 31.3 kB 31.3 kB
app-route-ex..prod.js gzip 21.1 kB 21.1 kB
app-route-tu..prod.js gzip 21.1 kB 21.1 kB
app-route-tu..prod.js gzip 21 kB 21 kB
app-route.ru...dev.js gzip 32.9 kB 32.9 kB
app-route.ru..prod.js gzip 21 kB 21 kB
pages-api-tu..prod.js gzip 9.62 kB 9.62 kB
pages-api.ru...dev.js gzip 11.5 kB 11.5 kB
pages-api.ru..prod.js gzip 9.61 kB 9.61 kB
pages-turbo...prod.js gzip 20.8 kB 20.8 kB
pages.runtim...dev.js gzip 26.4 kB 26.4 kB
pages.runtim..prod.js gzip 20.8 kB 20.8 kB
server.runti..prod.js gzip 57.7 kB 57.7 kB
Overall change 1.45 MB 1.45 MB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js 09-14-transform_object_server_react Change
0.pack gzip 1.66 MB 1.66 MB ⚠️ +378 B
index.pack gzip 131 kB 131 kB ⚠️ +131 B
Overall change 1.79 MB 1.79 MB ⚠️ +509 B
Diff details
Diff for page.js

Diff too large to display

Diff for middleware.js

Diff too large to display

Diff for edge-ssr.js

Diff too large to display

Diff for image-HASH.js
@@ -1,7 +1,7 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [8358],
   {
-    /***/ 8696: /***/ (
+    /***/ 2121: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(4129);
+          return __webpack_require__(2990);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 4979: /***/ (module, exports, __webpack_require__) => {
+    /***/ 2994: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(6663)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(9845)
+        __webpack_require__(3411)
       );
-      const _getimgprops = __webpack_require__(5864);
-      const _imageconfig = __webpack_require__(947);
-      const _imageconfigcontextsharedruntime = __webpack_require__(5048);
-      const _warnonce = __webpack_require__(1022);
-      const _routercontextsharedruntime = __webpack_require__(8261);
+      const _getimgprops = __webpack_require__(7181);
+      const _imageconfig = __webpack_require__(7872);
+      const _imageconfigcontextsharedruntime = __webpack_require__(3853);
+      const _warnonce = __webpack_require__(4374);
+      const _routercontextsharedruntime = __webpack_require__(5198);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1914)
+        __webpack_require__(3470)
       );
-      const _usemergedref = __webpack_require__(7856);
+      const _usemergedref = __webpack_require__(6612);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -371,7 +371,7 @@
       /***/
     },
 
-    /***/ 7856: /***/ (module, exports, __webpack_require__) => {
+    /***/ 6612: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -440,7 +440,7 @@
       /***/
     },
 
-    /***/ 5864: /***/ (
+    /***/ 7181: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -456,9 +456,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(1022);
-      const _imageblursvg = __webpack_require__(1009);
-      const _imageconfig = __webpack_require__(947);
+      const _warnonce = __webpack_require__(4374);
+      const _imageblursvg = __webpack_require__(3452);
+      const _imageconfig = __webpack_require__(7872);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -830,7 +830,7 @@
       /***/
     },
 
-    /***/ 1009: /***/ (__unused_webpack_module, exports) => {
+    /***/ 3452: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -885,7 +885,7 @@
       /***/
     },
 
-    /***/ 8900: /***/ (
+    /***/ 4150: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -912,10 +912,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(9608);
-      const _getimgprops = __webpack_require__(5864);
-      const _imagecomponent = __webpack_require__(4979);
+      const _getimgprops = __webpack_require__(7181);
+      const _imagecomponent = __webpack_require__(2994);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(1914)
+        __webpack_require__(3470)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -947,7 +947,7 @@
       /***/
     },
 
-    /***/ 1914: /***/ (__unused_webpack_module, exports) => {
+    /***/ 3470: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -982,7 +982,7 @@
       /***/
     },
 
-    /***/ 4129: /***/ (
+    /***/ 2990: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -999,8 +999,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0-rc-206df66e-20240912/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(1143);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-206df66e-20240912_re_7unepphpsfbl5ofsayom3m6uma/node_modules/next/image.js
-      var next_image = __webpack_require__(5618);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-206df66e-20240912_re_mm6spwxwvmn7da4aknn66l2fta/node_modules/next/image.js
+      var next_image = __webpack_require__(4243);
       var image_default = /*#__PURE__*/ __webpack_require__.n(next_image); // CONCATENATED MODULE: ./pages/nextjs.png
       /* harmony default export */ const nextjs = {
         src: "/_next/static/media/nextjs.cae0b805.png",
@@ -1030,12 +1030,12 @@
       /***/
     },
 
-    /***/ 5618: /***/ (
+    /***/ 4243: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(8900);
+      module.exports = __webpack_require__(4150);
 
       /***/
     },
@@ -1045,7 +1045,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(8696)
+      __webpack_exec__(2121)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Commit: 7bc0f9a

@huozhi huozhi changed the title wip test test: add conditional transform or optimize server react Sep 14, 2024
@ijjk ijjk added the tests label Sep 14, 2024
@huozhi huozhi changed the title test: add conditional transform or optimize server react Improve the transform and re-enable optimizeServerReact Sep 14, 2024
@huozhi huozhi requested review from shuding and kdy1 September 14, 2024 22:15
@huozhi huozhi marked this pull request as ready for review September 14, 2024 22:15
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

LGTM! Can we find a better name for the env that's more explicit?

Copy link
Member

@kdy1 kdy1 left a comment

Choose a reason for hiding this comment

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

Same 👍

@huozhi
Copy link
Member Author

huozhi commented Sep 16, 2024

renamed to __NEXT_PRIVATE_MINIMIZE_MARCO_FALSE

  • __NEXT and _PRIVATE represents "internal"
  • MINIMIZE represents which phase to replace
  • MACRO for functionality (C++ MACRO)
  • FALSE for value

@huozhi huozhi merged commit 5c5ea40 into canary Sep 16, 2024
107 of 109 checks passed
@huozhi huozhi deleted the 09-14-transform_object_server_react branch September 16, 2024 10:21
@github-actions github-actions bot added the locked label Oct 3, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
created-by: Next.js team PRs by the Next.js team. locked tests Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants