feat(routing): migrate route rules to h3-rules - #4411
Conversation
Route-rule types are now owned by the extracted `h3-rules` package. Re-export `RouteRuleConfig`/`RouteRules`/`MatchedRouteRule(s)` from it, keep `NitroRouteConfig`/`NitroRouteRules` as deprecated aliases, and augment `h3-rules` with Nitro's `isr`/`prerender`/`static` rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Delegate `normalizeRouteRules` to `h3-rules` (the `/**` scope `base` field replaces the internal `_redirectStripBase`/`_proxyStripBase` flags). Narrow the netlify/cloudflare/vercel preset generators for the now-explicit `redirect`/`proxy: false` reset; generated platform configs are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compile the `findRouteRules` lookup with `h3-rules/compiler` (tree-shaking to only the rule handlers the app uses) and resolve rules at runtime with the h3-rules matcher, replacing the hand-rolled dual-path merge in `getRouteRules`. Add a `route-rule-handlers` module that re-exports the h3-rules handlers plus a `cache` handler bound to Nitro's cache runtime (stable `nitro/route-rules` keys), and delete the in-tree `route-rules` runtime. The matcher is lazily instantiated so apps without route rules tree-shake h3-rules out entirely. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Point the normalization unit test at the h3-rules-backed resolver (the `canonicalPath`/`isPathInScope` suites now live upstream in h3-rules; the GHSA dual-path behavior stays covered by the `/rules/*` fixture e2e), adjust the virtual-routing template stub, and note in the docs that route-rule semantics are defined in h3-rules. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughNitro delegates route-rule normalization, matching, and handler implementations to Changesh3-rules migration
Estimated code review effort: 4 (Complex) | ~55 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
commit: |
Mark the injected `cache` rule handler `/* @__PURE__ */` so an app using only non-cache route rules (e.g. `headers`) no longer pulls `ocache`/`ohash`/ `unstorage` into the bundle. Importing `headers` from `route-rule-handlers` previously evaluated the top-level `createCacheRuleHandler(...)` call, dragging in the whole cache runtime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.agents/architecture.md (1)
91-91: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
corsto the h3-rules built-ins list for consistency with routing docs.
docs/1.docs/5.routing.mdlistscorsamong the h3-rules-provided handlers, but this line omits it from the built-ins enumeration. Addcorsto(headers, redirect, proxy, basicAuth, cors)if it is indeed an upstream handler.📝 Suggested update
-- `route-rule-handlers.ts` — Rule handlers for the compiled matcher: h3-rules built-ins (headers, redirect, proxy, basicAuth) plus a `cache` handler bound to Nitro's cache runtime. Rule matching/normalization live in the [`h3-rules`](https://github.com/h3js/h3-rules) package. +- `route-rule-handlers.ts` — Rule handlers for the compiled matcher: h3-rules built-ins (headers, redirect, proxy, basicAuth, cors) plus a `cache` handler bound to Nitro's cache runtime. Rule matching/normalization live in the [`h3-rules`](https://github.com/h3js/h3-rules) package.🤖 Prompt for 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. In @.agents/architecture.md at line 91, The h3-rules built-ins list in the architecture notes is missing `cors`, which makes it inconsistent with the routing documentation. Update the `route-rule-handlers.ts` description to include `cors` alongside the existing built-ins (`headers`, `redirect`, `proxy`, `basicAuth`), keeping the `cache` handler note unchanged if it remains Nitro-specific. Verify the wording still matches the upstream `h3-rules` handler set and the routing docs.
🤖 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 `@docs/1.docs/5.routing.md`:
- Around line 440-441: The routing docs sentence incorrectly attributes the
`cache` handler to `h3-rules`; update the wording in the routing section to
separate the `headers` / `redirect` / `proxy` / `basicAuth` / `cors` handlers
that come from `h3-rules` from the Nitro-specific `cache` handler. Use the
existing routing documentation text around the `Route rule matching` description
and the `cache` mention to make the ownership clear without changing the meaning
of the other handlers.
---
Nitpick comments:
In @.agents/architecture.md:
- Line 91: The h3-rules built-ins list in the architecture notes is missing
`cors`, which makes it inconsistent with the routing documentation. Update the
`route-rule-handlers.ts` description to include `cors` alongside the existing
built-ins (`headers`, `redirect`, `proxy`, `basicAuth`), keeping the `cache`
handler note unchanged if it remains Nitro-specific. Verify the wording still
matches the upstream `h3-rules` handler set and the routing docs.
🪄 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: 21f923d5-1892-4534-ae40-915eb9726b9b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.agents/architecture.mddocs/1.docs/5.routing.mdpackage.jsonpnpm-workspace.yamlsrc/build/virtual/routing.tssrc/config/resolvers/route-rules.tssrc/presets/cloudflare/utils.tssrc/presets/netlify/utils.tssrc/presets/vercel/utils.tssrc/runtime/internal/app.tssrc/runtime/internal/route-rule-handlers.tssrc/runtime/internal/route-rules.tssrc/runtime/virtual/routing.tssrc/types/route-rules.tstest/unit/route-rules.test.tstest/unit/virtual-routing.test.ts
💤 Files with no reviewable changes (1)
- src/runtime/internal/route-rules.ts
…sult The cache handler's memo map lives in route-rule-handlers module scope, not in the matcher; the lazy init exists only to keep createMatcherFromFind out of module scope for tree-shaking. Also drop the stale optional marker on getRouteRules().routeRules — the h3-rules matcher always returns a rule map. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adopt the new compiler API (`compileRouteRules` + `runtimeRules`), so built-in handlers import straight from `h3-rules`/`h3-rules/proxy` and only the `cache` rule is overridden to Nitro's cache runtime. Picks up 0.1.0 behavior deltas: real h3 `handleCors` for `cors: true` (so preset generators no longer bake CORS headers into static config), `swr: false` as a cache reset marker, and a closed `RouteRuleConfig` (config typos type-error again). Tests/snapshots updated accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`cors` is now applied at runtime by h3's `handleCors` (not baked into static headers), accepts a `CorsOptions` object, and won't cover assets served straight from the CDN edge. Also note `swr: false` as a cache reset marker in the rules reference table. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Compile with `preMerge: true` so each pattern's subsumption chain is collapsed at build time and per-request resolution takes only the most-specific matched layer. Fail-safe: a non-chain-clean rule set warns and falls back to plain compilation. - Wrap the runtime matcher with `memoizeRouteRulesMatcher` (bounded LRU, cap 1024, oldest-evicted) so hot paths skip the dual-path resolve. Both operate around the dual-path (raw+canonical) union in `createMatcherFromFind`, so the encoded-separator hardening (GHSA-5w89-w975-hf9q) is unaffected — verified that the compiled matcher resolves the encoded-separator scope cases identically with and without preMerge. Full suite green on both builders (937 passed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
h3-rules parses `"METHOD /path"` keys and the compiled matcher dispatches on method at runtime (method-scoped rules merge on top of method-agnostic ones for the same path). Add a fixture rule + node e2e proving a `POST /rules/method-scoped/**` basicAuth gate applies to POST only, and document the `METHOD ` key prefix in the routing guide. Uses a runtime-only `basicAuth` rule so the method-prefixed key does not leak into platform-native static config (the netlify/cloudflare/vercel header/redirect generators still treat keys as plain paths) — noted as a caveat in the docs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This PR migrates Nitro route rules to use
h3-rules