fix: handle undefined keyOrFn in RateLimiter

[marwand]

Resolves issue #10

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Summary by CodeRabbit

• Bug Fixes
  • Fixed an issue in the getValue flow where a key could become undefined in certain edge cases, ensuring proper functionality when optional parameters are not provided.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The getValue flow in hookAPI's index file was updated to use the nullish coalescing operator when assigning the key value. Instead of unconditionally overwriting the key, the assignment now preserves the existing key when keyOrFn is undefined or null.

Changes

Cohort / File(s)	Summary
Key assignment operator fix src/client/index.ts	Modified key assignment to use nullish coalescing operator (??) instead of unconditional assignment, preserving existing key when keyOrFn is undefined/null

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

• Key change involves understanding the difference between nullish coalescing (??) and direct assignment—verify the intent is to preserve the existing key only when the input is null/undefined
• Consider edge cases: confirm this aligns with intended behavior for getValue flow and doesn't introduce unintended side effects with falsy values

Poem

🐰 A coalescing hop, a nullish fix so neat,
When keyOrFn's not there, the old key stays sweet,
No more undefined paths in our hookAPI's way,
A rabbit's small patch makes the logic play!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: handle undefined keyOrFn in RateLimiter' directly matches the core change: using nullish coalescing to prevent key from becoming undefined when keyOrFn is undefined.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc4e92 and 23c77c8.

📒 Files selected for processing (1)

• src/client/index.ts (1 hunks)

🔇 Additional comments (1)

src/client/index.ts (1)

241-260: The nullish coalescing operator correctly preserves args.key when keyOrFn is undefined.

When options[0] is provided without a key field, keyOrFn becomes undefined. The fix using key = keyOrFn ?? key; ensures the existing args.key value is preserved instead of being overwritten with undefined, which is the correct behavior.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/get-convex/rate-limiter/@convex-dev/rate-limiter@13

commit: 23c77c8

[ianmacartney]

released in @convex-dev/rate-limiter@0.3.1

[ianmacartney]

Actually I just realized this can expose data unintentionally - e.g. knowing the rate limit of any other user by email. I'm going to instead pass the key as an argument to the function, so you can return the key from there to pass it through, but can also validate it

[ianmacartney]

So in @convex-dev/rate-limiter@0.3.2 you must now pass the key back from the key function, e.g. key: (ctx, keyFromClient) => keyFromClient