From 2ba708357bdea2bcf847a92f3e3bc1598be22b3c Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 17:39:20 +0200 Subject: [PATCH 1/9] refactor(core): try...catch dynamicImport This ensures calling dynamicImport will rather result in rejected promise than in unhandled exception. --- packages/api/core/helper/dynamic-import.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index f4c2a347c2..6e8850d125 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -1,5 +1,9 @@ const url = require('url'); exports.dynamicImport = function dynamicImport(path) { - return import(url.pathToFileURL(path)); + try { + return import(url.pathToFileURL(path)); + } catch (error) { + return Promise.reject(error); + } }; From bfb25ba2cc3f85fae2df93d48302004c0a488ddf Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 17:50:24 +0200 Subject: [PATCH 2/9] feat(core): support ESM module packages Replace some require() calls with dynamicImport() to support loading makers/publishers/plugins etc. that are ESM makers --- packages/api/core/helper/dynamic-import.d.ts | 2 ++ packages/api/core/helper/dynamic-import.js | 4 ++++ .../core/src/api/init-scripts/find-template.ts | 2 +- packages/api/core/src/api/make.ts | 4 ++-- packages/api/core/src/api/package.ts | 15 +++++++-------- packages/api/core/src/api/publish.ts | 4 ++-- packages/api/core/src/util/forge-config.ts | 10 ++-------- .../{require-search.ts => import-search.ts} | 14 ++++++++------ packages/api/core/src/util/plugin-interface.ts | 18 +++++++++--------- 9 files changed, 37 insertions(+), 36 deletions(-) rename packages/api/core/src/util/{require-search.ts => import-search.ts} (77%) diff --git a/packages/api/core/helper/dynamic-import.d.ts b/packages/api/core/helper/dynamic-import.d.ts index dc2e8b6ba9..841640d534 100644 --- a/packages/api/core/helper/dynamic-import.d.ts +++ b/packages/api/core/helper/dynamic-import.d.ts @@ -1 +1,3 @@ export declare function dynamicImport(path: string): Promise; +/** Like {@linkcode dynamicImport()}, but falls back to require on failure. */ +export declare function dynamicImportMaybe(path: string): Promise; diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index 6e8850d125..4dd65d5c4c 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -7,3 +7,7 @@ exports.dynamicImport = function dynamicImport(path) { return Promise.reject(error); } }; + +exports.dynamicImportMaybe = function dynamicImportMaybe(path) { + return exports.dynamicImport(path).catch(() => require(path)); +}; diff --git a/packages/api/core/src/api/init-scripts/find-template.ts b/packages/api/core/src/api/init-scripts/find-template.ts index cd9cef7058..7807e52344 100644 --- a/packages/api/core/src/api/init-scripts/find-template.ts +++ b/packages/api/core/src/api/init-scripts/find-template.ts @@ -2,7 +2,7 @@ import { ForgeTemplate } from '@electron-forge/shared-types'; import debug from 'debug'; import resolvePackage from 'resolve-package'; -import { PossibleModule } from '../../util/require-search'; +import { PossibleModule } from '../../util/import-search'; const d = debug('electron-forge:init:find-template'); diff --git a/packages/api/core/src/api/make.ts b/packages/api/core/src/api/make.ts index 58510dfee1..d9c0b9cdb7 100644 --- a/packages/api/core/src/api/make.ts +++ b/packages/api/core/src/api/make.ts @@ -22,10 +22,10 @@ import logSymbols from 'log-symbols'; import getForgeConfig from '../util/forge-config'; import { getHookListrTasks, runMutatingHook } from '../util/hook'; +import importSearch from '../util/import-search'; import getCurrentOutDir from '../util/out-dir'; import parseArchs from '../util/parse-archs'; import { readMutatedPackageJson } from '../util/read-package-json'; -import requireSearch from '../util/require-search'; import resolveDir from '../util/resolve-dir'; import { listrPackage } from './package'; @@ -168,7 +168,7 @@ export const listrMake = ( throw new Error(`The following maker config has a maker name that is not a string: ${JSON.stringify(resolvableTarget)}`); } - const MakerClass = requireSearch(dir, [resolvableTarget.name]); + const MakerClass = await importSearch(dir, [resolvableTarget.name]); if (!MakerClass) { throw new Error( `Could not find module with name '${resolvableTarget.name}'. If this is a package from NPM, make sure it's listed in the devDependencies of your package.json. If this is a local module, make sure you have the correct path to its entry point. Try using the DEBUG="electron-forge:require-search" environment variable for more information.` diff --git a/packages/api/core/src/api/package.ts b/packages/api/core/src/api/package.ts index db916b079e..e148d11265 100644 --- a/packages/api/core/src/api/package.ts +++ b/packages/api/core/src/api/package.ts @@ -14,10 +14,10 @@ import { Listr, PRESET_TIMER } from 'listr2'; import getForgeConfig from '../util/forge-config'; import { getHookListrTasks, runHook } from '../util/hook'; +import importSearch from '../util/import-search'; import { warn } from '../util/messages'; import getCurrentOutDir from '../util/out-dir'; import { readMutatedPackageJson } from '../util/read-package-json'; -import requireSearch from '../util/require-search'; import resolveDir from '../util/resolve-dir'; const d = debug('electron-forge:packager'); @@ -27,7 +27,7 @@ const d = debug('electron-forge:packager'); */ function resolveHooks(hooks: (string | F)[] | undefined, dir: string) { if (hooks) { - return hooks.map((hook) => (typeof hook === 'string' ? (requireSearch(dir, [hook]) as F) : hook)); + return Promise.all(hooks.map(async (hook) => (typeof hook === 'string' ? ((await importSearch(dir, [hook])) as F) : hook))); } return []; @@ -216,13 +216,12 @@ export const listrPackage = ( const rebuildTasks = new Map>[]>(); const signalRebuildStart = new Map) => void)[]>(); - const afterFinalizePackageTargetsHooks: FinalizePackageTargetsHookFunction[] = [ (targets, done) => { provideTargets(targets); done(); }, - ...resolveHooks(forgeConfig.packagerConfig.afterFinalizePackageTargets, ctx.dir), + ...(await resolveHooks(forgeConfig.packagerConfig.afterFinalizePackageTargets, ctx.dir)), ]; const pruneEnabled = !('prune' in forgeConfig.packagerConfig) || forgeConfig.packagerConfig.prune; @@ -265,7 +264,7 @@ export const listrPackage = ( await fs.writeJson(path.resolve(buildPath, 'package.json'), copiedPackageJSON, { spaces: 2 }); done(); }, - ...resolveHooks(forgeConfig.packagerConfig.afterCopy, ctx.dir), + ...(await resolveHooks(forgeConfig.packagerConfig.afterCopy, ctx.dir)), ]; const afterCompleteHooks: HookFunction[] = [ @@ -273,13 +272,13 @@ export const listrPackage = ( signalPackageDone.get(getTargetKey({ platform: pPlatform, arch: pArch }))?.pop()?.(); done(); }, - ...resolveHooks(forgeConfig.packagerConfig.afterComplete, ctx.dir), + ...(await resolveHooks(forgeConfig.packagerConfig.afterComplete, ctx.dir)), ]; const afterPruneHooks = []; if (pruneEnabled) { - afterPruneHooks.push(...resolveHooks(forgeConfig.packagerConfig.afterPrune, ctx.dir)); + afterPruneHooks.push(...(await resolveHooks(forgeConfig.packagerConfig.afterPrune, ctx.dir))); } afterPruneHooks.push((async (buildPath, electronVersion, pPlatform, pArch, done) => { @@ -293,7 +292,7 @@ export const listrPackage = ( done(); }) as HookFunction, ]; - afterExtractHooks.push(...resolveHooks(forgeConfig.packagerConfig.afterExtract, ctx.dir)); + afterExtractHooks.push(...(await resolveHooks(forgeConfig.packagerConfig.afterExtract, ctx.dir))); type PackagerArch = Exclude; diff --git a/packages/api/core/src/api/publish.ts b/packages/api/core/src/api/publish.ts index 81a21e67d6..0dc3ed2d23 100644 --- a/packages/api/core/src/api/publish.ts +++ b/packages/api/core/src/api/publish.ts @@ -19,9 +19,9 @@ import fs from 'fs-extra'; import { Listr } from 'listr2'; import getForgeConfig from '../util/forge-config'; +import importSearch from '../util/import-search'; import getCurrentOutDir from '../util/out-dir'; import PublishState from '../util/publish-state'; -import requireSearch from '../util/require-search'; import resolveDir from '../util/resolve-dir'; import { listrMake, MakeOptions } from './make'; @@ -198,7 +198,7 @@ export default autoTrace( } else { const resolvablePublishTarget = publishTarget as IForgeResolvablePublisher; // eslint-disable-next-line @typescript-eslint/no-explicit-any - const PublisherClass: any = requireSearch(dir, [resolvablePublishTarget.name]); + const PublisherClass: any = await importSearch(dir, [resolvablePublishTarget.name]); if (!PublisherClass) { throw new Error( `Could not find a publish target with the name: ${resolvablePublishTarget.name}. Make sure it's listed in the devDependencies of your package.json` diff --git a/packages/api/core/src/util/forge-config.ts b/packages/api/core/src/util/forge-config.ts index f6619461ca..044242e47f 100644 --- a/packages/api/core/src/util/forge-config.ts +++ b/packages/api/core/src/util/forge-config.ts @@ -6,7 +6,7 @@ import * as interpret from 'interpret'; import { template } from 'lodash'; import * as rechoir from 'rechoir'; -import { dynamicImport } from '../../helper/dynamic-import.js'; +import { dynamicImportMaybe } from '../../helper/dynamic-import.js'; import { runMutatingHook } from './hook'; import PluginInterface from './plugin-interface'; @@ -128,13 +128,7 @@ export default async (dir: string): Promise => { const forgeConfigPath = path.resolve(dir, forgeConfig as string); try { // The loaded "config" could potentially be a static forge config, ESM module or async function - let loaded; - try { - loaded = (await dynamicImport(forgeConfigPath)) as MaybeESM; - } catch (err) { - // eslint-disable-next-line @typescript-eslint/no-var-requires - loaded = require(forgeConfigPath) as MaybeESM; - } + const loaded = (await dynamicImportMaybe(forgeConfigPath)) as MaybeESM; const maybeForgeConfig = 'default' in loaded ? loaded.default : loaded; forgeConfig = typeof maybeForgeConfig === 'function' ? await maybeForgeConfig() : maybeForgeConfig; } catch (err) { diff --git a/packages/api/core/src/util/require-search.ts b/packages/api/core/src/util/import-search.ts similarity index 77% rename from packages/api/core/src/util/require-search.ts rename to packages/api/core/src/util/import-search.ts index f326c9d4c2..1eb4a9ffa3 100644 --- a/packages/api/core/src/util/require-search.ts +++ b/packages/api/core/src/util/import-search.ts @@ -2,7 +2,9 @@ import path from 'path'; import debug from 'debug'; -const d = debug('electron-forge:require-search'); +import { dynamicImportMaybe } from '../../helper/dynamic-import.js'; + +const d = debug('electron-forge:import-search'); // https://github.com/nodejs/node/blob/da0ede1ad55a502a25b4139f58aab3fb1ee3bf3f/lib/internal/modules/cjs/loader.js#L353-L359 type RequireError = Error & { @@ -11,14 +13,14 @@ type RequireError = Error & { requestPath: string | undefined; }; -export function requireSearchRaw(relativeTo: string, paths: string[]): T | null { +export async function importSearchRaw(relativeTo: string, paths: string[]): Promise { // Attempt to locally short-circuit if we're running from a checkout of forge if (__dirname.includes('forge/packages/api/core/') && paths.length === 1 && paths[0].startsWith('@electron-forge/')) { const [moduleType, moduleName] = paths[0].split('/')[1].split('-'); try { const localPath = path.resolve(__dirname, '..', '..', '..', '..', moduleType, moduleName); d('testing local forge build', { moduleType, moduleName, localPath }); - return require(localPath); + return await dynamicImportMaybe(localPath); } catch { // Ignore } @@ -32,7 +34,7 @@ export function requireSearchRaw(relativeTo: string, paths: string[]): T | nu for (const testPath of testPaths) { try { d('testing', testPath); - return require(testPath); + return await dynamicImportMaybe(testPath); } catch (err) { if (err instanceof Error) { const requireErr = err as RequireError; @@ -51,7 +53,7 @@ export type PossibleModule = { default?: T; } & T; -export default (relativeTo: string, paths: string[]): T | null => { - const result = requireSearchRaw>(relativeTo, paths); +export default async (relativeTo: string, paths: string[]): Promise => { + const result = await importSearchRaw>(relativeTo, paths); return typeof result === 'object' && result && result.default ? result.default : (result as T | null); }; diff --git a/packages/api/core/src/util/plugin-interface.ts b/packages/api/core/src/util/plugin-interface.ts index b5b442e679..9308cb624c 100644 --- a/packages/api/core/src/util/plugin-interface.ts +++ b/packages/api/core/src/util/plugin-interface.ts @@ -16,7 +16,7 @@ import debug from 'debug'; import { StartOptions } from '../api'; -import requireSearch from './require-search'; +import importSearch from './import-search'; const d = debug('electron-forge:plugins'); @@ -25,12 +25,12 @@ function isForgePlugin(plugin: IForgePlugin | unknown): plugin is IForgePlugin { } export default class PluginInterface implements IForgePluginInterface { - private plugins: IForgePlugin[]; + private plugins: Promise[]; private config: ResolvedForgeConfig; constructor(dir: string, forgeConfig: ResolvedForgeConfig) { - this.plugins = forgeConfig.plugins.map((plugin) => { + this.plugins = forgeConfig.plugins.map(async (plugin) => { if (isForgePlugin(plugin)) { return plugin; } @@ -41,7 +41,7 @@ export default class PluginInterface implements IForgePluginInterface { throw new Error(`Expected plugin[0] to be a string but found ${pluginName}`); } // eslint-disable-next-line @typescript-eslint/no-explicit-any - const Plugin = requireSearch(dir, [pluginName]); + const Plugin = await importSearch(dir, [pluginName]); if (!Plugin) { throw new Error(`Could not find module with name: ${pluginName}. Make sure it's listed in the devDependencies of your package.json`); } @@ -61,7 +61,7 @@ export default class PluginInterface implements IForgePluginInterface { }); for (const plugin of this.plugins) { - plugin.init(dir, forgeConfig); + void plugin.then((plugin) => plugin.init(dir, forgeConfig)); } this.triggerHook = this.triggerHook.bind(this); @@ -69,7 +69,7 @@ export default class PluginInterface implements IForgePluginInterface { } async triggerHook(hookName: Hook, hookArgs: ForgeSimpleHookSignatures[Hook]): Promise { - for (const plugin of this.plugins) { + for await (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeSimpleHookFn[] | ForgeSimpleHookFn; if (hooks) { @@ -89,7 +89,7 @@ export default class PluginInterface implements IForgePluginInterface { ): Promise { const tasks: ForgeListrTaskDefinition[] = []; - for (const plugin of this.plugins) { + for await (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeSimpleHookFn[] | ForgeSimpleHookFn; if (hooks) { @@ -123,7 +123,7 @@ export default class PluginInterface implements IForgePluginInterface { ...item: ForgeMutatingHookSignatures[Hook] ): Promise { let result: ForgeMutatingHookSignatures[Hook][0] = item[0]; - for (const plugin of this.plugins) { + for await (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeMutatingHookFn[] | ForgeMutatingHookFn; if (hooks) { @@ -140,7 +140,7 @@ export default class PluginInterface implements IForgePluginInterface { async overrideStartLogic(opts: StartOptions): Promise { let newStartFn; const claimed: string[] = []; - for (const plugin of this.plugins) { + for await (const plugin of this.plugins) { if (typeof plugin.startLogic === 'function' && plugin.startLogic !== PluginBase.prototype.startLogic) { claimed.push(plugin.name); newStartFn = plugin.startLogic; From a5b640d365c8c98240b66257da4921a3a974c786 Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 23:13:45 +0200 Subject: [PATCH 3/9] fix(core): limit path resolving in helper Avoid resolving path to file url in dynamicImport to support importing modules by identifiers. --- packages/api/core/helper/dynamic-import.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index 4dd65d5c4c..a8117a1842 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -1,8 +1,9 @@ const url = require('url'); +const fs = require('fs'); exports.dynamicImport = function dynamicImport(path) { try { - return import(url.pathToFileURL(path)); + return import(fs.existsSync(path) ? url.pathToFileURL(path) : path); } catch (error) { return Promise.reject(error); } From 3e1c364906446cefc440a73ba83c5a11a05f4481 Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 23:19:16 +0200 Subject: [PATCH 4/9] fix(core): re-order dynamicImportMaybe methods Try require first, then import, to avoid .default property for CJS. --- packages/api/core/helper/dynamic-import.d.ts | 2 +- packages/api/core/helper/dynamic-import.js | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/api/core/helper/dynamic-import.d.ts b/packages/api/core/helper/dynamic-import.d.ts index 841640d534..4d25e1299a 100644 --- a/packages/api/core/helper/dynamic-import.d.ts +++ b/packages/api/core/helper/dynamic-import.d.ts @@ -1,3 +1,3 @@ export declare function dynamicImport(path: string): Promise; -/** Like {@linkcode dynamicImport()}, but falls back to require on failure. */ +/** Like {@link dynamicImport()}, except it tries out {@link require()} first. */ export declare function dynamicImportMaybe(path: string): Promise; diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index a8117a1842..806b6a2a9b 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -9,6 +9,14 @@ exports.dynamicImport = function dynamicImport(path) { } }; -exports.dynamicImportMaybe = function dynamicImportMaybe(path) { - return exports.dynamicImport(path).catch(() => require(path)); +exports.dynamicImportMaybe = async function dynamicImportMaybe(path) { + try { + return require(path); + } catch (e1) { + try { + return await exports.dynamicImport(path); + } catch { + throw e1; + } + } }; From cd30e290b1295743546cb94ba821b8fa82c8cafe Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 23:26:29 +0200 Subject: [PATCH 5/9] fix(core) fix tests in core --- packages/api/core/test/fast/publish_spec.ts | 2 +- .../api/core/test/fast/require-search_spec.ts | 27 ++++++++++++------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/packages/api/core/test/fast/publish_spec.ts b/packages/api/core/test/fast/publish_spec.ts index f66dfe0b93..f5d5a43334 100644 --- a/packages/api/core/test/fast/publish_spec.ts +++ b/packages/api/core/test/fast/publish_spec.ts @@ -60,7 +60,7 @@ describe('publish', () => { config.publishers = publishers; return config; }, - '../util/require-search': (_: string, [name]: [string]) => { + '../util/import-search': async (_: string, [name]: [string]) => { if (name === 'void') { return fakePublisher(voidStub); } diff --git a/packages/api/core/test/fast/require-search_spec.ts b/packages/api/core/test/fast/require-search_spec.ts index 21e96afaf9..f3f725ded2 100644 --- a/packages/api/core/test/fast/require-search_spec.ts +++ b/packages/api/core/test/fast/require-search_spec.ts @@ -1,22 +1,29 @@ import { expect } from 'chai'; import findConfig from '../../src/util/forge-config'; -import requireSearch from '../../src/util/require-search'; +import importSearch from '../../src/util/import-search'; -describe('require-search', () => { - it('should resolve null if no file exists', () => { - const resolved = requireSearch(__dirname, ['../../src/util/wizard-secrets']); +describe('import-search', () => { + it('should resolve null if no file exists', async () => { + const resolved = await importSearch(__dirname, ['../../src/util/wizard-secrets']); expect(resolved).to.equal(null); }); - it('should resolve a file if it exists', () => { - const resolved = requireSearch(__dirname, ['../../src/util/forge-config']); + it('should resolve a file if it exists', async () => { + const resolved = await importSearch(__dirname, ['../../src/util/forge-config']); expect(resolved).to.equal(findConfig); }); - it('should throw if file exists but fails to load', () => { - expect(() => { - requireSearch(__dirname, ['../fixture/require-search/throw-error']); - }).to.throw('test'); + it('should throw if file exists but fails to load', async () => { + let test: () => unknown; + try { + const ret = await importSearch(__dirname, ['../fixture/require-search/throw-error']); + test = () => ret; + } catch (error) { + test = () => { + throw error; + }; + } + expect(test).to.throw('test'); }); }); From 2eeb90983adb85f44ca6c95d69da946cbb564507 Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 28 Apr 2024 23:33:50 +0200 Subject: [PATCH 6/9] refactor(core): merge import error messages Merge error messages for imports to easier debug potential failures. --- packages/api/core/helper/dynamic-import.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index 806b6a2a9b..bb9cf21557 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -15,7 +15,8 @@ exports.dynamicImportMaybe = async function dynamicImportMaybe(path) { } catch (e1) { try { return await exports.dynamicImport(path); - } catch { + } catch (e2) { + e1.message = '\n1. ' + e1.message + '\n2. ' + e2.message; throw e1; } } From 38b3b9b65fe7b951ba88c1f8dc2605ff1985888f Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Thu, 2 May 2024 21:29:26 +0200 Subject: [PATCH 7/9] refactor(core): improve tests Rename require-search test to import-search and improve testing the promise rejection. Co-authored-by: Erick Zhao --- ...{require-search_spec.ts => import-search_spec.ts} | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) rename packages/api/core/test/fast/{require-search_spec.ts => import-search_spec.ts} (70%) diff --git a/packages/api/core/test/fast/require-search_spec.ts b/packages/api/core/test/fast/import-search_spec.ts similarity index 70% rename from packages/api/core/test/fast/require-search_spec.ts rename to packages/api/core/test/fast/import-search_spec.ts index f3f725ded2..670c97f810 100644 --- a/packages/api/core/test/fast/require-search_spec.ts +++ b/packages/api/core/test/fast/import-search_spec.ts @@ -15,15 +15,7 @@ describe('import-search', () => { }); it('should throw if file exists but fails to load', async () => { - let test: () => unknown; - try { - const ret = await importSearch(__dirname, ['../fixture/require-search/throw-error']); - test = () => ret; - } catch (error) { - test = () => { - throw error; - }; - } - expect(test).to.throw('test'); + const promise = importSearch(__dirname, ['../fixture/require-search/throw-error']); + await expect(promise).to.be.rejectedWith('test'); }); }); From 5e53f4c298a696085d841541f8c021619ca2c415 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Thu, 27 Jun 2024 13:46:38 -0700 Subject: [PATCH 8/9] generic refactors --- packages/api/core/helper/dynamic-import.js | 4 +- packages/api/core/src/api/package.ts | 4 +- packages/api/core/src/util/forge-config.ts | 2 +- .../api/core/src/util/plugin-interface.ts | 56 +++++++++++-------- 4 files changed, 39 insertions(+), 27 deletions(-) diff --git a/packages/api/core/helper/dynamic-import.js b/packages/api/core/helper/dynamic-import.js index bb9cf21557..fd4fadfb1f 100644 --- a/packages/api/core/helper/dynamic-import.js +++ b/packages/api/core/helper/dynamic-import.js @@ -1,9 +1,9 @@ const url = require('url'); const fs = require('fs'); -exports.dynamicImport = function dynamicImport(path) { +exports.dynamicImport = async function dynamicImport(path) { try { - return import(fs.existsSync(path) ? url.pathToFileURL(path) : path); + return await import(fs.existsSync(path) ? url.pathToFileURL(path) : path); } catch (error) { return Promise.reject(error); } diff --git a/packages/api/core/src/api/package.ts b/packages/api/core/src/api/package.ts index e148d11265..3d7e1ca9af 100644 --- a/packages/api/core/src/api/package.ts +++ b/packages/api/core/src/api/package.ts @@ -25,9 +25,9 @@ const d = debug('electron-forge:packager'); /** * Resolves hooks if they are a path to a file (instead of a `Function`). */ -function resolveHooks(hooks: (string | F)[] | undefined, dir: string) { +async function resolveHooks(hooks: (string | F)[] | undefined, dir: string) { if (hooks) { - return Promise.all(hooks.map(async (hook) => (typeof hook === 'string' ? ((await importSearch(dir, [hook])) as F) : hook))); + return await Promise.all(hooks.map(async (hook) => (typeof hook === 'string' ? ((await importSearch(dir, [hook])) as F) : hook))); } return []; diff --git a/packages/api/core/src/util/forge-config.ts b/packages/api/core/src/util/forge-config.ts index 044242e47f..573de81a0f 100644 --- a/packages/api/core/src/util/forge-config.ts +++ b/packages/api/core/src/util/forge-config.ts @@ -155,7 +155,7 @@ export default async (dir: string): Promise => { const templateObj = { ...packageJSON, year: new Date().getFullYear() }; renderConfigTemplate(dir, templateObj, resolvedForgeConfig); - resolvedForgeConfig.pluginInterface = new PluginInterface(dir, resolvedForgeConfig); + resolvedForgeConfig.pluginInterface = await PluginInterface.create(dir, resolvedForgeConfig); resolvedForgeConfig = await runMutatingHook(resolvedForgeConfig, 'resolveForgeConfig', resolvedForgeConfig); diff --git a/packages/api/core/src/util/plugin-interface.ts b/packages/api/core/src/util/plugin-interface.ts index 9308cb624c..63789eb6b5 100644 --- a/packages/api/core/src/util/plugin-interface.ts +++ b/packages/api/core/src/util/plugin-interface.ts @@ -25,30 +25,42 @@ function isForgePlugin(plugin: IForgePlugin | unknown): plugin is IForgePlugin { } export default class PluginInterface implements IForgePluginInterface { - private plugins: Promise[]; + private plugins: IForgePlugin[] = []; + private _pluginPromise: Promise = Promise.resolve(); private config: ResolvedForgeConfig; - constructor(dir: string, forgeConfig: ResolvedForgeConfig) { - this.plugins = forgeConfig.plugins.map(async (plugin) => { - if (isForgePlugin(plugin)) { - return plugin; - } + static async create(dir: string, forgeConfig: ResolvedForgeConfig): Promise { + const int = new PluginInterface(dir, forgeConfig); + await int._pluginPromise; + return int; + } - if (typeof plugin === 'object' && 'name' in plugin && 'config' in plugin) { - const { name: pluginName, config: opts } = plugin; - if (typeof pluginName !== 'string') { - throw new Error(`Expected plugin[0] to be a string but found ${pluginName}`); + private constructor(dir: string, forgeConfig: ResolvedForgeConfig) { + this._pluginPromise = Promise.all( + forgeConfig.plugins.map(async (plugin): Promise => { + if (isForgePlugin(plugin)) { + return plugin; } - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const Plugin = await importSearch(dir, [pluginName]); - if (!Plugin) { - throw new Error(`Could not find module with name: ${pluginName}. Make sure it's listed in the devDependencies of your package.json`); + + if (typeof plugin === 'object' && 'name' in plugin && 'config' in plugin) { + const { name: pluginName, config: opts } = plugin; + if (typeof pluginName !== 'string') { + throw new Error(`Expected plugin[0] to be a string but found ${pluginName}`); + } + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const Plugin = await importSearch(dir, [pluginName]); + if (!Plugin) { + throw new Error(`Could not find module with name: ${pluginName}. Make sure it's listed in the devDependencies of your package.json`); + } + return new Plugin(opts); } - return new Plugin(opts); - } - throw new Error(`Expected plugin to either be a plugin instance or a { name, config } object but found ${JSON.stringify(plugin)}`); + throw new Error(`Expected plugin to either be a plugin instance or a { name, config } object but found ${JSON.stringify(plugin)}`); + }) + ).then((plugins) => { + this.plugins = plugins; + return; }); // TODO: fix hack // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -61,7 +73,7 @@ export default class PluginInterface implements IForgePluginInterface { }); for (const plugin of this.plugins) { - void plugin.then((plugin) => plugin.init(dir, forgeConfig)); + plugin.init(dir, forgeConfig); } this.triggerHook = this.triggerHook.bind(this); @@ -69,7 +81,7 @@ export default class PluginInterface implements IForgePluginInterface { } async triggerHook(hookName: Hook, hookArgs: ForgeSimpleHookSignatures[Hook]): Promise { - for await (const plugin of this.plugins) { + for (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeSimpleHookFn[] | ForgeSimpleHookFn; if (hooks) { @@ -89,7 +101,7 @@ export default class PluginInterface implements IForgePluginInterface { ): Promise { const tasks: ForgeListrTaskDefinition[] = []; - for await (const plugin of this.plugins) { + for (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeSimpleHookFn[] | ForgeSimpleHookFn; if (hooks) { @@ -123,7 +135,7 @@ export default class PluginInterface implements IForgePluginInterface { ...item: ForgeMutatingHookSignatures[Hook] ): Promise { let result: ForgeMutatingHookSignatures[Hook][0] = item[0]; - for await (const plugin of this.plugins) { + for (const plugin of this.plugins) { if (typeof plugin.getHooks === 'function') { let hooks = plugin.getHooks()[hookName] as ForgeMutatingHookFn[] | ForgeMutatingHookFn; if (hooks) { @@ -140,7 +152,7 @@ export default class PluginInterface implements IForgePluginInterface { async overrideStartLogic(opts: StartOptions): Promise { let newStartFn; const claimed: string[] = []; - for await (const plugin of this.plugins) { + for (const plugin of this.plugins) { if (typeof plugin.startLogic === 'function' && plugin.startLogic !== PluginBase.prototype.startLogic) { claimed.push(plugin.name); newStartFn = plugin.startLogic; From d1c1aee87d35f139ef925770af87bfc135768dd5 Mon Sep 17 00:00:00 2001 From: SpacingBat3 Date: Sun, 14 Jul 2024 21:44:00 +0200 Subject: [PATCH 9/9] fix(core): Move plugins init to promise. This should fix regression caused by bd6ea70473790e5436c20a9ea5bf851782e59e03. --- packages/api/core/src/util/plugin-interface.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/api/core/src/util/plugin-interface.ts b/packages/api/core/src/util/plugin-interface.ts index 63789eb6b5..1cabfa6f5d 100644 --- a/packages/api/core/src/util/plugin-interface.ts +++ b/packages/api/core/src/util/plugin-interface.ts @@ -60,6 +60,9 @@ export default class PluginInterface implements IForgePluginInterface { }) ).then((plugins) => { this.plugins = plugins; + for (const plugin of this.plugins) { + plugin.init(dir, forgeConfig); + } return; }); // TODO: fix hack @@ -71,11 +74,6 @@ export default class PluginInterface implements IForgePluginInterface { configurable: false, writable: false, }); - - for (const plugin of this.plugins) { - plugin.init(dir, forgeConfig); - } - this.triggerHook = this.triggerHook.bind(this); this.overrideStartLogic = this.overrideStartLogic.bind(this); }