Logger improvements (#1472)

package.json

@@ -51,6 +51,7 @@
     "node-forge": "^0.7.1",
     "node-libs-browser": "^2.0.0",
     "opn": "^5.1.0",
+    "ora": "^2.1.0",
     "physical-cpu-count": "^2.0.0",
     "postcss": "^6.0.19",
     "postcss-value-parser": "^3.3.0",

src/Bundler.js

@@ -13,7 +13,6 @@ const logger = require('./Logger');
 const PackagerRegistry = require('./packagers');
 const localRequire = require('./utils/localRequire');
 const config = require('./utils/config');
-const emoji = require('./utils/emoji');
 const loadEnv = require('./utils/env');
 const PromiseQueue = require('./utils/PromiseQueue');
 const installPackage = require('./utils/installPackage');
@@ -222,7 +221,7 @@ class Bundler extends EventEmitter {
     this.error = null;
 
     logger.clear();
-    logger.status(emoji.progress, 'Building...');
+    logger.progress('Building...');
 
     try {
       // Start worker farm, watcher, etc. if needed
@@ -252,7 +251,7 @@ class Bundler extends EventEmitter {
         asset.invalidateBundle();
       }
 
-      logger.status(emoji.progress, `Producing bundles...`);
+      logger.progress(`Producing bundles...`);
 
       // Create a root bundle to hold all of the entry assets, and add them to the tree.
       this.mainBundle = new Bundle();
@@ -279,7 +278,7 @@ class Bundler extends EventEmitter {
         this.hmr.emitUpdate(changedAssets);
       }
 
-      logger.status(emoji.progress, `Packaging...`);
+      logger.progress(`Packaging...`);
 
       // Package everything up
       this.bundleHashes = await this.mainBundle.package(
@@ -292,7 +291,7 @@ class Bundler extends EventEmitter {
 
       let buildTime = Date.now() - startTime;
       let time = prettifyTime(buildTime);
-      logger.status(emoji.success, `Built in ${time}.`, 'green');
+      logger.success(`Built in ${time}.`);
       if (!this.watcher) {
         bundleReport(this.mainBundle, this.options.detailedReport);
       }
@@ -513,7 +512,7 @@ class Bundler extends EventEmitter {
     }
 
     if (!this.error) {
-      logger.status(emoji.progress, `Building ${asset.basename}...`);
+      logger.progress(`Building ${asset.basename}...`);
     }
 
     // Mark the asset processed so we don't load it twice
@@ -717,7 +716,7 @@ class Bundler extends EventEmitter {
     }
 
     logger.clear();
-    logger.status(emoji.progress, `Building ${Path.basename(path)}...`);
+    logger.progress(`Building ${Path.basename(path)}...`);
 
     // Add the asset to the rebuild queue, and reset the timeout.
     for (let asset of assets) {

src/Logger.js

@@ -4,11 +4,12 @@ const prettyError = require('./utils/prettyError');
 const emoji = require('./utils/emoji');
 const {countBreaks} = require('grapheme-breaker');
 const stripAnsi = require('strip-ansi');
+const ora = require('ora');
 
 class Logger {
   constructor(options) {
     this.lines = 0;
-    this.statusLine = null;
+    this.spinner = null;
     this.setOptions(options);
   }
 
@@ -29,16 +30,20 @@ class Logger {
   }
 
   countLines(message) {
-    return message.split('\n').reduce((p, line) => {
-      if (process.stdout.columns) {
-        return p + Math.ceil((line.length || 1) / process.stdout.columns);
-      }
+    return stripAnsi(message)
+      .split('\n')
+      .reduce((p, line) => {
+        if (process.stdout.columns) {
+          return p + Math.ceil((line.length || 1) / process.stdout.columns);
+        }
 
-      return p + 1;
-    }, 0);
+        return p + 1;
+      }, 0);
   }
 
   writeRaw(message) {
+    this.stopSpinner();
+
     this.lines += this.countLines(message) - 1;
     process.stdout.write(message);
   }
@@ -48,6 +53,7 @@ class Logger {
       this.lines += this.countLines(message);
     }
 
+    this.stopSpinner();
     this._log(message);
   }
 
@@ -72,21 +78,24 @@ class Logger {
       return;
     }
 
-    let {message, stack} = prettyError(err, {color: this.color});
-    this.write(this.chalk.yellow(`${emoji.warning}  ${message}`));
-    if (stack) {
-      this.write(stack);
-    }
+    this._writeError(err, emoji.warning, this.chalk.yellow);
   }
 
   error(err) {
     if (this.logLevel < 1) {
       return;
     }
 
-    let {message, stack} = prettyError(err, {color: this.color});
+    this._writeError(err, emoji.error, this.chalk.red.bold);
+  }
 
-    this.status(emoji.error, message, 'red');
+  success(message) {
+    this.log(`${emoji.success}  ${this.chalk.green.bold(message)}`);
+  }
+
+  _writeError(err, emoji, color) {
+    let {message, stack} = prettyError(err, {color: this.color});
+    this.write(color(`${emoji}  ${message}`));
     if (stack) {
       this.write(stack);
     }
@@ -104,42 +113,30 @@ class Logger {
     }
 
     readline.cursorTo(process.stdout, 0);
-    this.statusLine = null;
-  }
-
-  writeLine(line, msg) {
-    if (!this.color) {
-      return this.log(msg);
-    }
-
-    let n = this.lines - line;
-    let stdout = process.stdout;
-    readline.cursorTo(stdout, 0);
-    readline.moveCursor(stdout, 0, -n);
-    stdout.write(msg);
-    readline.clearLine(stdout, 1);
-    readline.cursorTo(stdout, 0);
-    readline.moveCursor(stdout, 0, n);
+    this.stopSpinner();
   }
 
-  status(emoji, message, color = 'gray') {
+  progress(message) {
     if (this.logLevel < 3) {
       return;
     }
 
-    let hasStatusLine = this.statusLine != null;
-    if (!hasStatusLine) {
-      this.statusLine = this.lines;
+    let styledMessage = this.chalk.gray.bold(message);
+    if (!this.spinner) {
+      this.spinner = ora({
+        text: styledMessage,
+        stream: process.stdout,
+        enabled: !this.isTest
+      }).start();
+    } else {
+      this.spinner.text = styledMessage;
     }
+  }
 
-    this.writeLine(
-      this.statusLine,
-      this.chalk[color].bold(`${emoji}  ${message}`)
-    );
-
-    if (!hasStatusLine) {
-      process.stdout.write('\n');
-      this.lines++;
+  stopSpinner() {
+    if (this.spinner) {
+      this.spinner.stop();
+      this.spinner = null;
     }
   }
 
@@ -202,7 +199,7 @@ if (process.send && process.env.PARCEL_WORKER_TYPE === 'remote-worker') {
     LoggerProxy.prototype[method] = (...args) => {
       worker.addCall(
         {
-          location: require.resolve('./Logger'),
+          location: __filename,
           method,
           args
         },

src/utils/installPackage.js

@@ -3,7 +3,6 @@ const promisify = require('./promisify');
 const resolve = promisify(require('resolve'));
 const commandExists = require('command-exists');
 const logger = require('../Logger');
-const emoji = require('./emoji');
 const pipeSpawn = require('./pipeSpawn');
 const PromiseQueue = require('./PromiseQueue');
 const path = require('path');
@@ -12,7 +11,7 @@ const fs = require('./fs');
 async function install(modules, filepath, options = {}) {
   let {installPeers = true, saveDev = true, packageManager} = options;
 
-  logger.status(emoji.progress, `Installing ${modules.join(', ')}...`);
+  logger.progress(`Installing ${modules.join(', ')}...`);
 
   let packageLocation = await config.resolve(filepath, ['package.json']);
   let cwd = packageLocation ? path.dirname(packageLocation) : process.cwd();

src/workerfarm/WorkerFarm.js

@@ -158,14 +158,12 @@ class WorkerFarm extends EventEmitter {
 
     const mod = require(location);
     try {
-      let func;
+      result.contentType = 'data';
       if (method) {
-        func = mod[method];
+        result.content = await mod[method](...args);
       } else {
-        func = mod;
+        result.content = await mod(...args);
       }
-      result.contentType = 'data';
-      result.content = await func(...args);
     } catch (e) {
       result.contentType = 'error';
       result.content = errorUtils.errorToJson(e);

test/logger.js

@@ -46,7 +46,7 @@ describe('Logger', () => {
 
     l.log('message');
     l.persistent('message');
-    l.status('🚨', 'message');
+    l.progress('message');
     l.logLevel = 1;
     l.warn('message');
     l.logLevel = 0;
@@ -56,16 +56,16 @@ describe('Logger', () => {
 
     l.logLevel = 1;
     l.error({message: 'message', stack: 'stack'});
-    assert.equal(log.length, 1);
+    assert.equal(log.length, 2);
 
     l.logLevel = 2;
     l.warn('message');
-    assert.equal(log.length, 2);
+    assert.equal(log.length, 3);
 
     l.logLevel = 3;
     l.log('message');
     l.persistent('message');
-    l.status('🚨', 'message');
+    l.progress('message');
     assert.equal(log.length, 5);
   });
 
@@ -75,36 +75,32 @@ describe('Logger', () => {
 
     // clear is a no-op
     l.lines = 4;
-    l.statusLine = 'hello';
     l.clear();
     assert.equal(l.lines, 4);
-    assert.equal(l.statusLine, 'hello');
-
-    // write-line calls log
-    const spy = sinon.spy(l, 'log');
-    l.writeLine(1, 'hello');
-    assert(spy.called);
   });
 
   it('should reset on clear', () => {
     const l = new Logger.constructor({color: true, isTest: false});
     stub(l);
 
+    // stub readline so we don't actually clear the test output
+    const sandbox = sinon.createSandbox();
+    sandbox.stub(require('readline'));
+
     l.lines = 10;
-    l.statusLine = 'hello';
     l.clear();
 
     assert.equal(l.lines, 0);
-    assert.equal(l.statusLine, null);
+    sandbox.restore();
   });
 
-  it('should log emoji and message via status', () => {
+  it('should use ora for progress', () => {
     const l = new Logger.constructor({color: false});
-    stub(l);
-    l.status('🚨', 'hello');
 
-    assert(log[0].includes('🚨'));
-    assert(log[0].includes('hello'));
+    l.progress('message');
+
+    assert(l.spinner);
+    assert(l.spinner.text.includes('message'));
   });
 
   it('should use internal _log function for writes', () => {
@@ -124,22 +120,4 @@ describe('Logger', () => {
 
     assert(spy.called);
   });
-
-  it('should use stdout directly for writeLine', () => {
-    const l = new Logger.constructor({color: true});
-    const sandbox = sinon.createSandbox();
-    const log = [];
-
-    try {
-      sandbox.stub(process.stdout, 'write').callsFake(message => {
-        log.push(message);
-      });
-
-      l.writeLine(0, 'hello');
-    } finally {
-      sandbox.restore();
-    }
-
-    assert(log.includes('hello'));
-  });
 });