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.Problem/Motivation
This issue was originally opened because an asset library depended upon the CSS files provided by the Seven theme. But it could not declare a dependency on those since it wasn't a library yet.
However, the problem is broader than that: there's a strong disconnect between how theme(r)s attach assets and how modules/developers attach assets. Everything is using asset libraries… except themes. But themes already support asset libraries! (There is a libraries property in themes that allows you to declare which libraries should always be loaded for this theme.)
The advantages:
- dependencies, hence a step towards getting rid of weights
- one uniformly applied system for modules and themes alike
- themes are also forced to think about categorisation in their CSS
- libraries can contain CSS and JS as logically associated units, and can declare dependencies on other libraries
- themes are encouraged to create a library per component/topic/logical unit and hence are encouraged to not load all their CSS on all pages anymore, but to attach it when appropriate in a preprocess hook and/or only on the relevant pages via
hook_page_attachments_alter()
The downsides:
- a layer of indirection in a theme's
*.info.ymlfile, hence perceived as more complex
Proposed resolution
Remove the stylesheets property from theme *.info.yml files, in favor of the libraries property (which already exists).
This does not affect stylesheets-override and stylesheets-remove, which continue to function as-is.
Beta phase evaluation
| Issue category | Bug because themes not using libraries provides an inconsistent DX/TX, prevents dependencies from being declared on theme CSS assets. |
|---|---|
| Prioritized changes | The main goal of this issue is performance and removing previously deprecated code. |
| Disruption | Disruptive for contributed and custom themes because it will require a BC break. The benefit is a consistent DX/TX, and the unblocking of #2368797: Optimize ajaxPageState to keep Drupal 8 sites fast on high-latency networks, prevent CSS/JS aggregation from taking down sites and use HTTP GET for AJAX requests, which brings better front-end performance. |
Remaining tasks
Get green & review.
User interface changes
None.
API changes
Removed the stylesheets property from theme *.info.yml files, in favor of the libraries property (which already exists)
Original report by @alexpott
Follow up from #2376147: Installer is missing all of the global Seven theme stylesheets
The maintenance-page library in seven depends on all the stylesheets in seven.info.yml - we should make a base library.
| Comment | File | Size | Author |
|---|---|---|---|
| #77 | themes_stylesheets_to_libraries-2377397-77.patch | 23.64 KB | Wim Leers |
Comments
Comment #1
alexpottComment #2
sqndr CreditAttribution: sqndr commentedLooks good; the same thing has been done for Bartik as well:
This patch does the same thing for Seven. Manually tested and all the Seven stylesheets all included on the maintenance page.
Comment #3
Wim LeersOne important difference is that Bartik only has SMACSS "theme" CSS files. But Seven AFAICT has SMACSS "base", "component" and "layout" CSS files as well.
I think this might need to be changed to:
… but somebody with more SMACSS experience should confirm that.
Comment #4
joelpittet@LewisNyman may need to weigh in on this, assigning him to have a peak. Though anybody who was involved in the whole base/theme/layout/component can weigh in here.
I'd assume that structuring the library to swap out the different levels could be handy in some cases though. Like lightweight pages or HTML email templates, etc.
Comment #5
Wim Leers#4: Yes, that's another possibility :) (One library per SMACSS level.)
Comment #6
sqndr CreditAttribution: sqndr commented#3++
Actually looks like a better idea to me. This way, we could only include
seven/base, instead of the css for all components. :)Comment #7
LewisNymanAll I want is for someone to explain to me if this is now the recommend best practice for all Drupal 8 themes? Because people will look at how core themes implement certain things for guidance. I think this method of adding CSS globally is more complex than the stylesheets property.
Comment #8
joelpittet@Wim Leers you are pretty good and splaining why libraries are the future:) *putting people on the spot:P*
Comment #9
joelpittetThe only thing I see that is better about 'libraries' is that they are named, and by naming them you get to use them as dependencies, and dependencies are much better than weights. IMO
Comment #10
Wim Leers#7:
Yes.
Exactly. And this is why Bartik was already converted.
It's slightly more complex, but it's also consistent with everything else in Drupal core. If you want assets (CSS/JS) to be loaded, use asset libraries. Period. In modules, in themes, in alter hooks, in preprocess functions, everywhere. So it's learning one pattern, that is applied consistently everywhere, rather than learning a mishmash of patterns:
*.libraries.ymlPLUS$build['#attached']['libraries']in render arrays/preprocess functions OR thelibrariesproperty in a theme's*.info.yml)$build['#attached]['css']+$build['#attached]['js']stylesheetsproperty in a theme's*.info.ymlA long, long time ago, in #1813186: Integrate scripts/stylesheets from info/layout files with libraries, we chose to remove the
scriptsproperty from a theme's*.info.yml, in favor of thatlibrariesproperty. Many themes ship with some JS. Hence for a theme to load that JS, you already need to use a library anyway.On July 27, 2014, mdrummond also wrote this at #2228745-12: Color module doesn't look for CSS files declared in a library (where we converted Bartik's
stylesheetsproperty intolibraries):When it got committed, he even said:
So I was always under the impression that front-end people are on board with this general approach, and that the conversion was only a matter of time. Perhaps that's wrong?
#9:
The advantages:
hook_page_attachments_alter()The downsides:
*.info.ymlfile, hence perceived as more complexComment #11
sqndr CreditAttribution: sqndr commented@Wim Leers:
Does this mean we're also going to remove the
stylesheetsproperty from *.info.yml? And what about the second method? Keep several ways in to do this will most likely bring confusing. It means my own theming guide + the theming guide on Drupal 8 needs an update regarding adding stylesheets: https://www.drupal.org/node/2349827Comment #12
Wim LeersThat's the only sensible way forward, IMHO.
Comment #13
catchI'd personally like to see us move to using libraries everywhere, it helps to unblock issues such as #1014086: Stampedes and cold cache performance issues with css/js aggregation.
stylesheets-remove and stylesheets-override should still be OK though that's more of a shortcut to alter hooks which is different anyway.
Comment #14
sqndr CreditAttribution: sqndr commented@Wim Leers; sounds fair to me.
So to conclude: all themes and modules should define there css files as a library and depend on the library in the *.info.yml file. This is the only right way to include stylesheets inside themes/modules. This has been changed in all modules. It has also been changed in Bartik, but not yet in the Seven theme. That's what this issue is about. When defining the libraries stylesheets, the SMACSS methodology should be used if the theme/module's css folder has been organised in such as way. And they should, because it's part of the coding standards. The means there's 5 extra keys available:
theme/base/component/layout/states:The
stylesheetsproperty for *.info.yml should be removed because of this. Thestylesheets-removeandstylesheets-overridecan stay because that's more of a shortcut to alter hooks which is different anyway.What about that?
Comment #15
LewisNymanOk this answers my original question. Let's have a real discussion about this, in this issue or a meta, because I think we need to have a focused discussion about the advantages and the harm to themer experience. We're adding another Drupalism and another barrier to frontend developers. We're adding another Drupalism and another barrier to frontend developers. CSS isn't directly covered by #2298551: Documentation updates adding/adjusting inline/external JS in themes
The libraries system was designed to deal with a complex use case (dependancies) but it feels like we're applying to a separate use case. CSS does not have dependancies on other CSS like Javascript does. The
maintenance-page.cssdoes not depend on any other CSS to function.Comment #16
Wim Leers#14: "exactly!" to everything you said :)
About attaching individual CSS and JS files: that's also going away, first step at #2378095: Convert all remaining attached individual CSS/JS assets to attached asset libraries, an issue to remove that is coming up…
As per #13, I'm expanding the scope of this issue.
Not only does this block #1014086: Stampedes and cold cache performance issues with css/js aggregation as catch said in #13, it also blocks #2368797: Optimize ajaxPageState to keep Drupal 8 sites fast on high-latency networks, prevent CSS/JS aggregation from taking down sites and use HTTP GET for AJAX requests, which is critical.
Issue summary updated.
Comment #17
Wim Leers#15: We cross-posted. Here are my thoughts.
Yes, we add one layer of indirection. But for the following front-end related reasons:
$variables['#attached']inhook_preprocess()), rather than loading all CSS on all pagesI'm convinced this helps to futureproof Drupal from a front-end perspective, which will increasingly be about reusable, reactive, responsive widgets/components/snippets/blocks. Asset libraries then map perfectly to that use case, and will become feasible once Shadow DOM and Element Queries become available.
I could see a few additions now or in the future to improve the TX further:
libraries-extendproperty in theme*.info.ymlfiles:… to attach additional CSS/JS to existing libraries (if those existing libraries are present)
attach-library-to-hookproperty in theme*.info.ymlfiles:… to automatically attach a library instead of having to implement
hook_preprocess_HOOK()for each.Comment #18
Wim LeersThis patch:
stylesheetsproperty from theme*.info.ymlfiles (as in: removes support for it and removes it from all core themes)node-add.csswhen it's actually necessary.It does not yet:
views-ui.css)(because I'm not sure yet everybody wants that).
Comment #20
Wim LeersActually, there were only 3 failures. Testbot is currently failing due to #2376039: Undefined property ContainerAwareEventDispatcherTest::results in run-tests.sh. Fixing those…
Comment #21
sqndr CreditAttribution: sqndr commentedAs you mentioned (@ Wim Leers), by using the libraries we
enforced SMACSS categorization. In my opinion this should be fixed as well. It was the main reason why you changed the ticket as well in #3. I hope others will agree.This would however be great as well. You don't need all the css files on every page. We should be able to include small parts of the css, only where we need them.
@Wim Leers: Great work! ;)
Comment #22
Wim LeersActually, those are two different things. :)
I did apply SMACCS level. I did not create one library per SMACCS level, which was proposed by Joël Pittet in #4:
So I think you slightly misunderstood me? :)
From the patch:
SMACSS metadata: check!
Comment #23
Fabianx CreditAttribution: Fabianx commentedThere is another reason, which makes this boundary-critical for Performance reasons:
We want to implement AJAX / ESI, etc. requests that only load the necessary libraries. (e.g. https://docs.google.com/a/tag1consulting.com/presentation/d/1Q44eWLI2qvZ...)
However what you normally would do is you would overwrite the CSS of e.g. a module in your theme.
module-a.css:
theme/css/mytheme.css
So the last one wins.
Now lets assume the module.css is loaded _after_ the theme css, then you suddenly have:
Huh, what happened just now?
We now have the wrong CSS order.
That means CSS has dependencies (or at least an order), too. - in a way.
The ways Wim suggested will solve this, because you can:
- extend the library, so your CSS is guaranteed to come after the module libraries CSS.
- overwrite module-a.css directly via stylesheets-remove or stylesheets-overwrite (that works already now)
So by using libraries everywhere, we can much more easily define ordering, too - which is important for CSS.
Comment #24
Wim LeersComment #25
joelpittetOne thing I always do in D7, always remove stylesheets and scripts (stylesheets more so). It's part of my workflow: if CSS interferes at all with my design or it's large and inefficient remove, copy and rewrite into a sass partial.
Essentially what magic module or omega4 provides.
I'm also a big fan of dependencies vs weights, they are easy to understand and get at the root of the problem, IMO. Whether 'libraries' is the best approach is up for debate as you may be including just one js file or one css file which seems small for a 'library' with 1 book for take out:P
I know WordPress does work like libraries and dependencies though they are doing it more along the lines of drupal_add_js()/drupal_add_css() was, just each one is given an identifier (library name) and an array of dependencies.
(@Wim Leers I was mentioning that briefly in Amsterdam) Just because that is the way it seems to be leaning towards it may be interesting to see how others have tackled the problem:
@see https://developer.wordpress.org/reference/functions/wp_enqueue_style/
@see https://developer.wordpress.org/reference/functions/wp_enqueue_script/
Comment #26
Wim Leers#25: Thanks, interesting information! Especially that WordPress apparently was already requiring dependencies. They are ahead of us in that respect, then.
Having libraries declared in declarative YML files (Drupal 8 ) rather than in code (WordPress & Drupal 7) is better though: it ensures you have a predictable (declarative) system, rather than one where PHP logic can dynamically create a different set of libraries on every request. It allows for cacheability and predictability, and hence less frustration when debugging asset-related problems :)
Comment #27
LewisNymanThanks for outlining everything in the issue summary! Excellent work. I'll comment on a few but it would be great to get the a wider perspective of the frontend community, especially ones who aren't familiar with D8 core.
Do themes need weighting for their CSS? I have never had this problem.
I thought our aggregation group prevent this from happening? Is it possible for module CSS to load after the theme? (Unless you intentionally modify the group.)
Consistency does sound like a good idea, from the perspective of a core developer who has an overview of everything. But most themers are not module developers (I think). I'm worried about applying this solution to a use case that does not warrant it, just for the sake of consistency. Hopefully we get more feedback from others here.
This is not a good thing. We shouldn't force a particular architecture on frontend developers who use Drupal. What happens if they prefer another system and what if something better comes along? The best thing Drupal can do is be agnostic.
I don't think this is what you mean though, because it's not forcing SMACSS categorisation, it's just forcing some kind of categorisation, which is fine. I've updated the summary to remove mentions of SMACSS.
This is fine, but here we are talking about generic styling. CSS does not have a dependancy on other CSS. It feels like a we are applying the solution to another problem.
This is a good goal, but a total opposite from how all/most theme developers use CSS right now, which is why I think this is a big issue.
Let's look at the most popular base themes:
Zen, Omega, Aurora, and Fusion all ship with one referenced CSS file their starterkit info file. AT and Tao include a base file and generic 'style.css'.
Zen, Omega, Aurora, architect their styling using Sass partials, which then compile into one CSS file. This means you can split up your styling into whatever logically makes sense to you and Drupal doesn't care. I think bypassing Drupal's asset handling is a draw here, because it's confusing and annoying. It's much easier to remove all stylesheets and manage/compress them yourself.
I'm not against this idea, and would like to see it taken further in Seven so we aren't loading all the component stylesheets globally. I just think it needs some more discussion.
Comment #28
Wim LeersThis is getting long, so replying per subject.
Weights vs dependencies
Fabianx was specifically talking about the case of ESI/AJAX/Bigpipe ("AJAXing in" additional content, with associated CSS/JS). In that case, the basic page with the main content would be rendered as-is, but blocks would be loaded later, after the initial page had loaded. In that case, the theme's "every page" CSS would already be loaded as part of the basic page with the main content.
However, when e.g. a block FOO is loaded, which comes with its own CSS, it would then be overriding the theme's CSS for FOO. Because the theme didn't specify a separate library for FOO, and hence it was loaded as part of the theme's "every page" CSS.
It is very likely that this is going to be the technique to make Drupal 8 blazingly fast for authenticated users. I think it's even quite possible that in a Drupal 8 point release (8.1, 8.2…), Drupal will ship with this by default.
The why: only consistency?
I understand the concern. And I initially was concerned about it too! But having talked to several front-end people, this doesn't seem to be a problem, really. I also want to see more feedback though.
Note that this is not only for the sake of consistency. It's also to enforce front-end people to think about categorization, to encourage smaller components, and hence to load less CSS (plus referenced images) and improve front-end performance. Plus, it blocks #2368797: Optimize ajaxPageState to keep Drupal 8 sites fast on high-latency networks, prevent CSS/JS aggregation from taking down sites and use HTTP GET for AJAX requests, which is also important for front-end performance.
I've thought about turning a theme's
stylesheetsproperty into a library automagically internally, but that has several flaws:librariesThemes only have generic styling and don't need dependencies
Dependency implies order. If A depends on B, then B must be loaded before A. The order in which CSS is loaded is of great importance… I'm sure you agree with that?
Next, I strongly disagree that this is "generic styling". If it really was generic styling, and all themes only did generic styling, there wouldn't be a problem. But even a core theme like Seven violates this. Its
stylesheetsproperty lists the following CSS files, which are hardly generic (I could argue this for even more, but these are the clearest offenders):css/components/field-ui.csscss/components/modules-page.csscss/components/tour.theme.csscss/components/update-status.csscss/components/views-ui.csscss/layout/node-add.csscss/theme/appearance-page.cssOn top of that, I disagree with the general statement here and what you wrote in #15 (). This is not true; without the general Seven stylesheets,
seven/maintenance-pagedoes not achieve the intended effect:maintenance-page.cssbuilds on top of the "every page" Seven CSS files.In exactly the same way,
install-page.cssbuilds on top of the "every page" Seven CSS files ANDmaintenance-page.css, this is whyseven/install-pagedeclares a dependency onseven/maintenance-page.Asset libraries imply componentization in Drupal, themers do componentization in precompiled CSS
Absolutely.
Omega's ships with 4, but they're still not per-component for sure.
So if I understand you correctly, your biggest concerns are that A) Drupal would encourage themes to create a library per component/logical unit, whereas they're used to just load all CSS on all pages, B) Drupal would encourage to handle the "componentization" in Drupal itself, whereas many themes use SASS or other CSS preprocessors, split it up into files per component and then compile it into a single CSS file and hence encourage "componentization" in their precompiled CSS.
Please confirm or correct.
If that's the case, then I have four thoughts:
libraries-extendand/orattach-library-to-hookproperties (naming TBD).singlecssD8 contrib module that allowed you to use this workflow if you wanted to? It can't be the default because we can't automagically split up huge monolithic CSS files, but we can automagically combine many small ones… and we can cater for a "compile it all into a single CSS file" approach.Comment #29
cosmicdreams CreditAttribution: cosmicdreams commentedIt's also worth noting that HTTP/2 will bring new asset optimization strategies and largely deprecate the compile & compress strategy that many systems currently deploy:
Prioritization will be key. See:
https://plus.google.com/+IlyaGrigorik/posts/47FNmFEQpjN
https://nghttp2.org/blog/2014/04/27/how-dependency-based-prioritization-...
I wonder if we're building system-level enforcement of a development methodology that will be better enforced by web standards. Should we leave asset delivery optimization to HTTP/2?
Comment #30
Wim LeersYes! :)
This will only prioritize which assets are loaded first (those in the critical path to do a first paint etc.). For example, this would ensure that CSS/JS are loaded before any images are loaded.
It does not help with not loading unnecessary assets. It also does not change the order in which CSS or JS files are loaded (if it would, it'd break the web/violate specs).
Besides HTTP/2 Prioritization, there will also be HTTP/2 Server Push and HTTP/2 Server Hint… and for both of those, we will need dependency information to figure out which assets to push to the client and which assets to hint to the client.
See https://www.igvita.com/2013/06/12/innovating-with-http-2.0-server-push/, http://www.chromium.org/spdy/spdy-whitepaper
Comment #31
davidhernandezI like all of the advantages posted in the summary, especially 2 and 4.
The disadvantage I don't think is too big a deal. It does make it slightly more complicated to add your CSS file, but I don't think there is anything intuitive about the way we do it now. Every new themer I talk to will ask me which template file to edit so they can add their CSS to
<head>. They don't know anything about the "way" to do it until they read the instructions.I completely agree with this statement from Lewis, enough to emphasis it again. We don't want to force a particular workflow or organization on others.
I'm trying to read through everything, and understand everything being proposed, but getting a little lost with so much to read :), so forgive me if this a stupid question. This doesn't prevent a themer from putting all their CSS in one file, correct? They would just put one file under theme or base?
I know it is isn't performant or smart or professional or whatever, but it is something a significant percentage of users are going to do. People who run small sites, or are just getting started, are not going to have complex development methodologies. We have to support simple as well as sophisticated.
Comment #32
Wim LeersCorrect. Nothing prevents you from having a library with a single CSS file if you choose to do so.
Absolutely!
Comment #33
leahtard CreditAttribution: leahtard commentedAs the front-end community (not just the front-end Drupal community) moves towards Modular CSS implementations (SMACSS, OOCSS, etc) and online style guides, I see this as a hindrance or irrelevant to this production workflow. Here is an example:
1. A front-end developer creates reusable class that styles certain grouping of content.
2. We add these styles to the style guide for other developers to reference.
3. A back-end developer or a junior developer comes along (maybe with limited front-end skills) and they create a new grouping of content. After referencing the style guide, they see this type of content has already been styled and a class is available.
If that class was only loaded on "relevant pages" this workflow would not be possible. I feel our CSS should be agnostic to the page an element is sitting on. Yes, as themer, I would still have the ability to simply load all of my css on all pages. I just question if this would be useful. Interested in other opinions!
Cheers, Leah
Comment #34
markabur CreditAttribution: markabur commentedI do this sometimes now, in D7. E.g. if there is an image gallery content type and it has a significant amount of unique CSS that isn't used anywhere else in the site, I'll load it conditionally, as a separate stylesheet. But for me this is really rare, and I can't think of more than a couple of examples where this sort of componentization is worthwhile or even possible.
My current workflow uses a lot of SASS partials, and they all depend on _base.scss, which is where I define colors, font stacks, etc. that are used sitewide, and which need to be available to the other partials when compiling to CSS. On the surface, many of my partials look like good candidates for becoming separate libraries (_slideshow.scss, _navigation.scss, etc.), but in reality they depend on that _base.css file and would be a real hassle to maintain separately (less DRY).
Comment #35
joelpittetSeems that this problem has been taken care of mostly with precompilers (SASS/LESS/etc). I do exactly what is mentioned in #34 as well and because it's compiled into a maybe 3 CSS files for my theme with an ungoddly amount of partials grouped in a SMACSS like structure. This helps me keep things DRY, orgranized and maintainable AND when caching is turned off (during developement) I'm only loading one or 2 CSS files and it can be compressed (less work for for Drupal to do) and faster load time during development.
And I'll echo @LewisNyman and @davidhernandez comment, SMACSS/OOCSS are nice and we should use them, but I strongly suggest we don't enforce/force that structure on anybody as it may not fit their use-case or a better strategy may come along shortly.
Comment #36
joelpittet@Wim Leers look at how we discribe to others how to build a theme for some guidance:
http://www.appnovation.com/blog/creating-drupal-8-theme-sasssingularity-...
https://www.drupal.org/node/2165673
https://www.acquia.com/blog/ultimate-guide-drupal-8-episode-5-front-end-...
http://drupal.stackexchange.com/questions/104091/how-to-add-css-javasrip...
With the promise that if we add this to our fluffiness.info.yml, we will see those styles applied:
Though I (and @dawhener at BADCamp) were struggling with libraries because of the extra steps to accomplish this.
This would become (please god correct me if I'm wrong @Wim Leers because I litterally threw my hands up in the air and gave up trying to do it with libraries, though to be fair it could have been a typo and not enough coffee).
Followed by:
Comment #37
joelpittetI guess the addition would be in the info file...
so
Instead of the fluffiness_preprocess_page
Comment #38
davidhernandez#37 I think that is right, if you want to always include the library. If you want to discriminate when the library was attached you would still need to use the preprocess, correct?
Comment #39
Dragan Eror CreditAttribution: Dragan Eror commented@LewisNyman
#27
CSS does not have dependancy on other CSS, you are right, but does dependency on other PHP, JS or Library.
I agree with this idea.
Few examples:
if there is no jQuery on the page I do not need styles for some crazy menu that depends on jQuery.
I can also add my personal styles to my theme to make panel pages look better, which I do not need if there is no panels module and library which comes with panels module.
I want to style comments. I do not need to load those styles if there are no comments on page.
Comment #40
Fabianx CreditAttribution: Fabianx commented#36, #37: The assumptions are correct.
Please also note that you have to learn this workflow for adding some javascript anyway - so the learning curve will be needed one way or another anyway.
I think this is a great chance to provide both workflows, which is worth its own discussion.
Compotenized and dependent CSS is _already_ a reality and the examples of SASS and LESS show clearly that CSS _has_ dependencies and order.
Because both in SASS and LESS you include other partials, which are your dependencies per se.
So if you wanted to create a mytheme/slideshow library, you would have that thing depend on mytheme/base.
So dependencies are very similar to _partials in SASS.
But it means that not the whole 'style.css' is loaded over the wire, but only those CSS files that are actually needed.
And while that clearly is a paradigm change it is:
a) very mobile friendly and HTTP 2.0 proof (where aggregation is a moot point)
b) matches the current SASS workflow really nicely
Overall it might even be possible to write a SASS plugin that creates drupal libraries in addition to the CSS files that are no partials ...
Comment #41
jwilson3Ajax example was used above as one of the advantages for this dependency structure... so, if you have a dependency on base and you load the slideshow with ajax, how would it know not to pull in the base again (presumably its already loaded on the page).
I would think rather the mytheme/slideshow has some dependency on a stylesheet (or something else?) from views_slideshow, not from my theme's base.
Comment #42
Fabianx CreditAttribution: Fabianx commented#41:
We would use something similar to https://docs.google.com/a/tag1consulting.com/presentation/d/1Q44eWLI2qvZ....
Incremental loads and negative information providing. So we would always know that theme/base is already loaded and not load it again.
The idea is that after AJAX requests there is one single call like:
/js?+slideshow-mytheme/base
to get aggregated styles (for HTTP 1.1).
---
In the ideal case your slideshow library would _extend_ the views slideshow library, so your styles are always added when the slideshow library is added (and always added after the slideshow library CSS, so no ordering issues).
If however you have a custom component and you theme it completely in your theme (with no CSS output), and you need some base styles always, then your mytheme/slideshow might indeed rely on certain base styles.
This makes it e.g. possible to use the same slideshow styles on an admin page and makes preview easier. (in the case you add it dynamically via #attached library from a module for example.)
#17 / @Wim:
I think your libraries-extend is even more useful if you can add dynamic dependencies, too:
Edit: Just thinking loud, please disregard for this issue, but what if we allowed to instead of merely extending to provide core/contextual-links yourself and just extend from the "aliased" version.
and then
library-provides:
core/contextual-links: mytheme/contextual-links
which does the necessary hook_library_alter(). This would be more OOP like ... As I said just thinking out loud, currently still like the library-extends key better ...
Comment #43
mortendk CreditAttribution: mortendk commentedSo looking at it from the different persective's of "Themer-that-dont-care-how-drupal-works-just-want-to-add-my-sass-generated-css-file": It dosn't really matter in the theme, its gonna be one file, if that takes up 4 lines of codes instead of 1, yes its gonna be a little bit annoying the first time, but its not gonna be a day to day operation to do that.
I think were going into the we wanna do this for 100%, lets step back and remember the 80% rule for the frontend.
So from my persepctive i think were good with adding in css files this way, if its a win for consistency, developers & generally happiness im fine with it.
The one thing thats gonne be weird is now removing the existing css files, and not using stylesheet-remove in the info file, so working with css files thats gonna be a bit strange, but we can totally fix that with documentation.
Comment #44
davidhernandezThis doesn't change. It will still be in the info file.
If this goes in, it will need an extremely well publicized change record and we will need to update documentation everywhere.
Also, because of the significance of the change, should this only be in a beta release?
Comment #45
RainbowArray1. Yay, consistency and ease of explanation. Woot.
2. Yay that people don't have to provide a few dozen CSS files for each component if they don't want to do so.
In most cases, I'm using Sass to aggregate base + component + layout partials into one or maybe a few CSS files because with the prevalence of HTTP 1.1 right now, saving server round trips is by far the most important performance optimization.
Yes, HTTP 2 may well change that strategy, so having the option to split things up is a good thing. I'm not necessarily convinced that themers are going to work out matching up one component CSS file with whatever set of conditions are needed to create that component (because it's likely not going to be merely the existence of the use of a module on a particular page) so that it will truly work like a dependency, but hey, that's fine to have that possibility out there.
I guess with ESI caching maybe having a different aggregated CSS file per page, so that the CSS is as small as possible, is a good thing, again as an option, not a requirement. I need to read more about HTTP 1.1 vs 2 to better understand the impact of that on server roundtrips, as latency is often a much bigger factor for performance than file size. But anyhow... as long as there's flexibility, then great.
Also, just to be clear, this isn't saying that the only way CSS can be added to a site is through a libraries.yml file, right? Because sometimes you need to do processing before you can add something in: using Typekit, and you need to dynamically load in your kit ID, for example. Or you need to add in some inline CSS in head for a specific purpose in certain situations.
Anyhow, as long as front enders do have flexibility in how they work with this (except for adding in via the .info.yml file, which I agree is an unnecessary duplication), I'm +1.
Comment #46
joelpittetNot a big fan of these two clauses, any way I can avoid categorization while declaring a CSS file?
I'm thinking more like:
And for consistency's sake if I'm not wrong currently I can't attach a library globally in an module's info file. Can we add that?
I stubbed out a change record, feel free to update https://www.drupal.org/node/2379475
Comment #47
Wim Leers#33: Thanks for taking the time to write up your thoughts! :)
I'm a bit confused though. First you say the front-end community is moving towards SMACSS. Then you say that asset libraries/componentization is counter to that.
IMHO it's actually the contrary.
Also note that it's entirely okay to have one or more libraries loaded on all pages! Things like typography, basic layout, and so on are of course necessary on every page.
It's only things like styling Views (unless almost every page is a View) or a specific View or a specific page that we should take care to only load the library when necessary.
It's not clear to me what your concern is, but it's either:
Can you clarify that, so that I only have to answer your actual question rather than all three cases? :) I can answer all cases, and for every one, the answer is that your workflow is still supported.
#34: You wouldn't have to main
_slideshow.cssseparately, they'd continue to depend on that_base.cssfile. You'd just put (the compiled)_base.cssin amytheme/baselibrary and (the compiled)_slideshow.cssin amytheme/slideshowlibrary and declare a dependency:#35 + #36 + #37: Indeed, #37 is the correct solution. Please see
bartik.info.ymlandbartik.libraries.yml; they're the example that's in core. This issue is about making all themes use the same approach as Bartik.#38:
Indeed!
Yes! :)
#42: I like that idea of that
extendsproperty!#43: Yep, you can still do everything exactly the same CSS-wise: if you don't want to create multiple libraries, then you don't have to. You can still have all your stylesheets in a single library. In fact, this is what Bartik does!
As the issue summary and #44 already say, you still continue to have
stylesheets-removeandstylesheets-overrid; the only thing that's removed from a theme's*.info.ymlfile is thestylesheetsproperty, in favor of the already existinglibrariesproperty. For consistency, and becausestylesheetsis merely a subset of whatlibrariescan do anyway.We'll continue to have JS settings for this kind of dynamic settings. For the advanced things, there's also still
hook_library_info_alter()+hook_library_alter().#45: RE not being a fan of SMACSS: I can understand that, but that's nothing new. It's not being introduced here. We've had that for at least a year already, see https://www.drupal.org/node/1887922 and https://www.drupal.org/node/1887918#separate-concerns. That being said, it's totally possible to not use SMACSS; in that case just always use the
themecategory.This is wrong; Drupal has never had the ability for modules to add CSS to every page. Because a module is inherently, well… modular and only affects certain pages or even only parts of certain pages. It's highly exceptional for a module needing to add assets to all pages. For themes, it's the other way around. Hence the
librariesproperty is only supported inTHEME.info.yml, not inMODULE.info.yml.Thanks for the change record, I've refined it further — hope you like it. I've added sections with optional things: multiple libraries (e.g. one for node styling), libraries that use dependencies, SMACCS categoriziation, and even library-per-SMACSS-category (handy for subthemes). They're all optional. They're all interesting possibilities for experimentation that we get if we do this.
See https://www.drupal.org/node/2379475.
(There are some massive code block formatting problems, but that's due to the code filter on d.o that's doing crazy shit :()
Overall
themecategory. I was under the impression that this was widely discussed and agreed upon, but apparently that's not true. I'm now guessing we only adopted it for Drupal core, but we don't want to enforce it on contrib. That works too, of course.Comment #48
Fabianx CreditAttribution: Fabianx commentedI think the concern of #33 with a modular CSS approach is that something like http://warpspire.com/kss/styleguides/ is no longer possible.
However:
If you use SASS, then you can have e.g. a partial for _views.scss and a file + library views.scss (which just includes this partial) with library theme/views -> views.css.
But you can then still have a style-guide.scss that includes all the partials to automatically create your styleguide as usual.
Just because you have a catch-all style.css now does not mean we need to continue that :).
And nice base themes will be able to be created :).
Comment #49
davidhernandezThat change record is looking really nice. Thank you!
Can I get more explanation for the last section? "Optional: library-per-SMACCS-category" I don't quite understand the significance of the example. Is it saying that naming the library the same as the category lets a subtheme override it, but a custom library name cannot be overridden?
Also, it sounds like the categories are rigid, correct? (base, component, theme...) A themer can't decide their own to use? I also agree with Joel that it would be nice not to force categories at all if someone didn't want to use them.
Comment #50
Wim Leers#49: I added that example because it was mentioned by sqndr in #6. It's up to the themer to decide how we wants to organize his CSS files in libraries. It can be 1 library, but it can also be 20. It's only saying that one way of organizing a theme's CSS more granularly is to comply with SMACSS and then create one library per SMACCS category. Again, you can do that, but you don't have to. I was under the impression that multiple people liked that idea a lot, but perhaps that impression was wrong.
It's not saying . It only says that some base themes might like the idea of having one library per SMACSS category, because then a subtheme would e.g. only override the
themename/layoutlibrary that contains all SMACSS "layout" CSS files. That's conceptually very easy to understand. And hence that could be one way of doing things.It is just one example of how a theme can be creative with libraries to organize files better, and particularly how base themes can use a certain strategy for libraries to make it easier to implement subthemes.
This is why the change record uses the
themeSMACSS category in case you don't care about SMACSS categorization. Again, from #47:Comment #51
Dragan Eror CreditAttribution: Dragan Eror commentedGuys, stop trolling, everything was already clear 20 comments ago. Any further discussion is just waisting of time (for writing and reading).
Defining CSS in info file is anyway "Drupal way of doing things" and the only one who might not like this idea are "old school Drupal people" who do not want to change their habits.
@alexpot, @Wim Leers thanks for this, one more thing we need here, just well written, easy understandable documentation (with several examples).
Comment #52
davidhernandez@Dragon Eror, no one is trolling. People are trying to understand what this change means before forming an opinion of it, which is an important thing to do with such significant changes. All your trolling remark does is tell people who read this issue and haven't yet participated that their comments and questions are unwelcome.
Comment #53
catchYou could do this in Drupal 7, but it was committed without profiling, and too late to remove from Drupal 7 when I profiled it - the change required loading the entire .info contents on every request for every module. So it was one of the earlier Drupal 8 patch to rip that support out again.
Comment #54
Wim Leers#53: oh, HAH! Nice! That must be one very unknown gem :P
Comment #55
Dragan Eror CreditAttribution: Dragan Eror commented@davidhernandez No, I was referring to people who submit comment without reading all previous comments and repeating same questions. With that starts to be hard to follow important parts of discussion. Of course comments are welcome, but please, read everything from beginning slowly before you ask something what is already explained in earlier comments.
@Wim Leers I forgot to tell I'm willing to help with documentations...
Comment #56
davidhernandezFair enough, but lets be patient. Wim's responses are very long, and very thorough, and we're not all as smart as him. :) We can work on getting some of this explanation in the change record. I think that will be really helpful for other people. That's of course assuming we're willing to make a decision? I'm comfortable stating my opinion, but leave "voting" to Joel and Fabian and others who are the actual theme system maintainers.
Comment #57
lauriiiI think this issue is important and should be properly discussed so its good to have comments here.
#53 Lol!
I like the path we are moving on this issue because of the consistency with modules. I didn't read everything from the discussion so sorry if this is duplicate but it could be nice if you could easily remove whole libraries in a *.info.yml file. Then when you have a base theme that has a lot of styles but you don't want to have anything else than the base level on your own theme you could just choose to remove everything else.
Comment #58
Jeff Burnz CreditAttribution: Jeff Burnz commentedTo be frank this is how I am working in D8 already so I am pretty down with how libraries work and how to use them. Its all good and I love dependancies, just magic and eased my pain a lot.
@27/28 one reason many base/starter themes load one big sheet is that it's a "reasonable compromise", in the grand scheme of all things to consider when building generic base themes, not that its the best way by any means. Users have it ingrained that one stylesheet is best for performance, therefor best for SEO, there is massive pressure to reduce the number of stylesheets loaded to that magic number: 1.
+1 based on the issue summary points 1, 2 and 4.
3, 5 I am not fussed on, we can encourage people this way or that - I think thats OK, for me this does not matter, I do it my way and libraries lets me do that, so thats just great.
Comment #59
Jeff Burnz CreditAttribution: Jeff Burnz commented@57 yes but "remove whole libraries in info" is probably not for modules as well? But at the same time we should move towards consistency, not away from it, for example you can surely do this in hook_page_attachements_alter() and just blow them away or replace with your own. At least thats how I see it, move towards using the same tools for the same jobs, I use those tools and have for years, I am not a module dev, I just build themes, but I dislike working in info files, they seem like voodoo magic to often.
Comment #60
markabur CreditAttribution: markabur commentedQuick clarification: by definition, a SASS partial such as _base.scss won't compile into a standalone CSS file. So _base.css, _slideshow.css, etc. are not possible in a SASS workflow. And generally speaking, the _base.scss file won't have any CSS in it, so compiling it as a standalone file wouldn't make sense even if it was possible.
Comment #61
RainbowArrayQuick note on the change notice at https://www.drupal.org/node/2379475:
These aren't quite the same as the media query declaration of all is missing in this example. all and only can be important trigger words if you want to exclude older browsers from processing the CSS. Not something you want to do all the time, but just want to make sure that is still a possibility.
One other really important thing to note here. In looking at the documentation for how libraries work for JS (https://www.drupal.org/theme-guide/8/adding-javascript) and CSS (https://www.drupal.org/theme-guide/8/adding-stylesheets), it's noted that not only does the CSS and JS need to be added to the libraries.yml, and the library needs to be added in .info.yml, but also sitewide libraries need to be added via a HOOK_page_attachments_alter function.
It would be really nice if libraries.yml had an all category to put sitewide assets into in order to avoid this. Attaching CSS in an info file was nice and simple: now we need entries in the .info.yml file, libraries.yml files and the .theme file just to add a sitewide CSS file. Maybe I'm misunderstanding this, but it seems like somewhat clunky TX.
Also, while the issue summary talks about consistency as the goal of this issue, which is fine and good, it looks like from the timeline that https://www.drupal.org/node/2368797 was what triggered this discussion. Reducing what goes into the first TCP packet is also important, but it seems important to note that this is one of the key reasons we're creating a more complicated workflow for themers, and from what I can understand, making libraries the only way to add CSS for a theme.
I'll also quote what I said in https://www.drupal.org/node/2298551#comment-9366295:
As has been pointed out, moving everything to libraries.yml doesn't prevent people from having Sass aggregate partials to one CSS file and attaching that as a library. In a lot of cases, I think a single aggregate CSS file is probably going to better for performance when working with HTTP 1.1, but your mileage may vary.
Comment #62
Jeff Burnz CreditAttribution: Jeff Burnz commented@ #61, I have been adding a library in the info file and it does not need to be added in hook_page_attachments_alter() as well, or in preprocess, if you add your theme library in the info.yml file it gets loaded everywhere that theme is used, however I am not clear on what you mean by "site wide".
I can clarify my comments earlier to show how I am using libraries.
1. I have a lot of components, maybe 30 or so, it will depend on the theme.
2. I use libraries to group the components and load them conditionally.
3. Some libraries have dependancies, such as JS, which in turn have dependancies, e.g. - core/modernizr etc.
It's those conditions that are the real crucial bit and this is where I start getting on the fence a bit, is it better going forward to load, for example, my "forum library" only for forum pages, or every page. From what I can tell you need to have a pretty big CSS file to offset the cost of the http request. AFAICT drupal will create a new aggregate if I use a path/state strategy in my conditions, so this will force browser to fetch a new stylesheet, which will largely contain what they already had + the additional "forum library". Correct?
At the moment I am going with a strategy of module exists sort of thing, e.g. taxonomy, forum, etc and just load the library for every page. All loading is done in preprocess.
As you can see I am not using SASS to aggregate a set of partials, in fact there are almost no partials. Everything is categorised but the actual grouping is done in the library.
In effect I am decoupling components and loading, which gives me a lot of flexibility which is what a base theme needs to have.
The dependancy chain is a big win here, I can simply load one library and ensure everything I need is loaded in the right order at the right time, it's simply fantastic to be able to do this so easily.
Comment #63
Fabianx CreditAttribution: Fabianx commented#60: Correct, what I meant is, you have e.g. a libraries/base-styles.scss file, which includes _base-styles.scss partial (if it had base styles) and a slideshow.scss file (which includes the _slideshow partial).
Out of the box this is a probably too complicated workflow, but in the future more might be possible.
#62:
Thanks for sharing and I agree, especially using module exists, which is very similar to extending an existing library (as proposed by Wim), to attaching libraries is a good way to build a base theme in 2014, especially, because you can depend on the base library, so things are consistent.
In general libraries everywhere will make a lot of things in the caching / render layer simpler and with that futureproof it for performance.
In D7 this is kinda hell to get the order right and ensure the right things are attached for advanced caching, so if we do this issue to only give me a favor I am fine too :-D.
Comment #64
star-szrI haven't had a chance to read all the comments here but I am in favour of what is proposed in the issue summary and the draft change record.
I think we should remove the pros and cons section from the change record, that information can be gleaned from the issue summary if folks are interested. And on that subject, very very minor but shouldn't there be a space here:
should be (?):
I made one other minor edit but didn't want to make that edit because I'm not sure about it.
Comment #65
joelpittet@Wim Leers you are a machine, you like responded to everybody! Kudos and @Wim Leers++
Comment #66
joelpittet@Cottser I agree and I've removed the pros and cons section. Good call.
Comment #67
joelpittet@Wim Leers regarding #47
I'm going to disagree with this being an exception, because IMHO many modules should provide global CSS and I believe it would improve performance because it would get aggregated into the same CSS files and thus letting the browser cache it instead of the browser having to cache the specific page that it's attached to.
Now I'm not saying that should be the case in all circumstances, and I do consede it's rare, but I do think the use-case if not currently previlent should be more so for that reason. And the modules I'm going to pick on are ones like flag, and all the 3rd party modules like twitter, instagram, campaign monitor, mailchimp and chosen.
They could all and most do provide their own CSS that should be applied to all pages, and as a themer I'll likely (as previously mentioned) rip them right out and move them into a partial to clean up my precieved inacciquecies in their implementations. Having a default that looks decent out of the box for a demo is great.
All of the above can still be done with #attached in a preprocess or build render array. The intent is to propose for consistency modules should have library declarations too. Maybe I should follow-up this request or do you think it could be included here?
Comment #68
Fabianx CreditAttribution: Fabianx commented#67: Follow-up please, the reason this was ripped out is performance, because else you need to load all .info files to memory during a request. And while it can be cached, it is still a lot of scanning.
Comment #69
joelpittet@Fabianx ah that makes a bunch of sense, thanks for the details. And somehow I missed #53 which said the same thing with different words:) thanks @catch also!
Comment #70
Wim Leers#58 + #59 + #62: Thanks for explaining how libraries help you a lot as a themer :)
#60: Noted; but I'm sure you can quite easily have SASS generate multiple CSS files.
#61: the
allmedia query is the default. Regarding a theme attaching a library on all pages (unconditionally): that can be done simpler, using thelibrarieskey in a theme's*.info.ymlfile. I updated the documentation page now: https://www.drupal.org/node/2216195/revisions/view/7857605/7865503 + https://www.drupal.org/node/2216195/revisions/view/7865503/7865513. https://www.drupal.org/theme-guide/8/adding-javascript is very much complete. And once this goes in, https://www.drupal.org/theme-guide/8/adding-stylesheets is very much unnecessary, since JS and CSS assets are handled in exactly the same way; if this issue lands, I'll update https://www.drupal.org/theme-guide/8/adding-javascript to cover both CSS and JS.#67: That's an eternal discussion that's finally going to become moot once HTTP/2 becomes widely deployed :) But today, in a HTTP/1 world, it depends on the site's complexity, it depends on the "average user", and it depends on so much more. If Drupal is given global (non-modular) CSS assets, then it doesn't have the ability to attach them only when necessary. If it's given modular CSS assets, then it can still choose to attach them globally. Historically, this has been nigh impossible. BUT! One huge advantage that I forgot to mention in this issue is the following: if all modules and themes provide modular/componentized CSS assets, and declare them as libraries… then Drupal 8 has a complete overview of all the available CSS assets on the site, which means we can write an
allcssmodule that simply attaches all CSS on every page! In Drupal 7 and before, it was impossible for Drupal to get a complete overview of all assets that could ever be loaded. Thanks to declaring all assets in libraries, Drupal has complete insight, and that allows us to make such interesting use case-dependent optimizations.I vouch to write such a module, if anybody wants it. (And I think I'll personally want it for my own site, which is a use case where that'd be more optimal.)
It seems like most people are now on board with this change? If so, is there anything left to be done before this is RTBC?
Comment #71
sqndr CreditAttribution: sqndr commentedWouldn't it be nice if we changed it in Bartik as well? Bartik still has a
baseas library name for the css. Would be nice if it wasstylingtoo. No?Comment #72
davidhernandezStop, I can't handle this much win! :)
Yes. Has anyone reviewed the patch?
Comment #73
LewisNymanIMO styling also seems like a bad name. Every CSS file is styling. Maybe something like 'global'?
Comment #74
Wim Leers#71: agreed!
#73:
globaldoesn't imply it's global styling. What aboutglobalstylingthen?Comment #75
LewisNymanI've modified this part of the beta evaluation because you don't need to loading the CSS over libraries to be SMACSS compliant. Seven is currently the most SMACSS compliant part of core and it isn't using libraries.
Comment #76
LewisNymanWim and I quickly discussed this on IRC and I think we agree globalstyling is the most descriptive name for this library.
Comment #77
Wim Leers#75: fair point!
Per #73 + #74 + #76: standardizing on
global-styling(everywhere else we split up multiple words by using dashes, so doing the same here).Comment #78
Fabianx CreditAttribution: Fabianx commentedGiven all feedback has been addressed and it was only asked for a review, I did a review and found nothing outstanding.
Therefore: RTBC
Comment #79
catchGiven libraries can be declared in libraries.info.yml by themes, the consistency here outweighs the potential updates that contrib/custom themes may need to do.
If we were only making the change for this reason, then I'd be iffy on beta suitability, but given it helps a lot to unblock both #1014086: Stampedes and cold cache performance issues with css/js aggregation and #2368797: Optimize ajaxPageState to keep Drupal 8 sites fast on high-latency networks, prevent CSS/JS aggregation from taking down sites and use HTTP GET for AJAX requests (both longstanding performance issues in core, AjaxPageState makes me sad every time I view source on any Drupal site), that tips it well over the edge.
Committed/pushed to 8.0.x, thanks!
Comment #81
nod_Glad to see that in. Especially glad to hear about things like #62 (despite a relative lack of communication/explanation about the topic), that's exactly why library use was generalized and as catch said, it's much needed for JS stuff down the line.
Comment #82
catchAlso just a note on #62:
There's one aggregate for 'every page' assets, and one aggregate for 'per page' assets - this counts for both JavaScript and CSS.
This means the only duplication happens when:
- something is added with the per-page flag but is actually conditional.
- there are lots and lots of different per-page assets on different pages
Otherwise there should be a single site-wide (or at least theme-wide) every page aggregate, and a handful of per-page with different items in.
Comment #83
joachim CreditAttribution: joachim commentedDocumentation needs to be updated:
- https://www.drupal.org/theme-guide/8/adding-stylesheets
- https://www.drupal.org/node/2349827
and possibly other pages too.
Comment #84
Wim LeersI've now updated both.