Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem
-
There is no sensible
'version'for JS/CSS registered by (contrib) modules via hook_library_info().'version' => '1.0', -
The loaded files will not update, unless the version number is manually increased with every single change to a file.
'version' => '1.0', 'js' => array( $path . '/admin_menu.js' => $options, ), ... :: ... <script src="/modules/admin_menu/admin_menu.js?v=1.0"></script>(The query string was based on filemtime previously, properly detecting any changes.)
-
Library identifiers à la
'drupal.admin'are unclear and undocumented, and the current identifiers found throughout core make no sense at all.// This library DOES NOT provide 'Drupal.Admin'. WTF? $libraries['drupal.admin'] = array( -
It's entirely unclear how to add conditional JS/CSS that extends some other JS/CSS.
$attached['css'][$path . '/admin_menu.css'] = $options; if ($user->uid == 1) { $attached['css'][$path . '/admin_menu.uid1.css'] = $options; } -
jQuery, jQuery.once, and Drupal libraries by System do not specify
'every_page' => TRUEanymore, breaking the entirety of JavaScript aggregation optimizations that have been carefully designed.$libraries['drupal.admin'] = array( 'js' => array( $path . '/admin_menu.js' => array('every_page' => TRUE), ), 'css' => array( $path . '/admin_menu.css' => array('every_page' => TRUE), ), 'dependencies' => array( array('system', 'drupalSettings'), array('system', 'jquery'), array('system', 'drupal'), array('system', 'jquery.once'), ), ... :: ... <script src="/modules/admin_menu/admin_menu.js?v=1.0"></script> <script src="/core/misc/jquery.js?v=1.8.2"></script> <script src="/core/misc/jquery.once.js?v=1.2"></script> <script src="/core/misc/drupal.js?v=8.0-dev"></script> -
Various core libraries are registered with hard-coded weights that do not leave any room for contrib to inject themselves in between:
'core/misc/tabledrag.js' => array('group' => JS_LIBRARY, 'weight' => -1), -
ALL JS and CSS added through drupal_add_library() gets the group of
JS_LIBRARY, which destroys the entire sense of the previous separation between library vs. consumer/integration files.'css' => array( $path . '/admin_menu.css' => $options, ), ... :: ... <style media="all"> @import url("http://drupal8.test/sites/all/modules/admin_menu/admin_menu.css?mbhc0i"); </style> <style media="all"> @import url("http://drupal8.test/core/modules/field/theme/field.css?mbhc0i"); @import url("http://drupal8.test/core/modules/user/user.css?mbhc0i"); @import url("http://drupal8.test/sites/all/modules/admin_menu/admin_menu.uid1.css?mbhc0i"); </style>
Comments
Comment #1
sunComment #2
sunYuck. Added problem 6. and 7.
Comment #3
sunI don't think we can release with this, so tagging accordingly. Don't want to bump to critical, since this will take some time to resolve...
Comment #4
robloachevery_page: #977844: Remove the 'every_page' option for CSS/JS assets: it is confusing, even damagingweight:#1805552: Remove custom weights from library definitionsComment #5
nod_I'll keep it short, hopefully giving enough feedback.
The aggregation strategies can be thought out after feature freeze I'd say, what needs to happen now it cleaning the API and making it truly pluggable. The actual aggregation strategies can be refined later on.
Comment #6
panchoRe #3:
"I don't think we can release with this, so tagging accordingly." means "critical", and I tend to agree. Anyway, we're way beyond thresholds both regarding critical and major bugs.
So all this "major plus 'Release blocker' tag" leads to is covering our real technical debt, as discussed on #1810428: [Policy, no patch] Adjust major and critical task thresholds during code thaw Which isn't good, even more because this specific issue will take quite some time to be properly solved.
Comment #7
nod_update since last time:
Comment #8
nod_I don't feel any of this is critical, several points are going to be addressed with other issues. Demoting to major and adding revisit before release tag in case we miss something with the other issues.
Comment #8.0
nod_Updated issue summary.
Comment #9
catchAnything left here that's not covered by #1996238: Replace hook_library_info() by *.libraries.yml file?
Comment #10
sunI went through a whole bunch of related issues and turned them into child issues of this issue.
→ Turning this issue into a meta.
Don't have the energy to rewrite the issue summary right now.
Comment #11
jhedstromIssue summary needs updating given the removal of hook_library_info().
Comment #12
wim leersI think the only thing left to do here, is rationalizing/analyzing our CSS aggregation.
The
every_pagesystem never actually made sense, because it is site-specific, even though it was always specified by an individual module. How can a module know whether e.g. the block the asset is for will be present on every page? It cannot, because it's site-specific.Therefore, the only aggregation strategy that makes sense AFAICT, is analyzing our JS asset dependency graph, and always loading those assets that are most frequently depended upon. But even that is flawed: what if some JS that's only intended for admin users is amongst the most frequently depended upon JS assets?
Finally, and foremost: loading certain JS assets always for optimization purposes implies actively ignoring the dependency graph!
IMO improving front-end performance further in this area is something that can happen during a later release of Drupal 8. We already have sufficient front-end performance improvements: no JS for anonymous users out of the box, all JS loaded from the footer by default (opt-in to header instead of opt-out), some client-side caching, and so on. We should be correct/sane first, and optimize second.
Assigning to @nod_ to get his feedback, and give him the opportunity to close this one :)
Comment #20
nod_All right, I think compared to the situation in 2012 we're in a good place.
From the summary 2 and 3 could still be an issue, although the fact that we have libraries.yml files makes the discovery easier.
We can close this one and work on the followups is necessary.
Comment #21
wim leers🥳
Closed by @nod_ 5 years and 3 days after I suggested exactly that 😄 Blast from the past!
Comment #22
nod_very efficient :)