Due to recent JS/CSS changes in D8 and their [yet unresolved] consequences (see #1805452: [meta] Asset library declarations, registry, usage, DX, compatibility, documentation), modules should no longer use drupal_add_js/css() directly, but instead register corresponding libraries including their dependencies instead.
This makes the stylesheets[]
.info file property obsolete for modules. (only for modules)
Its introduction unfortunately caused performance problems anyway down the line (see #1061924: system_list() memory usage), so by removing support for stylesheets[]
in module .info files, we could even consider to revert some of the performance optimizations that happened for retrieving module info.
Essentially, we want to revert #328357: Allow module .info files to add CSS/JS
Comment | File | Size | Author |
---|---|---|---|
#15 | drupal8.module-info-assets.15.patch | 11.97 KB | sun |
#9 | drupal8.module-info-assets.9.patch | 11.25 KB | sun |
#7 | drupal8.module-info-assets.7.patch | 10.06 KB | sun |
#2 | drupal8.module-info.2.patch | 6.76 KB | sun |
Comments
Comment #1
sunComment #2
sunFor starters.
Anyone willing to pick this up?
Comment #4
aspilicious CreditAttribution: aspilicious commentedIf my module wants to load css on every page where should I call my library than?
Comment #5
sunThat's discussed in #1029460: Find a better hook than hook_init() for drupal_add_js() and drupal_add_css()
Comment #6
catchYes I think it was a mistake to add this in the first place especially given the massive performance regression and subsequent workarounds we had to add for what is a minor convenience. Module authors can write PHP.
Comment #7
sunAttached patch performs the full conversion.
Comment #9
sunWow. Forum module. Unholy things going on in there. Added a @todo.
Comment #10
sunI think this patch is ready.
Created a follow-up issue for Forum module: #1866900: Forum pages are adjusting breadcrumb too late
Comment #11
nod_It works. I'm a bit worried about adding a whole bunch of drupal_add_css() calls since you can't preprocess the thing to remove the CSS file from there as #attached would allow. Then again #attached can't really be used at this level and it can be removed from somewhere else, just in a pretty obscure way probably (unless the stylesheet-remove patch makes that trivial?).
But I mean, it works.
Comment #12
sunYeah, I'm totally aware of that issue.
However, I did not want to get into scope creep here. The
drupal_add_css()
calls need to be replaced withhook_library_info()
definitions and properly #attached libraries, but that is to be figured out in #1813186: Integrate scripts/stylesheets from info/layout files with libraries & related issues — in particular, we'll have to find a solution for theme functions and theme hook preprocessors to use #attached. That's a big and complex topic of its own though.For this issue here, we should just get rid of those module .info file properties. One step at a time :)
Comment #13
nod_Me happy then :)
Comment #14
sunSorry, I did not notice that there's additional processing in _system_rebuild_module_data(), which calls _system_info_add_path().
Comment #15
sunBack to RTBC with those.
Comment #16
catchCommitted/pushed to 8.x, thanks! This will need a change notice.
Comment #17
dawehnerFeel free to review: http://drupal.org/node/1876152
Comment #18
aspilicious CreditAttribution: aspilicious commentedCan we add an example how to add this css/js to every page with drupal_add_css and drupal_add_js?
Comment #19
Steven Jones CreditAttribution: Steven Jones commentedI tidied up the change notice a little, but haven't added the example wanted in #18.
Comment #20
jibranAdded Example
Fixing it as per #18
Comment #21
sunComment #22.0
(not verified) CreditAttribution: commentedUpdated issue summary.