Skip to content

docs: correct decode function name for getRouterParam helpers#1419

Merged
pi0 merged 1 commit into
h3js:mainfrom
patrickwehbe:fix-router-param-decode-docs
Jul 2, 2026
Merged

docs: correct decode function name for getRouterParam helpers#1419
pi0 merged 1 commit into
h3js:mainfrom
patrickwehbe:fix-router-param-decode-docs

Conversation

@patrickwehbe

@patrickwehbe patrickwehbe commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

The JSDoc for getRouterParam and getValidatedRouterParams says the decode option decodes params with decodeURI, but both functions delegate to getRouterParams, which decodes with decodeURIComponent:

// getRouterParams, src/utils/request.ts:178
params[key] = decodeURIComponent(params[key]);

getRouterParams' own JSDoc (line 161) already says decodeURIComponent, so the two sibling helpers contradicted it. The distinction matters because decodeURI leaves reserved characters like %2F, %23 and %3F encoded, while decodeURIComponent decodes them, so a reader following the docs would expect the wrong result.

Fixed the JSDoc on both helpers and regenerated the docs/2.utils/1.request.md automd block that is built from it. Comment-only change, no behavior change.

Summary by CodeRabbit

  • Documentation
    • Clarified route parameter decoding behavior when the decode option is enabled.

The JSDoc for getRouterParam and getValidatedRouterParams said the
decode option uses decodeURI, but both delegate to getRouterParams,
which decodes with decodeURIComponent (see line 178). The two differ on
reserved characters like %2F, so the docs were misleading. Aligned the
JSDoc, and the generated request.md docs, with the implementation and
with getRouterParams' own JSDoc.
@patrickwehbe
patrickwehbe requested a review from pi0 as a code owner June 22, 2026 17:49
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

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: aac05c98-3b97-41b4-9cdd-7514e8bb3476

📥 Commits

Reviewing files that changed from the base of the PR and between edb53fe and 8a8ff29.

📒 Files selected for processing (2)
  • docs/2.utils/1.request.md
  • src/utils/request.ts

📝 Walkthrough

Walkthrough

Two JSDoc comments in src/utils/request.ts and two corresponding prose blocks in docs/2.utils/1.request.md are updated to replace decodeURI with decodeURIComponent as the described decoding function used when decode: true is passed to getRouterParam and getValidatedRouterParams. No executable logic changes.

Changes

Decode function documentation correction

Layer / File(s) Summary
JSDoc and markdown docs: decodeURIdecodeURIComponent
src/utils/request.ts, docs/2.utils/1.request.md
JSDoc for getRouterParam and getValidatedRouterParams and their matching markdown documentation sections are corrected to state that decode: true uses decodeURIComponent.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Poem

🐇 A tiny tweak, a doc set right,
Component now shines in the light.
No slashes lost, no chars astray,
decodeURIComponent wins the day!
Hop hop hooray, the docs are true! 🎉

🚥 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 accurately reflects the main change—correcting documentation to specify that getRouterParam helpers use decodeURIComponent instead of decodeURI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ 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.

@pi0x pi0x 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.

Note: this review was performed by AI agents (Claude Code) on behalf of the maintainer, in two independent passes.

Pass 1 reviewed the diff for correctness, edge cases, and test coverage. Pass 2 (a fresh agent, adversarial by design) independently re-read the diff, checked for merge conflicts/CI status, and tried to find a reason to block approval before this review was posted. Both passes agree this change is correct, adequately tested, and safe to merge.

Caveat: this repo's real CI workflow (lint + build + vitest) requires maintainer approval to run on fork PRs and has not executed on this PR yet — only automated third-party checks (CodeRabbit, Socket Security) have run. Please trigger/approve the CI workflow before merging.

@pi0
pi0 merged commit 287664b into h3js:main Jul 2, 2026
5 checks passed
pi0x pushed a commit to mixelburg/h3 that referenced this pull request Jul 2, 2026
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.

3 participants