Skip to content

fix(types): allow undefined for trailing wildcard params - #198

Open
bytaesu wants to merge 1 commit into
h3js:mainfrom
bytaesu:2026-07-30/fix/trailing-wildcard-param-type
Open

fix(types): allow undefined for trailing wildcard params#198
bytaesu wants to merge 1 commit into
h3js:mainfrom
bytaesu:2026-07-30/fix/trailing-wildcard-param-type

Conversation

@bytaesu

@bytaesu bytaesu commented Jul 30, 2026

Copy link
Copy Markdown

Summary by CodeRabbit

  • New Features

    • Route parameters captured by trailing wildcards can now be optional when no value is present.
    • Route matching correctly handles wildcard patterns with trailing slashes or empty segments.
  • Tests

    • Added coverage for optional wildcard captures and trailing-slash route patterns.
    • Updated type checks to reflect potentially undefined wildcard parameters.

Copilot AI review requested due to automatic review settings July 30, 2026 11:27
@bytaesu
bytaesu requested a review from pi0 as a code owner July 30, 2026 11:27

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Trailing wildcard parameters now allow undefined in public route types. Type-level and runtime tests cover wildcard patterns that match empty trailing segments, while the match strategy contract reflects optional parameter values.

Changes

Trailing wildcard parameter types

Layer / File(s) Summary
Trailing wildcard type inference
src/types.ts
MatchedRoute.params and trailing wildcard keys in InferRouteParams now use string | undefined; other parameters remain string.
Runtime and type-level validation
test/find.test.ts, test/types.test-d.ts, test/wpt.test.ts
Tests cover empty trailing wildcard matches, optional wildcard captures, and the widened match strategy parameter contract.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • h3js/rou3#195: Covers trailing empty-segment handling that determines whether wildcard captures exist.

Suggested reviewers: copilot, pi0

Poem

A rabbit hops through /segment bright,
Finds an empty tail in the moonlit night.
“Undefined,” whispers the wildcard free,
Now types match reality.
Hop, hop—safer routes for me!

🚥 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 summarizes the main change: trailing wildcard params now allow undefined.
Linked Issues check ✅ Passed The type updates and tests match #197 by making trailing wildcard params infer string | undefined and reflect runtime behavior.
Out of Scope Changes check ✅ Passed The changes stay focused on route param typing and corresponding tests, with no obvious unrelated additions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

🧹 Nitpick comments (2)
src/types.ts (1)

27-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a regression assertion for MatchedRoute.params.

The supplied test/types.test-d.ts verifies InferRouteParams but not this changed public type. Add an assertion such as MatchedRoute["params"] equals Record<string, string | undefined> | undefined so the runtime { "0": undefined } contract cannot regress.

As per path instructions, test/**/*.{ts,d.ts} must maintain regression coverage and verify public API types there.

🤖 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 `@src/types.ts` at line 27, Add a type-level regression assertion in
test/types.test-d.ts for MatchedRoute["params"], verifying it equals
Record<string, string | undefined> | undefined. Keep the existing
InferRouteParams assertions intact and ensure the test covers the public
MatchedRoute params contract, including undefined values.

Source: Path instructions

test/wpt.test.ts (1)

263-266: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Align normalizeGroups with the widened match params contract.

MatchStrategy.match allows Record<string, string | undefined>, so match.groups values may be missing/undefined. Update normalizeGroups to accept and return groups containing undefined values instead of hiding that possibility with Record<string, string>.

🤖 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 `@test/wpt.test.ts` around lines 263 - 266, Update normalizeGroups to accept
and return group records typed as Record<string, string | undefined>, matching
the widened params contract of MatchStrategy.match. Preserve its existing
normalization behavior while allowing missing or undefined group values instead
of narrowing them to strings.
🤖 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.

Nitpick comments:
In `@src/types.ts`:
- Line 27: Add a type-level regression assertion in test/types.test-d.ts for
MatchedRoute["params"], verifying it equals Record<string, string | undefined> |
undefined. Keep the existing InferRouteParams assertions intact and ensure the
test covers the public MatchedRoute params contract, including undefined values.

In `@test/wpt.test.ts`:
- Around line 263-266: Update normalizeGroups to accept and return group records
typed as Record<string, string | undefined>, matching the widened params
contract of MatchStrategy.match. Preserve its existing normalization behavior
while allowing missing or undefined group values instead of narrowing them to
strings.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 262c5260-13d6-437c-8ab0-dfa52c867f38

📥 Commits

Reviewing files that changed from the base of the PR and between 39b0e9b and 6d5f06f.

📒 Files selected for processing (4)
  • src/types.ts
  • test/find.test.ts
  • test/types.test-d.ts
  • test/wpt.test.ts

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.

Trailing wildcard param types do not include undefined

2 participants