fix: improvement plugin type inference - #262
Conversation
WalkthroughThis update introduces advanced TypeScript type utilities and generics to the core plugin system, refactors plugin declarations in several packages to leverage enhanced type inference, and adds comprehensive type-level tests. The changes focus on improving static type safety for plugin dependencies and extensions, with no alterations to runtime logic. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PluginFactory as plugin()
participant PluginOptions
participant DependencyTypeUtils
User->>PluginFactory: Call plugin<TDeps, TId, TExt, ...>(options)
PluginFactory->>DependencyTypeUtils: Infer dependency extensions/types
DependencyTypeUtils-->>PluginFactory: Return merged extension types
PluginFactory->>PluginOptions: Construct plugin with inferred types
PluginOptions-->>PluginFactory: Return strongly-typed plugin
PluginFactory-->>User: Return plugin with merged extension typings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
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
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. 🪧 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 (
|
Deploying gunshi with
|
| Latest commit: |
9bfc606
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://77524828.gunshi.pages.dev |
| Branch Preview URL: | https://fix-plugin-typing.gunshi.pages.dev |
@gunshi/bone
@gunshi/definition
gunshi
@gunshi/plugin
@gunshi/plugin-completion
@gunshi/plugin-dryrun
@gunshi/plugin-global
@gunshi/plugin-i18n
@gunshi/plugin-renderer
@gunshi/resources
@gunshi/shared
commit: |
![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
🧹 Nitpick comments (2)
packages/gunshi/src/plugin/core.ts (2)
115-176: Sophisticated type system for plugin extensionsThe enhanced
PluginOptionsinterface provides excellent type safety by computing and flowing extension types through all plugin callbacks. While the generic parameters add complexity, they ensure compile-time verification of plugin compatibility.Consider documenting these complex type parameters with examples to help plugin developers understand the type flow.
320-321: Consider adding a comment explaining theanytype usageWhile the
anytype is necessary here to handle all overloads, consider adding a comment explaining why it's safe in this context to help future maintainers.// eslint-disable-next-line @typescript-eslint/no-explicit-any +// Implementation uses 'any' to handle all overload signatures - type safety is enforced at the API level export function plugin(options: any = {}): any {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
packages/gunshi/src/plugin/core.test-d.ts(1 hunks)packages/gunshi/src/plugin/core.test.ts(3 hunks)packages/gunshi/src/plugin/core.ts(5 hunks)packages/plugin-completion/src/index.ts(1 hunks)packages/plugin-i18n/src/index.ts(2 hunks)packages/plugin-renderer/src/index.ts(2 hunks)packages/plugin/package.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
**/*.ts: Use ES modules throughout the codebase
Follow existing code style (enforced by ESLint and Prettier)
Files:
packages/plugin-completion/src/index.tspackages/plugin-i18n/src/index.tspackages/gunshi/src/plugin/core.test-d.tspackages/gunshi/src/plugin/core.test.tspackages/gunshi/src/plugin/core.tspackages/plugin-renderer/src/index.ts
packages/gunshi/src/**/*.ts
📄 CodeRabbit Inference Engine (CLAUDE.md)
packages/gunshi/src/**/*.ts: All source code is in TypeScript with strict mode enabled
Type safety is a core feature - maintain strict TypeScript types throughout
Files:
packages/gunshi/src/plugin/core.test-d.tspackages/gunshi/src/plugin/core.test.tspackages/gunshi/src/plugin/core.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
packages/plugin/package.json (2)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
packages/plugin-i18n/src/index.ts (3)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to **/*.ts : Use ES modules throughout the codebase
packages/gunshi/src/plugin/core.test-d.ts (8)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Test files are in 'packages/gunshi/test/' with '.test.ts' extension
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Mock external dependencies when needed in tests
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Use 'describe' and 'it' blocks for test organization
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Snapshot tests are used for renderer output validation
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to **/*.ts : Use ES modules throughout the codebase
packages/gunshi/src/plugin/core.test.ts (8)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Test files are in 'packages/gunshi/test/' with '.test.ts' extension
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Mock external dependencies when needed in tests
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Use 'describe' and 'it' blocks for test organization
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Snapshot tests are used for renderer output validation
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to **/*.ts : Use ES modules throughout the codebase
packages/gunshi/src/plugin/core.ts (5)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Add tests for new features in the corresponding test file
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Test files are in 'packages/gunshi/test/' with '.test.ts' extension
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Mock external dependencies when needed in tests
packages/plugin-renderer/src/index.ts (4)
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : All source code is in TypeScript with strict mode enabled
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Snapshot tests are used for renderer output validation
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/test/**/*.test.ts : Test files are in 'packages/gunshi/test/' with '.test.ts' extension
Learnt from: CR
PR: kazupon/gunshi#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T07:12:47.997Z
Learning: Applies to packages/gunshi/src/**/*.ts : Type safety is a core feature - maintain strict TypeScript types throughout
🧬 Code Graph Analysis (2)
packages/plugin-i18n/src/index.ts (2)
packages/shared/src/utils.ts (1)
namespacedId(77-81)packages/plugin-i18n/src/types.ts (2)
I18nCommandContext(41-74)I18nPluginOptions(79-92)
packages/gunshi/src/plugin/core.test-d.ts (1)
packages/gunshi/src/plugin/core.ts (8)
ExtractDependencyId(27-31)PluginDependency(77-87)IsOptionalDependency(37-41)InferDependencyExtensions(61-71)PluginOptions(133-176)PluginFunction(93-95)PluginExtension(101-104)OnPluginExtension(110-113)
🔇 Additional comments (17)
packages/plugin/package.json (1)
3-3: LGTM! Enhanced description reflects improved plugin capabilities.The updated description "plugin development kit for gunshi" better represents the package's purpose than the previous "utilities for gunshi plugin", especially given the type inference improvements introduced in this PR.
packages/plugin-i18n/src/index.ts (2)
108-108: Excellent type safety improvement withas const.Using
as constensures the dependencies array is treated as an immutable tuple with literal types, enabling precise type inference in the plugin system. This aligns with the enhanced plugin typing introduced in the core.
187-187: Good removal of explicit type assertion.Allowing TypeScript to infer the return type naturally is more robust than explicit type assertions and provides better type safety. The compiler can now verify the return type matches the expected interface.
packages/plugin-completion/src/index.ts (2)
39-40: Consistent dependency declaration pattern.The
as constassertion on the dependencies array follows the same pattern established in other plugins, ensuring proper type inference and immutability. Good practice to centralize the dependency declaration.
48-56: Excellent explicit generic typing.The explicit generic parameters enhance type safety by specifying:
- Command context mapping for extensions
- Plugin ID for proper identification
- Dependencies for type inference
This leverages the improved plugin typing system introduced in the core.
packages/gunshi/src/plugin/core.test.ts (3)
10-10: Good cleanup of unused import.Removing the
ExtendContextimport after replacing it with inline types in the test assertions is proper housekeeping.
204-206: Appropriate inline type assertions.Replacing
ExtendContextwith explicit inline object types makes the test assertions more explicit and focused on the specific extension properties being tested. This maintains type safety while improving readability.Also applies to: 212-214
239-239: Consistent type assertion pattern.The inline
{ auth: {} }type assertion follows the same pattern as the earlier changes, maintaining consistency throughout the test file.Also applies to: 244-244
packages/gunshi/src/plugin/core.test-d.ts (5)
1-13: Excellent comprehensive type testing setup.The imports are well-organized and cover all the necessary type utilities from
core.ts. The switch to Vitest'sexpectTypeOffor type-level testing is appropriate for validating the enhanced plugin type system.
15-30: Thorough testing of ExtractDependencyId utility.The tests cover all the key scenarios:
- Generic PluginDependency interface
- Specific object with id
- Object with id and optional flag
This validates that the type utility correctly extracts dependency IDs from various input forms.
32-49: Comprehensive IsOptionalDependency testing.The test cases properly validate the optional dependency detection logic, covering both explicit true/false cases and the default behavior. This ensures the type system correctly identifies optional dependencies.
56-115: Excellent coverage of InferDependencyExtensions complexity.The test suite covers critical scenarios:
- Required and optional dependencies mixing
- Unavailable dependencies handling
- Empty dependencies edge case
- Mixed string and object dependency forms
- Long dependency chains
This thoroughly validates the recursive type inference logic that's central to the plugin system.
117-245: Comprehensive PluginOptions generic validation.The tests validate all aspects of the complex generic PluginOptions interface:
- Default type parameters
- Fully specified generics
- Required and optional dependency mixing
- Unmatched dependencies handling
- No dependencies edge case
This ensures the plugin options typing works correctly across all usage patterns. The commented note about
toEqualTypeOflimitations shows good awareness of testing constraints.packages/plugin-renderer/src/index.ts (2)
55-56: Correct implementation of typed dependenciesThe explicit declaration of dependencies as a const-asserted array enables proper type inference in the enhanced plugin system.
124-128: Clean simplification leveraging improved type inferenceThe removal of explicit generic type annotations demonstrates that the enhanced type system now properly infers types throughout the decorator chain.
packages/gunshi/src/plugin/core.ts (2)
23-72: Well-crafted type utilities for dependency inferenceThe type utilities correctly implement recursive type inference for plugin dependencies with proper handling of optional dependencies. The use of tail recursion pattern ensures TypeScript can optimize the type computation.
217-312: Type-safe plugin factory overloadsThe overloaded signatures correctly distinguish between plugins with and without extensions, ensuring proper typing based on the presence of the
extensionproperty.
|
|
||
| return { | ||
| text: localizable(ctx, cmd, i18n?.translate), | ||
| text: localizable(ctx as unknown as CommandContext, cmd, i18n?.translate), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the necessity of type casting to CommandContext
The cast ctx as unknown as CommandContext suggests a type mismatch. Consider if the localizable function's type signature could be adjusted to accept the actual context type instead.
🏁 Script executed:
#!/bin/bash
# Search for the localizable function definition to understand its type requirements
ast-grep --pattern 'export $_ function localizable($_) {
$$$
}'
# Also check for any type definitions
rg -A 5 "localizable.*CommandContext"Length of output: 475
🏁 Script executed:
#!/bin/bash
# Show import statements in plugin-renderer/src/index.ts
echo "Import statements in plugin-renderer/src/index.ts:"
sed -n '1,50p' packages/plugin-renderer/src/index.ts | rg "import"
# Search for export of localizable function across repo
echo "Searching for export of localizable function:"
rg -n "export .*localizable" -n .
# Search for any standalone definition of localizable
echo "Searching for any function localizable definition:"
rg -n "function localizable" -n .Length of output: 1123
🏁 Script executed:
#!/bin/bash
# Display lines around the localizable function definition to inspect its signature
echo "Context around localizable definition in packages/shared/src/localization.ts:"
sed -n '1,200p' packages/shared/src/localization.tsLength of output: 3015
Update localizable signature to support extended contexts
The cast ctx as unknown as CommandContext indicates that localizable only accepts the base CommandContext, despite having a generic C for extended contexts. We should change its signature so callers can pass in any context that extends CommandContext without casting.
Suggested diff in packages/shared/src/localization.ts:
-export function localizable<
- A extends Args,
- C = {}, // for CommandContext
- E extends Record<string, string> = {}, // for extended resources
- K = ResolveTranslationKeys<A, C, E>
->(
- ctx: CommandContext, cmd: Command, translate?: Translation<A, C, E, K>
+export function localizable<
+ A extends Args,
+ C extends CommandContext = CommandContext, // allow extended contexts
+ E extends Record<string, string> = {}, // for extended resources
+ K = ResolveTranslationKeys<A, C, E>
+>(
+ ctx: C,
+ cmd: Command,
+ translate?: Translation<A, C, E, K>
): Localization<A, C, E> {
…And in packages/plugin-renderer/src/index.ts, you can then drop the cast:
- text: localizable(ctx as unknown as CommandContext, cmd, i18n?.translate),
+ text: localizable(ctx, cmd, i18n?.translate),Files to update:
- packages/shared/src/localization.ts
- packages/plugin-renderer/src/index.ts
🤖 Prompt for AI Agents
In packages/shared/src/localization.ts, update the localizable function
signature to accept a generic context type C that extends CommandContext,
allowing it to handle extended contexts without casting. Then, in
packages/plugin-renderer/src/index.ts at line 118, remove the cast from ctx when
calling localizable, passing ctx directly. This change ensures type safety and
cleaner code by leveraging generics properly.
1 participant
Description
Linked Issues
Additional context
Summary by CodeRabbit
New Features
Refactor
Tests