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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

Title: Remove stylesheets[] .info file property support for modules » Remove stylesheets[] and scripts[] .info file property support for modules
sun’s picture

Status: Active » Needs review
FileSize
6.76 KB

For starters.

Anyone willing to pick this up?

Status: Needs review » Needs work

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

aspilicious’s picture

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

sun’s picture

catch’s picture

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.

sun’s picture

Assigned: Unassigned » sun
Status: Needs work » Needs review
FileSize
10.06 KB

Attached patch performs the full conversion.

Status: Needs review » Needs work

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

sun’s picture

Status: Needs work » Needs review
FileSize
11.25 KB

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

sun’s picture

I think this patch is ready.

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

nod_’s picture

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.

sun’s picture

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 :)

nod_’s picture

Status: Needs review » Reviewed & tested by the community

Me happy then :)

sun’s picture

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().

sun’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
11.97 KB

Back to RTBC with those.

catch’s picture

Title: Remove stylesheets[] and scripts[] .info file property support for modules » Change 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.

dawehner’s picture

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

aspilicious’s picture

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?

Steven Jones’s picture

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

jibran’s picture

Status: Needs review » Fixed

Added Example

CSS & JS can be added to every page.

/**
 * 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

sun’s picture

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

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.