Browse Source

chore: refactor deprecation apis (#14495)

Shelley Vohr 6 years ago
parent
commit
2157d09956
3 changed files with 146 additions and 144 deletions
  1. 7 3
      lib/browser/api/app.js
  2. 104 116
      lib/common/api/deprecate.js
  3. 35 25
      spec/api-deprecations-spec.js

+ 7 - 3
lib/browser/api/app.js

@@ -42,9 +42,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 {memory} of metrics) {
+    deprecate.removeProperty(memory, 'privateBytes')
+    deprecate.removeProperty(memory, 'sharedBytes')
+  }
+
+  return metrics
 }
 
 app.isPackaged = (() => {

+ 104 - 116
lib/common/api/deprecate.js

@@ -1,131 +1,119 @@
-// 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)
-  }
-}
-
-// The method is aliases and the old method is retained for backwards compat
-deprecate.alias = function (object, deprecatedName, existingName) {
-  let warned = false
-  const newMethod = function () {
-    if (!(warned || process.noDeprecation)) {
-      warned = true
-      deprecate.warn(deprecatedName, existingName)
-    }
-    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
-
-// Commented out until such time as it is needed
-// // 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)
-//   }
-// }
-
-// Remove a property with no replacement
-deprecate.removeProperty = (object, deprecatedName) => {
-  if (!process.noDeprecation) {
-    deprecate.log(`The '${deprecatedName}' property of ${object} has been deprecated and marked for removal.`)
-  }
-}
-
-// Remove a function with no replacement
-deprecate.removeFunction = (deprecatedName) => {
-  if (!process.noDeprecation) {
-    deprecate.log(`The '${deprecatedName}' function has been deprecated and marked for removal.`)
-  }
-}
+  },
+  renameFunction: function (fn, oldName, newName) {
+    let warned = false
+    return function () {
+      if (!(warned || process.noDeprecation)) {
+        warned = true
+        deprecate.warn(oldName, newName)
+      }
+      return fn.apply(this, arguments)
+    }
+  },
+  removeFunction: (oldName) => {
+    if (!process.noDeprecation) {
+      deprecate.log(`The '${oldName}' function has been deprecated and marked for removal.`)
+    }
+  },
+  alias: function (object, oldName, newName) {
+    let warned = false
+    const newFn = function () {
+      if (!(warned || process.noDeprecation)) {
+        warned = true
+        deprecate.warn(oldName, newName)
+      }
+      return this[newName].apply(this, arguments)
+    }
+    if (typeof object === 'function') {
+      object.prototype[newName] = newFn
+    } else {
+      object[oldName] = newFn
+    }
+  },
+  event: (emitter, oldName, newName) => {
+    let warned = false
+    return emitter.on(newName, function (...args) {
+      if (this.listenerCount(oldName) === 0) return
+      if (warned || process.noDeprecation) return
 
-// 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 (newName.startsWith('-')) {
+        deprecate.log(`'${oldName}' event has been deprecated.`)
+      } else {
+        deprecate.warn(`'${oldName}' event`, `'${newName}' event`)
+      }
+      this.emit(oldName, ...args)
+    })
+  },
+  removeProperty: (object, deprecated) => {
+    let warned = false
+    let warn = () => {
+      if (!(warned || process.noDeprecation)) {
+        warned = true
+        deprecate.log(`The '${deprecated}' property has been deprecated and marked for removal.`)
+      }
     }
-  }
 
-  if ((typeof object[newName] === 'undefined') &&
-      (typeof object[deprecatedName] !== 'undefined')) {
-    warn()
-    object[newName] = object[deprecatedName]
-  }
+    if (!(deprecated in object)) {
+      throw new Error('Cannot deprecate a property on an object which does not have that property')
+    }
 
-  return Object.defineProperty(object, deprecatedName, {
-    get: function () {
-      warn()
-      return this[newName]
-    },
-    set: function (value) {
+    let temp = object[deprecated]
+    return Object.defineProperty(object, deprecated, {
+      configurable: true,
+      get: () => {
+        warn()
+        return temp
+      },
+      set: (newValue) => {
+        warn()
+        temp = newValue
+      }
+    })
+  },
+  renameProperty: (object, oldName, newName) => {
+    let warned = false
+    let warn = () => {
+      if (!(warned || process.noDeprecation)) {
+        warned = true
+        deprecate.warn(oldName, newName)
+      }
+    }
+
+    if (!(newName in object) && (oldName in object)) {
       warn()
-      this[newName] = value
+      object[newName] = object[oldName]
     }
-  })
+
+    return Object.defineProperty(object, oldName, {
+      get: function () {
+        warn()
+        return this[newName]
+      },
+      set: function (value) {
+        warn()
+        this[newName] = value
+      }
+    })
+  }
 }
 
 module.exports = deprecate

+ 35 - 25
spec/api-deprecations-spec.js

@@ -5,7 +5,7 @@ const {deprecations, deprecate, nativeImage} = require('electron')
 const {expect} = chai
 chai.use(dirtyChai)
 
-describe('deprecations', () => {
+describe.only('deprecations', () => {
   beforeEach(() => {
     deprecations.setHandler(null)
     process.throwDeprecation = true
@@ -55,54 +55,64 @@ describe('deprecations', () => {
 
   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(newPropertyName).that.is.equal(value)
-    expect(o).to.have.a.property(oldPropertyName).that.is.equal(value)
+    expect(o).to.have.a.property(newProp).that.is.equal(value)
+    expect(o).to.have.a.property(oldProp).that.is.equal(value)
+  })
+
+  it('doesn\'t deprecate a property not on an object', () => {
+    const o = {}
+
+    expect(() => {
+      deprecate.removeProperty(o, 'iDontExist')
+    }).to.throw(/Cannot deprecate a property on an object which does not have that property/)
   })
 
   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 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', () => {