Skip to content

Commit

Permalink
fix: Change timing of loader requires to avoid duplicate logging
Browse files Browse the repository at this point in the history
  • Loading branch information
sttk authored and phated committed Nov 22, 2021
1 parent 3c7995a commit 4a6ad90
Show file tree
Hide file tree
Showing 4 changed files with 110 additions and 42 deletions.
16 changes: 2 additions & 14 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ var silentRequire = require('./lib/silent_require');
var buildConfigName = require('./lib/build_config_name');
var registerLoader = require('./lib/register_loader');
var getNodeFlags = require('./lib/get_node_flags');

var prepareConfig = require('./lib/prepare_config');

function Liftoff(opts) {
EE.call(this);
Expand Down Expand Up @@ -116,19 +116,8 @@ Liftoff.prototype.buildEnvironment = function(opts) {
}
}

// load any modules which were requested to be required
if (preload.length) {
// unique results first
preload.filter(function(value, index, self) {
return self.indexOf(value) === index;
}).forEach(function(dep) {
this.requireLocal(dep, findCwd(opts));
}, this);
}

var exts = this.extensions;
var eventEmitter = this;
registerLoader(eventEmitter, exts, configPath, cwd);

var configFiles = {};
if (isPlainObject(this.configFiles)) {
Expand Down Expand Up @@ -201,12 +190,11 @@ Liftoff.prototype.launch = function(opts, fn) {
this.emit('respawn', execArgv, child);
}
if (ready) {
prepareConfig(this, env, opts);
fn.call(this, env, argv);
}
}
}.bind(this));
};



module.exports = Liftoff;
17 changes: 17 additions & 0 deletions lib/prepare_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
var registerLoader = require('./register_loader');
var findCwd = require('./find_cwd');

function prepareConfig(self, env, opts) {
var basedir = findCwd(opts);
env.require.filter(toUnique).forEach(function(module) {
self.requireLocal(module, basedir);
});

registerLoader(self, self.extensions, env.configPath, env.cwd);
}

function toUnique(elem, index, array) {
return array.indexOf(elem) === index;
}

module.exports = prepareConfig;
21 changes: 21 additions & 0 deletions test/fixtures/respawn_and_require.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const Liftoff = require('../..');

const Test = new Liftoff({
name: 'test',
v8flags: ['--harmony'],
});

Test.on('respawn', function(flags, proc) {
console.log('saw respawn', flags);
});

Test.on('require', function(name) {
console.log('require', name);
});

Test.launch({
require: 'coffeescript/register',
forcedFlags: ['--lazy'],
}, function(env, argv) {
console.log('execute');
});
98 changes: 70 additions & 28 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,34 +33,6 @@ describe('Liftoff', function() {

describe('buildEnvironment', function() {

it('should attempt pre-loading local modules if they are requested', function() {
app.on('require', function(moduleName, module) {
expect(moduleName).to.equal('coffee-script/register');
expect(module).to.equal(require('coffee-script/register'));
});
var env = app.buildEnvironment({ require: ['coffee-script/register'] });
expect(env.require).to.deep.equal(['coffee-script/register']);
});

it('should attempt pre-loading a local module if it is requested', function() {
app.on('require', function(moduleName, module) {
expect(moduleName).to.equal('coffee-script/register');
expect(module).to.equal(require('coffee-script/register'));
});
var env = app.buildEnvironment({ require: 'coffee-script/register' });
expect(env.require).to.deep.equal(['coffee-script/register']);
});

it('should attempt pre-loading local modules based on extension option', function() {
app.on('require', function(moduleName, module) {
expect(moduleName).to.equal('coffee-script/register');
expect(module).to.equal(require('coffee-script/register'));
});
app.buildEnvironment({
configPath: 'test/fixtures/coffee/mochafile.coffee',
});
});

it('should locate local module using cwd if no config is found', function() {
var test = new Liftoff({ name: 'chai' });
var cwd = 'explicit/cwd';
Expand Down Expand Up @@ -310,6 +282,76 @@ describe('Liftoff', function() {

describe('requireLocal', function() {

it('should attempt pre-loading local modules if they are requested', function(done) {
var app = new Liftoff({ name: 'test' });
var logs = [];
app.on('require', function(moduleName, module) {
expect(moduleName).to.equal('coffeescript/register');
expect(module).to.equal(require('coffeescript/register'));
logs.push('require');
});
app.on('requireFail', function(moduleName, err) {
done(err);
});
app.launch({ require: ['coffeescript/register'] }, function(env) {
expect(env.require).to.deep.equal(['coffeescript/register']);
expect(logs).to.deep.equal(['require']);
done();
});
});

it('should attempt pre-loading a local module if it is requested', function(done) {
var app = new Liftoff({ name: 'test' });
var logs = [];
app.on('require', function(moduleName, module) {
expect(moduleName).to.equal('coffeescript/register');
expect(module).to.equal(require('coffeescript/register'));
logs.push('require');
});
app.on('requireFail', function(moduleName, err) {
done(err);
});
app.launch({ require: 'coffeescript/register' }, function(env) {
expect(env.require).to.deep.equal(['coffeescript/register']);
expect(logs).to.deep.equal(['require']);
done();
});
});

it('should attempt pre-loading local modules but fail', function(done) {
var app = new Liftoff({ name: 'test' });
var logs = [];
app.on('require', function(/* moduleName, module */) {
done();
});
app.on('requireFail', function(moduleName, err) {
expect(moduleName).to.equal('badmodule');
expect(err).to.not.equal(null);
logs.push('requireFail');
});
app.launch({ require: 'badmodule' }, function(env) {
expect(env.require).to.deep.equal(['badmodule']);
expect(logs).to.deep.equal(['requireFail']);
done();
});
});

it('should pre-load a local module only once even if be respawned', function(done) {
var fixturesDir = path.resolve(__dirname, 'fixtures');

exec('cd ' + fixturesDir + ' && node respawn_and_require.js', cb);
function cb(err, stdout, stderr) {
expect(err).to.equal(null);
expect(stderr).to.equal('');
expect(stdout).to.equal(
'saw respawn [ \'--lazy\' ]\n' +
'require coffeescript/register\n' +
'execute\n' +
'');
done();
}
});

it('should emit `require` with the name of the module and the required module', function(done) {
var requireTest = new Liftoff({ name: 'require' });
requireTest.on('require', function(name, module) {
Expand Down

0 comments on commit 4a6ad90

Please sign in to comment.