feat!: entry command lazy loading - #118
Conversation
WalkthroughThis update refactors the CLI framework to consistently use Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant CommandResolver
participant CommandExecutor
User->>CLI: invoke cli(argv, entry, options)
CLI->>CLI: resolveCliOptions(options, entry)
CLI->>CommandResolver: resolveCommand(sub, entry, options)
CommandResolver-->>CLI: { commandName, command, callMode }
CLI->>CommandExecutor: executeCommand(command, ctx, commandName)
CommandExecutor->>command: run(ctx)
CommandExecutor-->>CLI: result
CLI-->>User: CLI execution complete
Possibly related PRs
Suggested labels
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🪧 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: |
be4b27a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fdcfb0e4.gunshi.pages.dev |
| Branch Preview URL: | https://feat-entry-lazy-loading1.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: |
kazupon
changed the title
![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 (3)
src/cli.ts (3)
94-102:getCommandArgscan return an object with a live reference – consider defensive cloning
cmd.argsis returned directly when present.
If downstream code mutates the returned object (e.g.,ctx.args.foo = false) it will mutate the original definition attached to the command, which might be reused on subsequent invocations and cause subtle bugs in long-living CLI processes.- return cmd.args || create<A>() + return cmd.args ? { ...cmd.args } as A : create<A>()Copy-on-read keeps command definitions immutable and thread-safe.
230-236: Mutation of sub-command definition breaks immutabilityInside
resolveCommand, the sub-command object is mutated in-place:cmd.name = subIf the same
cmdinstance is reused elsewhere, this implicit state change can leak outside the CLI flow. Prefer returning a shallow-cloned object with thenamepopulated, or set the default lazily when the command is first defined.
248-258: Error message may be blank when command name is omittedWhen the entry command has no explicit name,
nameis passed as an empty string.
executeCommandthen throws:throw new Error(`'run' not found on Command \`${name}\``)which results in
'run' not found on Command– note the missing identifier.
Consider using a fallback such as'(anonymous)'for better debuggability:- throw new Error(`'run' not found on Command \`${name}\``) + throw new Error(`'run' not found on Command \`${name || "(anonymous)"}\``)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docs/guide/advanced/documentation-generation.md(1 hunks)docs/guide/essentials/declarative-configuration.md(1 hunks)src/cli.test.ts(4 hunks)src/cli.ts(5 hunks)src/constants.ts(2 hunks)src/context.test.ts(7 hunks)src/context.ts(6 hunks)src/generator.ts(2 hunks)src/renderer.test.ts(13 hunks)src/types.ts(2 hunks)src/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/constants.ts (1)
src/types.ts (1)
CliOptions(158-222)
src/context.ts (4)
src/types.ts (3)
CliOptions(158-222)CommandEnvironment(81-153)Command(322-351)src/utils.ts (2)
create(102-104)log(106-108)src/constants.ts (2)
COMMAND_OPTIONS_DEFAULT(48-63)NOOP(20-20)src/translation.ts (1)
createTranslationAdapter(10-14)
src/utils.ts (1)
src/types.ts (3)
LazyCommand(457-466)Commandable(471-471)Command(322-351)
src/cli.test.ts (2)
src/definition.ts (1)
lazy(37-49)src/types.ts (2)
Command(322-351)CliOptions(158-222)
🪛 LanguageTool
docs/guide/essentials/declarative-configuration.md
[uncategorized] ~226-~226: Loose punctuation mark.
Context: ...definition object itself. - cliOptions: The resolved CLI options passed to `cli...
(UNLIKELY_OPENING_PUNCTUATION)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (19)
docs/guide/advanced/documentation-generation.md (1)
56-56: Update description to include lazy command functions
The parameter doc forentrynow correctly states it can accept lazy command functions, aligning with the newLazyCommandsupport.docs/guide/essentials/declarative-configuration.md (1)
226-226: Align property name in documentation
The context property name is updated tocliOptions, matching the refactoring of theCommandOptionsinterface toCliOptions.🧰 Tools
🪛 LanguageTool
[uncategorized] ~226-~226: Loose punctuation mark.
Context: ...definition object itself. -cliOptions: The resolved CLI options passed to `cli...(UNLIKELY_OPENING_PUNCTUATION)
src/constants.ts (1)
7-7: Rename type import and default options constant
The import was changed fromCommandOptionstoCliOptions, andCOMMAND_OPTIONS_DEFAULTnow usesCliOptions<Args>for consistency with the renamed interface.Also applies to: 48-48
src/renderer.test.ts (3)
102-102: Consistent rename inrenderHeadertests
All calls tocreateCommandContextin therenderHeadersuite have been updated fromcommandOptionstocliOptions, matching the core API change.Also applies to: 125-125, 145-145, 163-163
215-215: Consistent rename inrenderUsagetests
ThecliOptionsproperty is used uniformly across allrenderUsagetest cases, ensuring the tests reflect the updated context API.Also applies to: 243-243, 287-287, 323-323, 371-371, 419-419, 469-469, 493-493
516-516: Consistent rename inrenderValidationErrorstest
The single test forrenderValidationErrorsnow passescliOptionsinstead of the oldcommandOptions.src/context.test.ts (1)
66-66: Rename parameter increateCommandContextcalls
All instances of thecommandOptionsparameter in this test suite have been renamed tocliOptions, consistent with the new interface and function signature.Also applies to: 157-157, 204-204, 265-265, 340-340, 413-413, 469-469
src/types.ts (2)
84-107: Good documentation updatesThe JSDoc comments have been correctly updated to reference
CliOptionsinstead ofCommandOptions, maintaining consistent API documentation.
155-158: Appropriate interface renaming from CommandOptions to CliOptionsThe renaming from
CommandOptionstoCliOptionsprovides a clearer description of the interface's purpose. This is a good naming improvement that better reflects that these are options for the CLI function rather than for a specific command.src/utils.ts (2)
22-24: Good addition of type guard for LazyCommandAdding a dedicated type guard function
isLazyCommandimproves type safety and makes the code more maintainable. The check for'commandName' in cmd && !!cmd.commandNameensures that only properly configured lazy commands are recognized.
32-32: Updated to use the new type guard functionGood refactoring to use the new
isLazyCommandtype guard instead of a generic function check. This makes the code more precise and type-safe.src/cli.test.ts (4)
9-9: Updated import to use CliOptionsThe import statement has been correctly updated to use
CliOptionsinstead ofCommandOptions.
276-283: Improved lazy command implementationThe test now properly uses the
lazyhelper function to create a lazy command, which is more consistent with the improved lazy command support in the codebase.
284-284: Updated command registration keyThe registration key for the lazy command has been correctly updated to use
command2.commandNameinstead ofcommand2.name, reflecting the structure of lazy commands.
563-563: Updated type assertionsThe type assertions for the options objects have been correctly updated to use
CliOptionsinstead ofCommandOptions.Also applies to: 612-612
src/context.ts (3)
33-33: Updated import to use CliOptionsThe import statement has been correctly updated to use
CliOptionsinstead ofCommandOptions.
87-87: Updated parameter type in CommandContextParamsThe parameter type in the
CommandContextParamsinterface has been updated to usecliOptionsinstead ofcommandOptions, maintaining consistency with the interface rename.
106-107: Comprehensive parameter and variable renamingAll occurrences of
commandOptionshave been consistently renamed tocliOptionsthroughout the file. This maintains the API consistency with the interface renaming.Also applies to: 123-123, 125-125, 128-128, 189-189, 213-213
src/cli.ts (1)
104-106:Details
❓ Verification inconclusive
Source-order of
COMMON_ARGSoverrides user options
Object.assign(create<A>(), args, COMMON_ARGS)copiesCOMMON_ARGSlast, so built-in flags (--help,--version…) overwrite any identically-named user-defined positional or option already present inargs. If the intention is to let framework defaults augment but never override the command’s declaration, swap the order:-return Object.assign(create<A>(), args, COMMON_ARGS) +return Object.assign(create<A>(), COMMON_ARGS, args)Please double-check the desired precedence rules.
Re-evaluate argument precedence in resolveArguments
The current merge orderObject.assign(create<A>(), args, COMMON_ARGS)applies
COMMON_ARGSlast, so built-in flags (e.g.--help,--version) will override identically named user options. If the intent is for framework defaults to only augment—not override—user inputs, swap the order:-return Object.assign(create<A>(), args, COMMON_ARGS) +return Object.assign(create<A>(), COMMON_ARGS, args)Please confirm which precedence rule you’d like to enforce.
- File: src/cli.ts, Lines 104–106
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Description
Linked Issues
Additional context
Summary by CodeRabbit
Documentation
New Features
Refactor
CommandOptionstoCliOptionsthroughout the codebase and updated related type names and references for consistency.Tests
cliOptionsproperty name and adjusted type usage accordingly.