Combine sass files

[ChristophWurst]

We need to get the number of requests down a bit as discussed in #2272. Therefore, I've started a PR to create an image sprite for the icons (#3385). Furthermore, I plan to combine the vendor scripts we load on all pages, like jQuery, it's plugins, Backbone and the like.

We do also load a bunch of style sheets, of which some are included on all pages as well. Therefore, I've been thinking about saving some requests there too. As discussed with @skjnldsv on IRC, the simplest solution from my point of view, would be to create some kind of meta files, where we import all the source files that we load individually now.

This would looks like this:

 core/css/server.scss            | 31 +++++++++++++++++++++++++++++++
 lib/private/legacy/template.php | 18 ++++++------------
 2 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/core/css/server.scss b/core/css/server.scss
new file mode 100644
index 0000000000..2048a881dd
--- /dev/null
+++ b/core/css/server.scss
@@ -0,0 +1,31 @@
+/**
+ * @author Christoph Wurst <christoph@winzerhof-wurst.at>
+ * @copyright Christoph Wurst <christoph@winzerhof-wurst.at>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+@import 'styles.scss';
+@import 'inputs.scss';
+@import 'header.scss';
+@import 'icons.scss';
+@import 'fonts.css';
+@import 'apps.scss';
+@import 'apps.scss';
+@import 'global.css';
+@import 'multiselect.scss';
+@import 'tooltip.scss';
diff --git a/lib/private/legacy/template.php b/lib/private/legacy/template.php
index a07bf214f3..07dbe7d6a6 100644
--- a/lib/private/legacy/template.php
+++ b/lib/private/legacy/template.php
@@ -106,19 +106,13 @@ class OC_Template extends \OC\Template\Base {
 				}
 			}
 
-			OC_Util::addStyle("tooltip",null,true);
-			OC_Util::addStyle('jquery-ui-fixes',null,true);
+			OC_Util::addStyle('jquery-ui-fixes', null, true);
+			OC_Util::addStyle('mobile', null, true);
+			OC_Util::addStyle('fixes', null, true);
+			OC_Util::addStyle('global', null, true);
+			OC_Util::addStyle('server', null, true);
+
 			OC_Util::addVendorStyle('jquery-ui/themes/base/jquery-ui',null,true);
-			OC_Util::addStyle("mobile",null,true);
-			OC_Util::addStyle("multiselect",null,true);
-			OC_Util::addStyle("fixes",null,true);
-			OC_Util::addStyle("global",null,true);
-			OC_Util::addStyle("apps",null,true);
-			OC_Util::addStyle("fonts",null,true);
-			OC_Util::addStyle("icons",null,true);
-			OC_Util::addStyle("header",null,true);
-			OC_Util::addStyle("inputs");
-			OC_Util::addStyle("styles",null,true);
 
 			// avatars
 			if (\OC::$server->getSystemConfig()->getValue('enable_avatars', true) === true) {

The result: on my dev instance 173 instead of 185 were necessary to load the files app with cleared cache (ctrl+f5). Note that this does only affect the server files. We could do the same with files, sharing, settings and so on and save even more requests.

@nextcloud/designers would do you think? Any objections? I know @skjnldsv doesn't like the approach 😀

[skjnldsv]

Why not include mobile, global, fixes... ?

Just as a comparison, could you provide before/after benchmarks? :D

[ChristophWurst]

Why not include mobile, global, fixes... ?

I wasn't sure if that's okay because those are css and not scss files. Can I just import them in the same fashion or should we rename them before?

Just as a comparison, could you provide before/after benchmarks? :D

As stated above, 173 instead of 185 requests 🚀

[skjnldsv]

siege -r6 -c10 -d10 --header="Authorization:Basic YWRtaW46YWRtaW4=" https://localhost/index.php/apps/files/

Without concatenation:

Transactions:		        9240 hits
Availability:		      100.00 %
Elapsed time:		       60.65 secs
Data transferred:	      192.46 MB
Response time:		        0.02 secs
Transaction rate:	      152.35 trans/sec
Throughput:		        3.17 MB/sec
Concurrency:		        3.16
Successful transactions:        9240
Failed transactions:	           0
Longest transaction:	        0.64
Shortest transaction:	        0.00

With concatenation:

Transactions:		        8760 hits
Availability:		      100.00 %
Elapsed time:		       51.89 secs
Data transferred:	      191.36 MB
Response time:		        0.01 secs
Transaction rate:	      168.82 trans/sec
Throughput:		        3.69 MB/sec
Concurrency:		        2.28
Successful transactions:        8760
Failed transactions:	           0
Longest transaction:	        0.83
Shortest transaction:	        0.00

[skjnldsv]

@ChristophWurst importing css files should be ok. The compiler should ignore it.

We need to be sure this process isn't too heavy for the server. And another issue, since the algorithm is relying on the mdate of the scss file to check if the cached css needs a recompilation or not won't work anymore if we use one single scss file 😞

[MorrisJobke]

We need to be sure this process isn't too heavy for the server. And another issue, since the algorithm is relying on the mdate of the scss file to check if the cached css needs a recompilation or not won't work anymore if we use one single scss file 😞

So we need to touch this file for every change in the imported css files?! 😕

[skjnldsv]

It won't work. @import doesn't care about the other files. Only server.scss will be used as a comparison

[go2sh]

Why would it be so bad to do the scss compilation as a build step? From a user point of view nothing changes. And if someone wants to run a dev (git) Version, screw the number of requests.

[skjnldsv]

Because we don't want to increase the level entry barrier. This has been discussed long time before launching the scss integration :)

[go2sh]

Hmm, I see. I just followed the discussion around this topic and spent some thoughts on it. It is quite hard to create a modern fully featured webapp, which nextcloud has become, with a low entry barrier. The hole asset workflow is very unique and the code is so complex and/or split, that npm install and npm run build (or what ever) doesn't add any complexity from my point of view...

But in the end it all comes down to this: Who wants to be modular, gets a lot of modules.

[juliusstoerrle]

I'm not sure if I understood the issue correctly, but can't we implement a button in the admin menu to recompile the full css?

[MorrisJobke]

I'm not sure if I understood the issue correctly, but can't we implement a button in the admin menu to recompile the full css?

No. we should do this completely in the background. The problem is also not about compiling it, but about concatenate the correct ones together in the correct order and properly detect that a change happened. No admin wants to do this manually and it is super error prone.