Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spin-off from #1737148: Explicitly declare all JS dependencies, don't use drupal_add_js:
Problem
hook_library_info()andhook_library_info_alter()is re-invoked and re-processed from scratch on every request indrupal_get_library(), although the registered information is not supposed to change.
Goal
- Improve performance.
- Prevent modules from registering library info that is dynamic (varies per request).
Proposed solution
- Cache hook_library_info() [per module].
| Comment | File | Size | Author |
|---|---|---|---|
| drupal8.library-info-cache.0.patch | 935 bytes | sun | |
Comments
Comment #1
sunComment #3
moshe weitzman CreditAttribution: moshe weitzman commentedI disagree with caches for info hooks like this. If a module is doing something slow during an info hook, it should do own caching. We need proof that performance is improved with this patch. As for
Prevent modules from registering library info that is dynamic (varies per request)., we don't babysit broken code, especially by adding caches. Use Doxygen and handbook for education.Comment #4
pounardmoshe +1 I definitely agree, this sounds like a useless cache. I prefer to see 100 module_invoke() running than a database query; I'm quite sure the database (or any other external storage server) query will be slower.
Comment #5
catchYeah unless this is shown to be slow, and the cache()->get() to be faster, I don't see the benefit either.
Comment #6
sunSeems I forgot one of the primary reasons for adding a cache here (probably because I wanted to make that a separate follow-up issue):
We need to properly resolve the dependencies between libraries. Right now:
1) The dependencies are "resolved" at runtime, but they are just simply processed in the order in which they are defined, not in the order of dependencies.
2) Nested dependencies (dependencies of dependencies) are not resolved, which means that every library has to specify all of its dependencies manually. Thus, it's very tricky to inject or remove a dependency via hook_library_alter().
3) Missing dependencies are determined at runtime only.
Comment #7
nod_2) They are resolved. We explicitly define them because it should be possible from contrib to build AMD definitions from the informations in hook_library_info(), see #1737148-99: Explicitly declare all JS dependencies, don't use drupal_add_js. You can have any level of dependencies, there is recursion going on in
drupal_process_attachedand when you end up with circular dependencies, things break.Comment #8
Wim LeersI can confirm that point 2, nested dependencies, are indeed resolved. The Aloha module leverages this.
EDIT: and both on D7 and D8, this works.
Comment #9
tim.plunkettFixing tags.
Comment #10
nod_Needs to be solved as a part of #1996238: Replace hook_library_info() by *.libraries.yml file.
Comment #11
nod_Comment #11.0
nod_Updated issue summary.