Skip to content

Commit

Permalink
fix: clarify ordering for dynamic hooks in run sequential
Browse files Browse the repository at this point in the history
We change the ordering of dynamic hooks in run sequential, so that it is
in effect the reverse of running chained.

Plugins are now run first, with dynamic handlers coming second.

Plugins run in resolved dependency order (if B depends on A, A runs
before B).

Dynamic handlers are run in the order they are registered.

The tests and docs have been updated to reflect this change.
  • Loading branch information
kanej committed Sep 16, 2024
1 parent 0c77c38 commit 3dd37fe
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 12 deletions.
8 changes: 2 additions & 6 deletions v-next/hardhat/src/internal/core/hook-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,16 +220,12 @@ export class HookManagerImplementation implements HookManager {
hookCategoryName: HookCategoryNameT,
hookName: HookNameT,
): Promise<Array<HardhatHooks[HookCategoryNameT][HookNameT]>> {
const reversedPluginHooks = (
await this.#getPluginHooks(hookCategoryName, hookName)
).reverse();

const dynamicHooks = await this.#getDynamicHooks(
const handlersInChainedOrder = await this.#getHandlersInChainedRunningOrder(
hookCategoryName,
hookName,
);

return [...dynamicHooks, ...reversedPluginHooks];
return handlersInChainedOrder.reverse();
}

async #getDynamicHooks<
Expand Down
6 changes: 3 additions & 3 deletions v-next/hardhat/src/types/hooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -309,10 +309,10 @@ export interface HookManager {

/**
* Runs all the handlers for a hook in the following priority order:
* - Dynamically registered handlers come first, in the reverse order they
* were registered.
* - Plugin handlers come last, in the resolved order of the plugins
* - 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.
Expand Down
6 changes: 3 additions & 3 deletions v-next/hardhat/test/internal/core/hook-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,10 @@ describe("HookManager", () => {
);

assert.deepEqual(result, [
"FromHandler2",
"FromHandler1",
"FromExamplePlugin1",
"FromExamplePlugin2",
"FromHandler1",
"FromHandler2",
]);
});
});
Expand Down Expand Up @@ -723,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 () => {
Expand Down

0 comments on commit 3dd37fe

Please sign in to comment.