Skip to content

feat(defineValidatedHandler): add params validation - #1501

Open
sandros94 wants to merge 7 commits into
h3js:mainfrom
sandros94:feat/params-validated-handler
Open

sandros94 wants to merge 7 commits into
h3js:mainfrom
sandros94:feat/params-validated-handler

Conversation

@sandros94

@sandros94 sandros94 commented Jul 22, 2026

Copy link
Copy Markdown
Member

Followup for #1491, discussed in #1437

This adds params validation in defineValidatedHandler, while also chaining promises as discussed in the prior PR, fast-tracking for sync operations.

The only limitation of this is that types do not actually rappresent what event.context.params are, since they are hardcoded into Record<string, string>. For this I would propose a followup PR which would also allow to properly type the H3EventContext and add room for future improvement that I've suggested in #1437 (comment)

Summary by CodeRabbit

  • New Features
    • Added route params validation to validated handlers, including optional decodeParams to decode values before validation.
    • Strengthened handler typing by exposing validated routerParams, and standardizing query string typing.
  • Bug Fixes
    • Validation now executes in a deterministic sequence (params → request → URL/query) and reliably short-circuits later async validations when earlier ones fail.
  • Tests
    • Added regression and unit/type coverage for params defaults, strict vs loose params retention, decoding behavior, and async short-circuit ordering.

@sandros94
sandros94 requested a review from pi0 as a code owner July 22, 2026 15:23
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6f0f4f5c-cd3e-412f-bdaa-fe578af8dda5

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd0228 and af3b483.

📒 Files selected for processing (1)
  • src/handler.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/handler.ts

📝 Walkthrough

Walkthrough

Adds route parameter validation to defineValidatedHandler, supports optional decoding, and refactors validation to preserve synchronous execution while handling asynchronous schemas. Router parameter typing and runtime regression coverage are expanded.

Changes

Validated handler validation

Layer / File(s) Summary
Validation utility flow
src/utils/internal/promise.ts, src/utils/internal/validate.ts
Adds conditional synchronous/promise chaining, validates route params, and refactors request and URL validation to use lazy result application.
Handler contract and sequencing
src/handler.ts, test/handler.test.ts
Adds typed params validation and decodeParams, runs params before request and URL validation, and tests ordering, decoding, defaults, and errors.
Router parameter typing and assertions
src/utils/request.ts, test/unit/types.test-d.ts
Makes getRouterParams generic and verifies validated route params are inferred as typed handler inputs.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • h3js/h3#1491: Overlaps in validated handler and internal validation flow changes.
  • h3js/h3#1500: Also updates validatedRequest behavior in the internal validation pipeline.

Suggested labels: enhancement

Suggested reviewers: pi0

Poem

A bunny hops through params bright,
Checks headers, queries, left and right.
Schemas chain without delay,
Bad fields stop the rest midway.
Typed ears twitch: “All tests pass!” 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: adding params validation to defineValidatedHandler.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 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 `@src/utils/internal/promise.ts`:
- Around line 7-9: Update the `chain` function to detect promise-like values by
checking for a callable `then` property instead of using `instanceof Promise`,
and invoke that thenable’s `then` with `fn`. Preserve direct synchronous
invocation for non-thenable values.

In `@src/utils/internal/validate.ts`:
- Around line 194-219: Update applyParams in validatedParams to merge validated
parameter values into the existing event.context.params object key-by-key,
matching applyHeaders and applyQuery, instead of replacing the container.
Preserve unvalidated router parameters while applying validated values and
returning the merged params.
🪄 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: 1114706e-ec2c-4db2-9edb-5b8e57fba25c

📥 Commits

Reviewing files that changed from the base of the PR and between 4a32c1b and 21dd2b0.

📒 Files selected for processing (6)
  • src/handler.ts
  • src/utils/internal/promise.ts
  • src/utils/internal/validate.ts
  • src/utils/request.ts
  • test/handler.test.ts
  • test/unit/types.test-d.ts

Comment thread src/utils/internal/promise.ts Outdated
Comment thread src/utils/internal/validate.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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/handler.test.ts`:
- Around line 461-462: Replace both runtime-matrix requests in the test around
the mounted route with ctx.fetch, using the handlers mounted through the test
context instead of app.request. Preserve the existing URLs and assertions while
ensuring both requests use the shared cross-runtime URL resolution path.
🪄 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: becf4674-6630-4fda-9b60-0c6f9d49862c

📥 Commits

Reviewing files that changed from the base of the PR and between d3870e4 and 520efe6.

📒 Files selected for processing (2)
  • src/utils/internal/validate.ts
  • test/handler.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/utils/internal/validate.ts

Comment thread test/handler.test.ts
sandros94 and others added 2 commits July 22, 2026 18:28
Coercing params schemas put non-strings in `context.params`; caller-side
`decode: true` crashed on them.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Document the sequential params → headers → query validation order and
short-circuit, the `context.params` write-back side effect, and the
`decodeParams` option. Also note why `StringsOnly` infers `never` for
coerced (non-string) params/query fields.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

2 participants