feat!: hooks are promisified

src/hooks.ts

@@ -1,12 +1,14 @@
-import { promisify } from 'util';
-import { FinalizePackageTargetsHookFunction, HookFunction, HookFunctionErrorCallback } from './types';
+import { FinalizePackageTargetsHookFunction, HookFunction } from './types';
 
-export async function promisifyHooks(hooks: HookFunction[] | FinalizePackageTargetsHookFunction[] | undefined, args?: unknown[]) {
-  if (!hooks || !Array.isArray(hooks)) {
+// eslint-disable-next-line @typescript-eslint/no-explicit-any
+type Fn = (...args: any[]) => Promise<void> | void;
+
+export async function runHooks<T extends Fn>(hooks: T[] | undefined, args?: Parameters<T>) {
+  if (hooks === undefined || !Array.isArray(hooks)) {
     return Promise.resolve();
   }
 
-  await Promise.all(hooks.map(hookFn => promisify(hookFn).apply(promisifyHooks, args)));
+  await Promise.all(hooks.map(hookFn => args ? hookFn(...args): hookFn()));
 }
 
 /**
@@ -20,30 +22,26 @@ export async function promisifyHooks(hooks: HookFunction[] | FinalizePackageTarg
  * packager({
  *   // ...
  *   afterCopy: [serialHooks([
- *     (buildPath, electronVersion, platform, arch, callback) => {
- *       setTimeout(() => {
- *         console.log('first function')
- *         callback()
- *       }, 1000)
+ *     async (buildPath, electronVersion, platform, arch) => {
+ *       await new Promise(resolve => setTimeout(() => {
+ *         console.log('first function');
+ *         resolve()
+ *       }, 1000))
  *     },
- *     (buildPath, electronVersion, platform, arch, callback) => {
+ *     (buildPath, electronVersion, platform, arch) => {
  *       console.log('second function')
- *       callback()
  *     }
  *   ])],
  *   // ...
  * })
  * ```
  */
-export function serialHooks(hooks: Parameters<typeof promisifyHooks>[0] = []) {
+export function serialHooks(hooks: Parameters<typeof runHooks>[0] = []) {
   return async function runSerialHook(...serialHookParams: Parameters<HookFunction | FinalizePackageTargetsHookFunction>) {
     const args = Array.prototype.slice.call(serialHookParams, 0, -1) as Parameters<HookFunction>;
-    const [done] = (Array.prototype.slice.call(serialHookParams, -1)) as [HookFunctionErrorCallback];
 
     for (const hook of hooks) {
       await (hook as HookFunction).apply(runSerialHook, args);
     }
-
-    return done();
   };
 }

src/packager.ts

@@ -3,12 +3,12 @@ import { populateIgnoredPaths } from './copy-filter';
 import { createDownloadCombos, downloadElectronZip } from './download';
 import fs from 'fs-extra';
 import { getMetadataFromPackageJSON } from './infer';
-import { promisifyHooks } from './hooks';
+import { runHooks } from './hooks';
 import path from 'path';
 import { createPlatformArchPairs, osModules, validateListFromOptions } from './targets';
 import { extractElectronZip } from './unzip';
 import { packageUniversalMac } from './universal';
-import { ComboOptions, DownloadOptions, OfficialPlatform, Options, SupportedArch, SupportedPlatform } from './types';
+import { ComboOptions, DownloadOptions, FinalizePackageTargetsHookFunction, OfficialPlatform, Options, SupportedArch, SupportedPlatform } from './types';
 import { App } from './platform';
 
 function debugHostInfo() {
@@ -75,11 +75,11 @@ export class Packager {
   async extractElectronZip(comboOpts: ComboOptions, zipPath: string, buildDir: string) {
     debug(`Extracting ${zipPath} to ${buildDir}`);
     await extractElectronZip(zipPath, buildDir);
-    await promisifyHooks(this.opts.afterExtract, [
+    await runHooks(this.opts.afterExtract, [
       buildDir,
-      comboOpts.electronVersion,
-      comboOpts.platform,
-      comboOpts.arch,
+      comboOpts.electronVersion as string,
+      comboOpts.platform as string,
+      comboOpts.arch as string,
     ]);
   }
 
@@ -240,7 +240,7 @@ export async function packager(opts: Options): Promise<string[]> {
 
   populateIgnoredPaths(opts);
 
-  await promisifyHooks(opts.afterFinalizePackageTargets, [
+  await runHooks<FinalizePackageTargetsHookFunction>(opts.afterFinalizePackageTargets, [
     createPlatformArchPairs(opts, platforms, archs).map(([platform, arch]) => ({ platform, arch })),
   ]);
   const appPaths = await packageAllSpecifiedCombos(opts, archs, platforms);

src/platform.ts

@@ -12,9 +12,9 @@ import {
   warning,
 } from './common';
 import { userPathFilter } from './copy-filter';
-import { promisifyHooks } from './hooks';
+import { runHooks } from './hooks';
 import crypto from 'crypto';
-import { ComboOptions } from './types';
+import { ComboOptions, HookFunction } from './types';
 
 export class App {
   asarIntegrity: Record<string, Pick<FileRecord['integrity'], 'algorithm' | 'hash'>> | undefined = undefined;
@@ -91,15 +91,15 @@ export class App {
     return path.join(this.originalResourcesDir, 'app.asar');
   }
 
-  get commonHookArgs() {
+  get commonHookArgs(): [string, string, string] {
     return [
-      this.opts.electronVersion,
-      this.opts.platform,
-      this.opts.arch,
-    ];
+      this.opts.electronVersion as string,
+      this.opts.platform as string,
+      this.opts.arch as string,
+    ] as const;
   }
 
-  get hookArgsWithOriginalResourcesAppDir() {
+  get hookArgsWithOriginalResourcesAppDir(): [string, string, string, string] {
     return [
       this.originalResourcesAppDir,
       ...this.commonHookArgs,
@@ -144,7 +144,7 @@ export class App {
       await this.buildApp();
     }
 
-    await promisifyHooks(this.opts.afterInitialize, this.hookArgsWithOriginalResourcesAppDir);
+    await runHooks<HookFunction>(this.opts.afterInitialize, this.hookArgsWithOriginalResourcesAppDir);
   }
 
   async buildApp() {
@@ -154,15 +154,15 @@ export class App {
   }
 
   async copyTemplate() {
-    await promisifyHooks(this.opts.beforeCopy, this.hookArgsWithOriginalResourcesAppDir);
+    await runHooks<HookFunction>(this.opts.beforeCopy, this.hookArgsWithOriginalResourcesAppDir);
 
     await fs.copy(this.opts.dir, this.originalResourcesAppDir, {
       filter: userPathFilter(this.opts),
       dereference: this.opts.derefSymlinks,
     });
-    await promisifyHooks(this.opts.afterCopy, this.hookArgsWithOriginalResourcesAppDir);
+    await runHooks<HookFunction>(this.opts.afterCopy, this.hookArgsWithOriginalResourcesAppDir);
     if (this.opts.prune) {
-      await promisifyHooks(this.opts.afterPrune, this.hookArgsWithOriginalResourcesAppDir);
+      await runHooks<HookFunction>(this.opts.afterPrune, this.hookArgsWithOriginalResourcesAppDir);
     }
   }
 
@@ -247,15 +247,15 @@ export class App {
 
     debug(`Running asar with the options ${JSON.stringify(this.asarOptions)}`);
 
-    await promisifyHooks(this.opts.beforeAsar, this.hookArgsWithOriginalResourcesAppDir);
+    await runHooks<HookFunction>(this.opts.beforeAsar, this.hookArgsWithOriginalResourcesAppDir);
 
     await asar.createPackageWithOptions(this.originalResourcesAppDir, this.appAsarPath, this.asarOptions);
     this.asarIntegrity = {
       [this.appRelativePlatformPath(this.appAsarPath)]: this.getAsarIntegrity(this.appAsarPath),
     };
     await fs.remove(this.originalResourcesAppDir);
 
-    await promisifyHooks(this.opts.afterAsar, this.hookArgsWithOriginalResourcesAppDir);
+    await runHooks<HookFunction>(this.opts.afterAsar, this.hookArgsWithOriginalResourcesAppDir);
   }
 
   getAsarIntegrity(path: string): Pick<FileRecord['integrity'], 'algorithm' | 'hash'> {
@@ -273,18 +273,18 @@ export class App {
 
     const extraResources = ensureArray(this.opts.extraResource);
 
-    const hookArgs = [
+    const hookArgs: [string, string, string, string] = [
       this.stagingPath,
       ...this.commonHookArgs,
     ];
 
-    await promisifyHooks(this.opts.beforeCopyExtraResources, hookArgs);
+    await runHooks<HookFunction>(this.opts.beforeCopyExtraResources, hookArgs);
 
     await Promise.all(extraResources.map(
       resource => fs.copy(resource, path.resolve(this.stagingPath, this.resourcesDir, path.basename(resource))),
     ));
 
-    await promisifyHooks(this.opts.afterCopyExtraResources, hookArgs);
+    await runHooks<HookFunction>(this.opts.afterCopyExtraResources, hookArgs);
   }
 
   async move() {
@@ -296,12 +296,12 @@ export class App {
     }
 
     if (this.opts.afterComplete) {
-      const hookArgs = [
+      const hookArgs: [string, string, string, string] = [
         finalPath,
         ...this.commonHookArgs,
       ];
 
-      await promisifyHooks(this.opts.afterComplete, hookArgs);
+      await runHooks<HookFunction>(this.opts.afterComplete, hookArgs);
     }
 
     return finalPath;

src/types.ts

@@ -56,8 +56,6 @@ export type SupportedPlatform = OfficialPlatform | 'all';
 
 export type IgnoreFunction = (path: string) => boolean;
 
-export type HookFunctionErrorCallback = (err?: Error | null) => void
-
 /**
  * A function that is called on the completion of a packaging stage.
  *
@@ -109,14 +107,13 @@ export type HookFunction =
     electronVersion: string,
     platform: TargetPlatform,
     arch: TargetArch,
-    callback: HookFunctionErrorCallback,
-  ) => void;
+  ) => Promise<void> | void;
 
 export type TargetDefinition = {
   arch: TargetArch;
   platform: TargetPlatform;
 }
-export type FinalizePackageTargetsHookFunction = (targets: TargetDefinition[], callback: HookFunctionErrorCallback) => void;
+export type FinalizePackageTargetsHookFunction = (targets: TargetDefinition[]) => Promise<void> | void;
 
 /** See the documentation for [`@electron/osx-sign`](https://npm.im/@electron/osx-sign#opts) for details.
  * @interface

test/darwin.js

@@ -243,11 +243,10 @@ if (!(process.env.CI && process.platform === 'win32')) {
     let expectedChecksum
     opts.icon = path.join(__dirname, 'fixtures', 'nonexistent')
     opts.afterExtract = [
-      async (extractPath, _electronVersion, _platform, _arch, callback) => {
+      async (extractPath) => {
         const hash = crypto.createHash('sha256')
         hash.update(await fs.readFile(path.join(extractPath, 'Electron.app', 'Contents', 'Resources', 'electron.icns')))
         expectedChecksum = hash.digest('hex')
-        callback()
       }
     ]
 
@@ -443,10 +442,9 @@ if (!(process.env.CI && process.platform === 'win32')) {
     ]
     const opts = {
       ...baseOpts,
-      afterExtract: [(buildPath, electronVersion, platform, arch, cb) => {
+      afterExtract: [(buildPath) => {
         return Promise.all(helpers.map(async helper => {
           await fs.remove(getHelperAppPath(buildPath, 'Electron', helper))
-          cb()
         }))
       }]
     }

test/hooks.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const config = require('./config.json')
-const { promisifyHooks, serialHooks } = require('../dist/hooks')
+const { runHooks, serialHooks } = require('../dist/hooks')
 const { packager } = require('../dist')
 const test = require('ava')
 const util = require('./_util')
@@ -18,11 +18,10 @@ async function hookTest (wantHookCalled, hookName, t, opts, validator) {
       hookCalled = true
       validator(t, ...args)
     }
-    : (buildPath, electronVersion, platform, arch, callback) => {
+    : (_buildPath, electronVersion, _platform, arch) => {
       hookCalled = true
       t.is(electronVersion, opts.electronVersion, `${hookName} electronVersion should be the same as the options object`)
       t.is(arch, opts.arch, `${hookName} arch should be the same as the options object`)
-      callback()
     }]
 
   // 2 packages will be built during this test
@@ -39,7 +38,7 @@ function createHookTest (hookName, validator) {
 
 test.serial('platform=all (one arch) for beforeCopy hook', createHookTest('beforeCopy'))
 test.serial('platform=all (one arch) for afterCopy hook', createHookTest('afterCopy'))
-test.serial('platform=all (one arch) for afterFinalizePackageTargets hook', createHookTest('afterFinalizePackageTargets', (t, targets, callback) => {
+test.serial('platform=all (one arch) for afterFinalizePackageTargets hook', createHookTest('afterFinalizePackageTargets', (t, targets) => {
   t.is(targets.length, 4, 'target list should have four items')
   t.is(targets[0].arch, 'x64')
   t.is(targets[0].platform, 'darwin')
@@ -49,46 +48,45 @@ test.serial('platform=all (one arch) for afterFinalizePackageTargets hook', crea
   t.is(targets[2].platform, 'mas')
   t.is(targets[3].arch, 'x64')
   t.is(targets[3].platform, 'win32')
-  callback()
 }))
 test.serial('platform=all (one arch) for afterPrune hook', createHookTest('afterPrune'))
 test.serial('platform=all (one arch) for afterExtract hook', createHookTest('afterExtract'))
 test.serial('platform=all (one arch) for afterComplete hook', createHookTest('afterComplete'))
 
-test('promisifyHooks executes functions in parallel', async t => {
+test('runHooks executes functions in parallel', async t => {
   let output = '0'
-  const timeoutFunc = (number, msTimeout) => {
-    return done => {
+  const timeoutFunc = async (number, msTimeout) => {
+    await new Promise(resolve => {
       setTimeout(() => {
         output += ` ${number}`
-        done()
+        resolve()
       }, msTimeout)
-    }
+    })
   }
   const testHooks = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map(number =>
-    timeoutFunc(number, number % 2 === 0 ? 1000 : 0)
+    () => timeoutFunc(number, number % 2 === 0 ? 1000 : 0)
   )
 
-  await promisifyHooks(testHooks)
+  await runHooks(testHooks)
   t.not(output, '0 1 2 3 4 5 6 7 8 9 10', 'should not be in sequential order')
 })
 
 test('serialHooks executes functions serially', async t => {
   let output = '0'
-  const timeoutFunc = (number, msTimeout) => {
-    return () => new Promise(resolve => { // eslint-disable-line promise/avoid-new
+  const timeoutFunc = async (number, msTimeout) => {
+    await new Promise(resolve => {
       setTimeout(() => {
         output += ` ${number}`
         resolve()
       }, msTimeout)
     })
   }
   const testHooks = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10].map(number =>
-    timeoutFunc(number, number % 2 === 0 ? 1000 : 0)
+    () => timeoutFunc(number, number % 2 === 0 ? 1000 : 0)
   )
 
-  const result = await serialHooks(testHooks)(() => output)
-  t.is(result, '0 1 2 3 4 5 6 7 8 9 10', 'should be in sequential order')
+  await serialHooks(testHooks)()
+  t.is(output, '0 1 2 3 4 5 6 7 8 9 10', 'should be in sequential order')
 })
 
 test.serial('prune hook does not get called when prune=false', util.packagerTest((t, opts) => {