Problem
- Some external libraries are contained in specific core modules, which prevents contrib from using them without having to depend on the respective core module.
Details
-
Previously,
core.libraries.ymldid not exist, so e.g., System module registering CKEditor module's library made little to no sense. -
By introducing the capability for the Drupal core base system itself to register libraries, the new world order is this:
- All external libraries supplied by Drupal core are placed into
/core/assets/vendor. - All of those libraries are registered by
core.libraries.yml. - Placing an external library into a core module is discouraged and requires a very sound, solid, and sophisticated reasoning.
- All external libraries supplied by Drupal core are placed into
-
Here is why:
- A contributed or custom module that would like to use the existing bundled library but does not like the usage/implementation of the core module, would (1) either have to needlessly depend on the core module to get the library registered or (2) re-register the library from scratch on its own (which breaks dependencies of other modules, since the library owner/provider would no longer be "core").
- Re-registering a core library in a contrib/custom module would be close to impossible to manage with regard to the declared library version. Drupal core might ship with an updated version, but that will not be reflected in the contrib module's declaration unless it is updated accordingly.
In short, this is required for e.g. Wysiwyg module to use the CKEditor library bundled with core, without having to depend on the ckeditor.module in core. Same for other libraries/modules/use-cases.
Task
- Move the picturefill.js library from Picture module into core.
- Move the jquery.joyride library from Tour module into core.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | lib.move_.2.patch | 3.97 KB | sun |
Comments
Comment #1
sunPicturefill is actually a bit weird — does anyone know why we diverged from upstream, instead of contributing upstream?
Comment #2
sunLet's ignore the topic of #1 here and discuss that in #2203431: [meta] Various asset (JavaScript) libraries have to be updated to a (minified) stable release prior to 8.0.0 instead.
Attached patch moves both libraries into core.
Comment #3
wim leersAgreed.
Any reason why we don't move CKEditor and others here already? :)
Comment #4
sunI added explicit @todos to the new .libraries.yml files in the original conversion. These two libraries are the only two that are not in
/core/assets/vendoryet. All others are in there already.The declaration of the
ckeditorlibrary was moved intocore.libraries.ymlin the parent/original issue already.Thus, moving these two is all that is required to resolve this issue.
Comment #5
sunComment #6
sunAny additional questions? :-)
Comment #7
wim leerslgtm
Comment #8
webchickI guess this works. It's a little weird for something in a super optional core module (Tour) to be in "core" core library definitions, though.
This solution obviously won't work for contrib, either, since contrib modules can't hack core.services.yml (beyond hook_library_info_alter() I suppose...), and we don't have a central "contrib.services.yml" to ensure one library registration per Drupal installation. So it's not clear to me how contrib is going to avoid all of the problems listed in the issue summary when for example both Views Slideshow and Media module want to use different versions of the same slider library. I guess we'll need thin module wrappers around every JS lib out there and these modules to declare dependencies on them?
At any rate, though, this hack will work for the libraries used by core, and it sounds like we've already started doing this anyway w/ CKEditor, which is also a dependency of an optional core module, so this is just making all the other libraries consistent with this approach.
Committed and pushed to 8.x.