Skip to content

Commit

Permalink
Check mtime of common package manager files
Browse files Browse the repository at this point in the history
On every invocation, check the modified timestamp of the common package
manager files `package.json`, `package-lock.json` and `yarn.lock`. The
newest timestamp is compared with the last eslint execution time. If a
modification occurred in the meantime, the cached instance of eslint is
discarded and a new one is created.
  • Loading branch information
mantoni committed Aug 24, 2018
1 parent 8d2b8fc commit 0176ec5
Show file tree
Hide file tree
Showing 4 changed files with 196 additions and 20 deletions.
31 changes: 24 additions & 7 deletions lib/linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function fail(message) {
return `${message}\n# exit 1`;
}

function resolveModules(cwd) {
function createCache(cwd) {
let eslintPath;
try {
eslintPath = resolve.sync('eslint', { basedir: cwd });
Expand All @@ -52,17 +52,34 @@ function resolveModules(cwd) {
});
}

exports.lint = function (cwd, args, text) {
function clearRequireCache(cwd) {
Object.keys(require.cache)
.filter(key => key.startsWith(cwd))
.forEach((key) => {
delete require.cache[key];
});
}

exports.lint = function (cwd, args, text, mtime) {
process.chdir(cwd);
const cwdDeps = eslintCache.get(cwd) || resolveModules(cwd);

let cache = eslintCache.get(cwd);
if (!cache) {
cache = createCache(cwd);
} else if (mtime > cache.last_run) {
clearRequireCache(cwd);
cache = createCache(cwd);
}
cache.last_run = Date.now();

const currentOptions = options.parse([0, 0].concat(args));
cwdDeps.chalk.enabled = currentOptions.color;
cache.chalk.enabled = currentOptions.color;
const files = currentOptions._;
const stdin = currentOptions.stdin;
if (!files.length && (!stdin || typeof text !== 'string')) {
return `${options.generateHelp()}\n`;
}
const engine = new cwdDeps.eslint.CLIEngine(
const engine = new cache.eslint.CLIEngine(
translateOptions(currentOptions, cwd)
);
if (currentOptions.printConfig) {
Expand Down Expand Up @@ -93,10 +110,10 @@ exports.lint = function (cwd, args, text) {
return (report.results[0] && report.results[0].output) || text;
}
if (currentOptions.fix) {
cwdDeps.eslint.CLIEngine.outputFixes(report);
cache.eslint.CLIEngine.outputFixes(report);
}
if (currentOptions.quiet) {
report.results = cwdDeps.eslint.CLIEngine.getErrorResults(report.results);
report.results = cache.eslint.CLIEngine.getErrorResults(report.results);
}
const format = currentOptions.format;
const formatter = engine.getFormatter(format);
Expand Down
48 changes: 37 additions & 11 deletions lib/server.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
'use strict';

const crypto = require('crypto');
const fs = require('fs');
const net = require('net');
const crypto = require('crypto');
const portfile = require('./portfile');
const linter = require('./linter');

Expand Down Expand Up @@ -39,6 +40,13 @@ function parseData(data) {
};
}

const stat_files = [
'package.json',
'package-lock.json',
'npm-shrinkwrap.json',
'yarn.lock'
];

exports.start = function () {

const token = crypto.randomBytes(8).toString('hex');
Expand All @@ -47,7 +55,25 @@ exports.start = function () {
const server = net.createServer({
allowHalfOpen: true
}, (con) => {
let latch = stat_files.length + 1;
let mtime = 0;
let data = '';

const decrementLatch = () => {
if (--latch === 0) {
const { cwd, args, text } = parseData(data);
let result;
try {
result = linter.lint(cwd, args, text, mtime);
} catch (e) {
fail(con, String(e));
return;
}
con.write(result);
con.end();
}
};

con.on('data', (chunk) => {
data += chunk;
});
Expand All @@ -73,17 +99,17 @@ exports.start = function () {
con.end(linter.getStatus());
return;
}
const { cwd, args, text } = parseData(data);
let result;
try {
result = linter.lint(cwd, args, text);
} catch (e) {
fail(con, e.toString());
return;
}
con.write(result);
con.end();
decrementLatch();
});
const onStat = (err, stat) => {
if (!err) {
mtime = Math.max(mtime, stat.mtimeMs);
}
decrementLatch();
};
for (const stat_file of stat_files) {
fs.stat(stat_file, onStat);
}
});

server.on('connection', (con) => {
Expand Down
59 changes: 57 additions & 2 deletions test/linter-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
'use strict';

const path = require('path');
const resolve = require('resolve');
const { assert, refute, sinon } = require('@sinonjs/referee-sinon');
const linter = require('../lib/linter');

Expand All @@ -19,17 +20,71 @@ describe('linter', () => {

describe('instance caching', () => {

beforeEach(() => {
sinon.spy(resolve, 'sync');
});

it('reuses instance from cache', () => {
linter.lint(cwd, ['--stdin'], '\'use strict\';');
linter.lint(cwd, ['--stdin'], '\'use strict\';', 1234);
const cache1 = linter.cache.get(cwd);
linter.lint(cwd, ['--stdin'], '\'use strict\';', 1234);
const cache2 = linter.cache.get(cwd);

assert.equals(linter.cache.length, 1);
assert.same(cache1, cache2, 'Cache recreated');
assert.same(cache1.eslint, cache2.eslint);
assert.calledTwice(resolve.sync);
assert.calledWithMatch(resolve.sync, 'eslint', { basedir: cwd });
assert.calledWith(resolve.sync, 'chalk');
});

it('uses new instance for different directory', () => {
const cwd2 = path.join(cwd, 'test');
linter.lint(cwd, ['--stdin'], '\'use strict\';');
linter.lint(path.join(cwd, 'test'), ['--stdin'], '\'use strict\';');
linter.lint(cwd2, ['--stdin'], '\'use strict\';');

assert.equals(linter.cache.length, 2);
assert.callCount(resolve.sync, 4);
assert.calledWithMatch(resolve.sync, 'eslint', { basedir: cwd });
assert.calledWithMatch(resolve.sync, 'eslint', { basedir: cwd2 });
});

it('creates new instance if mtime is larger than first call', () => {
const now = Date.now();
const clock = sinon.useFakeTimers(now);
linter.lint(cwd, ['--stdin'], '\'use strict\';', now - 1000);
const cache1 = linter.cache.get(cwd);

clock.tick(1000);
linter.lint(cwd, ['--stdin'], '\'use strict\';', now + 500);
const cache2 = linter.cache.get(cwd);

assert.equals(linter.cache.length, 1);
refute.same(cache1, cache2);
refute.same(cache1.eslint, cache2.eslint, 'require.cache cleared');
assert.callCount(resolve.sync, 4);
});

it('does not create new instance if mtime is lower than last call', () => {
const now = Date.now();
const clock = sinon.useFakeTimers(now);
linter.lint(cwd, ['--stdin'], '\'use strict\';', now - 1000);
const cache1 = linter.cache.get(cwd);

clock.tick(1000);
linter.lint(cwd, ['--stdin'], '\'use strict\';', now - 1000);
const cache2 = linter.cache.get(cwd);

clock.tick(1000);
// Newer than initial timestamp, but older than last run. Verifies the
// timestamp in the cache was renewed.
linter.lint(cwd, ['--stdin'], '\'use strict\';', now + 500);
const cache3 = linter.cache.get(cwd);

assert.equals(linter.cache.length, 1);
assert.same(cache1, cache2);
assert.same(cache2, cache3);
assert.callCount(resolve.sync, 2);
});

});
Expand Down
78 changes: 78 additions & 0 deletions test/server-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/*eslint-env mocha*/
'use strict';

const fs = require('fs');
const net = require('net');
const crypto = require('crypto');
const EventEmitter = require('events');
Expand Down Expand Up @@ -75,6 +76,7 @@ describe('server', () => {
});

it('closes connection without writing anything for empty request', () => {
sinon.replace(fs, 'stat', sinon.fake());
start();

request(connection);
Expand All @@ -85,6 +87,10 @@ describe('server', () => {

describe('stop', () => {

beforeEach(() => {
sinon.replace(fs, 'stat', sinon.fake());
});

it('closes connection and server for "stop" command', () => {
start();

Expand Down Expand Up @@ -139,10 +145,23 @@ describe('server', () => {
refute.called(net_server.close);
});

it('stops server without waiting for stat calls', () => {
start();

request(connection, `${token} stop`);

assert.calledOnce(net_server.close);
assert.calledOnce(connection.end);
});

});

describe('status', () => {

beforeEach(() => {
sinon.replace(fs, 'stat', sinon.fake());
});

it('prints linter status and closes connection', () => {
sinon.replace(linter, 'getStatus', sinon.fake.returns('Oh, hi!\n'));
start();
Expand All @@ -164,6 +183,16 @@ describe('server', () => {
refute.called(linter.getStatus);
});

it('gets "status" without waiting for stat calls', () => {
sinon.replace(linter, 'getStatus', sinon.fake.returns('Yes yo!'));
start();

request(connection, `${token} status`);

assert.calledOnce(linter.getStatus);
assert.calledOnceWith(connection.end, 'Yes yo!');
});

});

describe('lint', () => {
Expand All @@ -174,6 +203,7 @@ describe('server', () => {
};

it('invokes linter with JSON arguments', () => {
sinon.replace(fs, 'stat', sinon.fake.yields(new Error()));
sinon.replace(linter, 'lint', sinon.fake.returns('Oh, hi!\n'));
start();

Expand All @@ -185,6 +215,7 @@ describe('server', () => {
});

it('invokes linter with plain text arguments', () => {
sinon.replace(fs, 'stat', sinon.fake.yields(new Error()));
sinon.replace(linter, 'lint', sinon.fake.returns('Oh, hi!\n'));
start();

Expand All @@ -197,6 +228,7 @@ describe('server', () => {
});

it('handles exception from linter', () => {
sinon.replace(fs, 'stat', sinon.fake.yields(new Error()));
sinon.replace(linter, 'lint', sinon.fake.throws(new Error('Whatever')));
start();

Expand All @@ -206,15 +238,61 @@ describe('server', () => {
});

it('does not throw if connection died after exception from linter', () => {
sinon.replace(fs, 'stat', sinon.fake.yields(new Error()));
sinon.replace(linter, 'lint', sinon.fake.throws(new Error('Whatever')));
connection.end = sinon.fake.throws(new Error('Oh dear!'));
start();

refute.exception(() => {
request(connection, `${token} ${JSON.stringify(json)}`);
});
assert.calledOnce(connection.end); // Verify actually called
});

it('stats common package manager files on connect', () => {
sinon.replace(fs, 'stat', sinon.fake());
sinon.replace(linter, 'lint', sinon.fake());
start();

request(connection, `${token} ${JSON.stringify(json)}`);

assert.calledWith(fs.stat, 'package.json');
assert.calledWith(fs.stat, 'package-lock.json');
assert.calledWith(fs.stat, 'npm-shrinkwrap.json');
assert.calledWith(fs.stat, 'yarn.lock');
});

it('does not lint until stat calls yield', () => {
sinon.replace(fs, 'stat', sinon.fake());
sinon.replace(linter, 'lint', sinon.fake());
start();

request(connection, `${token} ${JSON.stringify(json)}`);

refute.called(linter.lint);

fs.stat.getCall(0).callback(new Error());
fs.stat.getCall(1).callback(new Error());
fs.stat.getCall(2).callback(new Error());
fs.stat.getCall(3).callback(new Error());

assert.calledOnce(linter.lint);
});

it('passes largest mtime value to linter', () => {
sinon.replace(fs, 'stat', sinon.fake());
sinon.replace(linter, 'lint', sinon.fake());
start();

request(connection, `${token} ${JSON.stringify(json)}`);

fs.stat.getCall(0).callback(null, { mtimeMs: 7 });
fs.stat.getCall(1).callback(null, { mtimeMs: 42 });
fs.stat.getCall(2).callback(null, { mtimeMs: 2 });
fs.stat.getCall(3).callback(null, { mtimeMs: 3 });

assert.calledOnceWith(linter.lint, json.cwd, json.args, json.text, 42);
});
});

});

0 comments on commit 0176ec5

Please sign in to comment.