feat: extract load commands as plugin extension - #184
Conversation
WalkthroughThe changes refactor command loading in the Gunshi project by removing the internal Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Context
participant LoaderPlugin
Renderer->>Context: Access extensions.loader.loadCommands()
Context->>LoaderPlugin: Call loadCommands()
LoaderPlugin->>Context: Retrieve subCommands from env
LoaderPlugin-->>Renderer: Return loaded commands
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (4)
✨ Finishing Touches
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 (
|
commit: |
kazupon
changed the title
Deploying gunshi with
|
| Latest commit: |
c95c83c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9739dbfc.gunshi.pages.dev |
| Branch Preview URL: | https://feat-command-loader-as-plugi.gunshi.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/gunshi/src/plugins/loader.ts (1)
51-53: Address the TODO comment for setup logicThe setup method contains a TODO comment indicating incomplete implementation. Consider what setup logic might be needed for the loader plugin.
Do you want me to help implement the setup logic or create an issue to track this task?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/gunshi/src/context.ts(2 hunks)packages/gunshi/src/plugins/index.ts(1 hunks)packages/gunshi/src/plugins/loader.ts(1 hunks)packages/gunshi/src/renderer.test.ts(2 hunks)packages/gunshi/src/renderer/usage.ts(3 hunks)packages/gunshi/src/types.ts(0 hunks)
💤 Files with no reviewable changes (1)
- packages/gunshi/src/types.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (13)
packages/gunshi/src/renderer.test.ts (2)
3-3: LGTM: Loader plugin import added correctlyThe import statement properly brings in the loader plugin needed for the test context.
495-497: ```shell
#!/bin/bashSearch for subCommands usage in all test files across the repo
rg -Hn "subCommands" -g "*.test.ts"
Search for loadCommands usage in all test files across the repo
rg -Hn "loadCommands" -g "*.test.ts"
</details> <details> <summary>packages/gunshi/src/plugins/index.ts (2)</summary> `9-9`: **LGTM: Loader plugin import added** The import statement correctly adds the loader plugin to the available plugins. --- `14-14`: **LGTM: Loader plugin properly integrated into plugin array** The loader plugin is correctly positioned in the plugins array between globals and defaultRenderer, which is appropriate for the command loading functionality. </details> <details> <summary>packages/gunshi/src/plugins/loader.ts (2)</summary> `16-22`: **LGTM: Well-defined interface for command loading** The `LoaderCommandContext` interface clearly defines the contract for command loading functionality with proper generic support. --- `27-43`: **LGTM: Solid implementation with caching** The extension implementation is well-structured with proper caching to avoid redundant command loading. The use of `Promise.all` for concurrent command resolution is efficient. </details> <details> <summary>packages/gunshi/src/context.ts (3)</summary> `34-34`: **LGTM: Import cleanup aligns with refactoring** The removal of `resolveLazyCommand` import is consistent with moving command loading functionality to the loader plugin. --- `242-252`: **LGTM: Improved extension handling** The refactored extension handling is more robust, using `defineProperty` to properly manage the extensions object on the core context. This provides better encapsulation and type safety. --- `253-253`: **LGTM: Simplified context creation** The simplified approach of directly freezing and returning the core object is cleaner than the previous conditional extension merging. </details> <details> <summary>packages/gunshi/src/renderer/usage.ts (4)</summary> `16-16`: **LGTM: Import added for loader extension interface** The import of `LoaderCommandContext` is necessary for the updated generic constraints. --- `143-145`: **LGTM: Generic constraint ensures loader extension availability** The updated generic constraint properly requires the presence of the loader extension, ensuring type safety for the command loading functionality. --- `147-147`: **LGTM: Safe access to loader extension with fallback** The use of optional chaining with fallback to empty array provides robust error handling in case the loader extension is not available. --- `213-217`: **LGTM: Consistent pattern for loader extension access** The `hasCommands` function follows the same pattern as `renderCommandsSection`, properly constraining generics and safely accessing the loader extension. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
1 participant
Description
Linked Issues
Additional context
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation