Skip to content

fix(route-meta): add order:pre to route-meta plugin hooks#4316

Open
kyjus25 wants to merge 2 commits into
nitrojs:mainfrom
kyjus25:fix/route-meta
Open

fix(route-meta): add order:pre to route-meta plugin hooks#4316
kyjus25 wants to merge 2 commits into
nitrojs:mainfrom
kyjus25:fix/route-meta

Conversation

@kyjus25

@kyjus25 kyjus25 commented Jun 5, 2026

Copy link
Copy Markdown

🔗 Linked issue

Fixes #3899

❓ Type of change

  • 📖 Documentation (updates to the documentation, readme, or JSdoc annotations)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

Fixes #3899

The route-meta plugin uses ?meta virtual imports to extract defineRouteMeta metadata from handler files. During Vite Rolldown builds, Rolldown's resolver processed these imports before the plugin could intercept them because the hooks lacked order: "pre", causing all defineRouteMeta calls to be silently ignored.

Authored in collaboration with DeepSeek V4 Pro

📝 Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@kyjus25 kyjus25 requested a review from pi0 as a code owner June 5, 2026 15:37
@vercel

vercel Bot commented Jun 5, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Nitro Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The route-meta plugin now sets order: "pre" for its resolveId, load, and transform hooks. The PR also adds an OpenAPI Vite fixture (route + vite.config) and Vitest that verifies generated OpenAPI JSON and the / _swagger and /_scalar UIs.

Changes

Route-Meta Plugin Hook Ordering

Layer / File(s) Summary
Plugin hook execution phase configuration
src/build/plugins/route-meta.ts
Added order: "pre" to the resolveId, load, and transform hook groups to ensure handlers run in the pre-phase for route-meta module resolution and transformation.

OpenAPI Vite Fixture and Tests

Layer / File(s) Summary
Fixture route and Vite config
test/vite/openapi-fixture/api/meta/test.ts, test/vite/openapi-fixture/vite.config.ts
Adds a test route that calls defineRouteMeta(...) with OpenAPI tags/description/params/responses and a default handler returning { status: "OK" }; adds a Vite config enabling experimental.openAPI and openAPI.meta for the fixture.
Vitest OpenAPI endpoint tests
test/vite/openapi.test.ts
Starts a Vite dev server against the fixture and asserts /_openapi.json contains the /api/meta/test path and metadata, and that /_swagger and /_scalar return HTML with the expected title and meta description.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • nitrojs/nitro#4002: Modifies the route-meta Rollup/Nitro plugin setup and relates to plugin initialization and rollup wiring.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title follows conventional commits format with 'fix' prefix and clearly describes the main change of adding order:pre to route-meta plugin hooks.
Description check ✅ Passed The description is directly related to the changeset, explaining the bug being fixed and why the order:pre change is necessary for virtual imports in Vite/Rollup builds.
Linked Issues check ✅ Passed The PR successfully addresses issue #3899 by adding order:pre to route-meta plugin hooks to ensure virtual imports are intercepted before Rollup's resolver, and includes regression tests validating defineRouteMeta works with OpenAPI.
Out of Scope Changes check ✅ Passed All changes are in-scope: the plugin hook modification addresses the core bug, the test fixture demonstrates the fix, and the regression test validates the solution works end-to-end with OpenAPI.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@pi0

pi0 commented Jun 5, 2026

Copy link
Copy Markdown
Member

Thanks for PR and transparency on AI usage.

Can you please add a regression test (to the existing main fixture) for this?

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/vite/openapi.test.ts`:
- Around line 17-23: Replace the unsafe string-cast port and the unsafe shaped
cast of the HTTP address: call server.listen with the numeric ephemeral port 0
(use server.listen(0)) and obtain the runtime address from
server.httpServer?.address(), treating it as the union type returned by Node
(AddressInfo | string | null) rather than forcing an object shape; then compute
serverURL from that union (handle null, a plain string address, and an
AddressInfo with .address/.port/.family, preserving the IPv6 bracket logic) so
serverURL is correctly derived without unsafe casts (changes affect
server.listen, server.httpServer?.address(), and the serverURL construction).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b8e3de28-7be2-4ed2-811c-ef8375131e4e

📥 Commits

Reviewing files that changed from the base of the PR and between ecd03d5 and f9b00f3.

📒 Files selected for processing (3)
  • test/vite/openapi-fixture/api/meta/test.ts
  • test/vite/openapi-fixture/vite.config.ts
  • test/vite/openapi.test.ts
✅ Files skipped from review due to trivial changes (1)
  • test/vite/openapi-fixture/api/meta/test.ts

Comment thread test/vite/openapi.test.ts
@kyjus25

kyjus25 commented Jun 5, 2026

Copy link
Copy Markdown
Author

Can you please add a regression test (to the existing main fixture) for this

Appreciate the feedback, sir! 🫡

Updated with openapi tests that check that the defineRouteMeta is recognized and that /_swagger and /_scalar routes serve HTML with the proper meta fields from the Nitro config.

The defineRouteMeta test fails if the order: pre additions are removed

Update: I wasn't able to add the test cases to the main fixture because the regression is specific to the native plugin pipeline ordering of createServer from Vite directly, which the main fixture doesn't use.

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.

defineRouteMeta is ignored in nitro v3 (OpenAPI meta missing)

2 participants