Asset amends (#8294)

refs #8221 🔥 Remove ghost=true concept from asset url helper ✨ 💯 Introduce CSS minification with cssnano - add new grunt-cssnano dependency - wire up grunt task to minify public/ghost.css 🎨 Rename minification config & hash params - Change minifyInProduction -> hasMinFile - this means this asset should have a .min file available - Change minifyAssets -> useMinFiles - this means that in this env we want to serve .min files if available 🎨 Update public/ghost.css to serve .min for prod - add the new `hasMinFile` property 🎨 Move minified asset handling to asset_url util - this logic should be in the util, not the asset helper - updated tests 📖 Error handler always needs asset helper - this removes the TODO and adds a more sensible comment - we also need to update our theme documentation around error templates 🔥 Don't use asset helper in ghost head - use getAssetUrl util instead! - removed TODO 📖 Update proxy docs 🎨 Simplify asset helper & add tests - this refactor is a step prior to moving this from metadata to being a url util - needed to skip some new tests 🐛 Add missing handler for css file
TryGhost · Apr 10, 2017 · a413d70 · a413d70
1 parent eb0cfb7
commit a413d70
Show file tree

Hide file tree

Showing 19 changed files with 719 additions and 167 deletions.
diff --git a/.gitignore b/.gitignore
@@ -69,6 +69,7 @@ config.*.json
 /core/built
 /core/server/admin/views/*
 /core/server/public/ghost-url.min.js
+/core/server/public/ghost.min.css
 
 # Coverage reports
 coverage.html
diff --git a/Gruntfile.js b/Gruntfile.js
@@ -323,6 +323,17 @@ var overrides      = require('./core/server/overrides'),
                 }
             },
 
+            cssnano: {
+                prod: {
+                    options: {
+                        sourcemap: false
+                    },
+                    files: {
+                        'core/server/public/ghost.min.css': 'core/server/public/ghost.css'
+                    }
+                }
+            },
+
             // ### grunt-subgrunt
             // Run grunt tasks in submodule Gruntfiles
             subgrunt: {
@@ -662,7 +673,7 @@ var overrides      = require('./core/server/overrides'),
         //
         // It is otherwise the same as running `grunt`, but is only used when running Ghost in the `production` env.
         grunt.registerTask('prod', 'Build JS & templates for production',
-            ['subgrunt:prod', 'uglify:prod', 'master-warn']);
+            ['subgrunt:prod', 'uglify:prod', 'cssnano:prod', 'master-warn']);
 
         // ### Live reload
         // `grunt dev` - build assets on the fly whilst developing

diff --git a/core/server/apps/private-blogging/lib/views/private.hbs b/core/server/apps/private-blogging/lib/views/private.hbs
@@ -15,7 +15,7 @@
         <link rel="shortcut icon" href="{{asset "favicon.ico"}}">
         <meta http-equiv="cleartype" content="on">
 
-        <link rel="stylesheet" href="{{asset "public/ghost.css"}}"/>
+        <link rel="stylesheet" href="{{asset "public/ghost.css" hasMinFile="true"}}"/>
     </head>
     <body>
         <div class="gh-app">

diff --git a/core/server/apps/subscribers/lib/views/subscribe.hbs b/core/server/apps/subscribers/lib/views/subscribe.hbs
@@ -15,7 +15,7 @@
     <link rel="shortcut icon" href="{{asset "favicon.ico"}}">
     <meta http-equiv="cleartype" content="on">
 
-    <link rel="stylesheet" href="{{asset "public/ghost.css"}}"/>
+    <link rel="stylesheet" href="{{asset "public/ghost.css" hasMinFile="true"}}"/>
 </head>
 <body>
 <div class="gh-app">

diff --git a/core/server/blog/app.js b/core/server/blog/app.js
@@ -53,6 +53,7 @@ module.exports = function setupBlogApp() {
 
     // Serve stylesheets for default templates
     blogApp.use(servePublicFile('public/ghost.css', 'text/css', utils.ONE_HOUR_S));
+    blogApp.use(servePublicFile('public/ghost.min.css', 'text/css', utils.ONE_HOUR_S));
 
     // Serve images for default templates
     blogApp.use(servePublicFile('public/404-ghost@2x.png', 'png', utils.ONE_HOUR_S));

diff --git a/core/server/config/defaults.json b/core/server/config/defaults.json
@@ -5,6 +5,8 @@
         "port": 2368
     },
     "privacy": false,
+    "useMinFiles": true,
+    "printErrorStack": false,
     "paths": {
         "contentPath": "content/"
     },

diff --git a/core/server/config/env/config.development.json b/core/server/config/env/config.development.json
@@ -17,7 +17,7 @@
         "useRpcPing": false,
         "useUpdateCheck": true
     },
-    "minifyAssets": false,
+    "useMinFiles": false,
     "printErrorStack": true,
     "caching": {
         "theme": {

diff --git a/core/server/config/env/config.testing-mysql.json b/core/server/config/env/config.testing-mysql.json
@@ -53,5 +53,5 @@
         "useTinfoil": true,
         "useStructuredData": true
     },
-    "minifyAssets": false
+    "useMinFiles": false
 }
diff --git a/core/server/config/env/config.testing.json b/core/server/config/env/config.testing.json
@@ -52,5 +52,5 @@
         "useTinfoil": true,
         "useStructuredData": true
     },
-    "minifyAssets": false
+    "useMinFiles": false
 }
diff --git a/core/server/data/meta/asset_url.js b/core/server/data/meta/asset_url.js
@@ -2,50 +2,51 @@ var config = require('../../config'),
     settingsCache = require('../../settings/cache'),
     utils = require('../../utils');
 
-function getAssetUrl(path, isAdmin, minify) {
-    var output = '';
+/**
+ * Serve either uploaded favicon or default
+ * @return {string}
+ */
+function getFaviconUrl() {
+    if (settingsCache.get('icon')) {
+        return utils.url.urlJoin(utils.url.getSubdir(), utils.url.urlFor('image', {image: settingsCache.get('icon')}));
+    }
+
+    return utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
+}
 
-    output += utils.url.urlJoin(utils.url.getSubdir(), '/');
+function getAssetUrl(path, hasMinFile) {
+    // CASE: favicon - this is special path with its own functionality
+    if (path.match(/\/?favicon\.(ico|png)$/)) {
+        // @TODO, resolve this - we should only be resolving subdirectory and extension.
+        return getFaviconUrl();
+    }
 
-    if (!path.match(/^favicon\.(ico|png)$/) && !path.match(/^public/) && !path.match(/^asset/)) {
-        if (isAdmin) {
-            output = utils.url.urlJoin(output, 'ghost/');
-        }
+    // CASE: Build the output URL
+    // Add subdirectory...
+    var output = utils.url.urlJoin(utils.url.getSubdir(), '/');
 
+    // Optionally add /assets/
+    if (!path.match(/^public/) && !path.match(/^asset/)) {
         output = utils.url.urlJoin(output, 'assets/');
     }
-    // Serve either uploaded favicon or default
-    // for favicon, we don't care anymore about the `/` leading slash, as we don't support theme favicons
-    if (path.match(/\/?favicon\.(ico|png)$/)) {
-        if (isAdmin) {
-            output = utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
-        } else {
-            if (settingsCache.get('icon')) {
-                output = utils.url.urlJoin(utils.url.getSubdir(), utils.url.urlFor('image', {image: settingsCache.get('icon')}));
-            } else {
-                output = utils.url.urlJoin(utils.url.getSubdir(), '/favicon.ico');
-            }
-        }
-    }
-    // Get rid of any leading slash on the path
-    path = path.replace(/^\//, '');
 
-    // replace ".foo" with ".min.foo" in production
-    if (minify) {
+    // replace ".foo" with ".min.foo" if configured
+    if (hasMinFile && config.get('useMinFiles') !== false) {
         path = path.replace(/\.([^\.]*)$/, '.min.$1');
     }
 
-    if (!path.match(/^favicon\.(ico|png)$/)) {
-        // we don't want to concat the path with our favicon url
-        output += path;
+    // Add the path for the requested asset
+    output = utils.url.urlJoin(output, path);
 
-        if (!config.get('assetHash')) {
-            config.set('assetHash', utils.generateAssetHash());
-        }
-
-        output = output + '?v=' + config.get('assetHash');
+    // Ensure we have an assetHash
+    // @TODO rework this!
+    if (!config.get('assetHash')) {
+        config.set('assetHash', utils.generateAssetHash());
     }
 
+    // Finally add the asset hash to the output URL
+    output += '?v=' + config.get('assetHash');
+
     return output;
 }
 

diff --git a/core/server/helpers/asset.js b/core/server/helpers/asset.js
@@ -3,24 +3,14 @@
 //
 // Returns the path to the specified asset. The ghost flag outputs the asset path for the Ghost admin
 var proxy = require('./proxy'),
-    config = proxy.config,
+    _ = require('lodash'),
     getAssetUrl = proxy.metaData.getAssetUrl,
     SafeString = proxy.SafeString;
 
 module.exports = function asset(path, options) {
-    var isAdmin,
-        minify;
-
-    if (options && options.hash) {
-        isAdmin = options.hash.ghost;
-        minify = options.hash.minifyInProduction;
-    }
-
-    if (config.get('minifyAssets') === false) {
-        minify = false;
-    }
+    var hasMinFile = _.get(options, 'hash.hasMinFile');
 
     return new SafeString(
-        getAssetUrl(path, isAdmin, minify)
+        getAssetUrl(path, hasMinFile)
     );
 };
diff --git a/core/server/helpers/ghost_head.js b/core/server/helpers/ghost_head.js
@@ -11,6 +11,7 @@ var proxy = require('./proxy'),
     Promise = require('bluebird'),
 
     getMetaData = proxy.metaData.get,
+    getAssetUrl = proxy.metaData.getAssetUrl,
     escapeExpression = proxy.escapeExpression,
     SafeString = proxy.SafeString,
     filters = proxy.filters,
@@ -66,12 +67,9 @@ function finaliseStructuredData(metaData) {
 }
 
 function getAjaxHelper(clientId, clientSecret) {
-    // @TODO: swap this for a direct utility, rather than using the helper? see #8221
-    // Note: this is here because the asset helper isn't registered when this file is first loaded
-    var assetHelper = proxy.hbs.handlebars.helpers.asset;
-
     return '<script type="text/javascript" src="' +
-        assetHelper('public/ghost-url.js', {hash: {minifyInProduction: true}}) + '"></script>\n' +
+        getAssetUrl('public/ghost-url.js', true) +
+        '"></script>\n' +
         '<script type="text/javascript">\n' +
         'ghost.init({\n' +
         '\tclientId: "' + clientId + '",\n' +

diff --git a/core/server/helpers/proxy.js b/core/server/helpers/proxy.js
@@ -33,7 +33,6 @@ module.exports = {
 
     // Config!
     // Keys used:
-    // minifyAssets in asset helper
     // isPrivacyDisabled & referrerPolicy used in ghost_head
     // Subscribe app uses routeKeywords
     config: {

diff --git a/core/server/middleware/error-handler.js b/core/server/middleware/error-handler.js
@@ -8,9 +8,12 @@ var _ = require('lodash'),
     _private = {},
     errorHandler = {};
 
+/**
+ * This is a bare minimum setup, which allows us to render the error page
+ * It uses the {{asset}} helper, and nothing more
+ */
 _private.createHbsEngine = function createHbsEngine() {
     var engine = hbs.create();
-    // @TODO get rid of this after #8126
     engine.registerHelper('asset', require('../helpers/asset'));
 
     return engine.express4();

diff --git a/core/server/views/error.hbs b/core/server/views/error.hbs
@@ -15,7 +15,7 @@
     <link rel="shortcut icon" href="{{asset "favicon.ico"}}">
     <meta http-equiv="cleartype" content="on">
 
-    <link rel="stylesheet" href="{{asset "public/ghost.css"}}"/>
+    <link rel="stylesheet" href="{{asset "public/ghost.css" hasMinFile="true"}}"/>
 
   </head>
   <body>