feat: enhance TypeScript coding style guidelines#369
feat: enhance TypeScript coding style guidelines#369jasondavey wants to merge 1 commit intoaffaan-m:mainfrom
Conversation
…es and best practices esp interfaces and types
📝 WalkthroughWalkthroughThis pull request adds a comprehensive TypeScript/JavaScript coding style guide covering explicit type annotations, interfaces vs. type aliases, immutability patterns, error handling with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="rules/typescript/coding-style.md">
<violation number="1" location="rules/typescript/coding-style.md:158">
P2: The `getErrorMessage` helper appears in both the "Avoid `any`" section and the "Error Handling" section with different fallback strings (`'Unknown error'` vs `'Unexpected error'`). In a style guide, readers are likely to copy one of these as a reference — having two slightly different versions of the same function creates unnecessary ambiguity. Align the fallback message so both examples are consistent.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return error.message | ||
| } | ||
|
|
||
| return 'Unexpected error' |
There was a problem hiding this comment.
P2: The getErrorMessage helper appears in both the "Avoid any" section and the "Error Handling" section with different fallback strings ('Unknown error' vs 'Unexpected error'). In a style guide, readers are likely to copy one of these as a reference — having two slightly different versions of the same function creates unnecessary ambiguity. Align the fallback message so both examples are consistent.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At rules/typescript/coding-style.md, line 158:
<comment>The `getErrorMessage` helper appears in both the "Avoid `any`" section and the "Error Handling" section with different fallback strings (`'Unknown error'` vs `'Unexpected error'`). In a style guide, readers are likely to copy one of these as a reference — having two slightly different versions of the same function creates unnecessary ambiguity. Align the fallback message so both examples are consistent.</comment>
<file context>
@@ -31,31 +140,50 @@ function updateUser(user, name) {
+ return error.message
+ }
+
+ return 'Unexpected error'
+}
+
</file context>
| return 'Unexpected error' | |
| return 'Unknown error' |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rules/typescript/coding-style.md (1)
141-170: Solid error handling pattern, but note console.error usage.The async/await with try-catch pattern and unknown error narrowing are correct and align with best practices. However, line 166 uses
console.errorwhile lines 191-192 recommend using proper logging libraries instead of console methods.Consider clarifying the logging approach
You might want to either:
- Add a comment in the example noting that console.error is used for simplicity but a logging library should be used in production, or
- Update the example to reference a logging library (even if pseudo-code), or
- Clarify in the Console.log section that console.error may be acceptable for server-side error logging
Example clarification:
async function loadUser(userId: string): Promise<User> { try { const result = await riskyOperation(userId) return result } catch (error: unknown) { // In production, use a proper logging library (e.g., winston, pino) logger.error('Operation failed:', error) throw new Error(getErrorMessage(error)) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rules/typescript/coding-style.md` around lines 141 - 170, The example's error logging in loadUser uses console.error; update it to demonstrate the recommended approach by replacing console.error with a call to a logging abstraction (e.g., logger.error) or add an inline comment clarifying console.error is only for simplicity and that a production logger (winston/pino) should be used; ensure the catch still narrows unknown via getErrorMessage and rethrows the Error, and reference loadUser, riskyOperation, and getErrorMessage so reviewers can find and update the example.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@rules/typescript/coding-style.md`:
- Around line 141-170: The example's error logging in loadUser uses
console.error; update it to demonstrate the recommended approach by replacing
console.error with a call to a logging abstraction (e.g., logger.error) or add
an inline comment clarifying console.error is only for simplicity and that a
production logger (winston/pino) should be used; ensure the catch still narrows
unknown via getErrorMessage and rethrows the Error, and reference loadUser,
riskyOperation, and getErrorMessage so reviewers can find and update the
example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aa8c6b19-150a-4821-ac64-8844fa568550
📒 Files selected for processing (1)
rules/typescript/coding-style.md
affaan-m
left a comment
affaan-m
left a comment
There was a problem hiding this comment.
Automated review: doc-only changes look good. Approving.
2 participants
This pull request adds a new section on types and interfaces to the TypeScript coding style guide, and expands guidance on error handling and input validation. The updates clarify best practices for typing public APIs, distinguishing between interfaces and type aliases, avoiding the use of any, typing React props, and using JSDoc in JavaScript files. Additionally, the error handling section now demonstrates safe narrowing of unknown errors, and the input validation section shows how to infer types from Zod schemas.
Type System Best Practices:
Error Handling:
Input Validation:
Summary by cubic
Enhances the TypeScript coding style guide with a new “Types and Interfaces” section, clearer error handling patterns, and
zod-based input validation with inferred types. Clarifies best practices for public APIs and React props to improve consistency and safety.any, React props typing, and JSDoc for.jsfiles.unknownerrors with guards/helpers.zodschemas to keep validation and types in sync.Written for commit d4c6449. Summary will update on new commits.
Summary by CodeRabbit