Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_56f30650-fcb7-4439-8d8f-667ca6ccbcee) |
There was a problem hiding this comment.
Code Review
This pull request removes obsolete comments and a TypeScript ignore directive from the useRouterQuery.ts file. The review feedback highlights a potential prototype pollution vulnerability and runtime error caused by calling result.hasOwnProperty(key) directly, recommending the use of Object.hasOwn instead, and points out that this utility function is duplicated in another file.
| // Consider setting atleast ES2015 as target | ||
| // @ts-expect-error | ||
| for (const [key, value] of entries) { | ||
| if (result.hasOwnProperty(key)) { |
There was a problem hiding this comment.
Using result.hasOwnProperty(key) directly on an object can lead to runtime errors or unexpected behavior if the query parameters contain keys that match prototype properties (e.g., hasOwnProperty, toString). For example, if a query parameter is hasOwnProperty=123, result.hasOwnProperty will be overwritten, and subsequent calls to result.hasOwnProperty(key) will throw a TypeError: result.hasOwnProperty is not a function, crashing the application.
To prevent this, use Object.hasOwn(result, key) (or Object.prototype.hasOwnProperty.call(result, key)).
Additionally, please note that this function is duplicated in packages/lib/fromEntriesWithDuplicateKeys.ts. You should apply the same fix there, or ideally, refactor the code to import fromEntriesWithDuplicateKeys from a single shared location to avoid duplication.
| if (result.hasOwnProperty(key)) { | |
| if (Object.hasOwn(result, key)) { |
Summary
Test plan
Note
Low Risk
Comment and type-suppression cleanup only; no runtime or query-parsing behavior change.
Overview
Removes a stale
@ts-expect-errorand related comment above thefor...ofloop infromEntriesWithDuplicateKeysinsideuseRouterQuery.ts.With the current TypeScript target (e.g. ES2022 in
@calcom/lib), iteratingsearchParams.entries()no longer needs a suppression, so the directive was unused and could break strict builds that flag unnecessary@ts-expect-errorcomments.Reviewed by Cursor Bugbot for commit 348bb56. Configure here.