Closed sjdemartini closed 3 months ago
oof, it turns out this breaks in a non-obvious way in consuming packages due to https://github.com/mui/material-ui/issues/35233 (specifically what's described in https://github.com/mui/material-ui/issues/35233). i.e., material-ui does not properly support imports of icons at their subpaths in an ESM context, same as https://stackoverflow.com/q/72008357/4543977. e.g. trying to use the ColorSwatchButton
in an external package with this version of mui-tiptap gives:
Warning: React.jsx: type is invalid -- expected a string (for built-in components) or a class/function (for composite components) but got: object.
Check the render method of `ColorSwatchButton`.
at http://localhost:9000/static/ui/node_modules/.vite/deps/mui-tiptap.js?v=924a118a:6591:25
...
Will try some workarounds.
Superseded by #259
Copying notes from #259 and adding more context about why this approach didn't work when I tried it: Both @mui/icons-material
(https://github.com/mui/material-ui/issues/35233) and lodash
(https://github.com/lodash/lodash/issues/5107) have problems being imported in a consuming package when using ESM. The path rewriting 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
If I force the @mui/icons-material
imports to add a file extension via the import renaming, which per https://nodejs.org/api/esm.html#esm_mandatory_file_extensions should be necessary for any package that doesn't have an "exports"
field (and @mui/icons-material
does not), it breaks with a different problem:
/path/to/mui-tiptap-in-node/node_modules/@mui/icons-material/esm/FormatColorFill.js:3
import createSvgIcon from './utils/createSvgIcon';
^^^^^^
SyntaxError: Cannot use import statement outside a module
since icons-material
doesn't in fact use proper ESM syntax internally. It's unclear whether this can be resolved without MUI fixing ESM on their end.
Should resolve https://github.com/sjdemartini/mui-tiptap/issues/256.
This changes our packaging approach to using bundling via tsup, whereas to date we've only used
tsc
to generate separate JS files. It seems bundling (but not including external dependencies) is standard from most major packages I audited, and importantlytsup
greatly simplifies how the build files are generated, ensuring CJS and ESM compatibility, in a way that was otherwise quite annoying.This also now adds source maps, which will likely be useful for users of mui-tiptap.
Other options considered:
tsup
worked much more easily, such as excluding external dependencies by default, and building for CJS specifically rather than UMD."type": "module"
and "NodeNext"moduleResolution
with typescript, which means all imports within mui-tiptap have to use.js
extensions, etc., which is quite annoying. I'd prefer minimal churn here.Some related reading:
With this,
npx @arethetypeswrong/cli
andnpx publint
now show as valid (now also part of CI):whereas the approach in https://github.com/sjdemartini/mui-tiptap/pull/257 was not :