refactor: better internal type safety for hooks

.gitignore

@@ -24,3 +24,5 @@ packages/api/cli/README.md
 packages/**/tsconfig.tsbuildinfo
 packages/**/.swc
 .webpack
+packages/api/core/test/fixture/app-with-scoped-name/out/make
+packages/plugin/webpack/test/fixtures/apps/native-modules/package-lock.json

packages/api/core/src/util/hook.ts

@@ -1,32 +1,47 @@
-import { ResolvedForgeConfig } from '@electron-forge/shared-types';
+import {
+  ForgeMutatingHookFn,
+  ForgeMutatingHookSignatures,
+  ForgeSimpleHookFn,
+  ForgeSimpleHookSignatures,
+  ResolvedForgeConfig,
+} from '@electron-forge/shared-types';
 import debug from 'debug';
 
 const d = debug('electron-forge:hook');
 
 // eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const runHook = async (forgeConfig: ResolvedForgeConfig, hookName: string, ...hookArgs: any[]): Promise<void> => {
+export const runHook = async <Hook extends keyof ForgeSimpleHookSignatures>(
+  forgeConfig: ResolvedForgeConfig,
+  hookName: Hook,
+  ...hookArgs: ForgeSimpleHookSignatures[Hook]
+): Promise<void> => {
   const { hooks } = forgeConfig;
   if (hooks) {
     d(`hook triggered: ${hookName}`);
     if (typeof hooks[hookName] === 'function') {
       d('calling hook:', hookName, 'with args:', hookArgs);
-      await hooks[hookName](forgeConfig, ...hookArgs);
+      await (hooks[hookName] as ForgeSimpleHookFn<Hook>)(forgeConfig, ...hookArgs);
     }
   }
   await forgeConfig.pluginInterface.triggerHook(hookName, hookArgs);
 };
 
-export async function runMutatingHook<T>(forgeConfig: ResolvedForgeConfig, hookName: string, item: T): Promise<T> {
+export async function runMutatingHook<Hook extends keyof ForgeMutatingHookSignatures>(
+  forgeConfig: ResolvedForgeConfig,
+  hookName: Hook,
+  ...item: ForgeMutatingHookSignatures[Hook]
+): Promise<ForgeMutatingHookSignatures[Hook][0]> {
   const { hooks } = forgeConfig;
   if (hooks) {
     d(`hook triggered: ${hookName}`);
     if (typeof hooks[hookName] === 'function') {
-      d('calling mutating hook:', hookName, 'with item:', item);
-      const result = await hooks[hookName](forgeConfig, item);
+      d('calling mutating hook:', hookName, 'with item:', item[0]);
+      const hook = hooks[hookName] as ForgeMutatingHookFn<Hook>;
+      const result = await hook(forgeConfig, ...item);
       if (typeof result !== 'undefined') {
-        item = result;
+        item[0] = result;
       }
     }
   }
-  return forgeConfig.pluginInterface.triggerMutatingHook(hookName, item);
+  return forgeConfig.pluginInterface.triggerMutatingHook(hookName, item[0]);
 }

packages/api/core/src/util/plugin-interface.ts

@@ -1,5 +1,14 @@
 import PluginBase from '@electron-forge/plugin-base';
-import { IForgePlugin, IForgePluginInterface, ResolvedForgeConfig, StartResult } from '@electron-forge/shared-types';
+import {
+  ForgeMutatingHookFn,
+  ForgeMutatingHookSignatures,
+  ForgeSimpleHookFn,
+  ForgeSimpleHookSignatures,
+  IForgePlugin,
+  IForgePluginInterface,
+  ResolvedForgeConfig,
+  StartResult,
+} from '@electron-forge/shared-types';
 import debug from 'debug';
 
 import { StartOptions } from '../api';
@@ -56,26 +65,29 @@ export default class PluginInterface implements IForgePluginInterface {
     this.overrideStartLogic = this.overrideStartLogic.bind(this);
   }
 
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  async triggerHook(hookName: string, hookArgs: any[]): Promise<void> {
+  async triggerHook<Hook extends keyof ForgeSimpleHookSignatures>(hookName: Hook, hookArgs: ForgeSimpleHookSignatures[Hook]): Promise<void> {
     for (const plugin of this.plugins) {
-      if (typeof plugin.getHook === 'function') {
-        const hook = plugin.getHook(hookName);
+      if (typeof plugin.getHooks === 'function') {
+        const hook = plugin.getHooks()[hookName] as ForgeSimpleHookFn<Hook>;
         if (hook) await hook(this.config, ...hookArgs);
       }
     }
   }
 
-  async triggerMutatingHook<T>(hookName: string, item: T): Promise<T> {
+  async triggerMutatingHook<Hook extends keyof ForgeMutatingHookSignatures>(
+    hookName: Hook,
+    ...item: ForgeMutatingHookSignatures[Hook]
+  ): Promise<ForgeMutatingHookSignatures[Hook][0]> {
+    let result: ForgeMutatingHookSignatures[Hook][0] = item[0];
     for (const plugin of this.plugins) {
-      if (typeof plugin.getHook === 'function') {
-        const hook = plugin.getHook(hookName);
+      if (typeof plugin.getHooks === 'function') {
+        const hook = plugin.getHooks()[hookName] as ForgeMutatingHookFn<Hook>;
         if (hook) {
-          item = await hook(this.config, item);
+          result = (await hook(this.config, ...item)) || result;
         }
       }
     }
-    return item;
+    return result;
   }
 
   async overrideStartLogic(opts: StartOptions): Promise<StartResult> {

packages/api/core/test/fast/hook_spec.ts

@@ -14,33 +14,47 @@ const fakeConfig = {
 describe('hooks', () => {
   describe('runHook', () => {
     it('should not error when running non existent hooks', async () => {
-      await runHook({ ...fakeConfig }, 'magic');
+      await runHook({ ...fakeConfig }, 'preMake');
     });
 
     it('should not error when running a hook that is not a function', async () => {
-      await runHook({ hooks: { myHook: 'abc' as unknown as ForgeHookFn }, ...fakeConfig }, 'abc');
+      await runHook({ hooks: { preMake: 'abc' as unknown as ForgeHookFn<'preMake'> }, ...fakeConfig }, 'preMake');
     });
 
     it('should run the hook if it is provided as a function', async () => {
       const myStub = stub();
       myStub.returns(Promise.resolve());
-      await runHook({ hooks: { myHook: myStub }, ...fakeConfig }, 'myHook');
+      await runHook({ hooks: { preMake: myStub }, ...fakeConfig }, 'preMake');
       expect(myStub.callCount).to.equal(1);
     });
   });
 
   describe('runMutatingHook', () => {
     it('should return the input when running non existent hooks', async () => {
-      expect(await runMutatingHook({ ...fakeConfig }, 'magic', 'input')).to.equal('input');
+      const info = {
+        foo: 'bar',
+      };
+      expect(await runMutatingHook({ ...fakeConfig }, 'readPackageJson', info)).to.equal(info);
     });
 
     it('should return the mutated input when returned from a hook', async () => {
       fakeConfig.pluginInterface.triggerMutatingHook = stub().returnsArg(1);
       const myStub = stub();
-      myStub.returns(Promise.resolve('magneto'));
-      const output = await runMutatingHook({ hooks: { myHook: myStub }, ...fakeConfig }, 'myHook', 'input');
-      expect(output).to.equal('magneto');
-      expect((fakeConfig.pluginInterface.triggerMutatingHook as SinonStub).firstCall.args[1]).to.equal('magneto');
+      myStub.returns(
+        Promise.resolve({
+          mutated: 'foo',
+        })
+      );
+      const info = {
+        foo: 'bar',
+      };
+      const output = await runMutatingHook({ hooks: { readPackageJson: myStub }, ...fakeConfig }, 'readPackageJson', info);
+      expect(output).to.deep.equal({
+        mutated: 'foo',
+      });
+      expect((fakeConfig.pluginInterface.triggerMutatingHook as SinonStub).firstCall.args[1]).to.deep.equal({
+        mutated: 'foo',
+      });
     });
   });
 });

packages/plugin/auto-unpack-natives/src/AutoUnpackNativesPlugin.ts

@@ -1,19 +1,18 @@
 import { PluginBase } from '@electron-forge/plugin-base';
-import { ForgeHookFn, ResolvedForgeConfig } from '@electron-forge/shared-types';
+import { ForgeHookFn, ForgeHookMap } from '@electron-forge/shared-types';
 
 import { AutoUnpackNativesConfig } from './Config';
 
 export default class AutoUnpackNativesPlugin extends PluginBase<AutoUnpackNativesConfig> {
   name = 'auto-unpack-natives';
 
-  getHook(hookName: string): ForgeHookFn | null {
-    if (hookName === 'resolveForgeConfig') {
-      return this.resolveForgeConfig;
-    }
-    return null;
+  getHooks(): ForgeHookMap {
+    return {
+      resolveForgeConfig: this.resolveForgeConfig,
+    };
   }
 
-  resolveForgeConfig = async (forgeConfig: ResolvedForgeConfig): Promise<ResolvedForgeConfig> => {
+  resolveForgeConfig: ForgeHookFn<'resolveForgeConfig'> = async (forgeConfig) => {
     if (!forgeConfig.packagerConfig) {
       forgeConfig.packagerConfig = {};
     }

packages/plugin/base/src/Plugin.ts

@@ -1,4 +1,4 @@
-import { ElectronProcess, ForgeHookFn, IForgePlugin, ResolvedForgeConfig, StartOptions } from '@electron-forge/shared-types';
+import { ElectronProcess, ForgeHookMap, IForgePlugin, ResolvedForgeConfig, StartOptions } from '@electron-forge/shared-types';
 
 export { StartOptions };
 
@@ -7,6 +7,8 @@ export default abstract class Plugin<C> implements IForgePlugin {
 
   /** @internal */
   __isElectronForgePlugin!: true;
+  /** @internal */
+  _resolvedHooks: ForgeHookMap = {};
 
   constructor(public config: C) {
     Object.defineProperty(this, '__isElectronForgePlugin', {
@@ -17,11 +19,14 @@ export default abstract class Plugin<C> implements IForgePlugin {
   }
 
   init(_dir: string, _config: ResolvedForgeConfig): void {
-    // By default, do nothing. This can be overridden.
+    // This logic ensures that we only call getHooks once regardless of how many
+    // times we trip hook logic in the PluginInterface.
+    this._resolvedHooks = this.getHooks();
+    this.getHooks = () => this._resolvedHooks;
   }
 
-  getHook(_hookName: string): ForgeHookFn | null {
-    return null;
+  getHooks(): ForgeHookMap {
+    return {};
   }
 
   async startLogic(_startOpts: StartOptions): Promise<ElectronProcess | string | string[] | false> {

packages/plugin/compile/src/CompilePlugin.ts

@@ -1,7 +1,7 @@
 import * as path from 'path';
 
 import { PluginBase, StartOptions } from '@electron-forge/plugin-base';
-import { ForgeHookFn } from '@electron-forge/shared-types';
+import { ForgeHookMap, ResolvedForgeConfig } from '@electron-forge/shared-types';
 
 import { CompilePluginConfig } from './Config';
 import { createCompileHook } from './lib/compile-hook';
@@ -15,19 +15,19 @@ export default class CompileElectronPlugin extends PluginBase<CompilePluginConfi
     super(c);
 
     this.init = this.init.bind(this);
-    this.getHook = this.getHook.bind(this);
+    this.getHooks = this.getHooks.bind(this);
     this.startLogic = this.startLogic.bind(this);
   }
 
-  init(dir: string): void {
+  init(dir: string, config: ResolvedForgeConfig): void {
+    super.init(dir, config);
     this.dir = dir;
   }
 
-  getHook(hookName: string): ForgeHookFn | null {
-    if (hookName === 'packageAfterCopy') {
-      return createCompileHook(this.dir);
-    }
-    return null;
+  getHooks(): ForgeHookMap {
+    return {
+      packageAfterCopy: createCompileHook(this.dir),
+    };
   }
 
   async startLogic(_opts: StartOptions): Promise<string[]> {

packages/plugin/compile/src/lib/compile-hook.ts

@@ -1,12 +1,12 @@
 import path from 'path';
 
 import { asyncOra } from '@electron-forge/async-ora';
-import { ResolvedForgeConfig } from '@electron-forge/shared-types';
+import { ForgeHookFn } from '@electron-forge/shared-types';
 import fs from 'fs-extra';
 
 export const createCompileHook =
-  (originalDir: string) =>
-  async (_config: ResolvedForgeConfig, buildPath: string): Promise<void> => {
+  (originalDir: string): ForgeHookFn<'packageAfterCopy'> =>
+  async (_config, buildPath): Promise<void> => {
     await asyncOra('Compiling Application', async () => {
       // eslint-disable-next-line @typescript-eslint/no-var-requires
       const compileCLI = require(path.resolve(originalDir, 'node_modules/electron-compile/lib/cli.js'));

packages/plugin/electronegativity/src/ElectronegativityPlugin.ts

@@ -1,11 +1,6 @@
 import runElectronegativity from '@doyensec/electronegativity';
 import { PluginBase } from '@electron-forge/plugin-base';
-import { ForgeHookFn, ResolvedForgeConfig } from '@electron-forge/shared-types';
-
-// To be more precise, postPackage options we care about.
-type PostPackageOptions = {
-  outputPaths: string[];
-};
+import { ForgeHookFn, ForgeHookMap } from '@electron-forge/shared-types';
 
 export type Confidence = 'certain' | 'firm' | 'tentative';
 export type CustomCheck = 'dangerousfunctionsjscheck' | 'remotemodulejscheck';
@@ -60,14 +55,13 @@ export type ElectronegativityConfig = {
 export default class ElectronegativityPlugin extends PluginBase<ElectronegativityConfig> {
   name = 'electronegativity';
 
-  getHook(hookName: string): ForgeHookFn | null {
-    if (hookName === 'postPackage') {
-      return this.postPackage;
-    }
-    return null;
+  getHooks(): ForgeHookMap {
+    return {
+      postPackage: this.postPackage,
+    };
   }
 
-  postPackage = async (_forgeConfig: ResolvedForgeConfig, options: PostPackageOptions): Promise<void> => {
+  postPackage: ForgeHookFn<'postPackage'> = async (_forgeConfig, options): Promise<void> => {
     await runElectronegativity(
       {
         ...this.config,

packages/plugin/local-electron/src/LocalElectronPlugin.ts

@@ -1,5 +1,5 @@
 import { PluginBase } from '@electron-forge/plugin-base';
-import { ForgeHookFn, ResolvedForgeConfig } from '@electron-forge/shared-types';
+import { ForgeHookFn, ForgeHookMap } from '@electron-forge/shared-types';
 import fs from 'fs-extra';
 
 import { LocalElectronPluginConfig } from './Config';
@@ -10,7 +10,7 @@ export default class LocalElectronPlugin extends PluginBase<LocalElectronPluginC
   constructor(c: LocalElectronPluginConfig) {
     super(c);
 
-    this.getHook = this.getHook.bind(this);
+    this.getHooks = this.getHooks.bind(this);
     this.startLogic = this.startLogic.bind(this);
   }
 
@@ -29,11 +29,10 @@ export default class LocalElectronPlugin extends PluginBase<LocalElectronPluginC
     return false;
   }
 
-  getHook(hookName: string): ForgeHookFn | null {
-    if (hookName === 'packageAfterExtract') {
-      return this.afterExtract;
-    }
-    return null;
+  getHooks(): ForgeHookMap {
+    return {
+      packageAfterExtract: this.afterExtract,
+    };
   }
 
   private checkPlatform = (platform: string) => {
@@ -50,7 +49,7 @@ export default class LocalElectronPlugin extends PluginBase<LocalElectronPluginC
     }
   };
 
-  private afterExtract = async (_config: ResolvedForgeConfig, buildPath: string, _electronVersion: string, platform: string, arch: string) => {
+  private afterExtract: ForgeHookFn<'packageAfterExtract'> = async (_config, buildPath, _electronVersion, platform, arch) => {
     if (!this.enabled) return;
 
     this.checkPlatform(platform);