feat(skills): add typescript-patterns skill (#543)#714
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
affaan-m has reached the 50-review limit for trial accounts. To continue receiving code reviews, upgrade your plan.
📝 WalkthroughWalkthroughA new TypeScript Patterns skill document has been added to define best practices for TypeScript development. The file includes metadata, usage guidance, and key patterns covering strict configuration, type-safe error handling, generic constraints, async/await conventions, utility types, runtime validation, and the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 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)
📝 Coding Plan
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}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@skills/typescript-patterns/SKILL.md`:
- Line 32: Replace the ambiguous guidance "Use async/await with typed catch
blocks" in SKILL.md with a precise instruction to use `unknown` for catch
parameters and then narrow the error before use (via instanceof, custom type
guards, or property checks); update the surrounding sentence to explicitly state
that catch variables should be treated as unknown and narrowed rather than
annotated with a concrete type, and keep the existing advice to prefer
Promise.allSettled for parallel operations that can partially fail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6e67c5cf-259b-4125-97ea-6251a2862456
📒 Files selected for processing (1)
skills/typescript-patterns/SKILL.md
| Constrain generics with extends to ensure type safety while maintaining flexibility. | ||
|
|
||
| ### Async Patterns | ||
| Use async/await with typed catch blocks. Prefer Promise.allSettled over Promise.all for parallel operations that can partially fail. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In modern TypeScript, can catch clause variables be explicitly type-annotated (other than any/unknown), and what is the recommended pattern for catch error handling?
💡 Result:
No, in modern TypeScript, catch clause variables cannot be explicitly type-annotated with types other than 'any' or 'unknown'. Attempting to use any other specific type (like Error or a custom type) results in the compiler error TS1196: "Catch clause variable type annotation must be 'any' or 'unknown' if specified." This restriction exists because JavaScript can throw values of any type (not just Error objects), and TypeScript cannot statically verify what will be thrown. TypeScript 4.0 introduced support for annotating catch variables as 'unknown' (safer than 'any'), and TypeScript 4.4 added the tsconfig option 'useUnknownInCatchVariables' (part of strict mode) to make unannotated catch variables default to 'unknown' instead of 'any'. The recommended pattern for catch error handling is: 1. Use 'catch (error: unknown)' or enable 'useUnknownInCatchVariables' so unannotated variables are 'unknown'. 2. Use type guards (instanceof Error, property checks, or custom type predicates) to narrow the type before accessing properties. 3. Avoid type assertions unless necessary, as they bypass safety. Example: try { riskyOperation; } catch (error: unknown) { if (error instanceof Error) { console.error(error.message); // Safe: error is Error } else { console.error('Unknown error:', error); } } Utility helper pattern (common best practice): function getErrorMessage(error: unknown): string { if (error instanceof Error) return error.message; return String(error); } This ensures type safety while handling any thrown value. For domain-specific errors, throw custom Error subclasses and check with instanceof.
Citations:
- 1: https://stackoverflow.com/questions/42618089/how-do-you-use-typed-errors-in-async-catch
- 2: https://stackoverflow.com/questions/69577451/how-do-you-type-an-error-property-in-a-catch-block-in-typescript
- 3: https://stackoverflow.com/questions/69021040/why-catch-clause-variable-type-annotation-must-be-any
- 4: https://dev.to/hannahanot/ts1196-catch-clause-variable-type-annotation-must-be-any-or-unknown-if-specified-423
- 5: https://www.typescriptlang.org/tsconfig/useUnknownInCatchVariables.html
- 6: Add flag to change
catchvariables' default types tounknownmicrosoft/TypeScript#41013
Clarify catch block error handling guidance for TypeScript accuracy.
Line 32 states "Use async/await with typed catch blocks," but this wording is ambiguous and could suggest explicit catch-parameter typing is a standard pattern. Rephrase to "use unknown in catch parameters and narrow the type before use" to align with TypeScript's actual capabilities (catch variables can only be annotated as any or unknown). This ensures developers understand the proper pattern: catch with unknown, then narrow via type guards (instanceof checks, property validation) before accessing error properties.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/typescript-patterns/SKILL.md` at line 32, Replace the ambiguous
guidance "Use async/await with typed catch blocks" in SKILL.md with a precise
instruction to use `unknown` for catch parameters and then narrow the error
before use (via instanceof, custom type guards, or property checks); update the
surrounding sentence to explicitly state that catch variables should be treated
as unknown and narrowed rather than annotated with a concrete type, and keep the
existing advice to prefer Promise.allSettled for parallel operations that can
partially fail.
|
Closing — 32 CI failures. SKILL.md format likely breaks catalog validators. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b3b16f779
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| --- | ||
| name: typescript-patterns | ||
| description: TypeScript best practices, type safety, generics, async patterns, and project configuration for Claude Code and Codex. | ||
| --- |
There was a problem hiding this comment.
Because npm test runs node scripts/ci/catalog.js --text (package.json:107), adding this 116th skill without updating the documented totals makes the default validation fail for every branch that includes this commit. Running the catalog check on top of this change reports skills: 116 while README.md and AGENTS.md still advertise 115, so CI will stay red until those counts are bumped.
Useful? React with 👍 / 👎.
| --- | ||
| name: typescript-patterns | ||
| description: TypeScript best practices, type safety, generics, async patterns, and project configuration for Claude Code and Codex. | ||
| --- |
There was a problem hiding this comment.
This new skill is not wired into any install path yet. Manifest-driven installs only copy entries from module.paths (scripts/lib/install-targets/helpers.js:260-277), lang:typescript still resolves via framework-language (manifests/install-components.json:53-58), and that module does not include skills/typescript-patterns (manifests/install-modules.json:105-138). In practice, users who install the TypeScript bundle—e.g. via npx ecc typescript—won't receive the skill you added here.
Useful? React with 👍 / 👎.
1 participant
Adds TypeScript patterns skill covering type safety, generics, async patterns, and configuration.
Summary by cubic
Adds a new TypeScript patterns skill to guide strict config, type-safe errors, generics, async patterns, and utility types. Helps teams write safer, clearer TypeScript.
skills/typescript-patterns/SKILL.mdwith guidance on stricttsconfig.json(strict, noUncheckedIndexedAccess, exactOptionalPropertyTypes).extends) and async patterns (typedcatch, preferPromise.allSettled).Pick,Omit,Partial,Required,Record) and examples usingzod, const assertions, and thesatisfiesoperator.Written for commit 9b3b16f. Summary will update on new commits.
Summary by CodeRabbit