search

sjdemartini / mui-tiptap

A Material UI (MUI) styled WYSIWYG rich text editor, using Tiptap
MIT License
319 stars 43 forks source link

Why does this project use react-icons? #119

Closed the-serious-programmer closed 1 year ago

the-serious-programmer commented 1 year ago

Wanted to use this package, but seeing react-icons being added as an extra dependency to my project is a bummer. Would love if there is an option to only use the mui icons that are already available in every mui project.

sjdemartini commented 1 year ago

@the-serious-programmer I understand the concern and agree it would be nice to omit react-icons. This was discussed a bit here https://github.com/sjdemartini/mui-tiptap/issues/51#issuecomment-1595894816 if you'd like more context. In short, @mui/icons-material does not have a complete enough icon set to cover the various actions associated with interacting with a table, for the TableBubbleMenu/TableMenuControls:

img

(There are a couple other icons used from react-icons besides those: in MenuButtonAddTable and MenuButtonCodeBlock where MUI's icons didn't seem sufficient.)

Fortunately, react-icons is tree-shakeable, so if you don't use TableBubbleMenu, it shouldn't have an impact on your bundle size (more details in the linked comment above). And only the icons that do get imported should be included in your bundle, so it's effectively just a way to get the SVGs into the project in a seamless way. It seems preferable to me to support an interface like TableBubbleMenu out of the box in mui-tiptap, rather than to avoid the dependency and not have that UI built in (since a main goal of mui-tiptap is to provide most UI functionality that Tiptap itself being headless does not include).

I think one option here for mui-tiptap would be to make react-icons a direct dependency rather than peerDependency, since there isn't really a concern about a consuming project having a conflicting react-icons version (in the way that we must have MUI and Tiptap packages be peers to ensure things work properly). Then users of mui-tiptap will not have to add an extra dependency, and it would still "just work." Would that be preferable to you and feel like a resolution? If you have an alternative suggestion, please let me know.

bryanjtc commented 1 year ago

Shifting react-icons to the project dependencies is indeed a step in the right direction. However, I'd like to highlight that even after installation and unpacking, the package still appears to be quite substantial, at 77.7mb. Given that only a handful of icons are being utilized, it might be more efficient to incorporate those specific icons directly into the bundle.

sjdemartini commented 1 year ago

@bryanjtc Thanks for chiming in! Including the SVG icons directly in the project makes mui-tiptap development a bit less convenient (e.g. if we need to include another icon, it won't be as simple as an import), but I think that may be a cost worth paying to reduce the dependencies and installation/download size. I'll explore this and follow up.

sjdemartini commented 1 year ago

Alright, the latest release v1.3.0 removes the react-icons peer dependency and defines these additional icons internally within mui-tiptap instead. Thanks again @the-serious-programmer and @bryanjtc for bringing this up and discussing!