Ckeditor5: Widget type around plugin relies on paragraph plugin features but doesn't require it

[mlewand]

📝 Provide detailed reproduction steps (if any)

WidgetTypeAround plugin uses insertParagraph command without requiring paragraph feature.

So if you add widget plugin without paragraph you'll end up with a type around UI element visible for the widget but clicking it throws an exception:

Uncaught CKEditorError: commandcollection-command-not-found {"commandName":"insertParagraph"}
Read more: https://ckeditor.com/docs/ckeditor5/latest/framework/guides/support/error-codes.html#error-commandcollection-command-not-found
    at CommandCollection.execute (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:35002:10)
    at ClassicEditor.execute (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:35822:25)
    at WidgetTypeAround._insertParagraph (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:103137:10)
    at https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:103509:9
    at Document.<anonymous> (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:103162:5)
    at Document.fire (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:98008:30)
    at MouseObserver.fire (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:74026:18)
    at MouseObserver.onDomEvent (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:74476:8)
    at ProxyEmitter.listenTo.useCapture (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:74009:11)
    at ProxyEmitter.fire (https://customDomain.xyz:1234/ckeditor5-engine/tests/manual/tickets/4600/1.js:98008:30)

See https://github.com/ckeditor/ckeditor5/blob/bd97c2649b4793f15a3996c86ca542c82967a4af/packages/ckeditor5-engine/tests/manual/tickets/4600/1.js file for a reference

✔️ Expected result

One of the two:

• Type around feature is not available if there is no paragraph feature.
• Type around has a dependency on paragraph.

I kind of lean towards the first one.

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[jodator]

Hmm... I think I remember why. A very similar thing happens in tables where we enforce <paragraph> inside <tableCell> and no plugin requires it:

https://github.com/ckeditor/ckeditor5/blob/7adbaf3414e1995403d2f734a6b0e2d0dbad61d3/packages/ckeditor5-table/src/table.js#L43

https://github.com/ckeditor/ckeditor5/blob/dc85a1e6495d3721c80128b546f6c0789b80d8ba/packages/ckeditor5-table/src/tableediting.js#L163

The rationale there was that the Table plugin does not make sense without Paragraph and AFAIR the assumption was that it will always be present. Similarly, we do not require other, core but not in engine, features like typing.

This is confusing for sure and I think that this would be an ideal candidate for an ADR to track it for the future.

[Reinmar]

In the past, we had man more such plugins that were "peer dependencies". This is – they should be loaded, but we didn't want to create a direct dependency to a concrete implementation. Clipboard was such an example – there are a couple features relying on its events, but we did not add that plugin as a dep of these plugins allowing for plugging a different implementation easily.

We changed that, though, because it's confusing. Also, webpack's module replacement plugin allows injecting your own implementations quite easily (although, I'm not sure if you can then import the original dependency in your replacement for it, if you'd like to extend it). 

However, the paragraph feature is a special case here. The paragraph element is used even in the engine. It's the "default block type" and must be supported in all editors except these enabled on paragraph-like elements. We could actually have some warning if paragraphs are not supported (the official or custom plugin is not loaded) if the editor has a container-like roots (can contain paragraphs inside). 

Also, I agree this deserves an ADR. It'd be a good moment to make our minds on this "peer dependencies" problem.

[Reinmar]

BTW, this is definitely DX, not UX.

[jodator]

We changed that, though, because it's confusing. Also, webpack's module replacement plugin allows injecting your own implementations quite easily (although, I'm not sure if you can then import the original dependency in your replacement for it, if you'd like to extend it).

In some cases it is doable. I did a similar trick in my JSX-like downcast converter API (soon to be published on the blog :crossed_fingers:). If the replaced plugin is required differently, then you can replace it. Also AFAIR it can be replaced for some part of the code only.