fix: deprecate promise functions properly (#16643)

lib/browser/api/app.js

@@ -36,7 +36,7 @@ Object.assign(app, {
   }
 })
 
-app.getFileIcon = deprecate.promisify(app.getFileIcon, 3)
+app.getFileIcon = deprecate.promisify(app.getFileIcon)
 
 const nativeAppMetrics = app.getAppMetrics
 app.getAppMetrics = () => {

lib/browser/api/session.js

@@ -20,6 +20,6 @@ Object.setPrototypeOf(Session.prototype, EventEmitter.prototype)
 Object.setPrototypeOf(Cookies.prototype, EventEmitter.prototype)
 
 Session.prototype._init = function () {
-  this.protocol.isProtocolHandled = deprecate.promisify(this.protocol.isProtocolHandled, 2)
+  this.protocol.isProtocolHandled = deprecate.promisify(this.protocol.isProtocolHandled)
   app.emit('session-created', this)
 }

lib/browser/api/web-contents.js

@@ -378,7 +378,7 @@ WebContents.prototype._init = function () {
   // render-view-deleted event, so ignore the listeners warning.
   this.setMaxListeners(0)
 
-  this.capturePage = deprecate.promisify(this.capturePage, 2)
+  this.capturePage = deprecate.promisify(this.capturePage)
 
   // Dispatch IPC messages to the ipc module.
   this.on('-ipc-message', function (event, [channel, ...args]) {

lib/common/api/deprecate.js

@@ -69,27 +69,27 @@ const deprecate = {
     })
   },
 
-  promisify: (fn, cbParamIndex) => {
+  promisify: (fn) => {
     const fnName = fn.name || 'function'
     const oldName = `${fnName} with callbacks`
     const newName = `${fnName} with Promises`
     const warn = warnOnce(oldName, newName)
 
     return function (...params) {
-      const cb = params.splice(cbParamIndex, 1)[0]
+      let cb
+      if (params.length > 0 && typeof params[params.length - 1] === 'function') {
+        cb = params.pop()
+      }
       const promise = fn.apply(this, params)
-
-      if (typeof cb !== 'function') return promise
+      if (!cb) return promise
       if (process.enablePromiseAPIs) warn()
       return promise
         .then(res => {
           process.nextTick(() => {
-            cb(null, res)
+            cb.length === 2 ? cb(null, res) : cb(res)
           })
         }, err => {
-          process.nextTick(() => {
-            cb(err)
-          })
+          process.nextTick(() => cb(err))
         })
     }
   },

lib/renderer/api/desktop-capturer.js

@@ -56,4 +56,4 @@ const getSources = (options) => {
   })
 }
 
-exports.getSources = deprecate.promisify(getSources, 1)
+exports.getSources = deprecate.promisify(getSources)

spec/api-app-spec.js

@@ -836,7 +836,7 @@ describe('app module', () => {
     })
   })
 
-  describe('getFileIcon() API', (done) => {
+  describe('getFileIcon() API', () => {
     const iconPath = path.join(__dirname, 'fixtures/assets/icon.ico')
     const sizes = {
       small: 16,
@@ -859,7 +859,7 @@ describe('app module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('fetches a non-empty icon (callback)', () => {
+    it('fetches a non-empty icon (callback)', (done) => {
       app.getFileIcon(iconPath, (icon) => {
         expect(icon.isEmpty()).to.be.false()
         done()
@@ -875,7 +875,7 @@ describe('app module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('fetches normal icon size by default (callback)', () => {
+    it('fetches normal icon size by default (callback)', (done) => {
       app.getFileIcon(iconPath, (icon) => {
         const size = icon.getSize()
 
@@ -903,7 +903,7 @@ describe('app module', () => {
       })
 
       // TODO(codebytere): remove when promisification is complete
-      it('fetches a normal icon (callback)', () => {
+      it('fetches a normal icon (callback)', (done) => {
         app.getFileIcon(iconPath, { size: 'normal' }, (icon) => {
           const size = icon.getSize()
 

spec/api-browser-window-spec.js

@@ -507,7 +507,7 @@ describe('BrowserWindow module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns a Promise with a Buffer (callback)', () => {
+    it('returns a Promise with a Buffer (callback)', (done) => {
       w.capturePage({
         x: 0,
         y: 0,
@@ -540,24 +540,25 @@ describe('BrowserWindow module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('preserves transparency (callback)', async () => {
-      const w = await openTheWindow({
+    it('preserves transparency (callback)', (done) => {
+      openTheWindow({
         show: false,
         width: 400,
         height: 400,
         transparent: true
-      })
-      const p = emittedOnce(w, 'ready-to-show')
-      w.loadURL('data:text/html,<html><body background-color: rgba(255,255,255,0)></body></html>')
-      await p
-      w.show()
-
-      w.capturePage((image) => {
-        const imgBuffer = image.toPNG()
-        // Check the 25th byte in the PNG.
-        // Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha
-        expect(imgBuffer[25]).to.equal(6)
-        done()
+      }).then(w => {
+        const p = emittedOnce(w, 'ready-to-show')
+        w.loadURL('data:text/html,<html><body background-color: rgba(255,255,255,0)></body></html>')
+        p.then(() => {
+          w.show()
+          w.capturePage((image) => {
+            const imgBuffer = image.toPNG()
+            // Check the 25th byte in the PNG.
+            // Values can be 0,2,3,4, or 6. We want 6, which is RGB + Alpha
+            expect(imgBuffer[25]).to.equal(6)
+            done()
+          })
+        })
       })
     })
   })

spec/api-deprecations-spec.js

@@ -138,7 +138,7 @@ describe('deprecations', () => {
 
     it('acts as a pass-through for promise-based invocations', async () => {
       enableCallbackWarnings()
-      promiseFunc = deprecate.promisify(promiseFunc, 1)
+      promiseFunc = deprecate.promisify(promiseFunc)
 
       const actual = await promiseFunc(expected)
       expect(actual).to.equal(expected)
@@ -147,7 +147,7 @@ describe('deprecations', () => {
 
     it('warns exactly once for callback-based invocations', (done) => {
       enableCallbackWarnings()
-      promiseFunc = deprecate.promisify(promiseFunc, 1)
+      promiseFunc = deprecate.promisify(promiseFunc)
 
       let callbackCount = 0
       const invocationCount = 3

spec/api-desktop-capturer-spec.js

@@ -65,6 +65,7 @@ describe('desktopCapturer', () => {
     const callback = (error, sources) => {
       callCount++
       expect(error).to.be.null()
+      expect(sources).to.not.be.null()
       if (callCount === 2) done()
     }
 

spec/api-protocol-spec.js

@@ -680,14 +680,14 @@ describe('protocol module', () => {
     })
   })
 
-  describe('protocol.isProtocolHandled', (done) => {
+  describe('protocol.isProtocolHandled', () => {
     it('returns true for about:', async () => {
       const result = await protocol.isProtocolHandled('about')
       assert.strictEqual(result, true)
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for about: (callback)', () => {
+    it('returns true for about: (callback)', (done) => {
       protocol.isProtocolHandled('about', (result) => {
         assert.strictEqual(result, true)
         done()
@@ -700,7 +700,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for file: (callback)', () => {
+    it('returns true for file: (callback)', (done) => {
       protocol.isProtocolHandled('file', (result) => {
         assert.strictEqual(result, true)
         done()
@@ -722,7 +722,7 @@ describe('protocol module', () => {
       assert.strictEqual(result, false)
     })
 
-    it('returns true for custom protocol', () => {
+    it('returns true for custom protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.registerStringProtocol(protocolName, emptyHandler, async (error) => {
         assert.strictEqual(error, null)
@@ -733,7 +733,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for custom protocol (callback)', () => {
+    it('returns true for custom protocol (callback)', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.registerStringProtocol(protocolName, emptyHandler, (error) => {
         assert.strictEqual(error, null)
@@ -744,7 +744,7 @@ describe('protocol module', () => {
       })
     })
 
-    it('returns true for intercepted protocol', () => {
+    it('returns true for intercepted protocol', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.interceptStringProtocol('http', emptyHandler, async (error) => {
         assert.strictEqual(error, null)
@@ -755,7 +755,7 @@ describe('protocol module', () => {
     })
 
     // TODO(codebytere): remove when promisification is complete
-    it('returns true for intercepted protocol (callback)', () => {
+    it('returns true for intercepted protocol (callback)', (done) => {
       const emptyHandler = (request, callback) => callback()
       protocol.interceptStringProtocol('http', emptyHandler, (error) => {
         assert.strictEqual(error, null)