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

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

Open
DiegoAndai opened this issue Apr 25, 2024 · 10 comments
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. Pages Router Related to Pages Router.

Comments

@DiegoAndai
Copy link

DiegoAndai commented Apr 25, 2024

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

  • I've tested with Next.js 14.2.3, 13.5.1, and 13.4.0.
  • I tried to create a minimal repro, but I couldn't figure it out. I'm sorry about this.
@DiegoAndai DiegoAndai added the bug Issue was opened via the bug report template. label Apr 25, 2024
@github-actions github-actions bot added the Pages Router Related to Pages Router. label Apr 25, 2024
@DiegoAndai DiegoAndai changed the title Dependencies with exports field break on pages router with esmExternals: false Dependencies with exports field break on pages router with esmExternals: false | 'loose' Apr 25, 2024
@DiegoAndai DiegoAndai changed the title Dependencies with exports field break on pages router with esmExternals: false | 'loose' Dependencies with exports field break on pages router with esmExternals: false Apr 25, 2024
@DiegoAndai
Copy link
Author

Hey @huozhi, may I ask you to look into this issue and #64796? Thanks in advance!

@github-actions github-actions bot added the linear: next Confirmed issue that is tracked by the Next.js team. label Jun 10, 2024
@eps1lon
Copy link
Member

eps1lon commented Jun 24, 2024

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
Copy link
Member

huozhi commented Jun 24, 2024

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?

@huozhi huozhi removed the linear: next Confirmed issue that is tracked by the Next.js team. label Jun 25, 2024
@github-actions github-actions bot added the linear: next Confirmed issue that is tracked by the Next.js team. label Jul 1, 2024
@DiegoAndai
Copy link
Author

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 😅: #49898, which we worked around with esmExternals: false: #49898 (comment). The linked issue is still reproducible in our docs with Next.js 14 and esmExternals: true

@titanve
Copy link

titanve commented Jul 4, 2024

Hi!

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

@huozhi
Copy link
Member

huozhi commented Jul 5, 2024

@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
Copy link
Author

@DiegoAndai the repro in #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 #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: emotion-js/emotion#3032 (comment).

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
Copy link
Member

huozhi commented Jul 10, 2024

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
Copy link
Author

@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
Copy link
Contributor

Janpot commented Jul 29, 2024

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.

  • We can't use transpilePackages because our package sources are contained in a src folder and it doesn't look like there is a way to make import specifiers resolve to a typescript file that isn't adjecent to the js file it would resolve to. Is there a way to make transpilePackages aware of where it can find TS sources? Does the Next.js team plan to make it possible in some way to make transpilePackages aware of the TS source location?
  • If not, then I assume transpiling monorepo packages from sources isn't a use-case Next.js would like to solve. Is there a suggested workflow to ensure a good DX (hot reloading of changes to monorepo workspaces as dependencies of a Next.js app). I'm assuming the following would be supported
    • Instead of transpiling dependencies as part of the Next.js compilation, transpile each workspace independently in parallel
    • Use transpilePackages to hot reload changes in the Next.js app when the output of the previous changes
    • How to preserve the DX of indicating compilation errors in the browser as would happen with sources within the Next.js app? Is there a way for dependencies to interact with the Next.js error UI?
    • Does transpilePackages respect publishConfig.directory?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue was opened via the bug report template. linear: next Confirmed issue that is tracked by the Next.js team. Pages Router Related to Pages Router.
Projects
None yet
Development

No branches or pull requests

5 participants