Refactor skins to be built on server-side (#6849)

* WIP

* using bootswatch from npm instead of bootswatch CDN url

* feat: on-demand client css building for skins

* added ability for client-side to select a skin

* updated loading and saving logic of bootstrapSkin on client side user settings

* fix: broken test for #6849

install/package.json

@@ -32,6 +32,7 @@
         "benchpressjs": "^1.2.5",
         "body-parser": "^1.18.2",
         "bootstrap": "^3.3.7",
+        "bootswatch": "^3",
         "chart.js": "^2.7.1",
         "cli-graph": "^3.2.2",
         "clipboard": "^2.0.1",

public/src/client/account/settings.js

@@ -48,26 +48,20 @@ define('forum/account/settings', ['forum/account/header', 'components', 'sounds'
 	};
 
 	function changePageSkin(skinName) {
-		var css = $('#bootswatchCSS');
+		var clientEl = Array.prototype.filter.call(document.querySelectorAll('link[rel="stylesheet"]'), function (el) {
-		if (skinName === 'noskin' || (skinName === 'default' && config.defaultBootswatchSkin === 'noskin')) {
+			return el.href.indexOf(config.relative_path + '/assets/client') !== -1;
-			css.remove();
+		})[0] || null;
-		} else {
+
-			if (skinName === 'default') {
+		// Update client.css link element to point to selected skin variant
-				skinName = config.defaultBootswatchSkin;
+		clientEl.href = config.relative_path + '/assets/client' + (skinName ? '-' + skinName : '') + '.css';
-			}
-			var cssSource = '//maxcdn.bootstrapcdn.com/bootswatch/3.3.7/' + skinName + '/bootstrap.min.css';
-			if (css.length) {
-				css.attr('href', cssSource);
-			} else {
-				css = $('<link id="bootswatchCSS" href="' + cssSource + '" rel="stylesheet" media="screen">');
-				$('head').append(css);
-			}
-		}
 
 		var currentSkinClassName = $('body').attr('class').split(/\s+/).filter(function (className) {
 			return className.startsWith('skin-');
 		});
-		$('body').removeClass(currentSkinClassName.join(' ')).addClass('skin-' + skinName);
+		$('body').removeClass(currentSkinClassName.join(' '));
+		if (skinName) {
+			$('body').addClass('skin-' + skinName);
+		}
 	}
 
 	function loadSettings() {

src/controllers/accounts/settings.js

@@ -112,8 +112,7 @@ settingsController.get = function (req, res, callback) {
 			];
 
 			userData.bootswatchSkinOptions = [
-				{ name: 'No skin', value: 'noskin' },
+				{ name: 'Default', value: '' },
-				{ name: 'Default', value: 'default' },
 				{ name: 'Cerulean', value: 'cerulean' },
 				{ name: 'Cosmo', value: 'cosmo'	},
 				{ name: 'Cyborg', value: 'cyborg' },

src/controllers/api.js

@@ -58,8 +58,7 @@ apiController.loadConfig = function (req, callback) {
 	config.categoryTopicSort = meta.config.categoryTopicSort || 'newest_to_oldest';
 	config.csrf_token = req.csrfToken && req.csrfToken();
 	config.searchEnabled = plugins.hasListeners('filter:search.query');
-	config.bootswatchSkin = meta.config.bootswatchSkin || 'noskin';
+	config.bootswatchSkin = meta.config.bootswatchSkin || '';
-	config.defaultBootswatchSkin = meta.config.bootswatchSkin || 'noskin';
 	config.enablePostHistory = (meta.config.enablePostHistory || 1) === 1;
 	config.notificationAlertTimeout = meta.config.notificationAlertTimeout || 5000;
 
@@ -85,6 +84,10 @@ apiController.loadConfig = function (req, callback) {
 			user.getSettings(req.uid, next);
 		},
 		function (settings, next) {
+			// Handle old skin configs
+			const oldSkins = ['noskin', 'default'];
+			settings.bootswatchSkin = oldSkins.includes(settings.bootswatchSkin) ? '' : settings.bootswatchSkin;
+
 			config.usePagination = settings.usePagination;
 			config.topicsPerPage = settings.topicsPerPage;
 			config.postsPerPage = settings.postsPerPage;
@@ -95,7 +98,7 @@ apiController.loadConfig = function (req, callback) {
 			config.categoryTopicSort = settings.categoryTopicSort || config.categoryTopicSort;
 			config.topicSearchEnabled = settings.topicSearchEnabled || false;
 			config.delayImageLoading = settings.delayImageLoading !== undefined ? settings.delayImageLoading : true;
-			config.bootswatchSkin = (meta.config.disableCustomUserSkins !== 1 && settings.bootswatchSkin && settings.bootswatchSkin !== 'default') ? settings.bootswatchSkin : config.bootswatchSkin;
+			config.bootswatchSkin = (meta.config.disableCustomUserSkins !== 1 && settings.bootswatchSkin && settings.bootswatchSkin !== '') ? settings.bootswatchSkin : '';
 			plugins.fireHook('filter:config.get', config, next);
 		},
 	], callback);

src/meta/css.js

@@ -5,6 +5,7 @@ var nconf = require('nconf');
 var fs = require('fs');
 var path = require('path');
 var async = require('async');
+var rimraf = require('rimraf');
 
 var plugins = require('../plugins');
 var db = require('../database');
@@ -13,6 +14,12 @@ var minifier = require('./minifier');
 
 var CSS = module.exports;
 
+CSS.supportedSkins = [
+	'cerulean', 'cyborg', 'flatly', 'journal', 'lumen', 'paper', 'simplex',
+	'spacelab', 'united', 'cosmo', 'darkly', 'readable', 'sandstone',
+	'slate', 'superhero', 'yeti',
+];
+
 var buildImports = {
 	client: function (source) {
 		return '@import "./theme";\n' + source + '\n' + [
@@ -93,21 +100,34 @@ function getBundleMetadata(target, callback) {
 		path.join(__dirname, '../../public/vendor/fontawesome/less'),
 	];
 
+	// Skin support
+	let skin;
+	if (target.startsWith('client-')) {
+		skin = target.split('-')[1];
+
+		if (CSS.supportedSkins.includes(skin)) {
+			target = 'client';
+		}
+	}
+
 	async.waterfall([
 		function (next) {
 			if (target !== 'client') {
 				return next(null, null);
 			}
 
-			db.getObjectFields('config', ['theme:type', 'theme:id'], next);
+			db.getObjectFields('config', ['theme:type', 'theme:id', 'bootswatchSkin'], next);
 		},
 		function (themeData, next) {
 			if (target === 'client') {
 				var themeId = (themeData['theme:id'] || 'nodebb-theme-persona');
 				var baseThemePath = path.join(nconf.get('themes_path'), (themeData['theme:type'] && themeData['theme:type'] === 'local' ? themeId : 'nodebb-theme-vanilla'));
 				paths.unshift(baseThemePath);
+
+				themeData.bootswatchSkin = skin || themeData.bootswatchSkin;
 			}
 
+
 			async.parallel({
 				less: function (cb) {
 					async.waterfall([
@@ -143,14 +163,24 @@ function getBundleMetadata(target, callback) {
 						},
 					], cb);
 				},
+				skin: function (cb) {
+					const skinImport = [];
+					if (themeData && themeData.bootswatchSkin) {
+						skinImport.push('\n@import "./bootswatch/' + themeData.bootswatchSkin + '/variables.less";');
+						skinImport.push('\n@import "./bootswatch/' + themeData.bootswatchSkin + '/bootswatch.less";');
+					}
+
+					cb(null, skinImport.join(''));
+				},
 			}, next);
 		},
 		function (result, next) {
+			var skinImport = result.skin;
 			var cssImports = result.css;
 			var lessImports = result.less;
 			var acpLessImports = result.acpLess;
 
-			var imports = cssImports + '\n' + lessImports + '\n' + acpLessImports;
+			var imports = skinImport + '\n' + cssImports + '\n' + lessImports + '\n' + acpLessImports;
 			imports = buildImports[target](imports);
 
 			next(null, { paths: paths, imports: imports });
@@ -160,6 +190,13 @@ function getBundleMetadata(target, callback) {
 
 CSS.buildBundle = function (target, fork, callback) {
 	async.waterfall([
+		function (next) {
+			if (target === 'client') {
+				rimraf(path.join(__dirname, '../../build/public/client*'), next);
+			} else {
+				setImmediate(next);
+			}
+		},
 		function (next) {
 			getBundleMetadata(target, next);
 		},
@@ -168,7 +205,7 @@ CSS.buildBundle = function (target, fork, callback) {
 			minifier.css.bundle(data.imports, data.paths, minify, fork, next);
 		},
 		function (bundle, next) {
-			var filename = (target === 'client' ? 'stylesheet' : 'admin') + '.css';
+			var filename = target + '.css';
 
 			fs.writeFile(path.join(__dirname, '../../build/public', filename), bundle.code, next);
 		},

src/meta/themes.js

@@ -158,6 +158,7 @@ Themes.set = function (data, callback) {
 				themeData['theme:staticDir'] = config.staticDir ? config.staticDir : '';
 				themeData['theme:templates'] = config.templates ? config.templates : '';
 				themeData['theme:src'] = '';
+				themeData.bootswatchSkin = '';
 
 				Meta.configs.setMultiple(themeData, next);
 

src/middleware/header.js

@@ -141,7 +141,7 @@ module.exports = function (middleware) {
 				results.user['email:confirmed'] = results.user['email:confirmed'] === 1;
 				results.user.isEmailConfirmSent = !!results.isEmailConfirmSent;
 
-				setBootswatchCSS(templateValues, res.locals.config);
+				templateValues.bootswatchSkin = parseInt(meta.config.disableCustomUserSkins, 10) !== 1 ? res.locals.config.bootswatchSkin || '' : '';
 
 				var unreadCount = {
 					topic: results.unreadCounts[''] || 0,
@@ -272,21 +272,5 @@ module.exports = function (middleware) {
 
 		return title;
 	}
-
-	function setBootswatchCSS(obj, config) {
-		if (config && config.bootswatchSkin !== 'noskin') {
-			var skinToUse = '';
-
-			if (!meta.config.disableCustomUserSkins) {
-				skinToUse = config.bootswatchSkin;
-			} else if (meta.config.bootswatchSkin) {
-				skinToUse = meta.config.bootswatchSkin;
-			}
-
-			if (skinToUse) {
-				obj.bootswatchCSS = '//maxcdn.bootstrapcdn.com/bootswatch/3.3.7/' + skinToUse + '/bootstrap.min.css';
-			}
-		}
-	}
 };
 

src/middleware/index.js

@@ -205,3 +205,15 @@ middleware.delayLoading = function (req, res, next) {
 
 	setTimeout(next, 1000);
 };
+
+middleware.buildSkinAsset = function (req, res, next) {
+	// If this middleware is reached, a skin was requested, so it is built on-demand
+	var target = path.basename(req.originalUrl).match(/(client-[a-z]+)/);
+	if (target) {
+		meta.css.buildBundle(target[0], true, function () {
+			next();
+		});
+	} else {
+		setImmediate(next);
+	}
+};

src/routes/index.js

@@ -153,6 +153,11 @@ module.exports = function (app, middleware, callback) {
 		statics.unshift({ route: '/assets/uploads', path: nconf.get('upload_path') });
 	}
 
+	// Skins
+	meta.css.supportedSkins.forEach(function (skin) {
+		app.use(relativePath + '/assets/client-' + skin + '.css', middleware.buildSkinAsset);
+	});
+
 	statics.forEach(function (obj) {
 		app.use(relativePath + obj.route, express.static(obj.path, staticOptions));
 	});
@@ -160,6 +165,19 @@ module.exports = function (app, middleware, callback) {
 		res.redirect(relativePath + '/assets/uploads' + req.path + '?' + meta.config['cache-buster']);
 	});
 
+	// only warn once
+	var warned = new Set();
+
+	// DEPRECATED (v1.12.0)
+	app.use(relativePath + '/assets/stylesheet.css', function (req, res) {
+		if (!warned.has(req.path)) {
+			winston.warn('[deprecated] Accessing `/assets/stylesheet.css` is deprecated to be REMOVED in NodeBB v1.12.0. ' +
+			'Use `/assets/client.css` to access this file');
+			warned.add(req.path);
+		}
+		res.redirect(relativePath + '/assets/client.css?' + meta.config['cache-buster']);
+	});
+
 	app.use(relativePath + '/assets/vendor/jquery/timeago/locales', middleware.processTimeagoLocales);
 	app.use(controllers['404'].handle404);
 	app.use(controllers.errors.handleURIErrors);

src/user/settings.js

@@ -78,7 +78,7 @@ module.exports = function (User) {
 				settings.restrictChat = parseInt(getSetting(settings, 'restrictChat', 0), 10) === 1;
 				settings.topicSearchEnabled = parseInt(getSetting(settings, 'topicSearchEnabled', 0), 10) === 1;
 				settings.delayImageLoading = parseInt(getSetting(settings, 'delayImageLoading', 1), 10) === 1;
-				settings.bootswatchSkin = settings.bootswatchSkin || meta.config.bootswatchSkin || 'default';
+				settings.bootswatchSkin = settings.bootswatchSkin || '';
 				settings.scrollToMyPost = parseInt(getSetting(settings, 'scrollToMyPost', 1), 10) === 1;
 
 				notifications.getAllNotificationTypes(next);
@@ -138,12 +138,9 @@ module.exports = function (User) {
 			incomingChatSound: data.incomingChatSound,
 			outgoingChatSound: data.outgoingChatSound,
 			upvoteNotifFreq: data.upvoteNotifFreq,
+			bootswatchSkin: data.bootswatchSkin,
 		};
 
-		if (data.bootswatchSkin) {
-			settings.bootswatchSkin = data.bootswatchSkin;
-		}
-
 		async.waterfall([
 			function (next) {
 				notifications.getAllNotificationTypes(next);

test/build.js

@@ -170,7 +170,7 @@ describe('Build', function (done) {
 	it('should build client side styles', function (done) {
 		build.build(['client side styles'], function (err) {
 			assert.ifError(err);
-			var filename = path.join(__dirname, '../build/public/stylesheet.css');
+			var filename = path.join(__dirname, '../build/public/client.css');
 			assert(file.existsSync(filename));
 			assert(fs.readFileSync(filename).toString().startsWith('/*! normalize.css'));
 			done();