fix: apply macros before merging hooks in group method

[tt-a1i]

Summary

Fixes #1586 - macro-defined beforeHandle hooks were running after nested plugin resolve hooks instead of before.

Root Cause: The group method was merging hooks via mergeHook(hook, localHook) before expanding macro options. The macro expansion happened later in add(), but by then the nested plugin hooks were already in the array, so macro hooks were appended at the end.

Fix:

1. Call applyMacro(hook) before mergeHook() in the group method (similar to how guard method already does this at line 4504)
2. Properly merge hook.standaloneValidator (from macro schema) with localHook.standaloneValidator to prevent macro-defined schema validation from being lost

Test Cases Added

Four test cases in test/core/macro-lifecycle.test.ts:

1. macro beforeHandle runs before nested plugin resolve: Verifies that when a group uses { isSignedIn: true } macro, the macro's beforeHandle runs before any nested plugin's resolve hook
2. hooks run in registration order: Verifies that resolve and beforeHandle share the same queue and run in registration order
3. macro in group with successful auth: Verifies the correct execution order when auth succeeds
4. preserve macro schema when merging: Verifies that macro-defined schema validation is preserved when used with nested plugins

Before Fix

Execution order: [ "auth.resolve", "data.resolve", "requireAuth.beforeHandle" ]

After Fix

Execution order: [ "auth.resolve", "requireAuth.beforeHandle", "data.resolve" ]

Test Results

• All existing tests pass (except pre-existing flaky stop.test.ts)
• All 4 new tests pass
• All macro and lifecycle related tests verified

Summary by CodeRabbit

• Bug Fixes

  • Ensured macros are expanded before group merging, fixing execution order in grouped routes and improving authorization and dependency resolution.
  • Corrected merging of standalone validators so combined validators from macros, local hooks, and group schemas are applied consistently.

• Tests

  • Added comprehensive tests validating macro and hook lifecycle ordering across grouped routes, registration order, and authorization flows.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Apply macro expansion to the local group hook before computing standalone schema and merging hooks, and add tests validating macro and hook lifecycle ordering across grouped routes and nested plugins.

Changes

Cohort / File(s)	Summary
Core group merge change src/index.ts	Add a macro expansion step (this.applyMacro(hook)) before computing hasStandaloneSchema, and revise merging logic to combine hook.standaloneValidator, localHook.standaloneValidator, and inline schema pieces into standaloneValidator only when applicable.
Macro lifecycle tests test/core/macro-lifecycle.test.ts	New test suite validating macro beforeHandle ordering vs nested plugin resolve, registration-order behavior within queues, macro interaction in grouped routes, error/authorization flows, and schema preservation during macro+plugin hook merges.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant GroupMacro as Group Macro (beforeHandle)
    participant PluginResolve as Nested Plugin (resolve)
    participant Handler as Route Handler

    Client->>GroupMacro: request enters group
    Note right of GroupMacro: macro expansion applied to local hook\n(before merging)
    GroupMacro->>PluginResolve: (queue ordering) beforeHandle runs first
    PluginResolve->>Handler: resolved context available
    Handler-->>Client: response

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review points:
  • src/index.ts: confirm macro is applied at the correct position and merging logic covers all standalone schema combinations without dropping validators.
  • test/core/macro-lifecycle.test.ts: verify test coverage matches the reported issue scenarios (#1586) and that assertions correctly reflect lifecycle ordering.
  • Check for side effects on other lifecycle queues (e.g., onAfterHandle, onError) and any interactions with dynamic/plugin registration ordering.

🐰
I hopped through hooks to set things right,
Applied my macro before the nested light.
No more surprises where resolves would leap,
Guards stand first — the path is safe to keep. 🥕

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title accurately summarizes the primary change: applying macros before merging hooks in the group method to fix hook execution ordering.
Linked Issues check	✅ Passed	The changes directly address issue #1586 by applying macro expansion before hook merging in the group method, ensuring macro-defined beforeHandle runs before nested plugin resolve hooks with proper execution ordering.
Out of Scope Changes check	✅ Passed	All code changes are directly scoped to fixing the hook execution ordering issue: macro expansion in group method and comprehensive lifecycle tests validating the fix.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/core/macro-lifecycle.test.ts (1)

95-141: Comprehensive test for authorized path.

This test effectively validates the fix when authentication succeeds, ensuring all hooks execute in the correct order: auth.resolve -> requireAuth.beforeHandle -> data.resolve. It nicely complements the unauthorized path test by confirming the fix works correctly in both success and failure scenarios.

Minor suggestion: The test description "should demonstrate the issue with macro in group" might be clearer as "should run macro beforeHandle between auth and data resolves when authorized" to better reflect that it's testing the successful flow after the fix.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 218c12a and 8abd7d7.

📒 Files selected for processing (2)

• src/index.ts (1 hunks)
• test/core/macro-lifecycle.test.ts (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

test/core/macro-lifecycle.test.ts (1)

src/index.ts (2)

• Elysia (190-8169)
• Elysia (8171-8171)

🔇 Additional comments (3)

src/index.ts (1)

4045-4047: LGTM! Correct fix for macro execution order.

The placement of applyMacro(hook) before mergeHook() ensures that macro-defined hooks (e.g., beforeHandle from a macro option like requireAuth: true) are expanded and registered before merging with nested plugin hooks. This fixes the execution order issue where macro beforeHandle was running after nested plugin resolve hooks.

This change aligns with the guard method's behavior (which calls applyMacro at line 4508), ensuring consistency across the API.

test/core/macro-lifecycle.test.ts (2)

5-59: Well-designed test case for unauthorized path.

This test effectively validates the core fix by verifying that:

1. authService.resolve runs first
2. isSignedIn.beforeHandle (macro-defined) runs next and throws
3. dbClient.resolve never executes (preventing the "User ID is required" error)

The execution order tracking and assertions correctly demonstrate that macro beforeHandle now executes before nested plugin resolve hooks, fixing the issue described in #1586.

---

61-93: Good validation of documented same-queue behavior.

This test confirms that resolve and beforeHandle hooks share the same execution queue and run in registration order, which is the documented behavior that makes the fix possible. The clear execution order assertion (resolve1 -> beforeHandle1 -> resolve2 -> beforeHandle2) validates this fundamental assumption.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

test/core/macro-lifecycle.test.ts (4)

50-51: Remove debug console.log statements.

These debug statements should be removed before merging to keep test output clean.

-		console.log('Execution order:', executionOrder)
-		console.log('Response:', body)
-

---

83-83: Remove debug console.log statement.

-		console.log('Execution order:', executionOrder)
-

---

131-132: Remove debug console.log statements.

-		console.log('Execution order:', executionOrder)
-		console.log('Error:', errorMessage)
-

---

97-97: Unused variable errorMessage.

The variable is assigned but never used in assertions. Consider either removing it or adding an assertion that verifies no error occurred.

-		let errorMessage = ''
-
 		const authPlugin = new Elysia({ name: 'auth' })

Or add an assertion:

 		expect(executionOrder).toEqual([
 			'auth.resolve',
 			'requireAuth.beforeHandle',
 			'data.resolve'
 		])
+		expect(errorMessage).toBe('')
 	})

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8abd7d7 and fc70333.

📒 Files selected for processing (2)

• src/index.ts (2 hunks)
• test/core/macro-lifecycle.test.ts (1 hunks)

🔇 Additional comments (4)

src/index.ts (2)

4045-4047: LGTM - Fix correctly applies macros before merging hooks.

This aligns the group() method behavior with guard(), which already applies macros before processing. The comment clearly explains the rationale for the change.

---

4069-4091: LGTM - Proper three-way merge of standalone validators.

The logic correctly combines validators from:

1. hook.standaloneValidator (from macro expansion)
2. localHook.standaloneValidator (from nested plugin)
3. Inline schema object (if hasStandaloneSchema is true)

The conditional ensures the array is only created when at least one source has validators, avoiding unnecessary empty arrays.

test/core/macro-lifecycle.test.ts (2)

5-59: Well-structured test covering the core fix.

This test effectively validates the fix from issue #1586 by verifying that:

1. The macro's beforeHandle runs before nested plugin's resolve
2. When beforeHandle throws, the nested resolve is never executed
3. The correct error message ("Unauthorized") is returned

The test setup with auth service, db client, and feature module mirrors real-world usage patterns.

---

143-191: Good coverage of schema preservation.

This test validates an important edge case: ensuring macro-defined schemas (like body validation) are preserved when merging with nested plugin hooks. The test correctly verifies both valid (200) and invalid (422) responses.