Rollup Fixes (#5232)

- Use PostCSS instead of Less plugin
- Fixes CSS append problem
- Show which css file changed instead of rollup.manifest.css
- Minify CSS in production
- Cannot import less/css files in js files, put them in build.json
- Let's avoid major changes in Frappe Framework now :)

frappe/public/build.json

@@ -126,7 +126,8 @@
 		"public/less/form.less",
 		"public/less/mobile.less",
 		"public/less/kanban.less",
-		"public/less/controls.less"
+		"public/less/controls.less",
+		"public/less/chat.less"
 	],
 	"css/frappe-rtl.css": [
 		"public/css/bootstrap-rtl.css",
@@ -350,6 +351,8 @@
 	],
 	"css/report.min.css": [
 		"public/less/report.less",
+		"node_modules/frappe-datatable/dist/frappe-datatable.css",
+		"public/less/frappe-datatable.less",
 		"public/css/tree_grid.css"
 	],
 	"js/report.min.js": [

frappe/public/js/frappe/chat.js

@@ -7,8 +7,6 @@ import hyper  from '../lib/hyper.min'
 
 import './socketio_client'
 
-import '../../less/chat.less'
-
 /* eslint semi: "never" */
 // Fuck semicolons - https://mislav.net/2010/05/semicolons
 
@@ -1055,7 +1053,7 @@ frappe.components.FAB.defaultProps
 	icon: "octicon octicon-plus"
 }
 frappe.components.FAB.SIZE
-= 
+=
 {
 	small:
 	{
@@ -1470,7 +1468,7 @@ class extends Component {
 			placeholder: __("Search or Create a New Chat"),
 				  class: "level",
 				 layout: props.layout,
-				actions: 
+				actions:
 			frappe._.compact([
 				{
 					  label: __("New"),
@@ -2460,11 +2458,11 @@ frappe.chat.setup  = () =>
 {
 	if ( frappe.session.user !== 'Guest' ) {
 		frappe.log = frappe.Logger.get('frappe.chat');
-	
+
 		frappe.log.info('Setting up frappe.chat');
 		frappe.log.warn('TODO: Handle realtime System Settings update.');
 		frappe.log.warn('TODO: frappe.chat.<object> requires a storage.');
-		
+
 		// Create/Get Chat Profile for session User, retrieve enable_chat
 		frappe.log.info('Creating a Chat Profile.');
 		frappe.chat.profile.create('enable_chat').then(({ enable_chat }) => {
@@ -2472,7 +2470,7 @@ frappe.chat.setup  = () =>
 			const should_render = frappe.sys_defaults.enable_chat && enable_chat;
 			frappe.chat.render(should_render);
 		});
-	
+
 		// Triggered when a User updates his/her Chat Profile.
 		// Don't worry, enable_chat is broadcasted to this user only. No overhead. :)
 		frappe.chat.profile.on.update((user, profile) => {

frappe/public/js/frappe/views/reports/report_view.js

@@ -2,8 +2,7 @@
  * frappe.views.ReportView
  */
 import DataTable from 'frappe-datatable';
-import 'frappe-datatable/dist/frappe-datatable.css';
-import '../../../../less/frappe-datatable.less';
+
 frappe.provide('frappe.views');
 
 frappe.views.ReportView = class ReportView extends frappe.views.ListView {

package.json

@@ -32,12 +32,13 @@
   "devDependencies": {
     "babel-runtime": "^6.26.0",
     "chalk": "^2.3.2",
+    "less": "^3.0.1",
     "rollup": "^0.55.3",
     "rollup-plugin-buble": "^0.19.2",
     "rollup-plugin-commonjs": "^8.3.0",
-    "rollup-plugin-less": "^0.1.3",
     "rollup-plugin-multi-entry": "^2.0.2",
     "rollup-plugin-node-resolve": "^3.0.2",
+    "rollup-plugin-postcss": "^1.4.0",
     "rollup-plugin-uglify": "^3.0.0"
   }
 }

rollup/build.js

@@ -10,12 +10,11 @@ const {
 	apps_list,
 	run_serially,
 	assets_path,
-	sites_path,
-	delete_file
+	sites_path
 } = require('./rollup.utils');
 
 const {
-	get_rollup_options
+	get_options_for
 } = require('./config');
 
 show_production_message();
@@ -30,23 +29,16 @@ function build_assets_for_all_apps() {
 }
 
 function build_assets(app) {
-	const build_json = get_build_json(app);
-	if (!build_json) return Promise.resolve();
+	const options = get_options_for(app);
+	if (!options.length) return Promise.resolve();
 	log(chalk.yellow(`\nBuilding ${app} assets...\n`));
 
-	const promises = Object.keys(build_json)
-		.map(output_file => {
-			const input_files = build_json[output_file]
-				.map(input_file => path.resolve(get_app_path(app), input_file));
-			const { inputOptions, outputOptions } = get_rollup_options(output_file, input_files);
-
-			if (output_file.endsWith('libs.min.js')) return Promise.resolve();
-
-			return build(inputOptions, outputOptions)
-				.then(() => {
-					log(`${chalk.green('✔')} Built ${output_file}`);
-				});
-		});
+	const promises = options.map(({ inputOptions, outputOptions, output_file}) => {
+		return build(inputOptions, outputOptions)
+			.then(() => {
+				log(`${chalk.green('✔')} Built ${output_file}`);
+			});
+	});
 
 	const start = Date.now();
 	return Promise.all(promises)
@@ -89,19 +81,9 @@ function ensure_js_css_dirs() {
 			fs.mkdirSync(path);
 		}
 	});
-
-	// clear files in css folder
-	const css_path = path.resolve(assets_path, 'css');
-	const files = fs.readdirSync(css_path);
-
-	files.forEach(file => {
-		delete_file(path.resolve(css_path, file));
-	});
 }
 
 function show_production_message() {
 	const production = process.env.FRAPPE_ENV === 'production';
-	if (production) {
-		log(chalk.green('Production mode'));
-	}
-}
+	log(chalk.yellow(`${production ? 'Production' : 'Development'} mode`));
+}

rollup/config.js

@@ -5,7 +5,7 @@ const log = console.log; // eslint-disable-line
 const multi_entry = require('rollup-plugin-multi-entry');
 const commonjs = require('rollup-plugin-commonjs');
 const node_resolve = require('rollup-plugin-node-resolve');
-const less = require('rollup-plugin-less');
+const postcss = require('rollup-plugin-postcss');
 const buble = require('rollup-plugin-buble');
 const uglify = require('rollup-plugin-uglify');
 const frappe_html = require('./frappe-html-plugin');
@@ -17,7 +17,7 @@ const {
 	bench_path,
 	get_public_path,
 	get_app_path,
-	delete_file,
+	get_build_json
 } = require('./rollup.utils');
 
 function get_rollup_options(output_file, input_files) {
@@ -36,17 +36,6 @@ function get_rollup_options_for_js(output_file, input_files) {
 		multi_entry(),
 		// .html -> .js
 		frappe_html(),
-		// less -> css
-		less({
-			output: css_output_file,
-			option: {
-				// so that other .less files can import variables.less from frappe directly
-				paths: [path.resolve(get_public_path('frappe'), 'less'), path.resolve(get_app_path('frappe'), '..')],
-				compress: production
-			},
-			include: [path.resolve(bench_path, '**/*.less'), path.resolve(bench_path, '**/*.css')],
-			exclude: []
-		}),
 		// ES6 -> ES5
 		buble({
 			objectAssign: 'Object.assign',
@@ -93,21 +82,20 @@ function get_rollup_options_for_js(output_file, input_files) {
 function get_rollup_options_for_css(output_file, input_files) {
 	const output_path = path.resolve(assets_path, output_file);
 
-	// clear css file to avoid appending problem
-	delete_file(output_path);
-
 	const plugins = [
 		// enables array of inputs
 		multi_entry(),
 		// less -> css
-		less({
-			output: output_path,
-			option: {
-				// so that other .less files can import variables.less from frappe directly
-				paths: [path.resolve(get_public_path('frappe'), 'less')],
-				compress: production
-			},
-			include: [path.resolve(bench_path, '**/*.less'), path.resolve(bench_path, '**/*.css')]
+		postcss({
+			extract: output_path,
+			use: [['less', {
+				// import other less/css files starting from these folders
+				paths: [
+					path.resolve(get_public_path('frappe'), 'less')
+				]
+			}]],
+			include: [path.resolve(bench_path, '**/*.less'), path.resolve(bench_path, '**/*.css')],
+			minimize: production
 		})
 	];
 
@@ -131,6 +119,30 @@ function get_rollup_options_for_css(output_file, input_files) {
 	};
 }
 
+function get_options_for(app) {
+	const build_json = get_build_json(app);
+	if (!build_json) return [];
+
+	return Object.keys(build_json)
+		.map(output_file => {
+			if (output_file.endsWith('libs.min.js')) return null;
+
+			const input_files = build_json[output_file]
+				.map(input_file => {
+					let prefix = get_app_path(app);
+					if (input_file.startsWith('node_modules/')) {
+						prefix = path.resolve(get_app_path(app), '..');
+					}
+					return path.resolve(prefix, input_file);
+				});
+			return Object.assign(
+				get_rollup_options(output_file, input_files), {
+					output_file
+				});
+		})
+		.filter(Boolean);
+}
+
 module.exports = {
-	get_rollup_options
+	get_options_for
 };

rollup/frappe-html-plugin.js

@@ -23,4 +23,4 @@ module.exports = function frappe_html() {
 			`;
 		}
 	};
-};
+};

rollup/watch.js

@@ -3,13 +3,11 @@ const chalk = require('chalk');
 const rollup = require('rollup');
 const log = console.log; // eslint-disable-line
 const {
-	apps_list,
-	get_app_path,
-	get_build_json
+	apps_list
 } = require('./rollup.utils');
 
 const {
-	get_rollup_options
+	get_options_for
 } = require('./config');
 
 watch_assets();
@@ -24,36 +22,42 @@ function watch_assets() {
 
 	watcher.on('event', event => {
 		switch(event.code) {
-			case 'START':
+			case 'START': {
 				log(chalk.yellow(`\nWatching...`));
 				break;
+			}
 
-			case 'BUNDLE_START':
-				log('Rebuilding', path.basename(event.output[0]));
+			case 'BUNDLE_START': {
+				const output = event.output[0];
+				if (output.endsWith('.js')) {
+					log('Rebuilding', path.basename(event.output[0]));
+				}
 				break;
+			}
 
-			default:
-				break;
+			default: break;
 		}
 	});
 }
 
 function get_watch_options(app) {
-	const build_json = get_build_json(app);
-	if (!build_json) return [];
-
-	const watchOptions = Object.keys(build_json)
-		.map(output_file => {
-			const input_files = build_json[output_file]
-				.map(input_file => path.resolve(get_app_path(app), input_file));
-			const { inputOptions, outputOptions } = get_rollup_options(output_file, input_files);
-
-			if (output_file.endsWith('libs.min.js')) return;
+	const options = get_options_for(app);
 
-			return Object.assign({}, inputOptions, {
-				output: outputOptions
-			});
-		}).filter(Boolean);
+	return options.map(({ inputOptions, outputOptions, output_file}) => {
+		return Object.assign({}, inputOptions, {
+			output: outputOptions,
+			plugins: [log_css_change({output: output_file})].concat(inputOptions.plugins)
+		});
+	});
+}
 
-	return watchOptions;
+function log_css_change({output}) {
+	return {
+		name: 'log-css-change',
+		ongenerate() {
+			if (!output.endsWith('.css')) return null;
+			log('Rebuilding', path.basename(output));
+			return null;
+		}
+	};
 }