Repository: pinojs/pino
Pull request #168 (2017-01-16 00:38:38)

Adds `lscache` as an instance property and fixes add level tests
This PR addresses issue #167 . The problem was that adding a new level to an existing instance relied on `instance.level = 42` to be invoked before the new level function would be present. This PR adds the function when `addLevel()` is invoked, adds `lscache` as an instance property, and fixes the tests so that each test uses a clean instance of Pino.


Patch:
diff --git a/pino.js b/pino.js
index f099e0f..16cc833 100644
--- a/pino.js
+++ b/pino.js
@@ -67,7 +67,7 @@ var isStandardLevelVal = (function () {
   }
 }())
 
-// level string catch
+// level string cache
 var lscache = Object.keys(nums).reduce(function (o, k) {
   o[k] = flatstr('"level":' + Number(k))
   return o
@@ -138,8 +138,9 @@ function pino (opts, stream) {
 
 defineLevelsProperty(pino)
 
-function Pino (opts, stream) {
-  this.stream = stream
+// Must be invoked via .call() to retain speed of inlining into the
+// Pino constructor.
+function applyOptions (opts) {
   this.serializers = opts.serializers
   this.stringify = opts.stringify
   this.end = opts.end
@@ -169,8 +170,15 @@ function Pino (opts, stream) {
   }
 }
 
+function Pino (opts, stream) {
+  // We define the levels property at construction so that state does
+  // not get shared between instances.
+  defineLevelsProperty(this)
+  this.stream = stream
+  applyOptions.call(this, opts)
+}
+
 Pino.prototype = new EventEmitter()
-defineLevelsProperty(Pino.prototype)
 
 Pino.prototype.fatal = genLog(levels.fatal)
 Pino.prototype.error = genLog(levels.error)
@@ -197,7 +205,7 @@ Object.defineProperty(Pino.prototype, 'levelVal', {
         this[key] = noop
         continue
       }
-      this[key] = isStandardLevel(key) ? Pino.prototype[key] : genLog(num)
+      this[key] = isStandardLevel(key) ? Pino.prototype[key] : genLog(this.levels.values[key])
     }
   }
 })
@@ -220,6 +228,10 @@ Object.defineProperty(Pino.prototype, 'level', {
   set: Pino.prototype._setLevel
 })
 
+Object.defineProperty(Pino.prototype, '_lscache', {
+  value: copy({}, lscache)
+})
+
 Object.defineProperty(
   Pino.prototype,
   'LOG_VERSION',
@@ -230,7 +242,7 @@ Pino.prototype.asJson = function asJson (obj, msg, num) {
   if (!msg && obj instanceof Error) {
     msg = obj.message
   }
-  var data = this._baseLog + lscache[num] + this.time()
+  var data = this._baseLog + this._lscache[num] + this.time()
   // to catch both null and undefined
   /* eslint-disable eqeqeq */
   if (msg != undefined) {
@@ -268,7 +280,7 @@ function getSlowTime () {
 
 Pino.prototype.child = function child (bindings) {
   if (!bindings) {
-    throw new Error('missing bindings for child Pino')
+    throw Error('missing bindings for child Pino')
   }
 
   var data = ','
@@ -297,7 +309,10 @@ Pino.prototype.child = function child (bindings) {
     formatOpts: this.formatOpts
   }
 
-  return new Pino(opts, this.stream)
+  var _child = Object.create(this)
+  _child.stream = this.stream
+  applyOptions.call(_child, opts)
+  return _child
 }
 
 Pino.prototype.write = function (obj, msg, num) {
@@ -328,7 +343,8 @@ Pino.prototype.addLevel = function addLevel (name, lvl) {
   if (this.levels.labels.hasOwnProperty(lvl)) return false
   this.levels.values[name] = lvl
   this.levels.labels[lvl] = name
-  lscache[lvl] = flatstr('"level":' + Number(lvl))
+  this._lscache[lvl] = flatstr('"level":' + Number(lvl))
+  this[name] = genLog(lvl)
   return true
 }
 


Test patch:
diff --git a/test/addlevel.test.js b/test/addlevel.test.js
index 85f93f6..bbcbf5e 100644
--- a/test/addlevel.test.js
+++ b/test/addlevel.test.js
@@ -1,8 +1,8 @@
 'use strict'
 
 var test = require('tap').test
-var pino = require('../')
 var sink = require('./helper').sink
+var pino = require('../')
 
 test('can add a custom level via constructor', function (t) {
   t.plan(2)
@@ -23,9 +23,29 @@ test('can add a custom level to a prior instance', function (t) {
     t.is(chunk.msg, 'bar')
   }))
 
-  log.addLevel('foo', 35)
-  t.is(typeof log.foo, 'function')
-  log.foo('bar')
+  log.addLevel('foo2', 35)
+  t.is(typeof log.foo2, 'function')
+  log.foo2('bar')
+})
+
+test('custom level via constructor does not affect other instances', function (t) {
+  t.plan(2)
+
+  var log = pino({level: 'foo3', levelVal: 36})
+  var other = pino()
+  t.is(typeof log.foo3, 'function')
+  t.is(typeof other.foo3, 'undefined')
+})
+
+test('custom level on one instance does not affect other instances', function (t) {
+  t.plan(2)
+
+  var log = pino()
+  log.addLevel('foo4', 37)
+  var other = pino()
+  log.addLevel('foo5', 38)
+  t.is(typeof other.foo4, 'undefined')
+  t.is(typeof other.foo5, 'undefined')
 })
 
 test('custom levels encompass higher levels', function (t) {
@@ -65,6 +85,19 @@ test('children can be set to custom level', function (t) {
   child.foo('bar')
 })
 
+test('custom levels exists on children', function (t) {
+  t.plan(2)
+
+  var parent = pino({}, sink(function (chunk, enc, cb) {
+    t.is(chunk.msg, 'bar')
+    t.is(chunk.child, 'yes')
+    cb()
+  }))
+  parent.addLevel('foo', 35)
+  var child = parent.child({child: 'yes'})
+  child.foo('bar')
+})
+
 test('rejects already known labels', function (t) {
   t.plan(1)
   var log = pino({level: 'info', levelVal: 900})
@@ -79,3 +112,22 @@ test('reject already known values', function (t) {
     t.is(e.message.indexOf('level value') > -1, true)
   }
 })
+
+test('level numbers are logged correctly after level change', function (t) {
+  t.plan(1)
+  var log = pino({level: 'foo', levelVal: 25}, sink(function (chunk, enc, cb) {
+    t.is(chunk.level, 25)
+  }))
+  log.level = 'debug'
+  log.foo('bar')
+})
+
+test('levels state is not shared between instances', function (t) {
+  t.plan(2)
+
+  var instance1 = pino({level: 'foo', levelVal: 35})
+  t.is(typeof instance1.foo, 'function')
+
+  var instance2 = pino()
+  t.is(instance2.hasOwnProperty('foo'), false)
+})