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.

Sign up for GitHub

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

Jump to bottom

fix(#36435): apply correct fix #36464

Merged
merged 4 commits into from Apr 26, 2022
Merged

fix(#36435): apply correct fix #36464

merged 4 commits into from Apr 26, 2022

Conversation

SukkaW
Copy link
Contributor

@SukkaW SukkaW commented Apr 26, 2022 •
edited

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

The PR is to supersede #36435 by applying a correct fix for #36183.

@SukkaW
Copy link
Contributor Author

SukkaW commented Apr 26, 2022 •
edited

cc @ijjk @Brooooooklyn

The swc can automatically apply the transpilation for default interop export:

// next/dist/client/link.js, next/dist/client/script.js
if (typeof exports.default === 'function' || (typeof exports.default === 'object' && exports.default !== null)) {
  Object.assign(exports.default, exports);
  module.exports = exports.default;
}

The reason next/link and next/script is unaffected by #36183, is they are re-exported from client/, where the interopClientDefaultExport flag is enabled.

next.js/packages/next/taskfile.js

Line 1836 in 26efcc5

.swc('client', { dev: opts.dev, interopClientDefaultExport: true })

However, next/head and next/dynamic are re-exported from shared/lib, where interopClientDefaultExport flag is not enabled:

next.js/packages/next/taskfile.js

Line 1926 in 26efcc5

.swc('server', { dev: opts.dev })

The PR applies the correct fix by enabling the interopClientDefaultExport flag for shared/lib, too.

@ijjk
Copy link
Member

ijjk commented Apr 26, 2022 •
edited

Failing test suites

Commit: 707aa09

yarn testheadless test/e2e/prerender.test.ts

  • Prerender > should handle manual revalidate for fallback: blocking
Expand output

● Prerender › should handle manual revalidate for fallback: blocking

expect(received).not.toBe(expected) // Object.is equality

Expected: not "time: 57814.297537999926"

  2060 |         const html4 = await res4.text()
  2061 |         const $4 = cheerio.load(html4)
> 2062 |         expect($4('#time').text()).not.toBe(initialTime)
       |                                        ^
  2063 |         expect(res4.headers.get('x-nextjs-cache')).toMatch(/(HIT|STALE)/)
  2064 |       })
  2065 |

  at Object.<anonymous> (e2e/prerender.test.ts:2062:40)

Read more about building and testing Next.js in contributing.md.

@SukkaW SukkaW marked this pull request as draft April 26, 2022 04:34
@SukkaW SukkaW marked this pull request as ready for review April 26, 2022 04:40
@ijjk
Copy link
Member

ijjk commented Apr 26, 2022 •
edited

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
buildDuration 15.2s 15.3s ⚠️ +100ms
buildDurationCached 6.1s 6.1s -16ms
nodeModulesSize 481 MB 481 MB ⚠️ +726 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
/ failed reqs 0 0 ✓
/ total time (seconds) 3.036 3.035 0
/ avg req/sec 823.49 823.71 +0.22
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.177 1.189 ⚠️ +0.01
/error-in-render avg req/sec 2124.04 2102.71 ⚠️ -21.33
Client Bundles (main, webpack) Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 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 ⚠️ +1 B
webpack-HASH.js gzip 1.44 kB 1.44 kB ✓
Overall change 72.3 kB 72.3 kB ⚠️ +1 B
Legacy Client Bundles (polyfills)
vercel/next.js canary SukkaW/next.js fix-36435 Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
_app-HASH.js gzip 1.36 kB 1.36 kB ✓
_error-HASH.js gzip 192 B 192 B ✓
amp-HASH.js gzip 309 B 309 B ✓
css-HASH.js gzip 327 B 327 B ✓
dynamic-HASH.js gzip 3.04 kB 3.08 kB ⚠️ +40 B
head-HASH.js gzip 378 B 357 B -21 B
hooks-HASH.js gzip 920 B 920 B ✓
image-HASH.js gzip 5.73 kB 5.73 kB ✓
index-HASH.js gzip 263 B 263 B ✓
link-HASH.js gzip 2.63 kB 2.63 kB ✓
routerDirect..HASH.js gzip 320 B 320 B ✓
script-HASH.js gzip 392 B 392 B ✓
withRouter-HASH.js gzip 319 B 319 B ✓
85e02e95b279..7e3.css gzip 107 B 107 B ✓
Overall change 16.3 kB 16.3 kB ⚠️ +19 B
Client Build Manifests Overall decrease ✓
vercel/next.js canary SukkaW/next.js fix-36435 Change
_buildManifest.js gzip 460 B 459 B -1 B
Overall change 460 B 459 B -1 B
Rendered Page Sizes Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
index.html gzip 531 B 532 B ⚠️ +1 B
link.html gzip 545 B 545 B ✓
withRouter.html gzip 525 B 525 B ✓
Overall change 1.6 kB 1.6 kB ⚠️ +1 B

Diffs

Diff for _buildManifest.js 
@@ -2,15 +2,15 @@ self.__BUILD_MANIFEST = {
   __rewrites: { beforeFiles: [], afterFiles: [], fallback: [] },
   "/": ["static\u002Fchunks\u002Fpages\u002Findex-a49797fad5e5a39b.js"],
   "/_error": ["static\u002Fchunks\u002Fpages\u002F_error-5d03de5a43fe90da.js"],
-  "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-3eb62f36e98a75e2.js"],
+  "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-c7cbac1df9ead194.js"],
   "/css": [
     "static\u002Fcss\u002F94fdbc56eafa2039.css",
     "static\u002Fchunks\u002Fpages\u002Fcss-f8d6ff68a6e8b080.js"
   ],
   "/dynamic": [
-    "static\u002Fchunks\u002Fpages\u002Fdynamic-2fc05fc466ebee90.js"
+    "static\u002Fchunks\u002Fpages\u002Fdynamic-7fda0a7b63c06ef1.js"
   ],
-  "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-b3f5cd04a41e9ce4.js"],
+  "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-ae8daed8e1e5c446.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-9dfe734f583d4926.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-4ebd299a1253d245.js"],
   "/link": ["static\u002Fchunks\u002Fpages\u002Flink-5b526b867abc83fc.js"],
Diff for amp-HASH.js 
@@ -47,6 +47,9 @@
       /* harmony import */ var next_amp__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(
         9135
       );
+      /* harmony import */ var next_amp__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/ __webpack_require__.n(
+        next_amp__WEBPACK_IMPORTED_MODULE_0__
+      );
 
       var config = {
         amp: "hybrid"
Diff for dynamic-HASH.js 
@@ -18,13 +18,8 @@
       /***/
     },
 
-    /***/ 7645: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 7645: /***/ function(module, exports, __webpack_require__) {
       "use strict";
-      var __webpack_unused_export__;
 
       function _defineProperty(obj, key, value) {
         if (key in obj) {
@@ -67,11 +62,11 @@
         }
         return target;
       }
-      __webpack_unused_export__ = {
+      Object.defineProperty(exports, "__esModule", {
         value: true
-      };
+      });
       exports["default"] = dynamic;
-      __webpack_unused_export__ = noSSR;
+      exports.noSSR = noSSR;
       var _react = _interopRequireDefault(__webpack_require__(7294));
       var _loadable = _interopRequireDefault(__webpack_require__(4588));
       function dynamic(dynamicOptions, options) {
@@ -160,6 +155,13 @@
             timedOut: false
           });
         };
+      }
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
       } //# sourceMappingURL=dynamic.js.map
 
       /***/
@@ -586,9 +588,11 @@
       /* harmony import */ var next_dynamic__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(
         5152
       );
+      /* harmony import */ var next_dynamic__WEBPACK_IMPORTED_MODULE_1___default = /*#__PURE__*/ __webpack_require__.n(
+        next_dynamic__WEBPACK_IMPORTED_MODULE_1__
+      );
 
-      var DynamicHello = (0,
-      next_dynamic__WEBPACK_IMPORTED_MODULE_1__["default"])(
+      var DynamicHello = next_dynamic__WEBPACK_IMPORTED_MODULE_1___default()(
         function() {
           return __webpack_require__
             .e(/* import() */ 925)
Diff for head-HASH.js 
@@ -76,9 +76,7 @@
       __unused_webpack_exports,
       __webpack_require__
     ) {
-      var head = __webpack_require__(3121);
-      Object.assign(head.default, head);
-      module.exports = head.default;
+      module.exports = __webpack_require__(3121);
 
       /***/
     }
Diff for main-HASH.js 
@@ -3681,11 +3681,7 @@
       /***/
     },
 
-    /***/ 1686: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 1686: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -3718,6 +3714,13 @@
         return isInAmpMode(
           _react.default.useContext(_ampContext.AmpStateContext)
         );
+      }
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
       } //# sourceMappingURL=amp.js.map
 
       /***/
@@ -3771,11 +3774,7 @@
       /***/
     },
 
-    /***/ 3121: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 3121: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
       function _defineProperty(obj, key, value) {
@@ -4020,7 +4019,14 @@
         );
       }
       var _default = Head;
-      exports["default"] = _default; //# sourceMappingURL=head.js.map
+      exports["default"] = _default;
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
+      } //# sourceMappingURL=head.js.map
 
       /***/
     },
Diff for index.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Diff for link.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Diff for withRouter.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Default Build with SWC (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
buildDuration 17.5s 17.6s ⚠️ +75ms
buildDurationCached 6s 6.1s ⚠️ +32ms
nodeModulesSize 481 MB 481 MB ⚠️ +726 B
Page Load Tests Overall decrease ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
/ failed reqs 0 0 ✓
/ total time (seconds) 3.041 3.036 0
/ avg req/sec 821.99 823.44 +1.45
/error-in-render failed reqs 0 0 ✓
/error-in-render total time (seconds) 1.166 1.171 ⚠️ +0.01
/error-in-render avg req/sec 2143.79 2134.88 ⚠️ -8.91
Client Bundles (main, webpack) Overall decrease ✓
vercel/next.js canary SukkaW/next.js fix-36435 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.1 kB -1 B
webpack-HASH.js gzip 1.45 kB 1.45 kB ✓
Overall change 72.9 kB 72.9 kB -1 B
Legacy Client Bundles (polyfills)
vercel/next.js canary SukkaW/next.js fix-36435 Change
polyfills-HASH.js gzip 31 kB 31 kB ✓
Overall change 31 kB 31 kB ✓
Client Pages Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
_app-HASH.js gzip 1.35 kB 1.35 kB ✓
_error-HASH.js gzip 179 B 179 B ✓
amp-HASH.js gzip 313 B 313 B ✓
css-HASH.js gzip 325 B 325 B ✓
dynamic-HASH.js gzip 3.02 kB 3.08 kB ⚠️ +59 B
head-HASH.js gzip 385 B 359 B -26 B
hooks-HASH.js gzip 921 B 921 B ✓
image-HASH.js gzip 5.78 kB 5.78 kB ✓
index-HASH.js gzip 261 B 261 B ✓
link-HASH.js gzip 2.74 kB 2.74 kB ✓
routerDirect..HASH.js gzip 322 B 322 B ✓
script-HASH.js gzip 393 B 393 B ✓
withRouter-HASH.js gzip 317 B 317 B ✓
85e02e95b279..7e3.css gzip 107 B 107 B ✓
Overall change 16.4 kB 16.5 kB ⚠️ +33 B
Client Build Manifests Overall increase ⚠️
vercel/next.js canary SukkaW/next.js fix-36435 Change
_buildManifest.js gzip 456 B 458 B ⚠️ +2 B
Overall change 456 B 458 B ⚠️ +2 B
Rendered Page Sizes Overall decrease ✓
vercel/next.js canary SukkaW/next.js fix-36435 Change
index.html gzip 530 B 529 B -1 B
link.html gzip 542 B 541 B -1 B
withRouter.html gzip 525 B 525 B ✓
Overall change 1.6 kB 1.59 kB -2 B

Diffs

Diff for _buildManifest.js 
@@ -2,15 +2,15 @@ self.__BUILD_MANIFEST = {
   __rewrites: { beforeFiles: [], afterFiles: [], fallback: [] },
   "/": ["static\u002Fchunks\u002Fpages\u002Findex-a49797fad5e5a39b.js"],
   "/_error": ["static\u002Fchunks\u002Fpages\u002F_error-5d03de5a43fe90da.js"],
-  "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-3eb62f36e98a75e2.js"],
+  "/amp": ["static\u002Fchunks\u002Fpages\u002Famp-c7cbac1df9ead194.js"],
   "/css": [
     "static\u002Fcss\u002F94fdbc56eafa2039.css",
     "static\u002Fchunks\u002Fpages\u002Fcss-f8d6ff68a6e8b080.js"
   ],
   "/dynamic": [
-    "static\u002Fchunks\u002Fpages\u002Fdynamic-2fc05fc466ebee90.js"
+    "static\u002Fchunks\u002Fpages\u002Fdynamic-7fda0a7b63c06ef1.js"
   ],
-  "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-b3f5cd04a41e9ce4.js"],
+  "/head": ["static\u002Fchunks\u002Fpages\u002Fhead-ae8daed8e1e5c446.js"],
   "/hooks": ["static\u002Fchunks\u002Fpages\u002Fhooks-9dfe734f583d4926.js"],
   "/image": ["static\u002Fchunks\u002Fpages\u002Fimage-4ebd299a1253d245.js"],
   "/link": ["static\u002Fchunks\u002Fpages\u002Flink-5b526b867abc83fc.js"],
Diff for amp-HASH.js 
@@ -47,6 +47,9 @@
       /* harmony import */ var next_amp__WEBPACK_IMPORTED_MODULE_0__ = __webpack_require__(
         9135
       );
+      /* harmony import */ var next_amp__WEBPACK_IMPORTED_MODULE_0___default = /*#__PURE__*/ __webpack_require__.n(
+        next_amp__WEBPACK_IMPORTED_MODULE_0__
+      );
 
       var config = {
         amp: "hybrid"
Diff for dynamic-HASH.js 
@@ -18,13 +18,8 @@
       /***/
     },
 
-    /***/ 7645: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 7645: /***/ function(module, exports, __webpack_require__) {
       "use strict";
-      var __webpack_unused_export__;
 
       function _defineProperty(obj, key, value) {
         if (key in obj) {
@@ -67,11 +62,11 @@
         }
         return target;
       }
-      __webpack_unused_export__ = {
+      Object.defineProperty(exports, "__esModule", {
         value: true
-      };
+      });
       exports["default"] = dynamic;
-      __webpack_unused_export__ = noSSR;
+      exports.noSSR = noSSR;
       var _react = _interopRequireDefault(__webpack_require__(7294));
       var _loadable = _interopRequireDefault(__webpack_require__(4588));
       function dynamic(dynamicOptions, options) {
@@ -160,6 +155,13 @@
             timedOut: false
           });
         };
+      }
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
       } //# sourceMappingURL=dynamic.js.map
 
       /***/
@@ -586,9 +588,11 @@
       /* harmony import */ var next_dynamic__WEBPACK_IMPORTED_MODULE_1__ = __webpack_require__(
         5152
       );
+      /* harmony import */ var next_dynamic__WEBPACK_IMPORTED_MODULE_1___default = /*#__PURE__*/ __webpack_require__.n(
+        next_dynamic__WEBPACK_IMPORTED_MODULE_1__
+      );
 
-      var DynamicHello = (0,
-      next_dynamic__WEBPACK_IMPORTED_MODULE_1__["default"])(
+      var DynamicHello = next_dynamic__WEBPACK_IMPORTED_MODULE_1___default()(
         function() {
           return __webpack_require__
             .e(/* import() */ 925)
Diff for head-HASH.js 
@@ -76,9 +76,7 @@
       __unused_webpack_exports,
       __webpack_require__
     ) {
-      var head = __webpack_require__(3121);
-      Object.assign(head.default, head);
-      module.exports = head.default;
+      module.exports = __webpack_require__(3121);
 
       /***/
     }
Diff for main-HASH.js 
@@ -3681,11 +3681,7 @@
       /***/
     },
 
-    /***/ 1686: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 1686: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
       Object.defineProperty(exports, "__esModule", {
@@ -3718,6 +3714,13 @@
         return isInAmpMode(
           _react.default.useContext(_ampContext.AmpStateContext)
         );
+      }
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
       } //# sourceMappingURL=amp.js.map
 
       /***/
@@ -3771,11 +3774,7 @@
       /***/
     },
 
-    /***/ 3121: /***/ function(
-      __unused_webpack_module,
-      exports,
-      __webpack_require__
-    ) {
+    /***/ 3121: /***/ function(module, exports, __webpack_require__) {
       "use strict";
 
       function _defineProperty(obj, key, value) {
@@ -4020,7 +4019,14 @@
         );
       }
       var _default = Head;
-      exports["default"] = _default; //# sourceMappingURL=head.js.map
+      exports["default"] = _default;
+      if (
+        typeof exports.default === "function" ||
+        (typeof exports.default === "object" && exports.default !== null)
+      ) {
+        Object.assign(exports.default, exports);
+        module.exports = exports.default;
+      } //# sourceMappingURL=head.js.map
 
       /***/
     },
Diff for index.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Diff for link.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Diff for withRouter.html 
@@ -19,7 +19,7 @@
       defer=""
     ></script>
     <script
-      src="/_next/static/chunks/main-c1c3c7b68a78f667.js"
+      src="/_next/static/chunks/main-bfcde4ffbe5273a8.js"
       defer=""
     ></script>
     <script
Commit: 0e5e5c3

Brooooooklyn
Brooooooklyn reviewed Apr 26, 2022
View reviewed changes
Copy link
Contributor

@Brooooooklyn Brooooooklyn left a comment

Choose a reason for hiding this comment

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

I'm wondering if https://github.com/vercel/next.js/blob/canary/packages/next/build/webpack/loaders/utils.ts#L3 should also add amp config constants, dynamic.

It's better to add some specs into test/e2e/type-module-interop/index.test.ts to verify it.

@SukkaW
Copy link
Contributor Author

SukkaW commented Apr 26, 2022

@Brooooooklyn The test cases for next/dynamic and next/amp is added.

@ijjk ijjk merged commit 487f7ab into vercel:canary Apr 26, 2022
@SukkaW SukkaW mentioned this pull request May 11, 2022
Merged
3 tasks
@timneutkens timneutkens mentioned this pull request May 16, 2022
Closed
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
@SukkaW SukkaW deleted the fix-36435 branch October 24, 2023 08:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Brooooooklyn Brooooooklyn Brooooooklyn approved these changes

@ijjk ijjk ijjk approved these changes

@timneutkens timneutkens Awaiting requested review from timneutkens timneutkens is a code owner

@shuding shuding Awaiting requested review from shuding shuding is a code owner

@huozhi huozhi Awaiting requested review from huozhi huozhi is a code owner

Assignees
No one assigned
Labels
type: next
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SukkaW @ijjk @Brooooooklyn