Merge pull request #96 from sass/feature/gh-88-skip-logs

Skip early and log when prefix mismatches
sass · Jan 16, 2017 · 3ecbe9f · 3ecbe9f
2 parents 3ef0775 + c3b12a2
commit 3ecbe9f
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 36 deletions.
diff --git a/README.md b/README.md
@@ -130,15 +130,19 @@ app.use(sassMiddleware({
 app.use(express.static(path.join(__dirname, 'public')));
 ```
 
-## Testing
+## Contributors
 
-    npm install mocha -g
+We <3 our contributors! A special thanks to all those who have clocked in some dev time on this project, we really appreciate your hard work. You can find [a full list of those people here](https://github.com/sass/node-sass-middleware/graphs/contributors).
 
-    mocha test
+### Building and Testing
 
-## Contributors
+```sh
+git clone git@github.com:sass/node-sass-middleware
+cd node-sass-middleware
 
-We <3 our contributors! A special thanks to all those who have clocked in some dev time on this project, we really appreciate your hard work. You can find [a full list of those people here](https://github.com/sass/node-sass-middleware/graphs/contributors).
+npm install
+npm test
+```
 
 ### Note on Patches/Pull Requests
 

diff --git a/middleware.js b/middleware.js
@@ -82,11 +82,17 @@ module.exports = function(options) {
   var maxAge = options.maxAge || 0;
 
   //Allow custom log function or default one
-  var log = options.log || function(severity, key, val) {
+  var log = options.log || function(severity, key, val, text) {
+    if (!debug && severity === 'debug') { // skip debug when debug is off
+      return;
+    }
+
+    text = text || '';
+
     if (typeof (console[severity]) === 'function') {
-      console[severity]('  \x1B[90m%s:\x1B[0m \x1B[36m%s\x1B[0m', key, val);
+      console[severity]('[sass]  \x1B[90m%s:\x1B[0m \x1B[36m%s %s\x1B[0m', key, val, text);
     } else {
-      console.error('  \x1B[90m%s:\x1B[0m \x1B[36m%s\x1B[0m', key, val);
+      console.error('[sass]  \x1B[90m%s:\x1B[0m \x1B[36m%s %s\x1B[0m', key, val, text);
     }
   };
 
@@ -101,9 +107,7 @@ module.exports = function(options) {
 
     // This function will be called if something goes wrong
     var error = function(err, errorMessage) {
-      if (debug) {
-        log('error', 'error', errorMessage || err);
-      }
+      log('error', 'error', errorMessage || err);
 
       if (options.error) {
         options.error(err);
@@ -117,14 +121,21 @@ module.exports = function(options) {
     }
 
     var path = url.parse(req.url).pathname;
-    if (options.prefix && 0 === path.indexOf(options.prefix)) {
-      path = path.substring(options.prefix.length);
-    }
 
     if (!/\.css$/.test(path)) {
+      log('debug', 'skip', path, 'nothing to do');
       return next();
     }
 
+    if (options.prefix) {
+      if (0 === path.indexOf(options.prefix)) {
+        path = path.substring(options.prefix.length);
+      } else {
+        log('debug', 'skip', path, 'prefix mismatch');
+        return next();
+      }
+    }
+
     var cssPath = join(dest, path),
         sassPath = join(src, path.replace(/\.css$/, sassExtension)),
         sassDir = dirname(sassPath);
@@ -137,10 +148,8 @@ module.exports = function(options) {
       sassDir = dirname(sassPath);
     }
 
-    if (debug) {
-      log('debug', 'source', sassPath);
-      log('debug', 'dest', options.response ? '<response>' : cssPath);
-    }
+    log('debug', 'source', sassPath);
+    log('debug', 'dest', options.response ? '<response>' : cssPath);
 
     // When render is done, respond to the request accordingly
     var done = function(err, result) {
@@ -159,12 +168,10 @@ module.exports = function(options) {
 
       var data = result.css;
 
-      if (debug) {
-        log('debug', 'render', options.response ? '<response>' : sassPath);
+      log('debug', 'render', options.response ? '<response>' : sassPath);
 
-        if (sourceMap) {
-          log('debug', 'render', this.options.sourceMap);
-        }
+      if (sourceMap) {
+        log('debug', 'render', this.options.sourceMap);
       }
       imports[sassPath] = result.stats.includedFiles;
 
@@ -203,6 +210,8 @@ module.exports = function(options) {
         }
 
         fs.writeFile(cssPath, data, 'utf8', function(err) {
+          log('debug', 'write', cssPath);
+
           if (err) {
             error(err);
           }
@@ -222,6 +231,8 @@ module.exports = function(options) {
           }
 
           fs.writeFile(sourceMapPath, result.map, 'utf8', function(err) {
+            log('debug', 'write', sourceMapPath);
+
             if (err) {
               error(err);
             }
@@ -235,10 +246,11 @@ module.exports = function(options) {
 
     // Compile to cssPath
     var compile = function() {
-      if (debug) { log('debug', 'read', cssPath); }
-
       fs.exists(sassPath, function(exists) {
+        log('debug', 'read', sassPath);
+
         if (!exists) {
+          log('debug', 'skip', sassPath, 'does not exist');
           return next();
         }
 
@@ -270,13 +282,14 @@ module.exports = function(options) {
     // Compare mtimes
     fs.stat(sassPath, function(err, sassStats) {
       if (err) { // sassPath can't be accessed, nothing to compile
+        log('debug', 'skip', sassPath, 'is unreadable');
         return next();
       }
 
       fs.stat(cssPath, function(err, cssStats) {
         if (err) {
           if ('ENOENT' === err.code) { // CSS has not been compiled, compile it!
-            if (debug) { log('debug', 'not found', cssPath); }
+            log('debug', 'compile', cssPath, 'was not found');
             return compile();
           }
 
@@ -285,15 +298,15 @@ module.exports = function(options) {
         }
 
         if (sassStats.mtime > cssStats.mtime) { // Source has changed, compile it
-          if (debug) { log('debug', 'modified', cssPath); }
+          log('debug', 'compile', sassPath, 'was modified');
           return compile();
         }
 
         // Already compiled, check imports
         checkImports(sassPath, cssStats.mtime, function(changed) {
           if (debug && changed && changed.length) {
             changed.forEach(function(path) {
-              log('debug', 'modified import %s', path);
+              log('debug', 'compile', path, '(import file) was modified');
             });
           }
           changed && changed.length ? compile() : next();

diff --git a/package.json b/package.json
@@ -31,7 +31,7 @@
   },
   "dependencies": {
     "mkdirp": "^0.5.1",
-    "node-sass": "^4.1.1"
+    "node-sass": "^4.3.0"
   },
   "devDependencies": {
     "connect": "^3.5.0",

diff --git a/test/middleware.js b/test/middleware.js
@@ -33,7 +33,6 @@ describe('Creating middleware', function() {
 
 });
 
-
 var spawnedServer;
 
 describe('Spawning example server', function() {
@@ -55,7 +54,7 @@ describe('Spawning example server', function() {
 
 describe('Log messages', function() {
   it('should use the default logger when none provided', function(done) {
-    var expected = '  \u001b[90msource:\u001b[0m \u001b[36m' + index_scssFile + '\u001b[0m';
+    var expected = '[sass]  \u001b[90msource:\u001b[0m \u001b[36m' + index_scssFile + ' \u001b[0m';
 
     http.request({ method: 'GET', host: 'localhost', port: process.env.PORT || '8000', path: '/index.css' })
         .end();
@@ -67,26 +66,81 @@ describe('Log messages', function() {
   });
 
   it('should use the provided custom logger', function(done) {
-    var token = null;
+    var loggerArguments;
 
     var server = connect()
       .use(middleware({
         src: fixture(),
         dest: fixture(),
         debug: true,
-        log: function(severity) {
-          token = severity;
+        log: function() {
+          loggerArguments = arguments;
         }
       }));
 
     request(server)
       .get('/index.css')
       .expect(200, function() {
         fs.unlink(index_cssFile);
-        token.should.equal('debug');
+        loggerArguments[0].should.equal('debug');
+        done();
+      });
+  });
+
+  it('should skip fast when requested path is missing the prefix', function(done) {
+    this.timeout(this.timeout() + 500);
+
+    var loggerArguments;
+    var dest = '/some/static-css/directory/file.css';
+
+    var server = connect()
+      .use(middleware({
+        src: fixture(),
+        dest: fixture(),
+        debug: true,
+        prefix: '/foo/bar',
+        log: function() {
+          loggerArguments = arguments;
+        }
+      }));
+
+    request(server)
+      .get(dest)
+      .expect(200, function() {
+        loggerArguments[1].should.equal('skip');
+        loggerArguments[2].should.equal(dest);
+        loggerArguments[3].should.equal('prefix mismatch');
         done();
       });
   });
+
+  it('should skip when requested path is not suffixed by css', function(done) {
+    this.timeout(this.timeout() + 500);
+
+    var loggerArguments;
+    var dest = '/assets/file.mp4';
+
+    var server = connect()
+      .use(middleware({
+        src: fixture(),
+        dest: fixture(),
+        debug: true,
+        prefix: '/foo/bar',
+        log: function() {
+          loggerArguments = arguments;
+        }
+      }));
+
+    request(server)
+      .get(dest)
+      .expect(200, function() {
+        loggerArguments[1].should.equal('skip');
+        loggerArguments[2].should.equal(dest);
+        loggerArguments[3].should.equal('nothing to do');
+        done();
+      });
+  });
+
 });
 
 function setupBeforeEach() {
@@ -315,7 +369,6 @@ describe('Using middleware to compile .scss', function() {
             });
           }());
         });
-
     });
 
   });