feat(hono): setup new propery handler generation strategy - #3518
Merged
Conversation
Contributor
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds a strategy-driven Hono handler regeneration system: ChangesHandler Regeneration Strategy System
Sequence DiagramsequenceDiagram
participant User
participant GenerateHandlerFile
participant StrategyRouter
participant ensureTypeScript
participant TypeScriptAPI
participant extractHandlerBodies
participant reconcileHandlerFile
User->>GenerateHandlerFile: generateHandlerFile(verbs, strategy)
GenerateHandlerFile->>StrategyRouter: decide by strategy
alt skip
StrategyRouter->>GenerateHandlerFile: return existing source
else full
StrategyRouter->>ensureTypeScript: ensureTypeScript()
ensureTypeScript->>TypeScriptAPI: dynamic import / parse check
TypeScriptAPI-->>ensureTypeScript: available?
ensureTypeScript-->>StrategyRouter: ok
StrategyRouter->>extractHandlerBodies: extractHandlerBodies(existing)
extractHandlerBodies->>TypeScriptAPI: parse AST & extract bodies
extractHandlerBodies-->>StrategyRouter: bodies
StrategyRouter->>GenerateHandlerFile: generate fresh file + splice bodies
GenerateHandlerFile->>User: new file with user bodies
else smart
StrategyRouter->>ensureTypeScript: ensureTypeScript()
ensureTypeScript->>TypeScriptAPI: dynamic import / parse check
TypeScriptAPI-->>ensureTypeScript: available?
ensureTypeScript-->>StrategyRouter: ok
StrategyRouter->>reconcileHandlerFile: reconcileHandlerFile(existing, desired)
reconcileHandlerFile->>TypeScriptAPI: parse AST & compute edits
reconcileHandlerFile->>reconcileHandlerFile: applyEdits()
reconcileHandlerFile-->>StrategyRouter: updated source
StrategyRouter->>User: merged file preserving user code
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Hono client’s handler regeneration behavior to prevent destructive overwrites of user-authored code by introducing a configurable handler merge strategy (defaulting to a new non-destructive “smart” strategy). It also adjusts validator generation for form-encoded request bodies and adds documentation + tests covering the new behavior and fallbacks.
Changes:
- Add
override.hono.handlerGenerationStrategy(smartdefault,skip,full) and normalize/type it across the codebase. - Implement AST-based reconciliation for existing handler files (optional
typescriptpeer dependency; warn + fall back toskipwhen absent). - Fix request-body validator target/context generation for
multipart/form-dataandapplication/x-www-form-urlencoded, and add end-to-end/unit test coverage + docs.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/vitest.snapshots.ts | Includes the new end-to-end handler preservation spec in the Vitest run. |
| tests/handler-preservation.spec.ts | End-to-end regression test ensuring user code survives regeneration when clean is disabled. |
| packages/orval/src/utils/options.ts | Normalizes handlerGenerationStrategy with default 'smart'. |
| packages/orval/src/utils/options.test.ts | Tests defaulting + preserving explicit handlerGenerationStrategy. |
| packages/hono/src/index.ts | Adds strategy-driven handler generation, TypeScript-optional behavior, and form-body validator/context fixes. |
| packages/hono/src/index.test.ts | Tests smart/skip/full behaviors and form-body validator target. |
| packages/hono/src/index.no-typescript.test.ts | Verifies warning + fallback behavior when typescript is unavailable. |
| packages/hono/src/handler-merge.ts | New AST-based reconciler and body extractor for handler preservation. |
| packages/hono/src/handler-merge.test.ts | Extensive reconciliation + safety regression tests (imports/validators/aliases/CRLF/etc.). |
| packages/hono/package.json | Declares typescript as an optional peer dependency. |
| packages/core/src/types.ts | Adds HonoHandlerStrategy type and options fields. |
| packages/core/src/test-utils/context.ts | Updates test context defaults to include handlerGenerationStrategy: 'smart'. |
| docs/content/docs/reference/configuration/output.mdx | Documents handlerGenerationStrategy and the TypeScript/clean caveats. |
| docs/content/docs/guides/hono.mdx | Updates Hono guide to reflect the new strategy-based behavior. |
| bun.lock | Records the optional peer dependency metadata for typescript. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
tests/handler-preservation.spec.ts (1)
77-90: ⚡ Quick winMake the mutation step less format-fragile.
This chained
replace(...)sequence depends on exact emitted strings, so harmless formatting changes in generated handlers can break the test. Prefer stable anchors (e.g., regex on semantic tokens or AST/text markers) so the test validates behavior rather than formatting.🤖 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 `@tests/handler-preservation.spec.ts` around lines 77 - 90, The test's chained string.replace calls are brittle; instead locate semantic anchors and perform targeted edits via regex or AST-aware operations: find the "export const createPetHandlers = factory.createHandlers(" invocation and inject "authenticate()," into its argument list (use a regex that allows whitespace/newlines), insert the USER_BODY line by matching the handler arrow start pattern like "=>\\s*\\{" and replacing with "=> {\\n return c.json(await save()); // USER_BODY" (again using a flexible regex), and append the async save() helper by matching the end of the module or a stable marker; reference symbols to change: fresh, edited, createPetHandlers, factory.createHandlers, authenticate(), save(), and USER_BODY. Ensure the replacements tolerate varying whitespace/formatting so the test validates behavior not exact formatting.packages/hono/src/index.no-typescript.test.ts (1)
10-19: Confirm mocks matchgenerateHandlerFile’s actual imports; consider isolating module-level warning state
packages/hono/src/index.tsimportsensureTypeScriptfrom./handler-mergeandlogWarningfrom@orval/core, so thevi.mock('./handler-merge'...)andvi.mock('@orval/core'...)overrides intercept the real calls.- Remaining fragility:
warnedMissingTypeScriptis module-scoped inindex.ts, so the “no warning” expectation for the “full” path depends on the prior “smart” test run; reset module state / re-import between cases to avoid order sensitivity.🤖 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 `@packages/hono/src/index.no-typescript.test.ts` around lines 10 - 19, The tests mock './handler-merge' and '`@orval/core`' but the suite is order-sensitive because index.ts keeps a module-scoped warnedMissingTypeScript flag; update the mocks to match the actual named exports used by generateHandlerFile (ensureTypeScript and logWarning) and make each test isolated by clearing the module cache and re-importing index.ts between cases (e.g., call vi.resetModules() and then import the module afresh) or refactor index.ts to expose/reset warnedMissingTypeScript so tests can reset it explicitly; ensure the mock for './handler-merge' returns an exported ensureTypeScript function and the '`@orval/core`' mock returns logWarning so imports line up exactly.
🤖 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 `@packages/hono/src/handler-merge.ts`:
- Around line 420-428: The insertion logic for pendingInsertions assumes there's
a newline after the last import; when source.indexOf('\n', lastImport.getEnd())
returns -1 new imports get prepended. Change the computation of insertPos (used
with pendingInsertions, importDeclarations, lastImport, source, edits) to guard
for -1: compute const newlinePos = source.indexOf('\n', lastImport.getEnd());
then set insertPos = newlinePos === -1 ? source.length : newlinePos + 1 so new
imports are appended after the last import even if it's at EOF.
- Around line 519-531: The insertion currently returns lineStart(...) for the
handler which wrongly places validators at the start of the line for single-line
handlers; update validatorInsertPos to compute handlerStart =
handler.getStart(sourceFile) and return handlerStart unless the handler already
begins at the start of its line (i.e., lineStart(source, handlerStart) ===
handlerStart), in which case keep returning the lineStart; keep the fallback of
call.getEnd() - 1 when no handler is found.
In `@packages/hono/src/index.test.ts`:
- Around line 49-56: The tests are constructing NormalizedHonoOptions without
the required handlerGenerationStrategy (type HonoHandlerStrategy), causing
TS2741; update the fixture objects (e.g., the args/fixtures that build hono: {
compositeRoute, validator, validatorOutputPath }) to include a
handlerGenerationStrategy property (for example 'smart' or the appropriate
HonoHandlerStrategy value) so the constructed NormalizedHonoOptions satisfies
the type, or alternatively ensure normalization/defaulting that sets
handlerGenerationStrategy runs before the object is typed as
NormalizedHonoOptions; refer to the NormalizedHonoOptions type and add
handlerGenerationStrategy to the failing fixtures (and the args helper if used)
to fix the compile error.
In `@packages/hono/src/index.ts`:
- Around line 528-539: extractHandlerBodies() currently returns an empty Map on
parse failure which makes the 'full' branch regenerate handlers with
DEFAULT_HANDLER_BODY and drop user code; change extractHandlerBodies to return
null (or another sentinel) on parse/parse-failure and update this branch in
packages/hono/src/index.ts to detect that sentinel (e.g., bodies === null) and
treat it like the 'skip' behavior instead of calling generateFreshHandlerFile;
reference extractHandlerBodies, generateFreshHandlerFile, DEFAULT_HANDLER_BODY
and ensure when bodies is a real Map (not null) you continue to call
generateFreshHandlerFile with bodyFor using
bodies.get(`${operationName}Handlers`).
---
Nitpick comments:
In `@packages/hono/src/index.no-typescript.test.ts`:
- Around line 10-19: The tests mock './handler-merge' and '`@orval/core`' but the
suite is order-sensitive because index.ts keeps a module-scoped
warnedMissingTypeScript flag; update the mocks to match the actual named exports
used by generateHandlerFile (ensureTypeScript and logWarning) and make each test
isolated by clearing the module cache and re-importing index.ts between cases
(e.g., call vi.resetModules() and then import the module afresh) or refactor
index.ts to expose/reset warnedMissingTypeScript so tests can reset it
explicitly; ensure the mock for './handler-merge' returns an exported
ensureTypeScript function and the '`@orval/core`' mock returns logWarning so
imports line up exactly.
In `@tests/handler-preservation.spec.ts`:
- Around line 77-90: The test's chained string.replace calls are brittle;
instead locate semantic anchors and perform targeted edits via regex or
AST-aware operations: find the "export const createPetHandlers =
factory.createHandlers(" invocation and inject "authenticate()," into its
argument list (use a regex that allows whitespace/newlines), insert the
USER_BODY line by matching the handler arrow start pattern like "=>\\s*\\{" and
replacing with "=> {\\n return c.json(await save()); // USER_BODY" (again
using a flexible regex), and append the async save() helper by matching the end
of the module or a stable marker; reference symbols to change: fresh, edited,
createPetHandlers, factory.createHandlers, authenticate(), save(), and
USER_BODY. Ensure the replacements tolerate varying whitespace/formatting so the
test validates behavior not exact formatting.
🪄 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: c06feafa-a9b7-4622-ada6-15a5bc8e4211
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
docs/content/docs/guides/hono.mdxdocs/content/docs/reference/configuration/output.mdxpackages/core/src/test-utils/context.tspackages/core/src/types.tspackages/hono/package.jsonpackages/hono/src/handler-merge.test.tspackages/hono/src/handler-merge.tspackages/hono/src/index.no-typescript.test.tspackages/hono/src/index.test.tspackages/hono/src/index.tspackages/orval/src/utils/options.test.tspackages/orval/src/utils/options.tstests/handler-preservation.spec.tstests/vitest.snapshots.ts
Collaborator
Author
|
Just flagging a pre-existing bug I noticed while working on this (not introduced The reason is that handler generation always receives the It's out of scope here so I'll fix it in a follow-up PR — just wanted to note it |
melloware
approved these changes
Jun 1, 2026
This was referenced Jun 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Handler files under
override.hono.handlersare where users put business logic.Since #3292 (shipped in 8.11.x), regeneration rebuilt the preamble + validator
chain from the spec and spliced back only the
async (c) => {…}body — silentlydeleting custom imports, middleware (
authenticate(),rateLimit()), andtop-level helpers on every run. In one project a single regeneration stripped 66
handlers (~2,880 deletions), removing auth middleware from protected endpoints.
This PR replaces that with a configurable, non-destructive strategy.
New option:
override.hono.handlerGenerationStrategyControls how an existing handler file is treated on regeneration (a
non-existent file is always generated fresh):
smart(default) — parse the file (TypeScript compiler API) and reconcileonly orval-owned regions: its own imports (names, module paths,
camelCase→PascalCase casing) and the
zValidator(...)arguments, and appendhandlers for new operations. Custom imports, middleware, bodies, and top-level
helpers are preserved.
skip— leave an existing file byte-for-byte unchanged.full— previous behavior: rebuild header/imports/validators, splice backthe body (drops custom imports/middleware/helpers).
smartis a general reconcile: adds missing imports/validators, removes stalevalidators, migrates renamed schemas, appends new-operation stubs.
Also fixed
multipart/form-dataorapplication/x-www-form-urlencodednow emitzValidator('form', …)+form:context instead of
'json'.Implementation
packages/hono/src/handler-merge.ts(AST reconcile). Only rewrites aplain named import whose names are all orval-owned; locates imports by module
specifier (no fuzzy name matching); resolves an aliased
zValidatorscoped tothe validator module; on parse failure falls back to
skip.typescriptis an optionalpeerDependency, loaded lazily viaimport('typescript'). If absent,smart/fulldegrade toskipwith awarning. The compiler is not bundled.
Default changes from the 8.11.x
full(destructive) tosmart(non-destructive).Set
handlerGenerationStrategy: 'full'for the old behavior.Tests
handler-merge.test.ts+index.test.ts: preservation ofimports/middleware/helpers/bodies; camelCase→PascalCase migration (incl. mixed
imports); add/remove/rename validators; new-operation append;
namespace/default/aliased import safety; the
getCommunityFilterContextclobberregression; inline-middleware handling; CRLF; typescript-unavailable fallback.
options.test.ts: default + explicit strategy.tests/handler-preservation.spec.ts: end-to-endgenerate()×2 with an editbetween (clean disabled).
Docs
output.mdx: documentshandlerGenerationStrategy(+ typescript-peer &cleancallouts).
hono.mdx: replaces the stale "refresh header/imports, preserve body" text.Summary by CodeRabbit
New Features
handlerGenerationStrategyfor Hono handlers:smart(default, non-destructive),skip(freeze), andfull(regenerate wrapper and splice back bodies). Request-body validator now usesformfor multipart/form-data and x-www-form-urlencoded.Documentation
Tests