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

Files: 
CommentFileSizeAuthor
#15 drupal8.module-info-assets.15.patch11.97 KBsun
PASSED: [[SimpleTest]]: [MySQL] 49,419 pass(es).
[ View ]
#9 drupal8.module-info-assets.9.patch11.25 KBsun
PASSED: [[SimpleTest]]: [MySQL] 49,328 pass(es).
[ View ]
#7 drupal8.module-info-assets.7.patch10.06 KBsun
FAILED: [[SimpleTest]]: [MySQL] 49,319 pass(es), 5 fail(s), and 0 exception(s).
[ View ]
#2 drupal8.module-info.2.patch6.76 KBsun
FAILED: [[SimpleTest]]: [MySQL] 49,201 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

Comments

Title:Remove stylesheets[] .info file property support for modulesRemove stylesheets[] and scripts[] .info file property support for modules

Status:Active» Needs review
StatusFileSize
new6.76 KB
FAILED: [[SimpleTest]]: [MySQL] 49,201 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

For starters.

Anyone willing to pick this up?

Status:Needs review» Needs work

The last submitted patch, drupal8.module-info.2.patch, failed testing.

If my module wants to load css on every page where should I call my library than?

Yes 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.

Assigned:Unassigned» sun
Status:Needs work» Needs review
StatusFileSize
new10.06 KB
FAILED: [[SimpleTest]]: [MySQL] 49,319 pass(es), 5 fail(s), and 0 exception(s).
[ View ]

Attached patch performs the full conversion.

Status:Needs review» Needs work

The last submitted patch, drupal8.module-info-assets.7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new11.25 KB
PASSED: [[SimpleTest]]: [MySQL] 49,328 pass(es).
[ View ]

Wow. Forum module. Unholy things going on in there. Added a @todo.

I think this patch is ready.

Created a follow-up issue for Forum module: #1866900: Forum pages are adjusting breadcrumb too late

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.

Yeah, 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 with hook_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 :)

Status:Needs review» Reviewed & tested by the community

Me happy then :)

Status:Reviewed & tested by the community» Needs review

Sorry, I did not notice that there's additional processing in _system_rebuild_module_data(), which calls _system_info_add_path().

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new11.97 KB
PASSED: [[SimpleTest]]: [MySQL] 49,419 pass(es).
[ View ]

Back to RTBC with those.

Title:Remove stylesheets[] and scripts[] .info file property support for modulesChange notice: Remove stylesheets[] and scripts[] .info file property support for modules
Priority:Normal» Critical
Status:Reviewed & tested by the community» Active

Committed/pushed to 8.x, thanks! This will need a change notice.

Feel free to review: http://drupal.org/node/1876152

Status:Active» Needs review

Can we add an example how to add this css/js to every page with drupal_add_css and drupal_add_js?

I tidied up the change notice a little, but haven't added the example wanted in #18.

Status:Needs review» Fixed

Added Example

CSS & JS can be added to every page.

<?php
/**
* Implements hook_page_build().
*/
function foo_page_build(&$page) {
 
$path = drupal_get_path('module', 'foo');
 
$page['#attached']['css'][$path . '/theme/foo.css'] = array('every_page' => TRUE);
 
$page['#attached']['js'][$path . '/js/foo.js'] = array('every_page' => TRUE);
}
?>

Fixing it as per #18

Title:Change notice: Remove stylesheets[] and scripts[] .info file property support for modulesRemove stylesheets[] and scripts[] .info file property support for modules
Priority:Critical» Normal

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue summary:View changes

Updated issue summary.