Improve attaching libraries when a plugin is needed

[quicksketch]

Right now in ckeditor_library_info(), all possible external plugins are added as dependencies of the main library. This includes backdrop-basic-styles, backdrop-image, and backdrop-link plugins. Although most instances will probably use these 3 plugins, and loading the plugin JS file doesn't mean it will be accessible in the toolbar or executed by CKEditor. Still, we shouldn't be attaching libraries we know aren't going to be used.

[indigoxela]

all possible external plugins are added as dependencies

That's not completely correct, though. Plugins implemented in this module... 😉

Additional plugins currently have to do the same.

[indigoxela]

Again ... an issue, I really tried to solve, but failed.

I can't see, how this could work without bigger changes in the filter module, which would in turn probably break CK4 and other editors.

Hoping for someone smarter to proceed here. 😉

[quicksketch]

@indigoxela I filed a PR at #82 that makes an attempt at this. Since all of our dependency logic is in ckeditor5_get_config(), I used that function to build our library dependencies. But since the library list is not necessary as a JavaScript setting, I wrapped ckeditor5_get_config() in two separate functions: one for getting JS settings and one for getting a library list.

Ultimately, the end-result is the same for CKEditor 5 core module when used by itself, but this should make contrib modules extending this one have a much easier time of attaching libraries, since they no longer would need to use hook_library_info_alter() to add their dependencies. Now they just specify the library in their hook_ckeditor5_plugin_info(), then ckeditor_library_info() will load that library information automatically.

[indigoxela]

Interesting approach, will test ASAP 👍

[quicksketch]

In the current approach the code checks if a library is in use across all text formats. Another option would be to call ckeditor5_plugins() and load all libraries used by any plugins. It wouldn't be quite as efficient on the front-end in the event you had an installed plugin but hadn't loaded its button into any text formats, but it might avoid some overly complex checking and possibly avoid recursive loops by contrib modules that implement filter format alter hooks.

[indigoxela]

🎉 Yes, this works as expected (tested with feef plugin, which I already had around). The previously necessary hook_library_info_alter to squeeze in the dependency's now obsolete. 👍

Mind to explain the danger of creating recursive loops with hook_filter_info_alter()? Or which hook did you mean? When would one run into that?

BTW, I think it would actually be OK to always load all libraries of installed plugins – I'd actually assume that:

• Only few cke5 plugins will ever exist in contrib land (for reasons)
• If they're installed, that's on purpose to actually use them
• The backdrop (core) link and image plugins will probably (almost) always be in use

So, if you have something in mind, that seems easier to maintain, sure - give it a try, if you like. 👍

I've left a question on the PR re the usage of the static cache in ckeditor5_get_config, which I don't understand. 😉

[quicksketch]

Mind to explain the danger of creating recursive loops with hook_filter_info_alter()? Or which hook did you mean? When would one run into that?

I'm having a little trouble coming up with a problematic scenario. I guess I'm just worried that hook_library_info() and hook_filter_format_info() are both hooks used relatively early in the bootstrap and are needed on nearly every request. Having one call the other seems like trouble.

If they're installed, that's on purpose to actually use them

One exception to this might be the Full Screen plugin in #76. If we bundle that plugin, it will always be loaded even if it's not used.

I'll take a spin at not calculating library usage and just add associated plugin libraries all the time and see if that is cleaner than the current approach.

[quicksketch]

I filed another PR at #83 that loads all plugin libraries. I think I'm more in favor of that approach.

I kept the static caching as I realized that ckeditor5_get_config() gets called once per format per field. And now that we're calling ckeditor5_plugins() in hook_library_info(), that function is now getting called twice per request as well.

[indigoxela]

Many thanks, will try to test soon.

Re the "Full Screen" (actually Maximize) plugin: it's relatively small, ~ 2KB js/css, and we still have the option to just make it a contrib project.

Sure, it would be more elegant to only attach plugin code as needed. But if that leads to complicated code, and given our CKE dll build has 1.4MB, where 2KB more don't really matter, I think it might be OK.

Just to make sure: we attach js/css, but js is never called, as the CKE instance isn't even aware of that code, right? How about CSS? I know, that's a theoretical questions, but could that lead to any problems (just being attached)?

[quicksketch]

Just to make sure: we attach js/css, but js is never called, as the CKE instance isn't even aware of that code, right? How about CSS?

Great questions. The JS would never actually be called. I've had the experience several times already where I try to use some of the (many) unused plugins that are bundled with the DLL build of CKEditor, but I forgot to specify it in the plugins config key.

The CSS on the other hand would be present on the page. Since CKEditor 5 does not use an iframe, it would affect any content within the editor that matches the given CSS selectors. I would expect however that the plugin would be mostly targeting classes added by the JavaScript plugin. Examples like the caption support, resize handles, link indicators, etc all target classes added by their respective plugins, so I don't think most plugins would affect what you see in the editor.

[quicksketch]

For now I took the simpler approach of #83 and closed out the alternative approach at #82.