Force node to use CJS to get around dependency import errors, and validate package types in CI #259
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Per recommendation from publint. https://publint.dev/rules#use_type > The package does not specify the "type" field. NodeJS may attempt to detect the package type causing a small performance hit. Consider adding "type": "commonjs".
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
force-pushed
the
use-cjs-in-node
branch
from
e84a7eb
to
a49802a
Compare
This was referenced Aug 21, 2024
sjdemartini
added a commit
that referenced
this pull request
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 pull request
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Third attempt to resolve #256 😅 (should supersede #258 and #257, see the former for more details/discussion)
Should resolve #256
Both
@mui/icons-material(mui/material-ui#35233) andlodash(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:This approach is inspired by what tss-react does (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):
and is considered valid by
arethetypeswrong:CJS vs ESM package resolution is an absolute mess—sad there's so much fragmentation and trouble getting things to play nicely with third party packages and different environments 😞