feat(next-typed-href): add requiredSearchParams option#58
Conversation
…refWithNuqs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthrough
Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
…withDefault fields optional Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/next-typed-href/src/nuqs.ts`:
- Around line 97-103: The generic Options inference loses literal boolean
information when callers pass a variable annotated as
DefineTypedHrefWithNuqsOptions (e.g. const opts: DefineTypedHrefWithNuqsOptions
= { requireSearchParams: true }), so requireSearchParams ends up widened to
boolean; fix by adding a type-level test and doc note: add a TypeScript type
test that calls defineTypedHrefWithNuqs<Routes, RouteParamsMap>() with an
annotated variable (not an inline object) to assert that
Options["requireSearchParams"] extends true enforces required searchParams, and
update the `@example/docs` for defineTypedHrefWithNuqs (and/or the typedef
DefineTypedHrefWithNuqsOptions) to recommend using a const/satisfies assertion
when you need literal inference. Include references to the generic Options and
the returned function defineTypedHrefWithNuqs / $href in the test and 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: fe82a729-2529-4391-a57a-f44efe2838f6
📒 Files selected for processing (2)
packages/next-typed-href/src/nuqs.test.tspackages/next-typed-href/src/nuqs.ts
…chParams Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fault Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/next-typed-href/src/nuqs.ts (1)
16-16: コメント言語を統一すると保守性が上がります。Line 16 のインラインコメントだけ日本語になっており、周辺の英語 JSDoc と混在しています。公開ライブラリとしては英語に寄せるとコントリビューター体験が安定します。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next-typed-href/src/nuqs.ts` at line 16, Replace the lone Japanese inline comment in nuqs.ts with an English equivalent to match the surrounding JSDoc; specifically update the comment that explains null-containing fields vs withDefault behavior to something like: "Fields that can contain null (i.e., no withDefault) are required; others (with withDefault) are optional." Keep the meaning intact and ensure the wording aligns with the file's existing English documentation style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/next-typed-href/src/nuqs.ts`:
- Line 16: Replace the lone Japanese inline comment in nuqs.ts with an English
equivalent to match the surrounding JSDoc; specifically update the comment that
explains null-containing fields vs withDefault behavior to something like:
"Fields that can contain null (i.e., no withDefault) are required; others (with
withDefault) are optional." Keep the meaning intact and ensure the wording
aligns with the file's existing English documentation style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d55bd666-338d-49b7-9df5-b48aa459bddd
📒 Files selected for processing (1)
packages/next-typed-href/src/nuqs.ts
akameco
left a comment
There was a problem hiding this comment.
LGTM!
3重のカリー化の部分がちょっと気になりますが、実装としては正しいと思います。
この辺の改良にちょっと案があるので、採用されるかは別にして後ほど提案PRします。
Summary
fix https://linear.app/plainbrew/issue/PC-67
defineTypedHrefWithNuqsを 3 重カリー化し、第 2 呼び出しでoptionsを受け取れるようにした{ requiredSearchParams: true }を渡すと、nuqs パーサーが定義されたルートでsearchParamsオブジェクトが必須になる.withDefault()なしのフィールドは必須、.withDefault()ありのフィールドは任意になるDefineTypedHrefWithNuqsOptions型を export に追加Breaking change
既存コードは
()()に変更が必要:呼び出し方
設計メモ
TypeScript の部分的型引数指定の制限(
<Routes, RouteParamsMap>を明示すると残りの型パラメータが引数から推論されない)を、3 重カリーにすることで回避。第 2 呼び出し時点では型引数がゼロなためconst Optionsが引数から正しく推論される。🤖 Generated with Claude Code