diff --git a/src/Logger.js b/src/Logger.js index 5206b59bab7..aef7932040e 100644 --- a/src/Logger.js +++ b/src/Logger.js @@ -44,12 +44,16 @@ class Logger { this.write(this.chalk.bold(message), true); } - warn(message) { + warn(err) { if (this.logLevel < 2) { return; } + let {message, stack} = prettyError(err, {color: this.color}); this.write(this.chalk.yellow(`${emoji.warning} ${message}`)); + if (stack) { + this.write(stack); + } } error(err) { diff --git a/src/WorkerFarm.js b/src/WorkerFarm.js index 6977733c2b4..98b764d3bcb 100644 --- a/src/WorkerFarm.js +++ b/src/WorkerFarm.js @@ -39,6 +39,7 @@ class WorkerFarm extends Farm { async initRemoteWorkers(options) { this.started = false; + this.warmWorkers = 0; let promises = []; for (let i = 0; i < this.options.maxConcurrentWorkers; i++) { @@ -52,36 +53,40 @@ class WorkerFarm extends Farm { } receive(data) { - if (!this.children[data.child]) { - // This handles premature death - // normally only accurs for workers - // that are still warming up when killed - return; - } - if (data.event) { this.emit(data.event, ...data.args); } else if (data.type === 'logger') { - logger.handleMessage(data); - } else { + if (this.shouldUseRemoteWorkers()) { + logger.handleMessage(data); + } + } else if (this.children[data.child]) { super.receive(data); } } + shouldUseRemoteWorkers() { + return this.started && this.warmWorkers >= this.activeChildren; + } + async run(...args) { // Child process workers are slow to start (~600ms). // While we're waiting, just run on the main thread. // This significantly speeds up startup time. - if (this.started && this.warmWorkers >= this.activeChildren) { + if (this.shouldUseRemoteWorkers()) { return this.remoteWorker.run(...args, false); } else { // Workers have started, but are not warmed up yet. // Send the job to a remote worker in the background, // but use the result from the local worker - it will be faster. if (this.started) { - this.remoteWorker.run(...args, true).then(() => { - this.warmWorkers++; - }); + this.remoteWorker.run(...args, true).then( + () => { + this.warmWorkers++; + }, + () => { + // ignore error + } + ); } return this.localWorker.run(...args, false); diff --git a/src/visitors/fs.js b/src/visitors/fs.js index aa83a2fbd2f..46c7099cd57 100644 --- a/src/visitors/fs.js +++ b/src/visitors/fs.js @@ -2,6 +2,7 @@ const t = require('babel-types'); const Path = require('path'); const fs = require('fs'); const template = require('babel-template'); +const logger = require('../Logger'); const bufferTemplate = template('Buffer(CONTENT, ENC)'); @@ -31,12 +32,32 @@ module.exports = { __dirname: Path.dirname(asset.name), __filename: asset.basename }; - let [filename, ...args] = path - .get('arguments') - .map(arg => evaluate(arg, vars)); - filename = Path.resolve(filename); + let filename, args, res; + + try { + [filename, ...args] = path + .get('arguments') + .map(arg => evaluate(arg, vars)); + + filename = Path.resolve(filename); + res = fs.readFileSync(filename, ...args); + } catch (err) { + if (err instanceof NodeNotEvaluatedError) { + // Warn using a code frame + err.fileName = asset.name; + asset.generateErrorMessage(err); + logger.warn(err); + return; + } + + // Add location info so we log a code frame with the error + err.loc = + path.node.arguments.length > 0 + ? path.node.arguments[0].loc.start + : path.node.loc.start; + throw err; + } - let res = fs.readFileSync(filename, ...args); let replacementNode; if (Buffer.isBuffer(res)) { replacementNode = bufferTemplate({ @@ -153,6 +174,12 @@ function getBindingPath(path, name) { return binding && binding.path; } +function NodeNotEvaluatedError(node) { + this.message = 'Cannot statically evaluate fs argument'; + this.node = node; + this.loc = node.loc.start; +} + function evaluate(path, vars) { // Inline variables path.traverse({ @@ -165,8 +192,9 @@ function evaluate(path, vars) { }); let res = path.evaluate(); + if (!res.confident) { - throw new Error('Could not statically evaluate fs call'); + throw new NodeNotEvaluatedError(path.node); } return res.value; diff --git a/test/fs.js b/test/fs.js index 46d48e5de9f..1698c8dfba6 100644 --- a/test/fs.js +++ b/test/fs.js @@ -76,6 +76,41 @@ describe('fs', function() { assert.equal(typeof output.test, 'function'); assert.equal(output.test(), 'test-pkg-ignore-fs-ok'); }); + + // TODO: check if the logger has warned the user + it('should ignore fs calls when the filename is not evaluable', async function() { + let b = await bundle( + __dirname + '/integration/fs-file-non-evaluable/index.js' + ); + let thrown = false; + + try { + run(b); + } catch (e) { + assert.equal(e.message, 'require(...).readFileSync is not a function'); + + thrown = true; + } + + assert.equal(thrown, true); + }); + + it('should ignore fs calls when the options are not evaluable', async function() { + let b = await bundle( + __dirname + '/integration/fs-options-non-evaluable/index.js' + ); + let thrown = false; + + try { + run(b); + } catch (e) { + assert.equal(e.message, 'require(...).readFileSync is not a function'); + + thrown = true; + } + + assert.equal(thrown, true); + }); }); describe('--target=node', function() { diff --git a/test/integration/fs-file-non-evaluable/index.js b/test/integration/fs-file-non-evaluable/index.js new file mode 100644 index 00000000000..0fa6d08bd49 --- /dev/null +++ b/test/integration/fs-file-non-evaluable/index.js @@ -0,0 +1 @@ +module.exports = require('fs').readFileSync(Date.now()) diff --git a/test/integration/fs-options-non-evaluable/index.js b/test/integration/fs-options-non-evaluable/index.js new file mode 100644 index 00000000000..d82ce92ba3d --- /dev/null +++ b/test/integration/fs-options-non-evaluable/index.js @@ -0,0 +1,5 @@ +const dir = __dirname + +module.exports = require('fs').readFileSync(dir + '/test.txt', { + encoding: (typeof Date.now()).replace(/number/, 'utf-8') +})