search

vercel / next.js

The React Framework
https://nextjs.org
MIT License
126.4k stars 26.9k forks source link

Dependencies with `exports` field break on pages router with `esmExternals: false` #65056

Open DiegoAndai opened 6 months ago

DiegoAndai commented 6 months ago

Link to the code that reproduces this issue

https://github.com/DiegoAndai/esm-test-13-nextjs-pages-mui-exports-ts

To Reproduce

  1. Clone https://github.com/DiegoAndai/esm-test-13-nextjs-pages-mui-exports-ts
  2. Run npm install
  3. Run npm run dev
  4. Open http://localhost:3000 in the browser
  5. You should see the error

Description

We're trying to add the exports field to the package.json of MUI core libraries (PR | Docs). This app is using the MUI libraries from the PR build (e.g., https://pkg.csb.dev/mui/material-ui/commit/fb7a4ff2/@mui/material). When testing the libraries on a Next.js app with pages router and esmExternals: false, we get the following error:

⨯ TypeError: _mui_utils_deepmerge__WEBPACK_IMPORTED_MODULE_3__ is not a function
Full error ``` ⨯ TypeError: _mui_utils_deepmerge__WEBPACK_IMPORTED_MODULE_3__ is not a function at createPalette (webpack-internal:///./node_modules/@mui/material/styles/createPalette.mjs:266:27) at createTheme (webpack-internal:///./node_modules/@mui/material/styles/createTheme.mjs:47:83) at eval (webpack-internal:///./node_modules/@mui/material/styles/defaultTheme.mjs:7:82) at ./node_modules/@mui/material/styles/defaultTheme.mjs (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/vendor-chunks/@mui.js:260:1) at __webpack_require__ (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/webpack-runtime.js:33:42) at eval (webpack-internal:///./node_modules/@mui/material/styles/styled.mjs:8:75) at ./node_modules/@mui/material/styles/styled.mjs (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/vendor-chunks/@mui.js:310:1) at __webpack_require__ (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/webpack-runtime.js:33:42) at eval (webpack-internal:///./node_modules/@mui/material/Button/Button.mjs:13:77) at ./node_modules/@mui/material/Button/Button.mjs (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/vendor-chunks/@mui.js:20:1) at __webpack_require__ (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/webpack-runtime.js:33:42) at eval (webpack-internal:///./node_modules/@mui/material/Button/index.mjs:6:69) at ./node_modules/@mui/material/Button/index.mjs (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/vendor-chunks/@mui.js:40:1) at __webpack_require__ (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/webpack-runtime.js:33:42) at __barrel_optimize__?names=Button!=!./node_modules/@mui/material/index.mjs (/Users/diegoandai/MUI/experiments/esm-test-13-nextjs-pages-mui-exports-ts/.next/server/pages/index.js:26:75) { page: '/' } GET / 500 in 128ms ```

This seems related to default import/export interop between esm and cjs: https://www.typescriptlang.org/docs/handbook/modules/appendices/esm-cjs-interop.html

The expected is for the app to run without erroring.

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 22.5.0: Thu Jun  8 22:22:22 PDT 2023; root:xnu-8796.121.3~7/RELEASE_X86_64
  Available memory (MB): 16384
  Available CPU cores: 16
Binaries:
  Node: 18.19.0
  npm: 10.4.0
  Yarn: 1.22.21
  pnpm: 8.14.1
Relevant Packages:
  next: 14.2.3 // Latest available version is detected (14.2.3).
  eslint-config-next: 14.2.3
  react: 18.3.0
  react-dom: 18.3.0
  typescript: 5.4.5
Next.js Config:
  output: N/A

Which area(s) are affected? (Select all that apply)

Pages Router

Which stage(s) are affected? (Select all that apply)

next dev (local)

Additional context

DiegoAndai commented 5 months ago

Hey @huozhi, may I ask you to look into this issue and https://github.com/vercel/next.js/issues/64796? Thanks in advance!

eps1lon commented 4 months ago

I tried to create a minimal repro, but I couldn't figure it out. I'm sorry about this.

With failed you mean everything worked in minimal repros?

Does it work if you run npm run dev --turbo?

When testing the libraries on a Next.js app with pages router and esmExternals: false, we get the following error:

And with esmExternals: true it works?

huozhi commented 4 months ago

14.2.x turbopack havent supported esmExternals so it's not able to run it.

The root cause is for pages router, since you disable the esmExternals it's resolving cjs on server side and client still resolving esm but will barrel optimization on client side. We can probably disable the barrel optimization when esmExternals is false. But it gonna slow down a lot on bundling.

Ideally you shouldn't need to disable esmExternals option, curious what's reason that makes you disable it?

DiegoAndai commented 3 months ago

With failed you mean everything worked in minimal repros?

Yes, I tried to reproduce the issue in a new Next.js app but couldn't.

And with esmExternals: true it works?

Yes, probably because of what @huozhi explained in the comment above.

Ideally you shouldn't need to disable esmExternals option, curious what's reason that makes you disable it?

I agree, but we are not able to use esmExternals: true because of another longstanding issue 😅: https://github.com/vercel/next.js/issues/49898, which we worked around with esmExternals: false: https://github.com/vercel/next.js/issues/49898#issuecomment-1762202697. The linked issue is still reproducible in our docs with Next.js 14 and esmExternals: true

titanve commented 3 months ago

Hi!

Any progress on this issue? It is affecting us as well. Thanks!

huozhi commented 3 months ago

@DiegoAndai the repro in #49898 is incorrect, it's require a ESM and expect it to be sync. What's the underlaying issue that make you to disable the flag? I think we need to fix that one

DiegoAndai commented 3 months ago

@DiegoAndai the repro in https://github.com/vercel/next.js/issues/49898 is incorrect, it's require a ESM and expect it to be sync.

What do you mean by "it's incorrect"?

What's the underlaying issue that make you to disable the flag?

If I disable it, I get the same error that prompted us to create https://github.com/vercel/next.js/issues/49898. This is the error:

 ⨯ ../packages/mui-icons-material/lib/ArrowDropDownRounded.js (11:30) @ eval
 ⨯ TypeError: (0 , _createSvgIcon.default) is not a function
    at eval (webpack-internal:///../packages/mui-icons-material/lib/ArrowDropDownRounded.js:11:64)
    at ../packages/mui-icons-material/lib/ArrowDropDownRounded.js (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:3703:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/brandingTheme.ts:16:98)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/brandingTheme.ts:1:21)
    at ../packages/mui-docs/src/branding/brandingTheme.ts (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:49:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/index.ts:3:72)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/index.ts:1:21)
    at ../packages/mui-docs/src/branding/index.ts (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:60:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///./pages/_document.js:21:76)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13) {
  page: '/'
}
   9 | var _createSvgIcon = _interopRequireDefault(require("./utils/createSvgIcon"));
  10 | var _jsxRuntime = require("react/jsx-runtime");
> 11 | var _default = exports.default = (0, _createSvgIcon.default)( /*#__PURE__*/(0, _jsxRuntime.jsx)("path", {
     |                              ^
  12 |   d: "m8.71 11.71 2.59 2.59c.39.39 1.02.39 1.41 0l2.59-2.59c.63-.63.18-1.71-.71-1.71H9.41c-.89 0-1.33 1.08-.7 1.71"
  13 | }), 'ArrowDropDownRounded');
TypeError: (0 , _createSvgIcon.default) is not a function
    at eval (webpack-internal:///../packages/mui-icons-material/lib/ArrowDropDownRounded.js:11:64)
    at ../packages/mui-icons-material/lib/ArrowDropDownRounded.js (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:3703:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/brandingTheme.ts:16:98)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/brandingTheme.ts:1:21)
    at ../packages/mui-docs/src/branding/brandingTheme.ts (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:49:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/index.ts:3:72)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13)
    at eval (webpack-internal:///../packages/mui-docs/src/branding/index.ts:1:21)
    at ../packages/mui-docs/src/branding/index.ts (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/pages/_document.js:60:1)
    at __webpack_require__ (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:33:42)
    at eval (webpack-internal:///./pages/_document.js:21:76)
    at __webpack_require__.a (/Users/diegoandai/MUI/DiegoAndai/material-ui/docs/export/server/webpack-runtime.js:97:13) {
  page: '/'
}

Here's more background: https://github.com/emotion-js/emotion/issues/3032#issuecomment-1550470774.

If you want to test it yourself, I've created a branch for it: https://github.com/DiegoAndai/material-ui/tree/disable-esm-externals-false. So you can

  1. git clone https://github.com/DiegoAndai/material-ui.git
  2. git checkout disable-esm-externals-false
  3. pnpm install
  4. pnpm docs:dev
  5. Go to localhost:3000 and the issue will show.

Note that this is still reproducible in next v14.

Thanks in advance for taking a look 😊.

huozhi commented 3 months ago

In #49898 's reproduction

There's a file cjs module requiring a ESM module, and they expect the require can get a sync module, which is not correct.

// ./common.js
module.exports = require("./esmish");
// ./esmish.js
export { default as unitless } from "@emotion/unitless";

This would break the module resolving when you expect them to work properly.

Do you know what's the similar case in mui or emotion-js that causes you have to disable esmExternals? In the future that option might not able to control anymore since the module can resolve properly if they are configured properly in package.json. That's why I say we don't want you to disable the flag. I'd love to learn more why it makes you do that.

DiegoAndai commented 3 months ago

@huozhi from the error I'm getting:

 X ../packages/mui-icons-material/lib/ArticleRounded.js (11:30) @ eval
 X TypeError: (0 , _createSvgIcon.default) is not a function

The issue seems to be this:

https://github.com/mui/material-ui/blob/next/packages/mui-icons-material/lib/ArticleRounded.js (and similar files) import from https://github.com/mui/material-ui/blob/next/packages/mui-icons-material/lib/utils/createSvgIcon.js, and the default export seems like is not set up properly. This is one instance but we probably have this setup throughout the mui codebase.

Might this be it?

Janpot commented 2 months ago

There's a file cjs module requiring a ESM module, and they expect the require can get a sync module, which is not correct.

@huozhi I've been investigating this a little bit, and your comment seems to point in the direction. Our setup involves a monorepo that contains workspaces for a next.js application and packages that are consumed by this application as well as published. Our setup contains some custom webpack configuration to transpile dependent workspaces by combining aliases with the default babel loader:

Inspecting the bundles with esmExternals: false, this setup produces a sync module (for e.g. @mui/utils), when this setting is removed, this produces an async module.

Some modules external to the repository also depend on packages from workspace. To make sure on the server they resolve these dependencies to the exact same version as the transpiled ones, they are omitted from the webpack externals as per:

Some of these packages (e.g. @mui/x-data-grid) don't export node.js compatible ESM modules, so it seems like webpack resolves to using their commonjs files. These call sync require on @mui/utils and that's where the problem happens.

I'm aware this is a bit of an exotic setup. I'm personally not a huge fan of the complexity, but it has great DX. We need a way out of this situation.