yellow team mailing list archive

[Thiago Veronezi <thiago@xxxxxxxxxxxx>]

Thiago Veronezi has proposed merging lp:~tveronezi/juju-gui/uglify into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1075521 in juju-ui: "Resource minification/aggregation js"
  https://bugs.launchpad.net/juju-gui/+bug/1075521

For more details, see:
https://code.launchpad.net/~tveronezi/juju-gui/uglify/+merge/133116

Resource minification/aggregation js

* We should aggregate and minimize the js sources in order to improve the load speed of the application;
* We dont want to use the yui combo loader feature;
* The final product will provide three js files: one for the YUI dependencies, one for our custom js code and one for third part js like d3.

https://codereview.appspot.com/6821090/

-- 
https://code.launchpad.net/~tveronezi/juju-gui/uglify/+merge/133116
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~tveronezi/juju-gui/uglify into lp:juju-gui.

=== modified file '.bzrignore'
--- .bzrignore	2012-10-31 09:16:13 +0000
+++ .bzrignore	2012-11-06 17:56:20 +0000
@@ -20,3 +20,8 @@
 yuidoc
 app/assets/manifest.appcache
 app/assets/sprite
+app/all-yui.js
+app/all-app.js
+app/all-third.js
+app/all-app-debug.js
+properties.js

=== modified file 'Makefile'
--- Makefile	2012-11-06 13:16:34 +0000
+++ Makefile	2012-11-06 17:56:20 +0000
@@ -9,7 +9,7 @@
 	node_modules/mocha node_modules/d3 node_modules/graceful-fs \
 	node_modules/should node_modules/jshint node_modules/expect.js \
 	node_modules/express node_modules/yui node_modules/yuidoc \
-	node_modules/grunt node_modules/node-spritesheet
+	node_modules/grunt node_modules/node-spritesheet node_modules/node-minify
 TEMPLATE_TARGETS=$(shell bzr ls -k file app/templates)
 SPRITE_SOURCE_FILES=$(shell bzr ls -R -k file app/assets/images)
 SPRITE_GENERATED_FILES=app/assets/sprite/sprite.css app/assets/sprite/sprite.png
@@ -37,7 +37,7 @@
 	@ln -sf `pwd`/node_modules/yui ./app/assets/javascripts/
 	@ln -sf `pwd`/node_modules/d3/d3.v2* ./app/assets/javascripts/
 
-install: appcache $(NODE_TARGETS) app/templates.js yuidoc spritegen
+install: appcache $(NODE_TARGETS) app/templates.js yuidoc spritegen combinejs
 
 gjslint: virtualenv/bin/gjslint
 	@virtualenv/bin/gjslint --strict --nojsdoc --jslint_error=all \
@@ -61,12 +61,26 @@
 
 spritegen: $(SPRITE_GENERATED_FILES)
 
+combinejs: 
+	@rm -f app/all-yui.js
+	@rm -f app/all-app.js
+	@rm -f app/all-third.js
+	@rm -f app/all-app-debug.js
+	@rm -f properties.js
+	@node merge-files.js
+
 prep: beautify lint
 
 test: install
 	@./test-server.sh
 
+debug: install
+	@cp properties-dev.js properties.js
+	@echo "Customize config.js to modify server settings"
+	@node server.js
+
 server: install
+	@cp properties-prod.js properties.js
 	@echo "Customize config.js to modify server settings"
 	@node server.js
 
@@ -75,6 +89,11 @@
 	@make -C docs clean
 	@rm -Rf bin/sprite/
 	@rm -Rf app/assets/sprite/
+	@rm -f app/all-yui.js
+	@rm -f app/all-app.js
+	@rm -f app/all-third.js
+	@rm -f app/all-app-debug.js
+	@rm -f properties.js
 
 $(APPCACHE): manifest.appcache.in
 	@cp manifest.appcache.in $(APPCACHE)
@@ -91,4 +110,5 @@
 appcache-force: appcache-touch appcache
 
 .PHONY: test lint beautify server install clean prep jshint gjslint \
-	appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint
+	appcache appcache-touch appcache-force yuidoc spritegen yuidoc-lint \
+	combinejs

=== modified file 'app/index.html'
--- app/index.html	2012-11-01 13:21:53 +0000
+++ app/index.html	2012-11-06 17:56:20 +0000
@@ -69,9 +69,9 @@
         <div id="vp-right-border"></div>
     </div>
     <!-- javascript away -->
-    <script src="/juju-ui/assets/javascripts/yui/yui/yui-debug.js"></script>
-    <script src="/juju-ui/modules.js"></script>
-    <script src="/config.js"></script>
+    <script src="/source/yui.js"></script>
+    <script src="/juju-ui/all-third.js"></script>
+    <script src="/source/modules.js"></script>
     <script>
       YUI(GlobalConfig).use(["juju-gui"], function(Y) {
         app = new Y.juju.App(juju_config);

=== added file 'app/modules-debug.js'
--- app/modules-debug.js	1970-01-01 00:00:00 +0000
+++ app/modules-debug.js	2012-11-06 17:56:20 +0000
@@ -0,0 +1,129 @@
+GlobalConfig = {
+  // Uncomment for debug versions of YUI.
+  filter: 'debug',
+  // Uncomment for verbose logging of YUI
+  debug: false,
+
+  // Use Rollups
+  combine: false,
+
+  groups: {
+    d3: {
+      modules: {
+        'd3': {
+          'fullpath': '/juju-ui/assets/javascripts/d3.v2.min.js'
+        }
+      }
+    },
+    juju: {
+      modules: {
+        // Primitives
+
+        'svg-layouts': {
+          fullpath: '/juju-ui/assets/javascripts/svg-layouts.js'
+        },
+
+        'reconnecting-websocket': {
+          fullpath: '/juju-ui/assets/javascripts/reconnecting-websocket.js'
+        },
+
+        // Views
+        'juju-view-utils': {
+          fullpath: '/juju-ui/views/utils.js'
+        },
+
+        'juju-charm-panel': {
+          fullpath: '/juju-ui/views/charm-panel.js'
+        },
+
+        'juju-notifications': {
+          fullpath: '/juju-ui/views/notifications.js'
+        },
+
+        'juju-view-environment': {
+          fullpath: '/juju-ui/views/environment.js'
+        },
+
+        'juju-view-service': {
+          fullpath: '/juju-ui/views/service.js'
+        },
+
+        'juju-view-unit': {
+          fullpath: '/juju-ui/views/unit.js'
+        },
+
+        'juju-view-charm-collection': {
+          fullpath: '/juju-ui/views/charm.js'
+        },
+
+        'juju-view-charm': {
+          fullpath: '/juju-ui/views/charm.js'
+        },
+
+        'juju-templates': {
+          fullpath: '/juju-ui/templates.js'
+        },
+
+        'juju-views': {
+          use: [
+            'juju-templates',
+            'juju-notifications',
+            'juju-view-utils',
+            'juju-view-environment',
+            'juju-view-service',
+            'juju-view-unit',
+            'juju-view-charm',
+            'juju-view-charm-collection'
+          ]
+        },
+
+        // Models
+        'juju-endpoints': {
+          fullpath: '/juju-ui/models/endpoints.js'
+        },
+
+        'juju-charm-models': {
+          requires: ['juju-charm-id'],
+          fullpath: '/juju-ui/models/charm.js'
+        },
+
+        'juju-models': {
+          requires: [
+            'model', 'model-list', 'juju-endpoints', 'juju-charm-models'],
+          fullpath: '/juju-ui/models/models.js'
+        },
+
+        // Connectivity
+        'juju-env': {
+          requires: ['reconnecting-websocket'],
+          fullpath: '/juju-ui/store/env.js'
+        },
+
+        'juju-notification-controller': {
+          fullpath: '/juju-ui/store/notifications.js'
+        },
+
+        'juju-charm-store': {
+          requires: ['juju-charm-id'],
+          fullpath: '/juju-ui/store/charm.js'
+        },
+
+        'juju-controllers': {
+          use: ['juju-env', 'juju-charm-store',
+            'juju-notification-controller']
+        },
+
+        // App
+        'juju-gui': {
+          fullpath: '/juju-ui/app.js',
+          requires: [
+            'juju-controllers',
+            'juju-views',
+            'juju-models',
+            'juju-view-charm-search'
+          ]
+        }
+      }
+    }
+  }
+};

=== modified file 'app/modules.js'
--- app/modules.js	2012-11-01 13:30:58 +0000
+++ app/modules.js	2012-11-06 17:56:20 +0000
@@ -1,69 +1,10 @@
 GlobalConfig = {
-  // Uncomment for debug versions of YUI.
-  filter: 'debug',
-  // Uncomment for verbose logging of YUI
   debug: false,
-
-  // Use Rollups
-  combine: false,
+  ignoreRegistered: true,
 
   groups: {
-    d3: {
-      modules: {
-        'd3': {
-          'fullpath': '/juju-ui/assets/javascripts/d3.v2.min.js'
-        }
-      }
-    },
     juju: {
       modules: {
-        // Primitives
-
-        'svg-layouts': {
-          fullpath: '/juju-ui/assets/javascripts/svg-layouts.js'
-        },
-
-        'reconnecting-websocket': {
-          fullpath: '/juju-ui/assets/javascripts/reconnecting-websocket.js'
-        },
-
-        // Views
-        'juju-view-utils': {
-          fullpath: '/juju-ui/views/utils.js'
-        },
-
-        'juju-charm-panel': {
-          fullpath: '/juju-ui/views/charm-panel.js'
-        },
-
-        'juju-notifications': {
-          fullpath: '/juju-ui/views/notifications.js'
-        },
-
-        'juju-view-environment': {
-          fullpath: '/juju-ui/views/environment.js'
-        },
-
-        'juju-view-service': {
-          fullpath: '/juju-ui/views/service.js'
-        },
-
-        'juju-view-unit': {
-          fullpath: '/juju-ui/views/unit.js'
-        },
-
-        'juju-view-charm-collection': {
-          fullpath: '/juju-ui/views/charm.js'
-        },
-
-        'juju-view-charm': {
-          fullpath: '/juju-ui/views/charm.js'
-        },
-
-        'juju-templates': {
-          fullpath: '/juju-ui/templates.js'
-        },
-
         'juju-views': {
           use: [
             'juju-templates',
@@ -77,51 +18,8 @@
           ]
         },
 
-        // Models
-        'juju-endpoints': {
-          fullpath: '/juju-ui/models/endpoints.js'
-        },
-
-        'juju-charm-models': {
-          requires: ['juju-charm-id'],
-          fullpath: '/juju-ui/models/charm.js'
-        },
-
-        'juju-models': {
-          requires: [
-            'model', 'model-list', 'juju-endpoints', 'juju-charm-models'],
-          fullpath: '/juju-ui/models/models.js'
-        },
-
-        // Connectivity
-        'juju-env': {
-          requires: ['reconnecting-websocket'],
-          fullpath: '/juju-ui/store/env.js'
-        },
-
-        'juju-notification-controller': {
-          fullpath: '/juju-ui/store/notifications.js'
-        },
-
-        'juju-charm-store': {
-          requires: ['juju-charm-id'],
-          fullpath: '/juju-ui/store/charm.js'
-        },
-
         'juju-controllers': {
-          use: ['juju-env', 'juju-charm-store',
-            'juju-notification-controller']
-        },
-
-        // App
-        'juju-gui': {
-          fullpath: '/juju-ui/app.js',
-          requires: [
-            'juju-controllers',
-            'juju-views',
-            'juju-models',
-            'juju-view-charm-search'
-          ]
+          use: ['juju-env', 'juju-charm-store', 'juju-notification-controller']
         }
       }
     }

=== modified file 'grunt.js'
--- grunt.js	2012-10-30 11:36:00 +0000
+++ grunt.js	2012-11-06 17:56:20 +0000
@@ -11,7 +11,7 @@
 
         },
         files: {
-          'bin' : 'app/assets/images/*'
+          'bin': 'app/assets/images/*'
         }
       }
     }

=== modified file 'lib/server.js'
--- lib/server.js	2012-10-02 11:29:56 +0000
+++ lib/server.js	2012-11-06 17:56:20 +0000
@@ -5,6 +5,7 @@
     fs = require('fs'),
     path = require('path'),
     config = require('../config').config.server,
+    properties = require('../properties'),
     public_dir = config.public_dir,
     Templates = require('./templates.js'),
     view = require('./view.js');
@@ -46,8 +47,20 @@
   });
 });
 
-server.get('/config.js', function(req, res) {
-  res.sendfile('app/config.js');
+server.get('/source/modules.js', function(req, res) {
+  if (properties.debugMode) {
+    res.sendfile('app/all-app-debug.js');
+  } else {
+    res.sendfile('app/all-app.js');
+  }
+});
+
+server.get('/source/yui.js', function(req, res) {
+  if (properties.debugMode) {
+    res.sendfile('app/assets/javascripts/yui/yui/yui-debug.js');
+  } else {
+    res.sendfile('app/all-yui.js');
+  }
 });
 
 server.get('*', function(req, res) {

=== added file 'merge-files.js'
--- merge-files.js	1970-01-01 00:00:00 +0000
+++ merge-files.js	2012-11-06 17:56:20 +0000
@@ -0,0 +1,165 @@
+'use strict';
+
+// http://stackoverflow.com/questions/5348685/node-js-require-inheritance
+global.YUI = require('yui').YUI;
+
+var fs = require('fs'),
+    syspath = require('path'),
+    compressor = require('node-minify'),
+    modules = {},
+    paths = [];
+
+function minify(file) {
+  var execution = new compressor.minify({
+    type: 'uglifyjs',
+    fileIn: file,
+    fileOut: file,
+    callback: function(err) {
+      if (err) {
+        console.log(err);
+      }
+    }
+  });
+}
+
+function loop(arr, callback) {
+  if (!arr) {
+    return;
+  }
+
+  if (Array.isArray(arr)) {
+    for (var i = 0; i < arr.length; i = i + 1) {
+      callback(arr[i], i);
+    }
+  } else {
+    for (var key in arr) {
+      if (arr.hasOwnProperty(key)) {
+        callback(arr[key], key);
+      }
+    }
+  }
+}
+
+// Reading the 'requires' attribute of all our custom js files
+(function() {
+  function readdir(path) {
+    var file, dirs = [], fileName, files = fs.readdirSync(path);
+
+    loop(files, function(value) {
+      fileName = path + '/' + value;
+      file = fs.statSync(fileName);
+
+      if (file.isFile()) {
+        if (syspath.extname(fileName).toLowerCase() === '.js') {
+          if ('./app/modules-debug.js' === fileName) {
+            console.log('SKIPPING FILE -> ' + fileName);
+          } else {
+            paths.push(fileName);
+          }
+        }
+      } else if (file.isDirectory()) {
+        console.log('DIRECTORY -> ' + fileName);
+        if ('./app/assets' === fileName) {
+          console.log('SKIPPING DIRECTORY -> ' + fileName);
+        } else {
+          dirs.push(fileName);
+        }
+      }
+    });
+
+    // We wrote all the files. Now it is time to read and write the files
+    // inside the children directories.
+    loop(dirs, function(directory) {
+      readdir(directory);
+    });
+  }
+
+  // reading all JS files under './app'
+  readdir('./app');
+  console.log('FILES loaded');
+
+  var originalAdd = YUI.add;
+  // This is a trick to get the 'requires' value from the module definition
+  YUI.add = function(name, fn, version, details) {
+    modules[name] = [];
+    if (details && details.requires) {
+      loop(details.requires, function(value) {
+        modules[name].push(value);
+      });
+    }
+  };
+
+  loop(paths, function(value) {
+    // It triggers the custom 'add' method above
+    require(value);
+  });
+  YUI.add = originalAdd;
+})();
+
+// Getting all the YUI dependencies that we need
+var reqs = (function() {
+  var yuiRequirements = [];
+  loop(modules, function(requires) {
+    loop(requires, function(value) {
+      if (!modules[value]) {
+        // This is not one of our modules but a yui one.
+        if (yuiRequirements.indexOf(value) < 0) {
+          // avoid duplicates
+          yuiRequirements.push(value);
+        }
+      }
+    });
+  });
+  console.log('REQS loaded');
+  return yuiRequirements;
+})();
+
+// Using the example http://yuilibrary.com/yui/docs/yui/loader-resolve.html
+(function() {
+  var Y, loader, out, str = [];
+  Y = YUI();
+  loader = new Y.Loader({
+    base: syspath.join(__dirname, './node_modules/yui/'),
+    ignoreRegistered: true,
+    require: reqs
+  });
+  out = loader.resolve(true);
+  out.js.forEach(function(file) {
+    console.log('file -> ' + file);
+    str.push(fs.readFileSync(file, 'utf8'));
+  });
+  fs.writeFileSync('./app/all-yui.js', str.join('\n'), 'utf8');
+  minify('./app/all-yui.js');
+})();
+
+// Creating the combined file for all the third part js code
+(function() {
+  var strDirectory = './app/assets/javascripts/', str = [];
+  str.push(fs.readFileSync(strDirectory + 'd3.v2.min.js', 'utf8'));
+  str.push(fs.readFileSync(strDirectory + 'reconnecting-websocket.js',
+      'utf8'));
+  str.push(fs.readFileSync(strDirectory + 'svg-layouts.js', 'utf8'));
+  fs.writeFileSync('./app/all-third.js', str.join('\n'), 'utf8');
+  minify('./app/all-third.js');
+})();
+
+//Creating the combined file for the modules-debug.js and config.js files
+(function() {
+
+  var str = [];
+  str.push(fs.readFileSync('./app/modules-debug.js', 'utf8'));
+  str.push(fs.readFileSync('./app/config.js', 'utf8'));
+  fs.writeFileSync('./app/all-app-debug.js', str.join('\n'), 'utf8');
+})();
+
+// Creating the combined file for all our files
+(function() {
+  var str = [];
+  loop(paths, function(file) {
+    console.log('file -> ' + file);
+    str.push(fs.readFileSync(file, 'utf8'));
+  });
+  fs.writeFileSync('./app/all-app.js', str.join('\n'), 'utf8');
+  minify('./app/all-app.js');
+})();
+

=== modified file 'package.json'
--- package.json	2012-10-30 11:33:21 +0000
+++ package.json	2012-11-06 17:56:20 +0000
@@ -30,7 +30,8 @@
     "graceful-fs": "1.1.x",
     "rimraf": "2.0.x",
     "grunt": ">=0.2.0",
-    "node-spritesheet": ">=0.2.3"
+    "node-spritesheet": ">=0.2.3",
+    "node-minify": "0.5.0"
   },
   "optionalDependencies": {}
 }

=== added file 'properties-dev.js'
--- properties-dev.js	1970-01-01 00:00:00 +0000
+++ properties-dev.js	2012-11-06 17:56:20 +0000
@@ -0,0 +1,1 @@
+exports.debugMode = true;

=== added file 'properties-prod.js'
--- properties-prod.js	1970-01-01 00:00:00 +0000
+++ properties-prod.js	2012-11-06 17:56:20 +0000
@@ -0,0 +1,1 @@
+exports.debugMode = false;

=== modified file 'test/index.html'
--- test/index.html	2012-11-01 13:12:28 +0000
+++ test/index.html	2012-11-06 17:56:20 +0000
@@ -4,7 +4,7 @@
   <meta charset="utf-8">
   <link rel="stylesheet" href="assets/mocha.css">
   <script src="../app/assets/javascripts/yui/yui/yui-debug.js"></script>
-  <script src="../app/modules.js"></script>
+  <script src="../app/modules-debug.js"></script>
   <script src="assets/chai.js"></script>
   <script src="assets/mocha.js"></script>
   <script src="utils.js"></script>