ueberdosis / tiptap

The headless rich text editor framework for web artisans.
https://tiptap.dev
MIT License
27.13k stars 2.26k forks source link

Popper & Tippy should not be in a bundled app by default #3170

Open VincentDauliac opened 2 years ago

VincentDauliac commented 2 years ago

What problem are you facing?

Event if I don't use the bubble & floating menus, popper.js and tippy.js are included in the final bundle of my app (vue3 vite app). While the (undocumented) enableCoreExtensions: false option on the editor is a good addition, this architecture does not allow to exclude popper and tippy with tree shaking.

What’s the solution you would like to see?

It would be great to have the possibility to use a "bare" editor and decide what core extension to import.

What alternatives did you consider?

I don't see any alternative, am i missing something ?

Anything to add? (optional)

No response

Are you sponsoring us?

bdbch commented 2 years ago

Thanks @VincentDauliac! Since we did the same with lowlight (users need to implement their own lowlight version now) I think it also makes sense with tippy. I'll take a look when I find some time - if someone else wants to look into this in the meantime feel free!

philiprenich commented 1 year ago

Tippy / Popper should be dependencies (or peers deps) of bubble and floating menus though, correct? It shouldn't be part of the main tiptap packages.

And if I'm understanding now NPM works well enough, peer would allow me to use the same version of popper I have elsewhere in my app and not duplicate the code/import. Is that right?

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days

philiprenich commented 1 year ago

@bdbch Not sure what the process is for acknowledged items like this, but thought I'd fight the Stale Bot once at least :) Do you have a reference for what happened lowlight? Maybe I can take a look at duplicating the methodology for this.

bdbch commented 1 year ago

With our most recent changes to prosemirror in general I think we can also move Tippy and everything non-tiptap related to peerDeps too. I'll see if I can push this in between the next release.

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 7 days

Nantris commented 1 year ago

Not stale.

YousefED commented 1 year ago

I would love to see this resolved as well. I think we can break this issue down into two problems:

1) should tippy be a peer dependency or not? 2) should tiptap/react depend on @tiptap/extension-bubble-menu and @tiptap/extension-floating-menu

imo fixing (2) would be the best. if we don't need the menus, those extensions shouldn't be included. That's in-line with how other extensions of tiptap work, they're all optional to include

Nantris commented 1 year ago

I agree that tiptap/react shouldn't be pulling in extensions which might rely on tiptap/react. If anything it should be the reverse that those extensions depend on tiptap/react.

Nantris commented 1 year ago

Is this planned? I know Tippy isn't huge but it's still between 26 and 80kb (unzipped.)

bdbch commented 1 year ago

Not planned yet. If anyone wants to pick this up I'd look over it. In general I agree that it should not be included in the bundle itself but you will need Tippy / Popper anyway as the floating/bubble react and vue implementations are requiring them.

Best solution I think would be to move those out into their own packages which only then require those dependencies (@tiptap/react-floating-menu and @tiptap/react-bubble-menu for example) but this would be a bigger change. That way if those menus are not used, popper and tippy wouldn't be included.

nperez0111 commented 2 months ago

We should also do the same for vue, but given that it is a breaking change it is deprioritized right now. Unless someone figures out a way to not make it a breaking change

Nantris commented 2 months ago

Thanks for the update on this! Makes sense that it's a low priority, and I appreciate the update from the devs!

benkroeger commented 2 months ago

at least when building with vite, neither popper nor tippy make it into the built output files as long as you don't use e.g. the BubbleMenu component from @tiptap/vue-3. Not sure regarding react though... but I assume build-tools behave similarly.

Simply mounting the BubbleMenu component somewhere in my project increases the output size by 40kb and includes tippy as well as popper.

To me, that looks like tree-shaking is working as expected here.

bdbch commented 2 months ago

I replaced tippy with Floating UI in #5398 - but still have it as a fixed dependency for the floating-menu and bubble-menu extensions.

I would be fine with moving it to peerDependencies and let the user handle the versioning / installation.

philiprenich commented 2 months ago

Yeah, I think it's useful to have it as a peer. I'm using tippy for an unrelated part of my app and it's unfortunate if it gets loaded in twice because of it being bundled with the menus.

Nantris commented 2 months ago

@philiprenich it would only be loaded once in almost all circumstances. One of the few exceptions is if you're requesting a version range that's incompatible with the version range TipTap requests.

bdbch commented 2 months ago

@philiprenich if we were to include Floating UI in 3.0.0 - would you rather configure the middlewares (https://floating-ui.com/docs/middleware) yourself and just have a blank slate or expect Tiptap to have default set active (for example the shift and flip middleware?).

Currently thinking about the options API for the new integration. I'm in for going peer, that would open us up to make the options more flexible.

philiprenich commented 2 months ago

@Nantris

@philiprenich it would only be loaded once in almost all circumstances. One of the few exceptions is if you're requesting a version range that's incompatible with the version range TipTap requests.

I'm not super familiar with tree-shaking. If I have Tippy in code that bundles to app.js and TipTap bundles to editor.js I'll still get it twice wouldn't I? Though as I type this out, I don't see how not having it as a dependency of TipTap would help anyway if I have two separate bundles that both use Tippy.

@bdbch Should TipTap's feature of being headless should guide this? I haven't used (yet) the Bubble or Floating menus, and while you don't want to ship something that feels broken, leaving the UX in the app developer's hands seems to be in line with the rest of TipTap. Shift and flip are pretty instrumental, but I wouldn't mind having to do that myself, especially if it meant doing it myself was the only way to not have them.

Nantris commented 2 months ago

@philiprenich it depends how it's bundled. Webpack is capable of making a shared bundle that both app.js and editor.js would rely on such that they'd only bundle the dependency once in that third .js file. But it could also be configured to the contrary. And indeed, as you note, this would be unrelated to whether it's listed as a dependency by TipTap.

benkroeger commented 2 months ago

when consuming the currently shipped code as ESModule, one consumes transpiled but NOT bundled js files - which allows the build process of a project to only pick those pieces that are actually used - hence, the final build of a project would only include popper, tippy or floating-ui when actually using components that import any of those libraries. Also, as it has been said already, there would only be multiple versions of each library in the final build when there is a version missmatch between project and tiptap.

from @tiptap/extension-bubble-menu/dist/index.js

import tippy from 'tippy.js';