Add helper.ts #70
Add helper.ts #70
Conversation
WalkthroughA new TypeScript helper module has been added, introducing five utility functions: string formatting, array summation, email validation, function debouncing, and array chunking. Each function is exported for external use, providing common utility operations for string, array, and function handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-Merge Checks (3 warnings)❌ Failed Checks (3 warnings)
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://app.smartcontext.ai/default/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
helper.ts (1)
1-3: LGTM! Consider edge case handling.The function logic is correct and handles the basic string formatting requirements well. The regex
/\s+/gproperly matches all whitespace sequences.Consider adding validation for edge cases like empty strings or strings that become empty after trimming.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
helper.ts(1 hunks)
🧰 Additional context used
🔍 MCP Research (2 servers)
Context7: - The closest relevant library to "helper.ts" is "/get-convex/convex-helpers," which contains TypeScript helper utilities but does not document the five functions introduced in the PR (formatString, calculateSum, isValidEmail, debounce, chunk) ("/get-convex/convex-helpers" docs).
- No records, discussions, standards, or previous decisions related to the new helper.ts module or its functions were found in the searched documentation ("/get-convex/convex-helpers" docs).
- No related work, dependencies, or downstream impacts specifically documented for these helper functions were found ("/get-convex/convex-helpers" docs).
Linear: - Issue ENG-2040 specifies adding helper.ts with multiple utility functions but excludes calculateSum as it exists elsewhere (ENG-2040, ID: 84652a3b-d55d-4894-9526-20eade880be8)
- The helper.ts file is planned for packages/common/src and includes functions like formatString, isValidEmail, chunk, debounce, with TypeScript annotations and JSDoc comments (ENG-2040 comments)
- The main index file will be updated to export these helpers; linked GitHub issue #6379 tracks implementation (ENG-2040; https://github.com/coderabbitai/mono/issues/6379)
- No prior documentation or incidents specifically about helper.ts found; adherence to existing codebase patterns is indicated (ENG-2040)
🔇 Additional comments (1)
helper.ts (1)
5-7: LGTM!The reduce implementation is correct and handles empty arrays properly with the initial value of 0. TypeScript typing ensures type safety for the input array.
| export function isValidEmail(email: string): boolean { | ||
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | ||
| return emailRegex.test(email); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more robust email validation approach.
The current regex is very basic and may reject valid email addresses or accept invalid ones. It doesn't handle cases like:
- Multiple consecutive dots
- Special characters in the local part
- Quoted strings
- International domain names
Consider using a well-tested email validation library or a more comprehensive regex pattern:
-export function isValidEmail(email: string): boolean {
- const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/;
- return emailRegex.test(email);
-}
+export function isValidEmail(email: string): boolean {
+ // More comprehensive regex or use a validation library
+ const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/;
+ return emailRegex.test(email);
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function isValidEmail(email: string): boolean { | |
| const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; | |
| return emailRegex.test(email); | |
| } | |
| export function isValidEmail(email: string): boolean { | |
| // More comprehensive regex or use a validation library | |
| const emailRegex = /^[a-zA-Z0-9.!#$%&'*+/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/; | |
| return emailRegex.test(email); | |
| } |
🤖 Prompt for AI Agents
In helper.ts around lines 9 to 12, the current email validation regex is too
simplistic and may incorrectly validate some email formats. Replace the regex
with a more comprehensive pattern that handles special characters, quoted
strings, multiple dots, and international domains, or better yet, integrate a
well-tested email validation library to ensure robust and accurate validation.
| export function debounce<T extends (...args: any[]) => any>( | ||
| func: T, | ||
| delay: number | ||
| ): (...args: Parameters<T>) => void { | ||
| let timeoutId: NodeJS.Timeout; | ||
| return (...args: Parameters<T>) => { | ||
| clearTimeout(timeoutId); | ||
| timeoutId = setTimeout(() => func(...args), delay); | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix uninitialized timeout and consider cross-platform compatibility.
The debounce logic is correct, but there are two improvements to consider:
timeoutIdis used uninitialized on the first call toclearTimeoutNodeJS.Timeouttype assumes Node.js environment
Apply this diff to address these issues:
export function debounce<T extends (...args: any[]) => any>(
func: T,
delay: number
): (...args: Parameters<T>) => void {
- let timeoutId: NodeJS.Timeout;
+ let timeoutId: NodeJS.Timeout | number | undefined;
return (...args: Parameters<T>) => {
- clearTimeout(timeoutId);
+ if (timeoutId) clearTimeout(timeoutId);
timeoutId = setTimeout(() => func(...args), delay);
};
}🤖 Prompt for AI Agents
In helper.ts around lines 14 to 23, the variable timeoutId is used before
initialization in clearTimeout, which can cause issues, and the type
NodeJS.Timeout assumes a Node.js environment limiting cross-platform
compatibility. To fix this, initialize timeoutId to undefined and update its
type to a more generic type like ReturnType<typeof setTimeout> or number to
ensure compatibility across environments. Also, add a check before calling
clearTimeout to ensure timeoutId is defined.
| export function chunk<T>(array: T[], size: number): T[][] { | ||
| const chunks: T[][] = []; | ||
| for (let i = 0; i < array.length; i += size) { | ||
| chunks.push(array.slice(i, i + size)); | ||
| } | ||
| return chunks; | ||
| } |
There was a problem hiding this comment.
Add input validation to prevent infinite loops.
The chunking logic is correct, but missing validation for size parameter could cause critical issues.
If size is 0 or negative, the loop i += size will create an infinite loop. Add input validation:
export function chunk<T>(array: T[], size: number): T[][] {
+ if (size <= 0) {
+ throw new Error('Chunk size must be greater than 0');
+ }
+ if (array.length === 0) {
+ return [];
+ }
const chunks: T[][] = [];
for (let i = 0; i < array.length; i += size) {
chunks.push(array.slice(i, i + size));
}
return chunks;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function chunk<T>(array: T[], size: number): T[][] { | |
| const chunks: T[][] = []; | |
| for (let i = 0; i < array.length; i += size) { | |
| chunks.push(array.slice(i, i + size)); | |
| } | |
| return chunks; | |
| } | |
| export function chunk<T>(array: T[], size: number): T[][] { | |
| if (size <= 0) { | |
| throw new Error('Chunk size must be greater than 0'); | |
| } | |
| if (array.length === 0) { | |
| return []; | |
| } | |
| const chunks: T[][] = []; | |
| for (let i = 0; i < array.length; i += size) { | |
| chunks.push(array.slice(i, i + size)); | |
| } | |
| return chunks; | |
| } |
🤖 Prompt for AI Agents
In helper.ts around lines 25 to 31, the chunk function lacks validation for the
size parameter, which can cause an infinite loop if size is zero or negative.
Add input validation at the start of the function to check if size is a positive
integer greater than zero. If not, throw an error or return an empty array to
prevent the loop from running indefinitely.
Summary by CodeRabbit