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 React from 14898b6a9 to c3048aab4 #64798

Merged
merged 6 commits into from
Apr 25, 2024
Merged

Update React from 14898b6a9 to c3048aab4 #64798

merged 6 commits into from
Apr 25, 2024

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Apr 19, 2024

Internal changes required for the sync

  • caught errors go through console.error not reportError so we need to spy on console.error and error events to collect errors for the redbox. Ideally we'd move away from console.error mocking and error event handlers in favor of onCaughtError and onUncaughtError. But these methods don't compose nicely. I'll have a proposal shortly to make them more useful.
  • I had to revert part of perf: remove react dom legacy from app router #56082 since that caused require('next/dist/compiled/react-dom/cjs/react-dom-server.edge.production.min.js')) to appear in page.js server bundles which resulted in the installed react-dom being used. It may be best to run app-dir tests without React installed to catch these issues earlier. For that to be effective, we'd also have to rewrite fixtures that mix pages and app dir. I'll double check what the impact of reverting perf: remove react dom legacy from app router #56082 is. Maybe Ensure next/dist/compiled/react-dom is never treated as an external #64992 is an alternate fix for this issue.

React upstream changes

Closes NEXT-3170

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Apr 19, 2024
@eps1lon eps1lon changed the title Update React from 14898b6a9 to c3048aab4. Update React from 14898b6a9 to c3048aab4 Apr 19, 2024
@ijjk
Copy link
Member

ijjk commented Apr 19, 2024

Tests Passed

@ijjk
Copy link
Member

ijjk commented Apr 19, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
buildDuration 18.1s 15.2s N/A
buildDurationCached 7.9s 6.6s N/A
nodeModulesSize 238 MB 239 MB ⚠️ +581 kB
nextStartRea..uration (ms) 404ms 401ms N/A
Client Bundles (main, webpack) Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
2453-HASH.js gzip 31.5 kB 31.8 kB ⚠️ +278 B
3304.HASH.js gzip 169 B 169 B
3f784ff6-HASH.js gzip 53.7 kB 53.5 kB N/A
8299-HASH.js gzip 5.09 kB 5.09 kB
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 227 B 228 B N/A
main-HASH.js gzip 31.5 kB 31.6 kB N/A
webpack-HASH.js gzip 1.64 kB 1.65 kB N/A
Overall change 82 kB 82.2 kB ⚠️ +278 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
_app-HASH.js gzip 193 B 194 B N/A
_error-HASH.js gzip 193 B 191 B N/A
amp-HASH.js gzip 510 B 510 B
css-HASH.js gzip 342 B 343 B N/A
dynamic-HASH.js gzip 2.51 kB 2.52 kB N/A
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 365 B 364 B N/A
hooks-HASH.js gzip 389 B 391 B N/A
image-HASH.js gzip 4.28 kB 4.28 kB N/A
index-HASH.js gzip 269 B 268 B N/A
link-HASH.js gzip 2.68 kB 2.69 kB N/A
routerDirect..HASH.js gzip 328 B 326 B N/A
script-HASH.js gzip 395 B 397 B N/A
withRouter-HASH.js gzip 323 B 323 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 1.2 kB 1.2 kB
Client Build Manifests
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
_buildManifest.js gzip 482 B 484 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
index.html gzip 529 B 528 B N/A
link.html gzip 541 B 541 B
withRouter.html gzip 524 B 523 B N/A
Overall change 541 B 541 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
edge-ssr.js gzip 108 kB 108 kB N/A
page.js gzip 3.04 kB 3.04 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
middleware-b..fest.js gzip 623 B 625 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 27.8 kB 27.8 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 839 B 839 B
Next Runtimes Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
app-page-exp...dev.js gzip 171 kB 171 kB N/A
app-page-exp..prod.js gzip 97.6 kB 98.3 kB ⚠️ +718 B
app-page-tur..prod.js gzip 99.4 kB 99.8 kB ⚠️ +452 B
app-page-tur..prod.js gzip 93.6 kB 94.2 kB ⚠️ +531 B
app-page.run...dev.js gzip 145 kB 157 kB ⚠️ +11.9 kB
app-page.run..prod.js gzip 92.1 kB 92.9 kB ⚠️ +768 B
app-route-ex...dev.js gzip 21.5 kB 21.4 kB N/A
app-route-ex..prod.js gzip 15.1 kB 15.1 kB N/A
app-route-tu..prod.js gzip 15.1 kB 15.1 kB N/A
app-route-tu..prod.js gzip 14.9 kB 14.9 kB N/A
app-route.ru...dev.js gzip 21.2 kB 21.2 kB N/A
app-route.ru..prod.js gzip 14.9 kB 14.9 kB N/A
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 21.4 kB 21.4 kB
pages.runtim...dev.js gzip 22.1 kB 22.1 kB
pages.runtim..prod.js gzip 21.4 kB 21.4 kB
server.runti..prod.js gzip 65.3 kB 65.3 kB
Overall change 687 kB 701 kB ⚠️ +14.4 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/react-18-sync Change
0.pack gzip 1.61 MB 1.61 MB N/A
index.pack gzip 106 kB 107 kB ⚠️ +476 B
Overall change 106 kB 107 kB ⚠️ +476 B
Diff details
Diff for middleware.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],
   {
-    /***/ 1552: /***/ (
+    /***/ 4070: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
@@ -9,7 +9,7 @@
       (window.__NEXT_P = window.__NEXT_P || []).push([
         "/image",
         function () {
-          return __webpack_require__(5237);
+          return __webpack_require__(396);
         },
       ]);
       if (false) {
@@ -18,7 +18,7 @@
       /***/
     },
 
-    /***/ 2016: /***/ (module, exports, __webpack_require__) => {
+    /***/ 8490: /***/ (module, exports, __webpack_require__) => {
       "use strict";
       /* __next_internal_client_entry_do_not_use__  cjs */
       Object.defineProperty(exports, "__esModule", {
@@ -40,15 +40,15 @@
         __webpack_require__(422)
       );
       const _head = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(6074)
+        __webpack_require__(2457)
       );
-      const _getimgprops = __webpack_require__(9571);
-      const _imageconfig = __webpack_require__(6567);
-      const _imageconfigcontextsharedruntime = __webpack_require__(419);
-      const _warnonce = __webpack_require__(4486);
-      const _routercontextsharedruntime = __webpack_require__(162);
+      const _getimgprops = __webpack_require__(7932);
+      const _imageconfig = __webpack_require__(5706);
+      const _imageconfigcontextsharedruntime = __webpack_require__(9483);
+      const _warnonce = __webpack_require__(9035);
+      const _routercontextsharedruntime = __webpack_require__(4829);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(6996)
+        __webpack_require__(7240)
       );
       // This is replaced by webpack define plugin
       const configEnv = {
@@ -379,7 +379,7 @@
       /***/
     },
 
-    /***/ 9571: /***/ (
+    /***/ 7932: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -395,9 +395,9 @@
           return getImgProps;
         },
       });
-      const _warnonce = __webpack_require__(4486);
-      const _imageblursvg = __webpack_require__(133);
-      const _imageconfig = __webpack_require__(6567);
+      const _warnonce = __webpack_require__(9035);
+      const _imageblursvg = __webpack_require__(2642);
+      const _imageconfig = __webpack_require__(5706);
       const VALID_LOADING_VALUES =
         /* unused pure expression or super */ null && [
           "lazy",
@@ -772,7 +772,7 @@
       /***/
     },
 
-    /***/ 133: /***/ (__unused_webpack_module, exports) => {
+    /***/ 2642: /***/ (__unused_webpack_module, exports) => {
       "use strict";
       /**
        * A shared function, used on both client and server, to generate a SVG blur placeholder.
@@ -827,7 +827,7 @@
       /***/
     },
 
-    /***/ 4085: /***/ (
+    /***/ 503: /***/ (
       __unused_webpack_module,
       exports,
       __webpack_require__
@@ -854,10 +854,10 @@
         },
       });
       const _interop_require_default = __webpack_require__(7456);
-      const _getimgprops = __webpack_require__(9571);
-      const _imagecomponent = __webpack_require__(2016);
+      const _getimgprops = __webpack_require__(7932);
+      const _imagecomponent = __webpack_require__(8490);
       const _imageloader = /*#__PURE__*/ _interop_require_default._(
-        __webpack_require__(6996)
+        __webpack_require__(7240)
       );
       function getImageProps(imgProps) {
         const { props } = (0, _getimgprops.getImgProps)(imgProps, {
@@ -889,7 +889,7 @@
       /***/
     },
 
-    /***/ 6996: /***/ (__unused_webpack_module, exports) => {
+    /***/ 7240: /***/ (__unused_webpack_module, exports) => {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -924,7 +924,7 @@
       /***/
     },
 
-    /***/ 5237: /***/ (
+    /***/ 396: /***/ (
       __unused_webpack_module,
       __webpack_exports__,
       __webpack_require__
@@ -941,8 +941,8 @@
 
       // EXTERNAL MODULE: ./node_modules/.pnpm/[email protected]/node_modules/react/jsx-runtime.js
       var jsx_runtime = __webpack_require__(1527);
-      // EXTERNAL MODULE: ./node_modules/.pnpm/[email protected][email protected]/node_modules/next/image.js
-      var next_image = __webpack_require__(1577);
+      // EXTERNAL MODULE: ./node_modules/.pnpm/[email protected][email protected]/node_modules/next/image.js
+      var next_image = __webpack_require__(73);
       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",
@@ -972,12 +972,8 @@
       /***/
     },
 
-    /***/ 1577: /***/ (
-      module,
-      __unused_webpack_exports,
-      __webpack_require__
-    ) => {
-      module.exports = __webpack_require__(4085);
+    /***/ 73: /***/ (module, __unused_webpack_exports, __webpack_require__) => {
+      module.exports = __webpack_require__(503);
 
       /***/
     },
@@ -987,7 +983,7 @@
     /******/ var __webpack_exec__ = (moduleId) =>
       __webpack_require__((__webpack_require__.s = moduleId));
     /******/ __webpack_require__.O(0, [2888, 9774, 179], () =>
-      __webpack_exec__(1552)
+      __webpack_exec__(4070)
     );
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Diff for 2453-HASH.js

Diff too large to display

Diff for 3f784ff6-HASH.js

Diff too large to display

Diff for main-HASH.js

Diff too large to display

Diff for app-page-exp..ntime.dev.js
failed to diff
Diff for app-page-exp..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page-tur..time.prod.js

Diff too large to display

Diff for app-page.runtime.dev.js
failed to diff
Diff for app-page.runtime.prod.js

Diff too large to display

Diff for app-route-ex..ntime.dev.js

Diff too large to display

Diff for app-route-ex..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route-tu..time.prod.js

Diff too large to display

Diff for app-route.runtime.dev.js

Diff too large to display

Diff for app-route.ru..time.prod.js

Diff too large to display

Commit: fbd6174

Would be nice to do this via onCaughtError but I copying the full impl of `defaultOnCaughtError` seems sketchy.
See https://vercel.slack.com/archives/C04TR3U872A/p1713969391445789 for quick proposal
Fixes displaying 2 errors in the redbox when just one was thrown.
It counted 2 because we had the actual error and React's "the above occured"
`next/dist/compiled/react-dom` can never appear in a require call i.e. be treated as an external because these modules don't import from vendored react-dom
but installed react-dom
@eps1lon eps1lon marked this pull request as ready for review April 24, 2024 19:16
@eps1lon eps1lon requested a review from wyattjoh as a code owner April 24, 2024 19:16
@@ -50,6 +50,7 @@ Or, run this command with no arguments to use the most recently published versio
`
)
}
newVersionInfo.releaseLabel = channel
Copy link
Member Author

Choose a reason for hiding this comment

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

Otherwise the script made react-experimental-builtin point to the Canary release channel.

Comment on lines +262 to +264
// TODO: restore optimizations to ignore the legacy build of react-dom/server in `server.browser` build
'react-dom/server.edge$': `next/dist/compiled/react-dom${bundledReactChannel}/server.edge`,
'react-dom/server.browser$': `next/dist/compiled/react-dom${bundledReactChannel}/server.browser`,
Copy link
Member Author

Choose a reason for hiding this comment

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

This reverts #56082 partially.`

However, next/dist/compiled/next-server/app-page.runtime.prod.js is only 5kB larger than on canary (330kB instead of 325kB). Doesn't seem like this change regresses anything significantly while improving correctness. @feedthejim should confirm though if there is another bundle that I should check.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Screenshot 2024-04-25 at 12 34 53

@eps1lon eps1lon merged commit 04571f3 into canary Apr 25, 2024
84 checks passed
@eps1lon eps1lon deleted the sebbie/react-18-sync branch April 25, 2024 10:35
@github-actions github-actions bot added the locked label May 9, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 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.

4 participants