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

Disable experimental.optimizeServer by default to fix failed server action #69788

Merged
merged 1 commit into from
Sep 6, 2024

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Sep 6, 2024

What

This PR fixes the server actions used under react hooks like useEffect are accidentally removed from server action manifest which resulted into a failed server action call.

Why

The server actions collection need to collect the used actions even in the 1st round of traversing. During fixing this I found it's broken due to the the react hooks are dropped in SSR bundle. That option is enabled since 14.2.0 by default and it will optimize the server side uesless hook call, however the action call under it will also be dropped along with the whole hook. Since we're collecting the used actions from server compiler in webpack, it's not possible to collect those actions if they're already dropped. So we disabled that flag in this PR by default unelss users enable by default.

Follow up improvement in the future: We either figure out a way that can refine the detection which react hook is better not optimized, or explore the client side used server action detection and reflect it to server actions manifest so that we don't have to rely on disabling experimental.optimizeServer.

Fixes #69756

Copy link
Member Author

huozhi commented Sep 6, 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

@huozhi huozhi changed the title handle use effect wip: handle use effect Sep 6, 2024
@ijjk
Copy link
Member

ijjk commented Sep 6, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
buildDuration 18.7s 15.8s N/A
buildDurationCached 8.9s 7.4s N/A
nodeModulesSize 357 MB 357 MB N/A
nextStartRea..uration (ms) 416ms 411ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
1062.HASH.js gzip 169 B 168 B N/A
2cd9da90-HASH.js gzip 52 kB 52 kB N/A
5867-HASH.js gzip 42.4 kB 42.4 kB N/A
8503-HASH.js gzip 5.25 kB 5.25 kB N/A
framework-HASH.js gzip 56.9 kB 56.9 kB N/A
main-app-HASH.js gzip 224 B 224 B
main-HASH.js gzip 32.6 kB 32.5 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 224 B 224 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action 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-05-fix_missing_server_action Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 191 B 191 B
amp-HASH.js gzip 511 B 510 B N/A
css-HASH.js gzip 342 B 343 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB N/A
edge-ssr-HASH.js gzip 265 B 266 B N/A
head-HASH.js gzip 362 B 364 B N/A
hooks-HASH.js gzip 392 B 392 B
image-HASH.js gzip 4.4 kB 4.4 kB
index-HASH.js gzip 268 B 267 B N/A
link-HASH.js gzip 2.81 kB 2.81 kB N/A
routerDirect..HASH.js gzip 329 B 327 B N/A
script-HASH.js gzip 398 B 396 B N/A
withRouter-HASH.js gzip 325 B 324 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 5.28 kB 5.28 kB
Client Build Manifests
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
_buildManifest.js gzip 749 B 749 B
Overall change 749 B 749 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
index.html gzip 522 B 523 B N/A
link.html gzip 536 B 537 B N/A
withRouter.html gzip 519 B 519 B
Overall change 519 B 519 B
Edge SSR bundle Size Overall increase ⚠️
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 179 kB 180 kB ⚠️ +916 B
Overall change 179 kB 180 kB ⚠️ +916 B
Middleware size
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
middleware-b..fest.js gzip 672 B 665 B N/A
middleware-r..fest.js gzip 156 B 157 B N/A
middleware.js gzip 29.9 kB 29.9 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-05-fix_missing_server_action 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 305 kB 305 kB
app-page.run..prod.js gzip 121 kB 121 kB
app-route-ex...dev.js gzip 31.2 kB 31.2 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 20.9 kB 20.9 kB
app-route.ru...dev.js gzip 32.9 kB 32.9 kB
app-route.ru..prod.js gzip 20.9 kB 20.9 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
vercel/next.js canary vercel/next.js 09-05-fix_missing_server_action Change
0.pack gzip 1.61 MB 1.61 MB N/A
index.pack gzip 131 kB 131 kB N/A
Overall change 0 B 0 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],
   {
-    /***/ 8908: /***/ (
+    /***/ 8176: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(2111);
+          return __webpack_require__(934);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 6308: /***/ (module, exports, __webpack_require__) => {
+    /***/ 5057: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,17 +40,17 @@
         __webpack_require__(1955)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(2383)
+        __webpack_require__(7163)
       );
-      const _getimgprops = __webpack_require__(5640);
-      const _imageconfig = __webpack_require__(2758);
-      const _imageconfigcontextsharedruntime = __webpack_require__(899);
-      const _warnonce = __webpack_require__(3878);
-      const _routercontextsharedruntime = __webpack_require__(869);
+      const _getimgprops = __webpack_require__(5447);
+      const _imageconfig = __webpack_require__(1650);
+      const _imageconfigcontextsharedruntime = __webpack_require__(1953);
+      const _warnonce = __webpack_require__(5054);
+      const _routercontextsharedruntime = __webpack_require__(4445);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(6501)
+        __webpack_require__(7406)
       );
-      const _usemergedref = __webpack_require__(3994);
+      const _usemergedref = __webpack_require__(3303);
       // This is replaced by webpack define plugin
       const configEnv = {
         deviceSizes: [640, 750, 828, 1080, 1200, 1920, 2048, 3840],
@@ -371,7 +371,7 @@
       /***/
     },
 
-    /***/ 3994: /***/ (module, exports, __webpack_require__) => {
+    /***/ 3303: /***/ (module, exports, __webpack_require__) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -440,7 +440,7 @@
       /***/
     },
 
-    /***/ 5640: /***/ (
+    /***/ 5447: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -456,9 +456,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(3878);
-      const _imageblursvg = __webpack_require__(4317);
-      const _imageconfig = __webpack_require__(2758);
+      const _warnonce = __webpack_require__(5054);
+      const _imageblursvg = __webpack_require__(1731);
+      const _imageconfig = __webpack_require__(1650);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -830,7 +830,7 @@
       /***/
     },
 
-    /***/ 4317: /***/ (__unused_webpack_module, exports) => {
+    /***/ 1731: /***/ (__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 @@
       /***/
     },
 
-    /***/ 8580: /***/ (
+    /***/ 9833: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -912,10 +912,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(9608);
-      const _getimgprops = __webpack_require__(5640);
-      const _imagecomponent = __webpack_require__(6308);
+      const _getimgprops = __webpack_require__(5447);
+      const _imagecomponent = __webpack_require__(5057);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(6501)
+        __webpack_require__(7406)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -947,7 +947,7 @@
       /***/
     },
 
-    /***/ 6501: /***/ (__unused_webpack_module, exports) => {
+    /***/ 7406: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -982,7 +982,7 @@
       /***/
     },
 
-    /***/ 2111: /***/ (
+    /***/ 934: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -999,8 +999,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/react@19.0.0-rc-7771d3a7-20240827/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(5815);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+main-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-7771d3a7-20240827_re_h4bgdqagqtli4q7bnw2ajnn4vm/node_modules/next/image.js
-      var next_image = __webpack_require__(1878);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/next@file+..+diff-repo+packages+next+next-packed.tgz_react-dom@19.0.0-rc-7771d3a7-20240827_re_qz7eh3ruj3cscolfxbcb265hza/node_modules/next/image.js
+      var next_image = __webpack_require__(7649);
       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 @@
       /***/
     },
 
-    /***/ 1878: /***/ (
+    /***/ 7649: /***/ (
       module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      module.exports = __webpack_require__(8580);
+      module.exports = __webpack_require__(9833);
 
       /***/
     },
@@ -1045,7 +1045,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(8908)
+      __webpack_exec__(8176)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for 5867-HASH.js

Diff too large to display

Commit: 76ac192

@ijjk
Copy link
Member

ijjk commented Sep 6, 2024

Tests Passed

@huozhi huozhi changed the title wip: handle use effect Fix failed server action under react hooks Sep 6, 2024
@huozhi huozhi force-pushed the 09-05-fix_missing_server_action branch from 8418fef to 76ac192 Compare September 6, 2024 15:28
@huozhi huozhi changed the title Fix failed server action under react hooks Disable experimental.optimizeServer by default Sep 6, 2024
@controversial
Copy link
Contributor

Thanks for the quick fix on this! The underlying cause (useEffects removed from the server bundle, causing the action calls to be tree-shaken) makes total sense.

We either figure out a way that can refine the detection which react hook is better not optimized, or explore the client side used server action detection and reflect it to server actions manifest

I think the latter approach will be the “correct” one - since server actions are intended to be called from the client, the client bundle should ideally be the one used for detecting which server actions are used/unused.

@huozhi
Copy link
Member Author

huozhi commented Sep 6, 2024

Thanks for the quick fix on this! The underlying cause (useEffects removed from the server bundle, causing the action calls to be tree-shaken) makes total sense.

We either figure out a way that can refine the detection which react hook is better not optimized, or explore the client side used server action detection and reflect it to server actions manifest

I think the latter approach will be the “correct” one - since server actions are intended to be called from the client, the client bundle should ideally be the one used for detecting which server actions are used/unused.

Yea this will be temporary. Since it would take more time on researching the ideal solution as we also want to enable it by default for now. We want to do hotfix for 14.2 quickly so this perf improvement won't crash the app. And once the ideal solution is there, we can re-enable again.

@huozhi huozhi marked this pull request as ready for review September 6, 2024 15:53
@huozhi huozhi merged commit 87e91d5 into canary Sep 6, 2024
105 of 110 checks passed
@huozhi huozhi deleted the 09-05-fix_missing_server_action branch September 6, 2024 17:08
@huozhi huozhi changed the title Disable experimental.optimizeServer by default Disable experimental.optimizeServer by default to fix failed server action Sep 9, 2024
huozhi added a commit that referenced this pull request Sep 10, 2024
@huozhi
Copy link
Member Author

huozhi commented Sep 14, 2024

Got a follow up solution in #70101 to re-enable the optimizeServer config with a more safe transform

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 29, 2024
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.

Server actions regression in 14.2.8: “Failed to find Server Action”
4 participants