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

ESM compatibility - unable to build #256

Closed
tehbelinda opened this issue Aug 16, 2024 · 4 comments · Fixed by #259
Closed

ESM compatibility - unable to build #256

tehbelinda opened this issue Aug 16, 2024 · 4 comments · Fixed by #259

Comments

@tehbelinda
Copy link

Describe the bug

When including extensions from mui tiptap in a node js environment with es modules, it fails to build

The error is:

import { FontSize, } from "mui-tiptap";
         ^^^^^^^^
SyntaxError: Named export 'FontSize' not found. The requested module 'mui-tiptap' is a CommonJS module, which may not support all module.exports as named exports.
CommonJS modules can always be imported via the default export, for example using:

import pkg from 'mui-tiptap';
const { FontSize, } = pkg;

Tried the default as suggested, but then that resulted in:

(node:5017) Warning: To load an ES module, set "type": "module" in the package.json or use the .mjs extension.
/project/workspace/node_modules/mui-tiptap/dist/esm/index.js:1
export { default as ControlledBubbleMenu, } from "./ControlledBubbleMenu";
^^^^^^

SyntaxError: Unexpected token 'export'

To Reproduce

https://codesandbox.io/p/devbox/pensive-volhard-hyhtls

Steps to reproduce the behavior:

  1. Run npm start to see the first error
  2. Comment out the Case 1 imports and uncomment the Case 2 import
  3. Run npm start again, see the second error

Expected behavior

Running npm start should successfully build and run the index.ts code.

Screenshots

N/A

System (please complete the following information)

  • mui-tiptap version: 1.9.5
  • Node version: v20.9.0

Additional context

I'm trying to convert HTML into a Tiptap format on a Hocuspocus nodejs server with TiptapTransformer (https://tiptap.dev/docs/guides/output-json-html), and would like to use the same Mui TipTap extensions that we use on our frontend for compatibility. I can import the rest of the Tiptap extensions without issues.

I noticed this package.json does have separate exports for import/require, so it seems like ESM should work. However it doesn't have "type": "module", could that be an issue?

@sjdemartini
Copy link
Owner

sjdemartini commented Aug 19, 2024

Thanks for reporting! I've opened a PR #257 which I think should resolve it, based on some reading I did, but I haven't yet tested that it resolves the issue in a node project. Once I do, I'll plan to merge and publish a new version.

sjdemartini added a commit that referenced this issue Aug 21, 2024
Should resolve #256

Both `@mui/icons-material`
(mui/material-ui#35233) and `lodash`
(lodash/lodash#5107) have problems being
imported in a consuming package when using ESM. The workarounds
attempted in #258 almost
seemed to work (didn't break a downstream bundled package using Vite),
but still caused problems for the original example node application
https://codesandbox.io/p/devbox/pensive-volhard-hyhtls, with errors
like:

```
Error: Cannot find module '/path/to/mui-tiptap-in-node/node_modules/@mui/icons-material/esm/FormatColorFill
```

This approach is inspired by what tss-react does
https://github.com/garronej/tss-react/blob/f5351e42e33f35f18415cfc1ffc6b08eb8ce4d25/package.json
(e.g. see here
garronej/tss-react@4699702
and garronej/tss-react#164).

With this change, this code now works in the node context (though
slightly odd):

```
import pkg from "mui-tiptap";
const { FontSize, HeadingWithAnchor, ResizableImage, TableImproved } = pkg;
```
sjdemartini added a commit that referenced this issue Aug 21, 2024
Should resolve #256.

Both `@mui/icons-material`
(mui/material-ui#35233) and `lodash`
(lodash/lodash#5107) have problems being
imported in a consuming package when using ESM. The workarounds
attempted in #258 almost
seemed to work (didn't break a downstream bundled package using Vite),
but still caused problems for the original example node application
https://codesandbox.io/p/devbox/pensive-volhard-hyhtls, with errors
like:

```
Error: Cannot find module '/path/to/mui-tiptap-in-node/node_modules/@mui/icons-material/esm/FormatColorFill
```

This approach is inspired by what tss-react does
https://github.com/garronej/tss-react/blob/f5351e42e33f35f18415cfc1ffc6b08eb8ce4d25/package.json
(e.g. see here
garronej/tss-react@4699702
and garronej/tss-react#164).

With this change, this code now works in the node context (though
slightly odd):

```
import pkg from "mui-tiptap";
const { FontSize, HeadingWithAnchor, ResizableImage, TableImproved } = pkg;
```
@sjdemartini
Copy link
Owner

sjdemartini commented Aug 21, 2024

@tehbelinda I've spent an inordinate amount of time trying to resolve this, and it's shockingly difficult to find a working solution. CJS vs ESM packaging/resolution is a frustrating mess! My original solution #257 didn't seem to work in practice, and #258 seemed to work in consuming bundled packages but then I tested on your node project where it still failed (even after trying to update imports for icons-material and lodash) 😢. So #259 seemed to be an approach that should hopefully work for various end-users, forcing CJS in the node environment.

I wouldn't be surprised if mui-tiptap still does not (and has never) properly packaged for ESM despite exporting files for that context, due to the quirks/incompatibilities with ESM in mui/icons-material and lodash, mentioned a bit in #259 (and attempted to resolve in #258). But I hadn't heard about specific problems to date, and from what I can tell this new approach of forcing CJS works 🤷

Published in v1.9.6. If you happen to have a better suggested fix or if this is still causing problems, I'd greatly appreciate suggestions on how it can be handled!

In the end, you'll want to use this syntax:

import pkg from 'mui-tiptap';
const { FontSize } = pkg;

@tehbelinda
Copy link
Author

appreciate you digging into this, it is working! only strange thing is that I had to add @emotion/styled to my dependencies, otherwise I would get this:

node:internal/modules/cjs/loader:1144
  const err = new Error(message);
              ^

Error: Cannot find module '@emotion/styled'
Require stack:
- /Users/bteh/edsa/dart-hocuspocus/node_modules/@mui/styled-engine/node/index.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/@mui/system/index.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/@mui/material/node/styles/adaptV4Theme.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/@mui/material/node/styles/index.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/@mui/material/node/index.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/mui-tiptap/dist/cjs/ControlledBubbleMenu.js
- /Users/bteh/edsa/dart-hocuspocus/node_modules/mui-tiptap/dist/cjs/index.js

not sure if this helps as you may have already seen this and I have never tried it myself, but this post suggests using rollup to support both: https://stackoverflow.com/a/75348391

@sjdemartini
Copy link
Owner

sjdemartini commented Aug 21, 2024

@tehbelinda Glad it's working! Yeah, that's expected, since @mui/material requires @emotion/styled as a peer dependency: https://mui.com/material-ui/getting-started/installation/#default-installation (this dep is listed in mui-tiptap's example install instructions, but it's not a peer of mui-tiptap, but rather a peer of a peer)

As for that Stack Overflow post, I did actually see that very post and did try rollup (via Vite library mode as mentioned in #258 (comment)). I didn't go too far with it, since it seemed less capable than tsup, which is what that PR was using. But I believe it will yield a similar result where @mui/icons-material and lodash do not work properly in a Node ESM context, as mentioned in #259. 😕

sjdemartini added a commit that referenced this issue Sep 15, 2024
When `"type": "commonjs"` is specified in our package.json, it
apparently causes the following problem in NextJS:

```
Module parse failed: 'import' and 'export' may appear only with 'sourceType: module' (1:0)
> export { default as ControlledBubbleMenu, } from "./ControlledBubbleMenu";
| export { default as LinkBubbleMenu, } from "./LinkBubbleMenu";
| export { default as MenuBar } from "./MenuBar";
```

This is odd, since we indeed *are* using "commonjs" type, as that's the
default, but it seems that fortunately merely omitting this resolves
import errors in NextJS, and still supports NodeJS.

Should resolve #264,
removing one of the changes from
#259, while still
hopefully keeping #256
fixed.
sjdemartini added a commit that referenced this issue Sep 15, 2024
When `"type": "commonjs"` is specified in our package.json, it
apparently causes the following problem in NextJS:

```
Module parse failed: 'import' and 'export' may appear only with 'sourceType: module' (1:0)
> export { default as ControlledBubbleMenu, } from "./ControlledBubbleMenu";
| export { default as LinkBubbleMenu, } from "./LinkBubbleMenu";
| export { default as MenuBar } from "./MenuBar";
```

This is odd, since we indeed *are* using "commonjs" type, as that's the
default, but it seems that fortunately merely omitting this resolves
import errors in NextJS, and still supports NodeJS.

Should resolve #264,
removing one of the changes from
#259, while still
hopefully keeping #256
fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment