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,41 @@
-import { ForgeConfig } from '@electron-forge/shared-types';
+import { ForgeConfig, ForgeMutatingHookFn, ForgeMutatingHookSignatures, ForgeSimpleHookFn, ForgeSimpleHookSignatures } 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: ForgeConfig, hookName: string, ...hookArgs: any[]): Promise<void> => {
+export const runHook = async <Hook extends keyof ForgeSimpleHookSignatures>(
+ forgeConfig: ForgeConfig,
+ 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: ForgeConfig, hookName: string, item: T): Promise<T> {
+export async function runMutatingHook<Hook extends keyof ForgeMutatingHookSignatures>(
+ forgeConfig: ForgeConfig,
+ 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 { ForgeConfig, IForgePlugin, IForgePluginInterface, StartResult } from '@electron-forge/shared-types';
+import {
+ ForgeConfig,
+ ForgeMutatingHookFn,
+ ForgeMutatingHookSignatures,
+ ForgeSimpleHookFn,
+ ForgeSimpleHookSignatures,
+ IForgePlugin,
+ IForgePluginInterface,
+ 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 { ForgeConfig, ForgeHookFn } 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: ForgeConfig): Promise<ForgeConfig> => {
+ resolveForgeConfig: ForgeHookFn<'resolveForgeConfig'> = async (forgeConfig) => {
  if (!forgeConfig.packagerConfig) {
  forgeConfig.packagerConfig = {};
  }

packages/plugin/base/src/Plugin.ts

@@ -1,4 +1,4 @@
-import { ElectronProcess, ForgeConfig, ForgeHookFn, IForgePlugin, StartOptions } from '@electron-forge/shared-types';
+import { ElectronProcess, ForgeConfig, ForgeHookMap, IForgePlugin, 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: ForgeConfig): 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 { ForgeConfig, ForgeHookMap } 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: ForgeConfig): 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 { ForgeConfig } from '@electron-forge/shared-types';
+import { ForgeHookFn } from '@electron-forge/shared-types';
 import fs from 'fs-extra';
 
 export const createCompileHook =
- (originalDir: string) =>
- async (_config: ForgeConfig, 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 { ForgeConfig, ForgeHookFn } 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: ForgeConfig, 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 { ForgeConfig, ForgeHookFn } 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: ForgeConfig, 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);