Skip to content

Conversation

@hunterwilhelm
Copy link
Contributor

@hunterwilhelm hunterwilhelm commented Nov 17, 2025

Goal: Enable macros to introspect route metadata (path and method) during route registration for documentation generation and analysis.

Features Added:

  1. Route metadata in macro introspection

    • Added MacroIntrospectionMetadata with path and method
    • introspect now receives route metadata as a second parameter
  2. Path resolution

    • Paths are normalized before introspection
    • Prefixes are resolved so metadata uses the final route path
    • Groups defer introspection until routes are added to the parent with fully resolved paths
  3. Guard-level macro introspection

    • Guard-level macros are introspected per route with the resolved path
    • Introspection runs once per route with complete metadata

Behavior Changes:

  • introspect receives (introspectOptions, metadata) where metadata contains { path: string, method: HTTPMethod }
  • Introspection runs only when route metadata is available
  • Paths include prefixes and group paths (e.g., /prefix/group/route/:id)
  • Works with nested groups, guards, and route combinations

Use Case: Enables macros to generate documentation, analyze routes, or perform static analysis using route path and method information.

Summary by CodeRabbit

  • New Features

    • Per-route macro introspection now receives route context (path and HTTP method) when applicable.
    • Introspection is executed per-route (only when full route metadata is available) and integrates across guards/groups without changing non-macro behavior.
    • MacroIntrospectionMetadata type is exposed in the public API.
  • Tests

    • Expanded tests covering macro introspection across guards, groups, nested routes, and edge cases.

@coderabbitai
Copy link

coderabbitai bot commented Nov 17, 2025

Walkthrough

Adds per-route macro introspection: captures macro options from guards/groups, normalizes and prefixes paths before invoking introspection, propagates route metadata (path, method) through macro application, and exposes MacroIntrospectionMetadata in public types. Tests exercise many introspection scenarios.

Changes

Cohort / File(s) Summary
Types & public API
src/types.ts, src/index.ts
Adds MacroIntrospectionMetadata { path, method }; updates MacroProperty.introspect signature to (option: Record<string, any>, context: MacroIntrospectionMetadata); imports and re-exports the new type.
Core introspection plumbing
src/index.ts
Introduces internal guardMacroOptions and skipMacroIntrospection; normalizes and prefixes paths before introspection; merges/restores guard macro options across group/route boundaries; invokes per-route introspect hooks when route metadata is available.
Tests
test/macro/macro.test.ts
Adds extensive tests validating introspection invocation and metadata across inline/longhand macros, guards, groups, nested routes, deduplication, disabled macros, and composition scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant Dev as Developer
    participant Guard as guard()
    participant Group as group()
    participant Add as add()
    participant Macro as applyMacro()
    participant Introspect as introspect hook

    Dev->>Guard: .guard({ ...macroOptions })
    Guard->>Guard: store guardMacroOptions

    Dev->>Group: .group(...)
    Group->>Group: set skipMacroIntrospection for child adds
    Group->>Group: merge guardMacroOptions for children

    Dev->>Add: .add({ path, method })
    Add->>Add: normalize path & apply prefix
    Add->>Add: merge/restore guardMacroOptions
    Add->>Macro: apply macros with route metadata
    Macro->>Introspect: introspect(option, { path, method })
    Introspect-->>Macro: optional result
    Macro-->>Add: expanded macro result
    Add-->>Dev: route registered
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Focus review on src/index.ts flows: correctness of merging/restoring guardMacroOptions, skipMacroIntrospection behavior, and timing/guarding of introspect calls.
  • Verify src/types.ts and src/index.ts exports for compatibility and correct signature changes.
  • Inspect test/macro/macro.test.ts for fragile ordering assumptions and coverage completeness.

Possibly related PRs

  • patch: 1.4.16 #1529 — Modifies macro introspection codepaths and types; likely overlaps with the new introspect hook behavior and type additions.

Poem

🐰 I hopped through guards and groups to spy the route's name,
I sniffed the path and verb and called each macro by name,
I tucked options, waited, then whispered metadata's tune,
A tiny rabbit cheer — introspection under moon. ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding full path and method metadata to macro introspection, which aligns with the PR's core objective of exposing route information to macros.
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

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

@hunterwilhelm
Copy link
Contributor Author

Fixes #1498

@pkg-pr-new
Copy link

pkg-pr-new bot commented Nov 17, 2025

Open in StackBlitz

npm i https://pkg.pr.new/elysiajs/elysia@1557

commit: 6543156

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 78f4828 and 3f106f9.

📒 Files selected for processing (3)
  • src/index.ts (9 hunks)
  • src/types.ts (2 hunks)
  • test/macro/macro.test.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
test/macro/macro.test.ts (1)
src/index.ts (3)
  • Elysia (186-8223)
  • Elysia (8225-8225)
  • options (6283-6372)
src/types.ts (1)
src/index.ts (2)
  • MacroIntrospectionMetadata (8318-8318)
  • HTTPMethod (8297-8297)
src/index.ts (2)
src/types.ts (1)
  • MacroIntrospectionMetadata (1946-1961)
src/utils.ts (2)
  • mergeDeep (52-103)
  • isNotEmpty (1078-1084)
🔇 Additional comments (7)
test/macro/macro.test.ts (7)

1442-1531: LGTM! Comprehensive coverage of introspection styles.

These tests effectively validate the three different ways to define macro introspection (shorthand styles 1 & 2, and longhand). The assertions correctly verify that introspection receives both the macro options and the route metadata (path and method).


1575-1603: LGTM! Validates introspect with resolve.

This test confirms that introspection works correctly when macros also define resolve hooks, which is an important edge case.


1806-1820: LGTM! Validates introspect is not called unnecessarily.

This test correctly verifies that introspection is only invoked for routes that actually use the macro.


1822-1879: LGTM! Comprehensive validation of multiple macro introspection.

This test effectively verifies that multiple macros can coexist and introspect independently, with each macro's introspect function called only for routes that use it.


1881-1903: LGTM! Validates introspect skips disabled macros.

This test correctly verifies that introspection is not called when a macro value is false.


1533-1573: Based on my verification, I can now provide the rewritten response:


The introspection behavior shown in this test is correct and intentional. The PR design includes two-phase introspection per route: guard-level macros are introspected once per route with full metadata, and route-level macros are also introspected once per route. When a route has conflicting macro options (like route-level a: false and guard-level a: true), both levels are appropriately introspected to capture complete metadata. The comments in the code (lines 286, 506) specifically refer to guard-level macros running "once per route"—not to a singular total introspection call per route. Multiple introspection calls are expected and correct when both guard and route-level macros apply to the same route.

Likely an incorrect or invalid review comment.


1605-1804: Incorrect review comment: Macro path resolution behavior is intentional, not an inconsistency.

The tests correctly reflect the designed behavior:

  • Groups defer introspection (via skipMacroIntrospection = true, line 4033): Macros defined inside groups delay introspection until routes are added to the parent, enabling fully-resolved paths including the prefix.

  • Guards do NOT defer introspection: The guard() method (line 4527) does not set skipMacroIntrospection. Instead, it captures macro options from guard hooks for per-route introspection. Macros defined inside guard callbacks introspect immediately with local paths, which is why prefixes are excluded (the prefix belongs to the parent, not the guard scope).

This design is documented in code comments:

  • Group comment (line 4031-4032): "Macro introspection will happen when routes are added to the parent with the fully-resolved path"
  • Guard comment (line 4537): "Capture guard-level macro options so we can introspect them per-route later"

The test expectations correctly verify this behavior:

  • Tests 6-7: Group-scoped macros include prefix + group paths
  • Tests 8-11: Guard-scoped macros exclude prefix (local scope only)

No changes needed.

Likely an incorrect or invalid review comment.

@hunterwilhelm hunterwilhelm force-pushed the feat/add-route-metadata-to-macro-instrospect branch from 3f106f9 to 0771705 Compare November 17, 2025 23:31
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3f106f9 and 0771705.

📒 Files selected for processing (3)
  • src/index.ts (11 hunks)
  • src/types.ts (2 hunks)
  • test/macro/macro.test.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/types.ts
🧰 Additional context used
🧬 Code graph analysis (2)
test/macro/macro.test.ts (1)
src/index.ts (4)
  • Elysia (186-8243)
  • Elysia (8245-8245)
  • options (6303-6392)
  • group (4013-4152)
src/index.ts (2)
src/types.ts (1)
  • MacroIntrospectionMetadata (1946-1961)
src/utils.ts (1)
  • isNotEmpty (1078-1084)

@hunterwilhelm hunterwilhelm force-pushed the feat/add-route-metadata-to-macro-instrospect branch from 0771705 to 6543156 Compare November 18, 2025 16:04
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/index.ts (1)

501-559: add() path normalization and guard-level introspection look correct

Normalizing the path (leading / and configured prefix) before introspection ensures macros see the same path the router uses, and gating guard macro introspection behind !this.skipMacroIntrospection matches the group/guard deferral design. Object-style macros with value === false are correctly treated as disabled, and route-level macros still go through applyMacro with { path, method } metadata.

If you ever want to avoid re-instantiating guard macro hooks per route, you could cache just the introspect functions (and their static options) when capturing guardMacroOptions, but that’s an optional micro-optimization, not required for correctness.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0771705 and 6543156.

📒 Files selected for processing (3)
  • src/index.ts (14 hunks)
  • src/types.ts (2 hunks)
  • test/macro/macro.test.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/types.ts
🧰 Additional context used
🧬 Code graph analysis (2)
test/macro/macro.test.ts (1)
src/index.ts (4)
  • Elysia (186-8315)
  • Elysia (8317-8317)
  • options (6375-6464)
  • group (4024-4163)
src/index.ts (2)
src/types.ts (2)
  • MacroIntrospectionMetadata (1946-1961)
  • AnyLocalHook (1680-1680)
src/utils.ts (3)
  • mergeDeep (52-103)
  • isNotEmpty (1078-1084)
  • mergeHook (225-339)
🔇 Additional comments (8)
test/macro/macro.test.ts (2)

2-3: Imports switch to bun:test is appropriate

Using describe/expect/it from bun:test and importing Elysia from ../../src is consistent with the rest of the test suite; no issues here.


1442-2025: New macro introspection tests give strong behavioral coverage

This block thoroughly exercises the new introspection flow: shorthand/longhand macros, guard-level macros (including conflicting/disabled cases), groups, nested guards/groups, routes without macros, multiple macros on a route, and stale guard({ macro: true }) state being cleared by guard({ macro: false }). The expectations match the implementation in src/index.ts, so this looks solid.

src/index.ts (6)

120-121: MacroIntrospectionMetadata import/export is consistent

Importing MacroIntrospectionMetadata from ./types and re‑exporting it alongside other public types wires the new metadata surface cleanly into the public API without affecting existing types.

Also applies to: 8410-8412


285-294: Internal guard/introspection state is scoped appropriately

Adding guardMacroOptions and skipMacroIntrospection as protected fields keeps the new state internal to Elysia while enabling the per-route guard introspection plumbing you need. The comments clarify their purpose, and they don’t leak into generics or public APIs.


4045-4059: Group handling correctly defers introspection and propagates macros

Marking group child instances with skipMacroIntrospection = true and then merging both extender.macro and instance.guardMacroOptions into the parent before replaying routes ensures:

  • macros defined inside groups are available when the parent introspects, and
  • guard-level macros inside groups are introspected with fully resolved paths (prefix + group segments).

Restoring guardMacroOptions afterward avoids leaking group-specific guard state beyond the merge.

Also applies to: 4075-4087, 4159-4161


4568-4598: 1-arg guard(...): stale guard macro options bug is fixed

The new updatedGuardMacroOptions logic properly:

  • deletes entries for object-style macros explicitly set to false, and
  • only updates this.guardMacroOptions when values change.

This aligns with the tests that guard({ a: false }) should stop introspecting with stale { a: true }, while still allowing function-style macros to use false as a legitimate option value.


4686-4720: 2-arg guard(hook, run) preserves semantics and avoids double introspection

Copying this.guardMacroOptions into the child instance, updating it based on hook, and then:

  • running guard-level introspection on the child as routes are added, and
  • temporarily setting this.skipMacroIntrospection = true and clearing this.guardMacroOptions while merging routes back,

ensures:

  • per-route guard introspection happens once with the child’s view of the path,
  • group-level deferral continues to work (since groups use skipMacroIntrospection on their child instances), and
  • macros are still applied to merged routes via applyMacro(mergedHook, mergedHook) without re-running introspect.

The new tests around conflicting guard macros and guard({ macro: false }) within nested guards/groups reflect this behavior well.

Also applies to: 4744-4752, 4768-4798, 4808-4811


5567-5577: applyMacro introspect handling cleanly separates guard vs route introspection

Extending applyMacro to accept optional metadata: MacroIntrospectionMetadata and only invoking value?.(introspectOptions, metadata) when metadata is present:

  • prevents guard-level macros from introspecting during guard registration (no metadata passed), and
  • ensures route-level macros introspect with { path, method } when called from add().

The recursive calls propagate the same metadata so nested macros also see correct route metadata. Deleting the macro key from localHook regardless of whether metadata is present preserves previous macro application semantics.

Also applies to: 5615-5625, 5643-5646

@hunterwilhelm
Copy link
Contributor Author

@SaltyAom I was able to update index.ts so that it now passes all of the new tests for this feature. However, I ended up making more changes than I would have preferred, including adding a few instance variables that I’m not fully convinced are necessary.

If there’s a more elegant or idiomatic way to approach this within Elysia’s design, I’d be happy to revise the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant