flarum / issue-archive

0 stars 0 forks source link

Bundling optional javascript depencencies, sortablejs, jquery #161

Open clarkwinkelmann opened 3 years ago

clarkwinkelmann commented 3 years ago

This is a general issue about how we could handle optional but common javascript dependencies.

Two libraries mostly come to mind:

The situation is well described by Sortable. Since there's no guarantee that Tags will be enabled, it forces all extensions that need it to bundle it: Tags, FoF/Mason, FoF/Masquerade, FoF/Links, Kilowhat/Formulaire all include a copy of that library.

The situation isn't too bad since most of those only require it on the admin side (Formulaire uses it on the forum side) but would be worst for libraries like jQuery which would most likely be used on the forum side as as such requires optimization.

The current extension API doesn't offer any easy solution to the problem. Currently the only way to reduce assets size while assuring the extension is loaded would be to load the library conditionally (either from CDN or separate assets) but that's not always convenient.

This is a problem other frameworks solve with a system of internal javascript dependencies. For example Wordpress has the $deps argument to wp_register_script, but I wouldn't call that solution very elegant.

Bundling any dependency obviously introduces version issues, because now you can no longer require a particular version of that library. But I think the benefit of not duplicating the code would outweigh the inability to be on the bleeding edge.

What I would propose is that we add a small subset of common javascript libraries to core. Those dependencies could be enabled from an extender by extensions that need them. Some syntax ideas:

(new Extend\Frontend('forum'))
    ->js(__DIR__.'/js/dist/forum.js')
    ->withJsLibrary('jquery')

or

(new Extend\Frontend('forum'))
    ->js(__DIR__.'/js/dist/forum.js')
    ->enableJQuery()
dsevillamartin commented 3 years ago

How would we include the code of these libraries? Would they be copied by webpack into some directory when building core JS?

clarkwinkelmann commented 3 years ago

Yes that's what I was imagining. Compile those libraries separately, almost like if they were extensions, and place them in another folder or just a different name in dist.

For packages that don't need to be global, they could be compiled with the same script we use for extensions. And if we implement any of flarum/issue-archive#165 this could be for this as well.

Another option could be to load those from CDN, but offer logic in Flarum that helps only add one <script src=""> tag instead of one for each extension. This could be a well defined list of libraries, or just a system in Assets where an extension registers a CDN resource URL, and if multiple extensions define a same URL, the subsequent ones are skipped.

tankerkiller125 commented 3 years ago

or just a system in Assets where an extension registers a CDN resource URL, and if multiple extensions define a same URL, the subsequent ones are skipped.

The problem I see with this idea is different extensions calling different versions of the same library OR calling the same library from different CDN providers.

clarkwinkelmann commented 3 years ago

The problem I see with this idea is different extensions calling different versions of the same library OR calling the same library from different CDN providers.

Definitely. I don't see this option being very popular. And a team like FoF could easily implement that across their extensions if needed. The true performance/de-duplication benefit would be with locally hosted libraries that must be in the assets files.

Generally, loading libraries on demand would make even more sense for some of those. sortableJS for example is rarely needed at page load, so it wouldn't be that bad if all extensions included a copy, and only loaded it when necessary.

It's only for extensions like jQuery or something used across many pages that you really want to include in the base assets AND not duplicate them for performance.

askvortsov1 commented 3 years ago

Maybe we could have 2 extender methods? One for JS code that should be included in the boot process / be immediately necessary (like jQuery), and another for code that should be loaded in via CDN after pages have loaded?

Then there's the problem of any dependencies that these packages, in turn, have. Would those be duplicated if jQuery and sortable were loaded in this way? Or would all dependencies need to be declared?

For the latter category of packaged (cdn ones), we could simply load in via the CDN link of the last booted extension with that package name?