diff --git a/lib/browser/api/app.js b/lib/browser/api/app.js index 3b8c0e38c88b..ed100281fc00 100644 --- a/lib/browser/api/app.js +++ b/lib/browser/api/app.js @@ -44,7 +44,9 @@ const nativeFn = app.getAppMetrics app.getAppMetrics = () => { let metrics = nativeFn.call(app) for (const metric of metrics) { - deprecate.removeProperty(metric, 'memory', true) + if ('memory' in metric) { + deprecate.removeProperty(metric, 'memory') + } } return metrics diff --git a/lib/common/api/deprecate.js b/lib/common/api/deprecate.js index 8275dfe27da6..f72939b6e9db 100644 --- a/lib/common/api/deprecate.js +++ b/lib/common/api/deprecate.js @@ -1,6 +1,20 @@ +'use strict' let deprecationHandler = null +function warnOnce (oldName, newName) { + let warned = false + 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.log(msg) + } + } +} + const deprecate = { setHandler: (handler) => { deprecationHandler = handler }, getHandler: () => deprecationHandler, @@ -18,100 +32,61 @@ const deprecate = { return console.warn(`(electron) ${message}`) } }, - 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 - warned = true - if (newName.startsWith('-')) { - deprecate.log(`'${oldName}' event has been deprecated.`) - } else { - deprecate.warn(`'${oldName}' event`, `'${newName}' event`) + 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) } - this.emit(oldName, ...args) }) }, - removeProperty: (object, deprecated, ignoreMissingProps = false) => { - let warned = false - let warn = () => { - if (!(warned || process.noDeprecation)) { - warned = true - deprecate.log(`The '${deprecated}' property has been deprecated and marked for removal.`) - } + + 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.`) } - if (!(deprecated in object)) { - if (ignoreMissingProps) return - throw new Error('Cannot deprecate a property on an object which does not have that property') - } - - let temp = object[deprecated] - return Object.defineProperty(object, deprecated, { + // 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 temp + return val }, - set: (newValue) => { + set: newVal => { warn() - temp = newValue + val = newVal } }) }, - 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)) { + 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() - object[newName] = object[oldName] + o[newName] = o[oldName] } - return Object.defineProperty(object, oldName, { - get: function () { + // wrap the deprecated property in an accessor to warn + // and redirect to the new property + return Object.defineProperty(o, oldName, { + get: () => { warn() - return this[newName] + return o[newName] }, - set: function (value) { + set: value => { warn() - this[newName] = value + o[newName] = value } }) } diff --git a/spec/api-deprecations-spec.js b/spec/api-deprecations-spec.js index 3afa777a2d02..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,26 +35,6 @@ 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 }) @@ -80,8 +62,8 @@ describe('deprecations', () => { const o = {} expect(() => { - deprecate.removeProperty(o, 'iDontExist') - }).to.throw(/Cannot deprecate a property on an object which does not have that property/) + deprecate.removeProperty(o, 'iDoNotExist') + }).to.throw(/iDoNotExist/) }) it('deprecates a property of an object', () => { @@ -100,6 +82,21 @@ describe('deprecations', () => { 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 })