Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip early and log when prefix mismatches #96

Merged
merged 2 commits into from
Jan 16, 2017
Merged

Skip early and log when prefix mismatches #96

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3edb69
Skip early and log when prefix mismatches
am11 Jan 15, 2017
c3b12a2
Update readme with build and test instructions
am11 Jan 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
59 changes: 36 additions & 23 deletions middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};

Expand All @@ -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);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The severity level error should not be conditioned by debug check. Besides now the condition to check debug flag for severity debug in moved to the log function.


if (options.error) {
options.error(err);
Expand All @@ -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);
Expand All @@ -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) {
Expand All @@ -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;

Expand Down Expand Up @@ -203,6 +210,8 @@ module.exports = function(options) {
}

fs.writeFile(cssPath, data, 'utf8', function(err) {
log('debug', 'write', cssPath);

if (err) {
error(err);
}
Expand All @@ -222,6 +231,8 @@ module.exports = function(options) {
}

fs.writeFile(sourceMapPath, result.map, 'utf8', function(err) {
log('debug', 'write', sourceMapPath);

if (err) {
error(err);
}
Expand All @@ -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();
}

Expand Down Expand Up @@ -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();
}

Expand All @@ -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');
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that %s here was not expanding, now the fourth parameter is interpolated in the flattened log message.

});
}
changed && changed.length ? compile() : next();
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
},
"dependencies": {
"mkdirp": "^0.5.1",
"node-sass": "^4.1.1"
"node-sass": "^4.3.0"
},
"devDependencies": {
"connect": "^3.5.0",
Expand Down
67 changes: 60 additions & 7 deletions test/middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ describe('Creating middleware', function() {

});


var spawnedServer;

describe('Spawning example server', function() {
Expand All @@ -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();
Expand All @@ -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() {
Expand Down Expand Up @@ -315,7 +369,6 @@ describe('Using middleware to compile .scss', function() {
});
}());
});

});

});
Expand Down