feat(cli): Non-interactive mode and new structure #3606
Conversation
…aming in add commands - WIP
…ments ## to do: - Docs - write spy tests - rerun every command and test that every single one works in interactive and non-interactive mode. - time-out after 1 min so ai agentic editors know that they shouldn't call interactive mode.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces a comprehensive overhaul of the Vendure CLI's command structure to support both interactive and non-interactive modes. It adds new modules for command registration, plugin resolution, and project validation, and refactors existing commands to accept options for non-interactive execution. Extensive documentation and usage examples are provided, along with improved error handling, prompt timeouts, and test coverage for migration operations. Changes
Sequence Diagram(s)High-level Control Flow: CLI Command Execution (Interactive vs Non-Interactive)sequenceDiagram
participant User
participant CLI
participant CommandRegistry
participant CommandHandler
User->>CLI: Run CLI command (e.g., add, migrate)
CLI->>CommandRegistry: Register commands
CLI->>CommandHandler: Invoke command with options
alt Non-interactive mode (flags provided)
CommandHandler->>CommandHandler: Validate and execute with options
CommandHandler->>CLI: Output result, exit
else Interactive mode (no flags)
CommandHandler->>User: Prompt for input (with timeout)
User->>CommandHandler: Provide input
CommandHandler->>CommandHandler: Execute with user input
CommandHandler->>CLI: Output result, exit
end
Plugin Resolution LogicsequenceDiagram
participant CommandHandler
participant PluginResolution
participant User
CommandHandler->>PluginResolution: resolvePluginFromOptions(options)
alt Plugin provided
PluginResolution-->>CommandHandler: Return plugin
else Plugin name provided
PluginResolution->>PluginResolution: Search plugins by name
alt Found
PluginResolution-->>CommandHandler: Return plugin
else Not found
PluginResolution-->>CommandHandler: Throw error (list available plugins)
end
else Non-interactive mode
PluginResolution-->>CommandHandler: Throw error (plugin required)
else Interactive mode
PluginResolution-->>CommandHandler: Indicate prompt needed
CommandHandler->>User: Prompt for plugin selection
User->>CommandHandler: Select plugin
CommandHandler->>PluginResolution: Return selected plugin
end
Migration Operation (Non-Interactive)sequenceDiagram
participant User
participant CLI
participant MigrationOperation
User->>CLI: Run migrate command with flags (--generate, --run, --revert)
CLI->>MigrationOperation: Call corresponding operation with options
MigrationOperation->>MigrationOperation: Validate project, analyze config
MigrationOperation->>CLI: Return result (success/error)
CLI->>User: Output result, exit
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! 📜 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 (15)
🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (7)
packages/cli/src/shared/cli-command-definition.ts (1)
1-19: Strong foundation for the new CLI architecture.The interfaces provide a clean, declarative approach to command definition. The nested sub-options feature (line 7) is particularly well-designed for complex CLI scenarios.
Consider improving type safety:
- defaultValue?: any; + defaultValue?: string | number | boolean;The
Record<string, any>type for the action function parameter (line 14) may be acceptable for CLI flexibility, but consider if a more specific type would be beneficial as the codebase evolves.packages/cli/src/commands/command-declarations.ts (1)
80-84: Consider adding error handling around dynamic imports.While the current implementation is clean, consider wrapping the dynamic import and command execution in a try-catch block to provide better error messages if the command modules fail to load or execute.
action: async options => { - const { addCommand } = await import('./add/add'); - await addCommand(options); - process.exit(0); + try { + const { addCommand } = await import('./add/add'); + await addCommand(options); + process.exit(0); + } catch (error) { + console.error(`Failed to execute add command: ${error.message}`); + process.exit(1); + } },packages/cli/src/shared/command-registry.ts (1)
35-50: Consider adding validation for option format.While the current implementation works well, consider adding validation to ensure option strings follow expected patterns (e.g., long options start with
--).function addOption(command: Command, option: CliCommandOption): void { const parts: string[] = []; if (option.short) { + if (!option.short.startsWith('-') || option.short.startsWith('--')) { + throw new Error(`Invalid short option format: ${option.short}. Should start with single dash.`); + } parts.push(option.short); } + if (!option.long.startsWith('--')) { + throw new Error(`Invalid long option format: ${option.long}. Should start with double dash.`); + } parts.push(option.long);packages/cli/src/commands/add/service/add-service.ts (1)
42-48: Improve error message for non-interactive mode.The error message mentions using
selectPluginin interactive mode, which is not applicable here.-throw new Error('Plugin must be specified when running in non-interactive mode. Use selectPlugin in interactive mode.'); +throw new Error('Plugin must be specified when running in non-interactive mode.');packages/cli/src/shared/vendure-config-ref.ts (1)
73-82: Simplify the helper method logic.The default value for
checkFileNameistruebut it's always called withfalse. Also, the nested ternary reduces readability.private isVendureConfigFile( sourceFile: SourceFile, - options: { checkFileName?: boolean } = {}, + options: { checkFileName: boolean } = { checkFileName: true }, ): boolean { - const checkFileName = options.checkFileName ?? true; - return ( - (checkFileName ? sourceFile.getFilePath().endsWith('vendure-config.ts') : true) && - !!sourceFile.getVariableDeclarations().find(v => this.isVendureConfigVariableDeclaration(v)) - ); + const hasVendureConfig = sourceFile + .getVariableDeclarations() + .some(v => this.isVendureConfigVariableDeclaration(v)); + + if (!hasVendureConfig) { + return false; + } + + if (!options.checkFileName) { + return true; + } + + return sourceFile.getFilePath().endsWith('vendure-config.ts'); }packages/cli/src/commands/add/job-queue/add-job-queue.ts (1)
82-87: Remove redundant non-interactive mode check.The check on lines 85-87 is redundant because
pluginis already assigned on line 82 using the nullish coalescing operator. In non-interactive mode, this check will never be true.plugin = plugin ?? (await selectPlugin(project, cancelledMessage)); - // In non-interactive mode, we cannot prompt for service selection - if (isNonInteractive && !plugin) { - throw new Error('Cannot select service in non-interactive mode - plugin must be specified'); - }packages/cli/src/commands/migrate/migration-operations.ts (1)
162-164: Improve Vendure dependency detection to avoid false positives.The current check might incorrectly identify third-party packages that contain '@vendure/' in their name as official Vendure packages.
const hasVendureDeps = Object.keys(dependencies).some( - dep => dep.includes('@vendure/') || dep === 'vendure', + dep => dep.startsWith('@vendure/') || dep === '@vendure/core', );This ensures only official Vendure packages are detected.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
packages/cli/src/README.md(1 hunks)packages/cli/src/cli.ts(2 hunks)packages/cli/src/commands/add/add-operations.ts(1 hunks)packages/cli/src/commands/add/add.ts(2 hunks)packages/cli/src/commands/add/api-extension/add-api-extension.ts(3 hunks)packages/cli/src/commands/add/codegen/add-codegen.ts(4 hunks)packages/cli/src/commands/add/entity/add-entity.ts(4 hunks)packages/cli/src/commands/add/job-queue/add-job-queue.ts(5 hunks)packages/cli/src/commands/add/plugin/create-new-plugin.ts(3 hunks)packages/cli/src/commands/add/plugin/types.ts(1 hunks)packages/cli/src/commands/add/service/add-service.ts(3 hunks)packages/cli/src/commands/add/ui-extensions/add-ui-extensions.ts(3 hunks)packages/cli/src/commands/add/ui-extensions/codemods/update-admin-ui-plugin-init/update-admin-ui-plugin-init.spec.ts(2 hunks)packages/cli/src/commands/command-declarations.ts(1 hunks)packages/cli/src/commands/migrate/migrate.ts(2 hunks)packages/cli/src/commands/migrate/migration-operations.ts(1 hunks)packages/cli/src/shared/cli-command-definition.ts(1 hunks)packages/cli/src/shared/command-registry.ts(1 hunks)packages/cli/src/shared/shared-prompts.ts(2 hunks)packages/cli/src/shared/vendure-config-ref.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (22.x, sqljs)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: codegen / codegen
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: build (20.x)
- GitHub Check: build (22.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (31)
packages/cli/src/commands/add/plugin/types.ts (1)
4-4: LGTM: Clean interface extension.The addition of the optional
configproperty appropriately supports the new non-interactive mode functionality while maintaining backward compatibility.packages/cli/src/commands/add/ui-extensions/codemods/update-admin-ui-plugin-init/update-admin-ui-plugin-init.spec.ts (1)
17-17: Verify VendureConfigRef constructor change doesn't break functionality.The removal of the
{ checkFileName: false }option suggests a breaking change to theVendureConfigRefconstructor API. Ensure that the new constructor signature properly handles the file name checking functionality that was previously controlled by this option.#!/bin/bash # Description: Verify VendureConfigRef constructor and its usage across the codebase # Expected: Find new constructor signature and ensure all usages are updated # Search for VendureConfigRef constructor definition ast-grep --pattern 'class VendureConfigRef { $$$ constructor($$$) { $$$ } $$$ }' # Search for VendureConfigRef instantiations to verify consistency rg -A 3 "new VendureConfigRef"Also applies to: 35-35
packages/cli/src/cli.ts (2)
6-7: Clean architectural improvement.The new imports support the centralized command registration approach, moving away from individual command definition to a more maintainable declarative structure.
30-31: I couldn’t confirm support for options, sub-commands, or handlers without seeing the full implementation. Let’s pull the contents of both files:#!/bin/bash # Show the registerCommands implementation cat packages/cli/src/shared/command-registry.ts # Show the cliCommands definitions cat packages/cli/src/commands/command-declarations.tspackages/cli/src/shared/shared-prompts.ts (3)
16-16: Good addition to support non-interactive mode.The optional
configparameter enables explicit configuration file specification, supporting the CLI's new non-interactive capabilities.
27-36: Excellent improvement for CLI project analysis.The addition of
rootDir: './src'compiler option ensures the CLI analyzes source files rather than compiled JavaScript files, which is exactly what's needed for CLI tooling. The structured approach to passing compiler options is clean and maintainable.
41-41: Proper config parameter propagation.Including the config option in the return value ensures it's available to downstream consumers, maintaining consistency with the non-interactive mode architecture.
packages/cli/src/commands/command-declarations.ts (1)
3-85: Well-structured command declaration with good separation of concerns.The declarative approach for CLI commands is excellent - it centralizes command definitions and makes the CLI more maintainable. The option structure is comprehensive and properly typed.
packages/cli/src/README.md (2)
22-29: Documentation inconsistency with actual implementation.The
CliCommandOptioninterface documented here uses aflagproperty, but the actual implementation incommand-declarations.tsuses separateshortandlongproperties.interface CliCommandOption { - flag: string; // Option flag (e.g., '-f, --file <path>') + short?: string; // Short flag (e.g., '-f') + long: string; // Long flag (e.g., '--file <path>') description: string; // Option description required?: boolean; // Whether the option is required defaultValue?: any; // Default value for the option + subOptions?: CliCommandOption[]; // Optional nested sub-options }Likely an incorrect or invalid review comment.
78-89: Update example to match actual implementation.The example uses the
flagproperty which doesn't match the actual interface. Update to useshortandlongproperties.options: [ { - flag: '-f, --file <path>', + short: '-f', + long: '--file <path>', description: 'Path to the file', required: true, }, { - flag: '-v, --verbose', + long: '--verbose', description: 'Enable verbose output', required: false, defaultValue: false, }, ],Likely an incorrect or invalid review comment.
packages/cli/src/commands/add/ui-extensions/add-ui-extensions.ts (3)
15-20: Good interface extension for non-interactive mode support.The addition of
pluginName,config, andisNonInteractiveoptions properly extends the interface to support both interactive and non-interactive usage patterns.
39-58: Excellent plugin resolution and validation logic.The plugin lookup by name with proper error handling and the enforcement of plugin specification in non-interactive mode are well-implemented. The error message listing available plugins is particularly helpful for users.
111-111: Consistent config parameter usage.The VendureConfigRef instantiation properly uses the optional config parameter, maintaining consistency with the enhanced interface.
packages/cli/src/commands/add/entity/add-entity.ts (3)
27-29: Consistent interface extension.The addition of
configandisNonInteractiveoptions maintains consistency with other command interfaces in the refactor.
44-47: Clear error handling for non-interactive mode.The validation that ensures a plugin is specified in non-interactive mode is essential and provides a clear error message.
89-95: Sensible defaults for non-interactive mode.Providing default feature flags (
customFields: true,translatable: false) when running in non-interactive mode without explicit features is a good UX decision.packages/cli/src/shared/command-registry.ts (2)
5-33: Excellent centralized command registration system.The
registerCommandsfunction elegantly handles the complex command structure with nested sub-options. The visual indentation for sub-options in help text is a nice UX touch.
44-47: Smart handling of optional value syntax.The regex replacement converting
<value>to[value]for optional options is a clever solution that maintains Commander.js compatibility while supporting the declarative option structure.packages/cli/src/commands/add/codegen/add-codegen.ts (4)
16-19: LGTM! Clear interface extension for non-interactive mode.The added properties properly support non-interactive CLI usage with explicit plugin and config specification.
33-33: LGTM! Config parameter properly propagated.
63-65: LGTM! Clean conditional logic for plugin selection.
111-114: LGTM! Consistent plugin processing.The refactored loop simplifies the code by always iterating through the plugins array.
packages/cli/src/commands/add/service/add-service.ts (4)
27-29: LGTM! Interface properly extended.
52-65: LGTM! Proper handling of interactive vs non-interactive modes.The code correctly defaults to 'basic' type and skips cancel checks in non-interactive mode.
67-70: LGTM! Proper option setup with sensible defaults.
94-115: LGTM! Smart service name handling.The code efficiently skips the prompt when a custom service name is provided.
packages/cli/src/commands/add/add.ts (3)
8-20: LGTM! Clean imports and interface definition.
21-31: LGTM! Clear separation of interactive and non-interactive modes.The logic correctly detects non-interactive mode based on the presence of any options.
63-113: LGTM! Clean refactoring of interactive mode logic.The code was successfully extracted into a separate function without changing its behavior.
packages/cli/src/commands/add/add-operations.ts (1)
10-35: LGTM! Well-structured interface for CLI operations.The interface provides a comprehensive set of options for all add operations with clear documentation.
packages/cli/src/shared/vendure-config-ref.ts (1)
27-40: LGTM! Excellent error messages for config file issues.The error messages clearly guide users on how to resolve multiple config files or missing config scenarios.
packages/cli/src/commands/add/api-extension/add-api-extension.ts
Outdated
Show resolved
Hide resolved
packages/cli/src/commands/add/api-extension/add-api-extension.ts
Outdated
Show resolved
Hide resolved
Docstrings generation was requested by @HouseinIsProgramming. * #3606 (comment) The following files were modified: * `packages/cli/src/commands/add/add-operations.ts` * `packages/cli/src/commands/add/add.ts` * `packages/cli/src/commands/add/api-extension/add-api-extension.ts` * `packages/cli/src/commands/add/codegen/add-codegen.ts` * `packages/cli/src/commands/add/entity/add-entity.ts` * `packages/cli/src/commands/add/job-queue/add-job-queue.ts` * `packages/cli/src/commands/add/plugin/create-new-plugin.ts` * `packages/cli/src/commands/add/service/add-service.ts` * `packages/cli/src/commands/add/ui-extensions/add-ui-extensions.ts` * `packages/cli/src/commands/migrate/migrate.ts` * `packages/cli/src/commands/migrate/migration-operations.ts` * `packages/cli/src/shared/command-registry.ts` * `packages/cli/src/shared/shared-prompts.ts`
|
Note Generated docstrings for this pull request at #3607 |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
packages/cli/src/commands/add/add-operations.ts (1)
240-254: Improve error handling consistency.The current approach of checking for specific error messages is fragile and creates tight coupling, as noted in previous reviews.
Consider using error types or codes instead:
} catch (error: any) { - // Re-throw validation errors so they can be properly handled with stack trace - if ( - error.message.includes('is required') || - error.message.includes('cannot be empty') || - error.message.includes('must be specified') - ) { - throw error; - } + // Re-throw validation errors + if (error.name === 'ValidationError' || error.code === 'VALIDATION_ERROR') { + throw error; + } return { success: false, message: error.message ?? 'Add operation failed', }; }Then update the validation errors to use a custom error class:
class ValidationError extends Error { name = 'ValidationError'; code = 'VALIDATION_ERROR'; }docs/docs/guides/developer-guide/cli/index.md (1)
44-44: Address previous reviewer feedback on terminology.A previous reviewer requested using "scripting" instead of "automation" in this context.
Consider updating:
- Non-interactive mode: Allows direct command execution with arguments and options, perfect for automation, CI/CD, and AI agents + Non-interactive mode: Allows direct command execution with arguments and options, perfect for scripting, CI/CD, and AI agents
🧹 Nitpick comments (5)
packages/cli/e2e/migrate-command.e2e-spec.ts (5)
83-97: Expand test coverage for invalid migration names.The test only covers basic invalid names. Consider adding more edge cases for comprehensive validation.
const invalidNames = [ '123-invalid', // starts with number 'test migration', // contains space 'test@migration', // special character + '', // empty string + 'a', // too short (less than 2 chars) + 'test/migration', // path separator + 'test\\migration', // backslash + 'test.migration', // dot + 'migration!', // exclamation + 'test>migration', // greater than + 'test<migration', // less than + 'a'.repeat(256), // extremely long name ];
117-122: Remove implementation details from test comments.The comment mentions "synchronize is false" which is an implementation detail that doesn't belong in tests. This makes the test fragile to implementation changes.
- // Since synchronize is false, generateMigration will create the initial migration - // The first time it runs, it will generate all tables - // Subsequent runs may report no changes - // Both are valid outcomes + // The operation should complete successfully + // It may generate migrations or report no changes depending on the database state
332-353: Enhance concurrent operations test to verify proper concurrency handling.The test runs operations concurrently but doesn't verify if they properly handle potential race conditions or file conflicts.
it('should handle concurrent operations gracefully', async () => { process.chdir(TEST_PROJECT_DIR); // Try to run multiple operations concurrently const operations = [ generateMigrationOperation({ name: 'Concurrent1', outputDir: MIGRATIONS_DIR }), generateMigrationOperation({ name: 'Concurrent2', outputDir: MIGRATIONS_DIR }), generateMigrationOperation({ name: 'Concurrent3', outputDir: MIGRATIONS_DIR }), ]; const results = await Promise.all(operations); // All should complete without throwing results.forEach(result => { expect(result).toHaveProperty('success'); expect(result).toHaveProperty('message'); }); // At least some should succeed const successCount = results.filter(r => r.success).length; expect(successCount).toBeGreaterThan(0); + + // Verify that generated files don't conflict + const files = await fs.readdir(MIGRATIONS_DIR); + const timestamps = files + .filter(f => f.endsWith('.ts')) + .map(f => f.split('-')[0]); + + // Check for unique timestamps (no race condition in timestamp generation) + const uniqueTimestamps = new Set(timestamps); + expect(uniqueTimestamps.size).toBe(timestamps.length); });
356-373: Improve interrupted migration simulation.The hardcoded partial migration content might not accurately represent a real interrupted migration scenario.
it('should recover from interrupted migration generation', async () => { process.chdir(TEST_PROJECT_DIR); // Create a partial migration file to simulate interruption const partialFile = path.join(MIGRATIONS_DIR, '1234567890-Partial.ts'); - await fs.writeFile(partialFile, 'export class Partial1234567890 {}'); + // Create a more realistic partial migration file + await fs.writeFile(partialFile, ` +import { MigrationInterface, QueryRunner } from "typeorm"; + +export class Partial1234567890 implements MigrationInterface { + public async up(queryRunner: QueryRunner): Promise<void> { + // Incomplete migration +`); // Should still be able to generate new migrations const result = await generateMigrationOperation({ name: 'RecoveryTest', outputDir: MIGRATIONS_DIR, }); // The operation should complete successfully expect(result.success).toBe(true); expect(result.message).toBeDefined(); });
399-400: Add newline at end of file.The file is missing a newline character at the end, which is a common convention.
}); }); +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.eslintrc.js(1 hunks)docs/docs/guides/developer-guide/cli/index.md(3 hunks)packages/cli/.gitignore(1 hunks)packages/cli/e2e/config/tsconfig.e2e.json(1 hunks)packages/cli/e2e/fixtures/test-entity.ts(1 hunks)packages/cli/e2e/fixtures/test-project/package.json(1 hunks)packages/cli/e2e/fixtures/test-project/src/vendure-config.ts(1 hunks)packages/cli/e2e/fixtures/test-project/tsconfig.json(1 hunks)packages/cli/e2e/fixtures/vendure-config.ts(1 hunks)packages/cli/e2e/migrate-command.e2e-spec.ts(1 hunks)packages/cli/e2e/vitest.e2e.config.mts(1 hunks)packages/cli/package.json(1 hunks)packages/cli/src/README.md(1 hunks)packages/cli/src/commands/add/add-operations.ts(1 hunks)packages/cli/src/commands/command-declarations.ts(1 hunks)packages/cli/tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (9)
- .eslintrc.js
- packages/cli/e2e/fixtures/test-project/tsconfig.json
- packages/cli/tsconfig.json
- packages/cli/e2e/fixtures/test-project/package.json
- packages/cli/.gitignore
- packages/cli/src/README.md
- packages/cli/e2e/config/tsconfig.e2e.json
- packages/cli/e2e/fixtures/vendure-config.ts
- packages/cli/e2e/fixtures/test-project/src/vendure-config.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/src/commands/command-declarations.ts
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: codegen / codegen
- GitHub Check: build (22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/cli/src/commands/add/add-operations.ts (1)
60-234: Comprehensive validation implementation looks excellent.The validation logic for all operations is thorough and consistent. Each operation properly validates required parameters with clear error messages and appropriate usage examples. This addresses the previous concern about missing validation for other operations.
docs/docs/guides/developer-guide/cli/index.md (1)
155-267: Excellent comprehensive documentation.The documentation clearly distinguishes between interactive and non-interactive modes with comprehensive examples, detailed flag descriptions, and proper validation notes. This significantly improves the developer experience for both manual and automated CLI usage.
packages/cli/package.json (1)
25-26: Clean e2e script addition.The new e2e script properly integrates with the Vitest e2e configuration and follows the existing script pattern.
packages/cli/e2e/vitest.e2e.config.mts (1)
1-31: Well-configured e2e test setup.The Vitest configuration is properly set up for CLI e2e testing:
- SWC plugin enables decorator support for TypeORM entities
- Single fork prevents test conflicts
- Appropriate timeout and test pattern matching
- Node environment suitable for CLI testing
packages/cli/e2e/fixtures/test-entity.ts (1)
1-15: Clean test entity implementation.The test entity follows proper TypeORM and Vendure conventions with appropriate field types and decorators. Well-suited for testing CLI migration and entity generation functionality.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/cli/src/commands/add/service/add-service.ts (1)
53-53: Improve non-interactive mode detection logic.The current detection
providedOptions?.serviceName !== undefinedmay incorrectly classify empty strings as interactive mode triggers. Consider using a more explicit check.- const isNonInteractive = providedOptions?.serviceName !== undefined || providedOptions?.isNonInteractive; + const isNonInteractive = (providedOptions?.serviceName !== undefined && providedOptions?.serviceName !== '') || providedOptions?.isNonInteractive;Alternatively, consider requiring an explicit
isNonInteractiveflag for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/cli/src/commands/add/add-operations.ts(1 hunks)packages/cli/src/commands/add/entity/add-entity.ts(5 hunks)packages/cli/src/commands/add/service/add-service.ts(4 hunks)packages/cli/src/commands/command-declarations.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/cli/src/commands/command-declarations.ts
- packages/cli/src/commands/add/add-operations.ts
- packages/cli/src/commands/add/entity/add-entity.ts
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: unit tests (20.x)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: codegen / codegen
- GitHub Check: e2e tests (22.x, mysql)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: e2e tests (22.x, postgres)
- GitHub Check: e2e tests (22.x, sqljs)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: unit tests (22.x)
- GitHub Check: build (22.x)
- GitHub Check: build (20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (13)
packages/cli/src/commands/add/service/add-service.ts (13)
16-16: LGTM: Import addition is appropriate.The
getPluginClassesimport aligns with the new plugin resolution functionality added below.
18-18: LGTM: Timeout utility import supports interactive enhancements.The
withInteractiveTimeoutimport is correctly used to wrap interactive prompts throughout the file.
28-32: LGTM: Interface extension supports non-interactive mode.The new optional properties appropriately extend the interface to support both interactive and non-interactive modes.
46-50: LGTM: Config option passed to analyzeProject.The config parameter is correctly passed through to support project analysis with custom configuration.
54-58: LGTM: Appropriate validation for non-interactive mode.The error message clearly indicates what's required and provides helpful guidance for the user.
63-75: LGTM: Plugin resolution by name is well-implemented.The logic correctly finds plugins by name and provides helpful error messages with available alternatives when the specified plugin is not found.
81-93: LGTM: Service type selection handles both modes appropriately.The logic correctly defaults to 'basic' in non-interactive mode and wraps the interactive prompt with timeout functionality.
94-97: LGTM: Cancellation handling respects non-interactive mode.The condition correctly skips cancellation handling when in non-interactive mode.
99-102: LGTM: Options assignment includes new config parameter.The config option is properly passed through to support the enhanced functionality.
107-118: LGTM: Entity selection by name is well-implemented.The logic correctly finds entities by name within the plugin and provides helpful error messages with available alternatives.
119-132: LGTM: Fallback to interactive entity selection is appropriate.The existing interactive logic is preserved when not in non-interactive mode or when no entity name is provided.
142-156: LGTM: Service name handling supports both modes.The logic correctly skips the prompt when a non-default service name is provided and maintains the validation logic for interactive mode.
158-163: LGTM: Cancellation handling consistent with non-interactive mode.The condition correctly skips cancellation handling when in non-interactive mode, maintaining consistency with the pattern established earlier in the file.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/cli/e2e/vitest.e2e.config.mts (1)
14-14: Consider improving type safety for the SWC plugin configuration.The
as anytype assertion suggests a potential type compatibility issue with the SWC plugin and Vitest.Consider using a more specific type assertion if possible:
- }) as any, + }) as Parameters<typeof defineConfig>[0]['plugins'][0],Or alternatively, check if there's a more recent version of
unplugin-swcthat provides better TypeScript compatibility with Vitest.packages/cli/e2e/add-command.e2e-spec.ts (1)
25-33: Consider improving type safety in the stubRun utility.The
as anycasting in the stubRun function, while functional, could be improved for better type safety.Consider using a more specific type constraint:
-function stubRun(cmd: { run: (...args: any[]) => any }): Spy { +function stubRun<T extends { run: (...args: any[]) => Promise<any> }>(cmd: T): Spy { // Cast to 'any' to avoid over-constraining the generic type parameters that // vitest uses on spyOn, which causes type inference issues in strict mode. // The runtime behaviour (spying on an object method) is what matters for // these tests – the precise compile-time types are not important. return vi - .spyOn(cmd as any, 'run') + .spyOn(cmd, 'run') .mockResolvedValue({ project: undefined, modifiedSourceFiles: [] } as any); }This maintains the flexibility while providing better type constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/cli/e2e/add-command.e2e-spec.ts(1 hunks)packages/cli/e2e/migrate-command.e2e-spec.ts(1 hunks)packages/cli/e2e/vitest.e2e.config.mts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/cli/e2e/migrate-command.e2e-spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build (22.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 20.x)
🔇 Additional comments (5)
packages/cli/e2e/vitest.e2e.config.mts (1)
1-31: Well-configured e2e test setup for CLI testing.The Vitest configuration is appropriately structured for end-to-end CLI testing with proper environment settings, timeout configuration, and SWC plugin integration for decorator support.
packages/cli/e2e/add-command.e2e-spec.ts (4)
1-201: Excellent comprehensive test coverage for the add command.This test suite provides thorough coverage of all add command operations including success scenarios, error handling, and non-interactive mode validation. The test structure is well-organized with proper setup and cleanup.
69-74: Good validation test for plugin name requirements.The test properly validates that empty plugin names are rejected, which is essential for non-interactive mode reliability.
96-101: Excellent test coverage for non-interactive mode constraints.This test properly validates that the add command correctly enforces plugin name requirements when running in non-interactive mode, which is crucial for the new CLI structure.
191-199: Good fallback behavior test for invalid operations.The test ensures that the command gracefully handles cases where no valid operation is specified, providing appropriate error messaging.
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
84d79b1 to
ed840b4
Compare
ed840b4 to
1e4de15
Compare
|
Description
TO-DO:
Checklist
📌 Always:
👍 Most of the time:
Summary by CodeRabbit
New Features
add,migrate, and sub-commands) with new options for specifying plugins, entities, services, job queues, API extensions, UI extensions, and custom config files.Documentation
Bug Fixes
Tests
Chores