diff --git a/v-next/hardhat/src/internal/core/hook-manager.ts b/v-next/hardhat/src/internal/core/hook-manager.ts index e044046a14..a312b6ae40 100644 --- a/v-next/hardhat/src/internal/core/hook-manager.ts +++ b/v-next/hardhat/src/internal/core/hook-manager.ts @@ -57,25 +57,6 @@ export class HookManagerImplementation implements HookManager { this.#context = context; } - public async getHandlers< - HookCategoryNameT extends keyof HardhatHooks, - HookNameT extends keyof HardhatHooks[HookCategoryNameT], - >( - hookCategoryName: HookCategoryNameT, - hookName: HookNameT, - ): Promise> { - const pluginHooks = await this.#getPluginHooks(hookCategoryName, hookName); - - const dynamicHooks = await this.#getDynamicHooks( - hookCategoryName, - hookName, - ); - - const r = [...dynamicHooks, ...pluginHooks]; - - return r; - } - public registerHandlers( hookCategoryName: HookCategoryNameT, hookHandlerCategory: Partial, @@ -114,7 +95,10 @@ export class HookManagerImplementation implements HookManager { params: InitialChainedHookParams, defaultImplementation: LastParameter, ): Promise>> { - const handlers = await this.getHandlers(hookCategoryName, hookName); + const handlers = await this.#getHandlersInChainedRunningOrder( + hookCategoryName, + hookName, + ); let handlerParams: Parameters; if (hookCategoryName !== "config") { @@ -153,7 +137,10 @@ export class HookManagerImplementation implements HookManager { ): Promise< Array>> > { - const handlers = await this.getHandlers(hookCategoryName, hookName); + const handlers = await this.#getHandlersInSequentialRunningOrder( + hookCategoryName, + hookName, + ); let handlerParams: any; if (hookCategoryName !== "config") { @@ -186,7 +173,11 @@ export class HookManagerImplementation implements HookManager { ): Promise< Array>> > { - const handlers = await this.getHandlers(hookCategoryName, hookName); + // The ordering of handlers is unimportant here, as they are run in parallel + const handlers = await this.#getHandlersInChainedRunningOrder( + hookCategoryName, + hookName, + ); let handlerParams: any; if (hookCategoryName !== "config") { @@ -205,6 +196,38 @@ export class HookManagerImplementation implements HookManager { ); } + async #getHandlersInChainedRunningOrder< + HookCategoryNameT extends keyof HardhatHooks, + HookNameT extends keyof HardhatHooks[HookCategoryNameT], + >( + hookCategoryName: HookCategoryNameT, + hookName: HookNameT, + ): Promise> { + const pluginHooks = await this.#getPluginHooks(hookCategoryName, hookName); + + const dynamicHooks = await this.#getDynamicHooks( + hookCategoryName, + hookName, + ); + + return [...dynamicHooks, ...pluginHooks]; + } + + async #getHandlersInSequentialRunningOrder< + HookCategoryNameT extends keyof HardhatHooks, + HookNameT extends keyof HardhatHooks[HookCategoryNameT], + >( + hookCategoryName: HookCategoryNameT, + hookName: HookNameT, + ): Promise> { + const handlersInChainedOrder = await this.#getHandlersInChainedRunningOrder( + hookCategoryName, + hookName, + ); + + return handlersInChainedOrder.reverse(); + } + async #getDynamicHooks< HookCategoryNameT extends keyof HardhatHooks, HookNameT extends keyof HardhatHooks[HookCategoryNameT], diff --git a/v-next/hardhat/src/types/hooks.ts b/v-next/hardhat/src/types/hooks.ts index 1d128e09d4..482e6d7362 100644 --- a/v-next/hardhat/src/types/hooks.ts +++ b/v-next/hardhat/src/types/hooks.ts @@ -252,23 +252,6 @@ export interface HardhatRuntimeEnvironmentHooks { * - In parallel, where all handlers are called at the same time. */ export interface HookManager { - /** - * Returns an array with the existing handlers for a hook, in priority order. - * - * The priority is defined like this: - * - Dynamically registered handlers come first, in the reverse order they - * were registered. - * - Plugin handlers come last, in the same reverse of the resolved plugins - * list, as seen in `HardhatConfig#plugins`. - */ - getHandlers< - HookCategoryNameT extends keyof HardhatHooks, - HookNameT extends keyof HardhatHooks[HookCategoryNameT], - >( - hookCategoryName: HookCategoryNameT, - hookName: HookNameT, - ): Promise>; - /** * Registers handlers for a category of hooks. */ @@ -288,8 +271,12 @@ export interface HookManager { /** * Runs the existing handlers of a hook in a chained fashion. * - * This chain has the same order than the one returned by `getHooks`, plus the - * default handler which is added at the end. + * This chain has the following priority order: + * - Dynamically registered handlers come first, in the reverse order they + * were registered. + * - Plugin handlers come last, in the same reverse of the resolved plugins + * list, as seen in `HardhatConfig#plugins`. + * - The default handler is called last. * * The first handler is called with `initialParams`, and then it can call * `next` to call the next handler in the chain. @@ -321,8 +308,11 @@ export interface HookManager { ): Promise>>; /** - * Runs all the handlers for a hook in the same order that `getHandlers` - * returns them. + * Runs all the handlers for a hook in the following priority order: + * - Plugin handlers come first, in the resolved order of the plugins + * list, hence if B has a dependency on A, the order will be A then B. + * - Dynamically registered handlers come last, in the order they + * were registered. * * @param hookCategoryName The name of the category of the hook whose * handlers should be run. diff --git a/v-next/hardhat/test/internal/core/hook-manager.ts b/v-next/hardhat/test/internal/core/hook-manager.ts index 10dc3b9056..db362e4189 100644 --- a/v-next/hardhat/test/internal/core/hook-manager.ts +++ b/v-next/hardhat/test/internal/core/hook-manager.ts @@ -1,4 +1,4 @@ -/* eslint-disable @typescript-eslint/consistent-type-assertions -- the +/* eslint-disable @typescript-eslint/consistent-type-assertions -- the sequential tests require casting - see the `runSequentialHandlers` describe */ import type { HardhatUserConfig } from "../../../src/config.js"; @@ -36,163 +36,356 @@ describe("HookManager", () => { describe("plugin hooks", () => { describe("running", () => { let hre: HardhatRuntimeEnvironment; - let forceConfigValidationErrorFromPlugin: boolean; - let sequence: string[]; - beforeEach(async () => { - sequence = []; - forceConfigValidationErrorFromPlugin = false; + describe("runHandlerChain", () => { + let sequence: string[]; + + beforeEach(async () => { + sequence = []; + + const examplePlugin1: HardhatPlugin = { + id: "example1", + hookHandlers: { + config: async () => { + const handlers: Partial = { + extendUserConfig: async ( + config: HardhatUserConfig, + next: ( + nextConfig: HardhatUserConfig, + ) => Promise, + ) => { + sequence.push("FromExamplePlugin1:before"); + const newConfig = await next(config); + sequence.push("FromExamplePlugin2:after"); + + return newConfig; + }, + }; + + return handlers; + }, + }, + }; + + const examplePlugin2: HardhatPlugin = { + id: "example2", + dependencies: [async () => examplePlugin1], + hookHandlers: { + config: async () => { + const handlers: Partial = { + extendUserConfig: async ( + config: HardhatUserConfig, + next: ( + nextConfig: HardhatUserConfig, + ) => Promise, + ) => { + sequence.push("FromExamplePlugin2:before"); + const newConfig = await next(config); + sequence.push("FromExamplePlugin2:after"); + + return newConfig; + }, + }; + + return handlers; + }, + }, + }; - const examplePlugin: HardhatPlugin = { - id: "example", - hookHandlers: { - config: async () => { - const handlers: Partial = { - validateUserConfig: async ( - _config: HardhatUserConfig, - ): Promise => { - if (forceConfigValidationErrorFromPlugin) { - return [ - { - path: [], - message: "FromPlugin", - }, - ]; - } - - return []; - }, - extendUserConfig: async ( - config: HardhatUserConfig, - next: ( - nextConfig: HardhatUserConfig, - ) => Promise, - ) => { - sequence.push("FromPlugin:before"); - const newConfig = await next(config); - sequence.push("FromPlugin:after"); - - return newConfig; - }, - }; + hre = await HardhatRuntimeEnvironmentImplementation.create( + { plugins: [examplePlugin1, examplePlugin2] }, + {}, + ); + }); - return handlers; + it("should run handlers first (in reverse order of registration), then plugins in reverse dependency order then the default", async () => { + hre.hooks.registerHandlers("config", { + extendUserConfig: async ( + config: HardhatUserConfig, + next: ( + nextConfig: HardhatUserConfig, + ) => Promise, + ) => { + sequence.push("FromHandler1:before"); + const newConfig = await next(config); + sequence.push("FromHandler1:after"); + + return newConfig; }, - hre: async () => { - const handlers = { - testExample: async ( - _context: HookContext, - _input: string, - ): Promise => { - return "FromPlugin"; - }, - } as Partial; + }); + + hre.hooks.registerHandlers("config", { + extendUserConfig: async ( + config: HardhatUserConfig, + next: ( + nextConfig: HardhatUserConfig, + ) => Promise, + ) => { + sequence.push("FromHandler2:before"); + const newConfig = await next(config); + sequence.push("FromHandler2:after"); + + return newConfig; + }, + }); + + // We clear the sequense here, as we used the plugin already during the + // initialization of the HRE + sequence = []; - return handlers; + await hre.hooks.runHandlerChain( + "config", + "extendUserConfig", + [{}], + async () => { + sequence.push("default"); + return {}; }, - }, - }; + ); - hre = await HardhatRuntimeEnvironmentImplementation.create( - { plugins: [examplePlugin] }, - {}, - ); + assert.deepEqual(sequence, [ + "FromHandler2:before", + "FromHandler1:before", + "FromExamplePlugin2:before", + "FromExamplePlugin1:before", + "default", + "FromExamplePlugin2:after", + "FromExamplePlugin2:after", + "FromHandler1:after", + "FromHandler2:after", + ]); + }); }); - it("should use plugins during handler runs", async () => { - hre.hooks.registerHandlers("config", { - extendUserConfig: async ( - config: HardhatUserConfig, - next: (nextConfig: HardhatUserConfig) => Promise, - ) => { - sequence.push("FromHandler:before"); - const newConfig = await next(config); - sequence.push("FromHandler:after"); + describe("runSequentialHandlers", () => { + describe("plugin/handler exection order interactions", () => { + beforeEach(async () => { + const examplePlugin1: HardhatPlugin = { + id: "example1", + hookHandlers: { + hre: async () => { + const handlers = { + testExample: async ( + _context: HookContext, + _input: string, + ): Promise => { + return "FromExamplePlugin1"; + }, + } as Partial; + + return handlers; + }, + }, + }; + + const examplePlugin2: HardhatPlugin = { + id: "example2", + hookHandlers: { + hre: async () => { + const handlers = { + testExample: async ( + _context: HookContext, + _input: string, + ): Promise => { + return "FromExamplePlugin2"; + }, + } as Partial; + + return handlers; + }, + }, + }; - return newConfig; - }, + hre = await HardhatRuntimeEnvironmentImplementation.create( + { plugins: [examplePlugin1, examplePlugin2] }, + {}, + ); + }); + + it("Should run handlers first in reverse registration order, then plugins in dependency order", async () => { + hre.hooks.registerHandlers("hre", { + testExample: async ( + _context: HookContext, + _input: string, + ): Promise => { + return "FromHandler1"; + }, + } as Partial); + + hre.hooks.registerHandlers("hre", { + testExample: async ( + _context: HookContext, + _input: string, + ): Promise => { + return "FromHandler2"; + }, + } as Partial); + + const result = await hre.hooks.runSequentialHandlers( + "hre", + "testExample" as any, + ["input"], + ); + + assert.deepEqual(result, [ + "FromExamplePlugin1", + "FromExamplePlugin2", + "FromHandler1", + "FromHandler2", + ]); + }); }); - // We clear the sequense here, as we used the plugin already during the - // initialization of the HRE - sequence = []; + /** + * This test was added in response to a bug giving the wrong + * execution order for plugins that use the same hook and depend on each + * other. + * If you have two plugins A and B that both update the `hre` through the `hre/created` + * hook, where B depends on A, then A should run first and B should run second. + * A concrete example would be a plugin that adds `hre.artifacts`, and then a plugin + * that mocks the artifacts. The mock plugin should be able to depend on the real + * plugin, and expect to be executed after the real plugin. + */ + describe("multiple plugin execution order", () => { + beforeEach(async () => { + const plugin1: HardhatPlugin = { + id: "plugin1", + hookHandlers: { + hre: async () => { + const handlers = { + created: async ( + _context: HookContext, + givenHre: HardhatRuntimeEnvironment, + ): Promise => { + givenHre.config.paths.tests = + "./test-folder-from-plugin1"; + }, + } as Partial; + + return handlers; + }, + }, + }; + + const overridingPlugin2: HardhatPlugin = { + id: "overriding-plugin2", + dependencies: [async () => plugin1], + hookHandlers: { + hre: async () => { + const handlers = { + created: async ( + _context: HookContext, + givenHre: HardhatRuntimeEnvironment, + ): Promise => { + givenHre.config.paths.tests = + "./test-folder-from-overriding-plugin2"; + }, + } as Partial; + + return handlers; + }, + }, + }; - await hre.hooks.runHandlerChain( - "config", - "extendUserConfig", - [{}], - async () => { - sequence.push("default"); - return {}; - }, - ); + hre = await HardhatRuntimeEnvironmentImplementation.create( + { plugins: [plugin1, overridingPlugin2] }, + {}, + ); + }); - assert.deepEqual(sequence, [ - "FromHandler:before", - "FromPlugin:before", - "default", - "FromPlugin:after", - "FromHandler:after", - ]); + it("Should invoke plugins in dependency order during a sequential run", async () => { + const result = await hre.hooks.runSequentialHandlers( + "hre", + "created", + [hre], + ); + + assert.equal(result.length, 2); + assert.equal( + hre.config.paths.tests, + "./test-folder-from-overriding-plugin2", + ); + }); + }); }); - it("Should use plugins during a sequential run", async () => { - hre.hooks.registerHandlers("hre", { - testExample: async ( - _context: HookContext, - _input: string, - ): Promise => { - return "FromHandler"; - }, - } as Partial); + describe("runParallelHandlers", () => { + let forceConfigValidationErrorFromPlugin: boolean; + + beforeEach(async () => { + forceConfigValidationErrorFromPlugin = false; + + const examplePlugin: HardhatPlugin = { + id: "example", + hookHandlers: { + config: async () => { + const handlers: Partial = { + validateUserConfig: async ( + _config: HardhatUserConfig, + ): Promise => { + if (forceConfigValidationErrorFromPlugin) { + return [ + { + path: [], + message: "FromPlugin", + }, + ]; + } + + return []; + }, + }; + + return handlers; + }, + }, + }; - const result = await hre.hooks.runSequentialHandlers( - "hre", - "testExample" as any, - ["input"], - ); + hre = await HardhatRuntimeEnvironmentImplementation.create( + { plugins: [examplePlugin] }, + {}, + ); + }); - assert.deepEqual(result, ["FromHandler", "FromPlugin"]); - }); + it("should use plugins during parallel handlers runs", async () => { + const originalConfig: HardhatUserConfig = {}; + + hre.hooks.registerHandlers("config", { + validateUserConfig: async ( + _config: HardhatUserConfig, + ): Promise => { + return [ + { + path: [], + message: "FromRegisteredHandler", + }, + ]; + }, + }); - it("should use plugins during parallel handlers runs", async () => { - const originalConfig: HardhatUserConfig = {}; + forceConfigValidationErrorFromPlugin = true; - hre.hooks.registerHandlers("config", { - validateUserConfig: async ( - _config: HardhatUserConfig, - ): Promise => { - return [ + const results = await hre.hooks.runParallelHandlers( + "config", + "validateUserConfig", + [originalConfig], + ); + + assert.deepEqual(results, [ + [ { path: [], message: "FromRegisteredHandler", }, - ]; - }, + ], + [ + { + path: [], + message: "FromPlugin", + }, + ], + ]); }); - - forceConfigValidationErrorFromPlugin = true; - - const results = await hre.hooks.runParallelHandlers( - "config", - "validateUserConfig", - [originalConfig], - ); - - assert.deepEqual(results, [ - [ - { - path: [], - message: "FromRegisteredHandler", - }, - ], - [ - { - path: [], - message: "FromPlugin", - }, - ], - ]); }); }); @@ -530,7 +723,7 @@ describe("HookManager", () => { ["input"] as any, ); - assert.deepEqual(result, ["third", "second", "first"]); + assert.deepEqual(result, ["first", "second", "third"]); }); it("Should let handlers access the passed context (for non-config hooks)", async () => {