From 89148bcf8dad735e68b644798a48acba50658dca Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Thu, 13 Sep 2018 21:39:00 -0700 Subject: [PATCH] refactor: deprecation handling apis (#14615) --- lib/browser/api/app.js | 10 +- lib/common/api/deprecate.js | 196 ++++++++++++++-------------------- spec/api-deprecations-spec.js | 97 +++++++++-------- 3 files changed, 142 insertions(+), 161 deletions(-) diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index a6d94a20a400..00c2dd9f804c 100644 --- a/lib/browser/api/app.js +++ b/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 = (() => { diff --git a/lib/common/api/deprecate.js b/lib/common/api/deprecate.js index 4a42b2a693ac..f72939b6e9db 100644 --- a/lib/common/api/deprecate.js +++ b/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) - } -} - -// 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.`) -} +'use strict' 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) => { +function warnOnce (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.`) - } else { - // The event has a new name now. Warn with that. - deprecate.warn(`'${oldName}' event`, `'${newName}' event`) - } - this.emit(oldName, ...args) - }) -} - -deprecate.setHandler = (handler) => { - deprecationHandler = handler -} - -deprecate.getHandler = () => deprecationHandler - -// 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. - -// // 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) -// } -// } - -deprecate.removeProperty = (object, deprecatedName) => { - if (!process.noDeprecation) { - deprecate.log(`The '${deprecatedName}' property has been deprecated.`) - } -} - -// Replace the old name of a property -deprecate.renameProperty = (object, deprecatedName, newName) => { - let warned = false - let warn = () => { - 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, newName) + deprecate.log(msg) } } +} - if ((typeof object[newName] === 'undefined') && - (typeof object[deprecatedName] !== 'undefined')) { - warn() - object[newName] = object[deprecatedName] - } - - return Object.defineProperty(object, deprecatedName, { - get: function () { - warn() - return this[newName] - }, - set: function (value) { - warn() - this[newName] = value +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 { + return console.warn(`(electron) ${message}`) } - }) + }, + + 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) + } + }) + }, + + 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.`) + } + + // 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 + } + }) + }, + + renameProperty: (o, oldName, newName) => { + const warn = warnOnce(oldName, 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] + } + + // 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 + } + }) + } } module.exports = deprecate diff --git a/spec/api-deprecations-spec.js b/spec/api-deprecations-spec.js index 6a18f7704074..01be01f4a183 100644 --- a/spec/api-deprecations-spec.js +++ b/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(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, '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, propertyName) + deprecate.removeProperty(o, prop) + 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', () => {