Merge pull request #5751 from NomicFoundation/fix/run-sequential-hook…

…-ordering

fix: rework `runSequentialHandlers` plugin ordering

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<Array<HardhatHooks[HookCategoryNameT][HookNameT]>> {
-    const pluginHooks = await this.#getPluginHooks(hookCategoryName, hookName);
-
-    const dynamicHooks = await this.#getDynamicHooks(
-      hookCategoryName,
-      hookName,
-    );
-
-    const r = [...dynamicHooks, ...pluginHooks];
-
-    return r;
-  }
-
   public registerHandlers<HookCategoryNameT extends keyof HardhatHooks>(
     hookCategoryName: HookCategoryNameT,
     hookHandlerCategory: Partial<HardhatHooks[HookCategoryNameT]>,
@@ -114,7 +95,10 @@ export class HookManagerImplementation implements HookManager {
     params: InitialChainedHookParams<HookCategoryNameT, HookT>,
     defaultImplementation: LastParameter<HookT>,
   ): Promise<Awaited<Return<HardhatHooks[HookCategoryNameT][HookNameT]>>> {
-    const handlers = await this.getHandlers(hookCategoryName, hookName);
+    const handlers = await this.#getHandlersInChainedRunningOrder(
+      hookCategoryName,
+      hookName,
+    );
 
     let handlerParams: Parameters<typeof defaultImplementation>;
     if (hookCategoryName !== "config") {
@@ -153,7 +137,10 @@ export class HookManagerImplementation implements HookManager {
   ): Promise<
     Array<Awaited<Return<HardhatHooks[HookCategoryNameT][HookNameT]>>>
   > {
-    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<Awaited<Return<HardhatHooks[HookCategoryNameT][HookNameT]>>>
   > {
-    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<Array<HardhatHooks[HookCategoryNameT][HookNameT]>> {
+    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<Array<HardhatHooks[HookCategoryNameT][HookNameT]>> {
+    const handlersInChainedOrder = await this.#getHandlersInChainedRunningOrder(
+      hookCategoryName,
+      hookName,
+    );
+
+    return handlersInChainedOrder.reverse();
+  }
+
   async #getDynamicHooks<
     HookCategoryNameT extends keyof HardhatHooks,
     HookNameT extends keyof HardhatHooks[HookCategoryNameT],

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<Array<HardhatHooks[HookCategoryNameT][HookNameT]>>;
-
   /**
    * 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<Awaited<Return<HookT>>>;
 
   /**
-   * 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.