refactor: deprecation handling apis (#14615)

lib/browser/api/app.js

@@ -60,9 +60,13 @@ Object.assign(app, {
 
 const nativeFn = app.getAppMetrics
 app.getAppMetrics = () => {
-  deprecate.removeProperty(nativeFn, 'privateBytes')
-  deprecate.removeProperty(nativeFn, 'sharedBytes')
-  return nativeFn.call(app)
+  let metrics = nativeFn.call(app)
+  for (const metric of metrics) {
+    deprecate.removeProperty(metric, 'memory', true)
+    if ('memory' in metric) {
+      deprecate.removeProperty(metric, 'memory')
+    }
+  }
 }
 
 app.isPackaged = (() => {

lib/common/api/deprecate.js

@@ -1,125 +1,95 @@
-// Deprecate a method.
-const deprecate = function (oldName, newName, fn) {
-  let warned = false
-  return function () {
-    if (!(warned || process.noDeprecation)) {
-      warned = true
-      deprecate.warn(oldName, newName)
-    }
-    return fn.apply(this, arguments)
-  }
-}
+'use strict'
+
+let deprecationHandler = null
 
-// The method is aliases and the old method is retained for backwards compat
-deprecate.alias = function (object, deprecatedName, existingName) {
+function warnOnce (oldName, newName) {
   let warned = false
-  const newMethod = function () {
-    if (!(warned || process.noDeprecation)) {
+  const msg = newName
+    ? `'${oldName}' is deprecated and will be removed. Please use '${newName}' instead.`
+    : `'${oldName}' is deprecated and will be removed.`
+  return () => {
+    if (!warned && !process.noDeprecation) {
       warned = true
-      deprecate.warn(deprecatedName, existingName)
+      deprecate.log(msg)
     }
-    return this[existingName].apply(this, arguments)
-  }
-  if (typeof object === 'function') {
-    object.prototype[deprecatedName] = newMethod
-  } else {
-    object[deprecatedName] = newMethod
-  }
-}
-
-deprecate.warn = (oldName, newName) => {
-  return deprecate.log(`'${oldName}' is deprecated. Use '${newName}' instead.`)
-}
-
-let deprecationHandler = null
-
-// Print deprecation message.
-deprecate.log = (message) => {
-  if (typeof deprecationHandler === 'function') {
-    deprecationHandler(message)
-  } else if (process.throwDeprecation) {
-    throw new Error(message)
-  } else if (process.traceDeprecation) {
-    return console.trace(message)
-  } else {
-    return console.warn(`(electron) ${message}`)
   }
 }
 
-// Deprecate an event.
-deprecate.event = (emitter, oldName, newName) => {
-  let warned = false
-  return emitter.on(newName, function (...args) {
-    // There are no listeners for this event
-    if (this.listenerCount(oldName) === 0) { return }
-    // noDeprecation set or if user has already been warned
-    if (warned || process.noDeprecation) { return }
-    warned = true
-    const isInternalEvent = newName.startsWith('-')
-    if (isInternalEvent) {
-      // The event cannot be use anymore. Log that.
-      deprecate.log(`'${oldName}' event has been deprecated.`)
+const deprecate = {
+  setHandler: (handler) => { deprecationHandler = handler },
+  getHandler: () => deprecationHandler,
+  warn: (oldName, newName) => {
+    return deprecate.log(`'${oldName}' is deprecated. Use '${newName}' instead.`)
+  },
+  log: (message) => {
+    if (typeof deprecationHandler === 'function') {
+      deprecationHandler(message)
+    } else if (process.throwDeprecation) {
+      throw new Error(message)
+    } else if (process.traceDeprecation) {
+      return console.trace(message)
     } else {
-      // The event has a new name now. Warn with that.
-      deprecate.warn(`'${oldName}' event`, `'${newName}' event`)
+      return console.warn(`(electron) ${message}`)
     }
-    this.emit(oldName, ...args)
-  })
-}
-
-deprecate.setHandler = (handler) => {
-  deprecationHandler = handler
-}
+  },
 
-deprecate.getHandler = () => deprecationHandler
+  event: (emitter, oldName, newName) => {
+    const warn = newName.startsWith('-') /* internal event */
+      ? warnOnce(`${oldName} event`)
+      : warnOnce(`${oldName} event`, `${newName} event`)
+    return emitter.on(newName, function (...args) {
+      if (this.listenerCount(oldName) !== 0) {
+        warn()
+        this.emit(oldName, ...args)
+      }
+    })
+  },
 
-// None of the below methods are used, and so will be commented
-// out until such time that they are needed to be used and tested.
+  removeProperty: (o, removedName) => {
+    // if the property's already been removed, warn about it
+    if (!(removedName in o)) {
+      deprecate.log(`Unable to remove property '${removedName}' from an object that lacks it.`)
+    }
 
-// // Forward the method to member.
-// deprecate.member = (object, method, member) => {
-//   let warned = false
-//   object.prototype[method] = function () {
-//     if (!(warned || process.noDeprecation)) {
-//       warned = true
-//       deprecate.warn(method, `${member}.${method}`)
-//     }
-//     return this[member][method].apply(this[member], arguments)
-//   }
-// }
+    // wrap the deprecated property in an accessor to warn
+    const warn = warnOnce(removedName)
+    let val = o[removedName]
+    return Object.defineProperty(o, removedName, {
+      configurable: true,
+      get: () => {
+        warn()
+        return val
+      },
+      set: newVal => {
+        warn()
+        val = newVal
+      }
+    })
+  },
 
-deprecate.removeProperty = (object, deprecatedName) => {
-  if (!process.noDeprecation) {
-    deprecate.log(`The '${deprecatedName}' property has been deprecated.`)
-  }
-}
+  renameProperty: (o, oldName, newName) => {
+    const warn = warnOnce(oldName, newName)
 
-// Replace the old name of a property
-deprecate.renameProperty = (object, deprecatedName, newName) => {
-  let warned = false
-  let warn = () => {
-    if (!(warned || process.noDeprecation)) {
-      warned = true
-      deprecate.warn(deprecatedName, newName)
+    // if the new property isn't there yet,
+    // inject it and warn about it
+    if ((oldName in o) && !(newName in o)) {
+      warn()
+      o[newName] = o[oldName]
     }
-  }
 
-  if ((typeof object[newName] === 'undefined') &&
-      (typeof object[deprecatedName] !== 'undefined')) {
-    warn()
-    object[newName] = object[deprecatedName]
+    // wrap the deprecated property in an accessor to warn
+    // and redirect to the new property
+    return Object.defineProperty(o, oldName, {
+      get: () => {
+        warn()
+        return o[newName]
+      },
+      set: value => {
+        warn()
+        o[newName] = value
+      }
+    })
   }
-
-  return Object.defineProperty(object, deprecatedName, {
-    get: function () {
-      warn()
-      return this[newName]
-    },
-    set: function (value) {
-      warn()
-      this[newName] = value
-    }
-  })
 }
 
 module.exports = deprecate

spec/api-deprecations-spec.js

@@ -1,6 +1,8 @@
+'use strict'
+
 const chai = require('chai')
 const dirtyChai = require('dirty-chai')
-const {deprecations, deprecate, nativeImage} = require('electron')
+const {deprecations, deprecate} = require('electron')
 
 const {expect} = chai
 chai.use(dirtyChai)
@@ -33,76 +35,81 @@ describe('deprecations', () => {
     expect(deprecations.getHandler()).to.be.a('function')
   })
 
-  it('returns a deprecation warning', () => {
-    const messages = []
-
-    deprecations.setHandler(message => {
-      messages.push(message)
-    })
-
-    deprecate.warn('old', 'new')
-    expect(messages).to.deep.equal([`'old' is deprecated. Use 'new' instead.`])
-  })
-
-  it('renames a method', () => {
-    expect(nativeImage.createFromDataUrl).to.be.undefined()
-    expect(nativeImage.createFromDataURL).to.be.a('function')
-
-    deprecate.alias(nativeImage, 'createFromDataUrl', 'createFromDataURL')
-
-    expect(nativeImage.createFromDataUrl).to.be.a('function')
-  })
-
   it('renames a property', () => {
     let msg
-    deprecations.setHandler((m) => { msg = m })
+    deprecations.setHandler(m => { msg = m })
 
-    const oldPropertyName = 'dingyOldName'
-    const newPropertyName = 'shinyNewName'
+    const oldProp = 'dingyOldName'
+    const newProp = 'shinyNewName'
 
     let value = 0
-    let o = { [newPropertyName]: value }
-    expect(o).to.not.have.a.property(oldPropertyName)
-    expect(o).to.have.a.property(newPropertyName).that.is.a('number')
+    const o = {[newProp]: value}
+    expect(o).to.not.have.a.property(oldProp)
+    expect(o).to.have.a.property(newProp).that.is.a('number')
 
-    deprecate.renameProperty(o, oldPropertyName, newPropertyName)
-    o[oldPropertyName] = ++value
+    deprecate.renameProperty(o, oldProp, newProp)
+    o[oldProp] = ++value
 
     expect(msg).to.be.a('string')
-    expect(msg).to.include(oldPropertyName)
-    expect(msg).to.include(newPropertyName)
+    expect(msg).to.include(oldProp)
+    expect(msg).to.include(newProp)
+
+    expect(o).to.have.a.property(newProp).that.is.equal(value)
+    expect(o).to.have.a.property(oldProp).that.is.equal(value)
+  })
 
-    expect(o).to.have.a.property(newPropertyName).that.is.equal(value)
-    expect(o).to.have.a.property(oldPropertyName).that.is.equal(value)
+  it('doesn\'t deprecate a property not on an object', () => {
+    const o = {}
+
+    expect(() => {
+      deprecate.removeProperty(o, 'iDoNotExist')
+    }).to.throw(/iDoNotExist/)
   })
 
   it('deprecates a property of an object', () => {
     let msg
     deprecations.setHandler(m => { msg = m })
 
-    const propertyName = 'itMustGo'
-    const o = { [propertyName]: 0 }
+    const prop = 'itMustGo'
+    let o = {[prop]: 0}
+
+    deprecate.removeProperty(o, prop)
 
-    deprecate.removeProperty(o, propertyName)
+    const temp = o[prop]
 
+    expect(temp).to.equal(0)
     expect(msg).to.be.a('string')
-    expect(msg).to.include(propertyName)
+    expect(msg).to.include(prop)
+  })
+
+  it('warns only once per item', () => {
+    const messages = []
+    deprecations.setHandler(message => { messages.push(message) })
+
+    const key = 'foo'
+    const val = 'bar'
+    let o = {[key]: val}
+    deprecate.removeProperty(o, key)
+
+    for (let i = 0; i < 3; ++i) {
+      expect(o[key]).to.equal(val)
+      expect(messages).to.have.length(1)
+    }
   })
 
   it('warns if deprecated property is already set', () => {
     let msg
-    deprecations.setHandler((m) => { msg = m })
+    deprecations.setHandler(m => { msg = m })
 
-    const oldPropertyName = 'dingyOldName'
-    const newPropertyName = 'shinyNewName'
-    const value = 0
+    const oldProp = 'dingyOldName'
+    const newProp = 'shinyNewName'
 
-    let o = { [oldPropertyName]: value }
-    deprecate.renameProperty(o, oldPropertyName, newPropertyName)
+    let o = {[oldProp]: 0}
+    deprecate.renameProperty(o, oldProp, newProp)
 
     expect(msg).to.be.a('string')
-    expect(msg).to.include(oldPropertyName)
-    expect(msg).to.include(newPropertyName)
+    expect(msg).to.include(oldProp)
+    expect(msg).to.include(newProp)
   })
 
   it('throws an exception if no deprecation handler is specified', () => {