-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/88 create cli tool for scaffolding #127
base: main
Are you sure you want to change the base?
Conversation
Greetings from Munich! 🥨 |
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes in this pull request introduce several configuration files and modifications to streamline project setup and management. Key updates include the addition of a command-line interface (CLI) in Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (13)
refarch-cli/.prettierignore (1)
1-3
: Consider adding more common ignore patterns.While the current patterns cover the basics, consider adding these common patterns to ensure comprehensive coverage:
dist target node_modules +coverage +*.log +.env*refarch-cli/.npmrc (1)
1-2
: Document security implications in READMESince this configuration affects lockfile content and package source verification, it would be helpful to document these implications in the project's README.
Consider adding a section in the README that explains:
- Why this configuration was chosen
- What it means for users installing the package
- Any potential security considerations when using the scaffolding tool
refarch-cli/tsconfig.json (2)
11-11
: Consider adding test files to exclude listThe exclude configuration is good for build artifacts and dependencies, but consider also excluding test files if they exist.
- "exclude": ["node_modules", "./dist/**/*"] + "exclude": ["node_modules", "./dist/**/*", "**/*.test.ts", "**/*.spec.ts"]
1-12
: Consider additional TypeScript configurations for CLI developmentSince this is a CLI tool, consider adding these configurations to enhance development:
"compilerOptions": { + "strict": true, + "resolveJsonModule": true, + "allowJs": true, // ... existing config }Rationale:
strict: true
- Enables all strict type checking optionsresolveJsonModule
- Useful for importing package.json or config filesallowJs
- Helpful if mixing JavaScript and TypeScript during developmentrefarch-cli/rollup.config.js (1)
13-18
: Consider enabling source maps for better debugging.The plugins configuration is correct, but adding source map support would improve the debugging experience.
Consider updating the TypeScript plugin configuration:
- typescript(), + typescript({ + sourceMap: true, + inlineSources: true + }),.gitignore (1)
35-36
: Consider adding more Node.js and Rollup-specific patterns.Since this is a Node.js project using Rollup, consider adding these commonly ignored patterns:
# Node node_modules +# Logs +logs +*.log +npm-debug.log* + +# Coverage directory used by tools like Istanbul +coverage + +# Rollup build outputs +.rollup.cache/ + +# Environment variables +.env +.env.localrefarch-cli/eslint.config.js (2)
8-14
: Consider adding more common ignore patternsWhile the current ignore patterns are good, consider adding more common patterns for a Node.js CLI tool.
ignores: [ "**/build/**", "**/dist/**", "index.js", "eslint.config.js", "rollup.config.js", + "**/node_modules/**", + "coverage/**", + ".git/**" ],
32-32
: Consider broadening the file patternsThe current configuration only targets
index.ts
. As the project grows, you might want to lint all TypeScript files.- files: ["index.ts"], + files: ["**/*.ts"],refarch-cli/package.json (1)
5-9
: Add essential npm scriptsConsider adding the following scripts:
test
: For running testsprepare
: For building the package during installationclean
: For cleaning the dist directory before buildApply this diff to add the suggested scripts:
"scripts": { + "clean": "rm -rf dist", - "build": "rollup -c", + "build": "npm run clean && rollup -c", "lint": "prettier . --check && eslint . ", - "fix": "prettier . --write && eslint . --fix" + "fix": "prettier . --write && eslint . --fix", + "test": "echo \"No tests specified\" && exit 1", + "prepare": "npm run build" },refarch-cli/index.ts (4)
18-18
: Typo in comment: 'Projekt' should be 'Project'In the comment at line 18, 'Projekt' should be corrected to 'Project' to fix the typo.
23-23
: Clarify the selection message for better user understandingThe message
"Select Project you want to generate with enter"
could be clearer. Consider rephrasing it to"Select the project you want to generate and press enter"
.
130-130
: UseforEach
instead ofmap
when not using the returned arrayAt line 130, using
map
without utilizing the returned array is less efficient. UseforEach
instead.Apply this diff to improve the code:
- replacements.map((options) => replaceInFileSync(options)); + replacements.forEach((options) => replaceInFileSync(options));
207-207
: UseforEach
instead ofmap
when not using the returned arrayAt line 207, using
map
without utilizing the returned array is inefficient. UseforEach
instead.Apply this diff to improve the code:
- replacements.map((options) => replaceInFileSync(options)); + replacements.forEach((options) => replaceInFileSync(options));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
refarch-cli/package-lock.json
is excluded by!**/package-lock.json
📒 Files selected for processing (9)
.gitignore
(1 hunks)refarch-cli/.npmrc
(1 hunks)refarch-cli/.prettierignore
(1 hunks)refarch-cli/.prettierrc.json
(1 hunks)refarch-cli/eslint.config.js
(1 hunks)refarch-cli/index.ts
(1 hunks)refarch-cli/package.json
(1 hunks)refarch-cli/rollup.config.js
(1 hunks)refarch-cli/tsconfig.json
(1 hunks)
🔇 Additional comments (12)
refarch-cli/.prettierignore (1)
2-2
: Verify if target
directory is necessary.
The target
directory is typically associated with Java/Maven projects. Since this appears to be a Node.js project using Rollup for building, this entry might be unnecessary unless there's a specific use case.
refarch-cli/.prettierrc.json (1)
1-1
: Verify package declaration and compatibility
Let's ensure the package is properly declared in package.json and check its availability.
✅ Verification successful
Package is properly configured and available
The package @muenchen/prettier-codeformat
is:
- Correctly declared in package.json as a devDependency with version 1.0.2
- Available in the npm registry with latest version 1.0.2
- Compatible with the project's setup as it's used in conjunction with Prettier 3.3.3
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package declaration and npm registry availability
# Check if package is declared in package.json
echo "Checking package.json for @muenchen/prettier-codeformat..."
rg -A 1 -B 1 "@muenchen/prettier-codeformat" package.json
# Check npm registry for package info
echo "Checking npm registry for package..."
curl -s https://registry.npmjs.org/@muenchen/prettier-codeformat | jq '.["dist-tags"].latest'
Length of output: 424
Script:
#!/bin/bash
# Let's check the actual package.json file location and content
fd -t f "package.json"
# Once we find the package.json, let's examine its dependencies
echo "Content of package.json files:"
fd -t f "package.json" -x cat {}
Length of output: 1118
refarch-cli/.npmrc (1)
1-2
: Verify npm version compatibility
The configuration looks good and helps prevent leaking registry URLs in lockfiles. However, we should verify compatibility with different npm versions since this is a CLI tool that will be distributed.
refarch-cli/tsconfig.json (2)
4-5
: LGTM: Module configuration aligns with modern Node.js practices
The configuration correctly uses:
"module": "NodeNext"
for latest Node.js module resolution"target": "ESNext"
for modern JavaScript features"moduleResolution": "NodeNext"
matching the module setting
Also applies to: 9-9
3-3
: Verify TypeScript declaration file generation
The compilation settings look good, but since this is a CLI tool that will be published as an npm package:
declaration: true
will generate .d.ts files which might not be necessary for a CLI tool- Consider adding
"types": "./dist/index.d.ts"
to package.json if TypeScript types are intended to be consumed
Also applies to: 6-8
✅ Verification successful
Declaration files are not needed for this CLI package
This is a pure CLI tool as evidenced by:
- The package.json defines it as a CLI through the
bin
field with thescaffold
command - No
types
field is specified in package.json - The package is not meant to be programmatically imported (no exports field)
Recommendations:
- Remove
"declaration": true
from tsconfig.json as TypeScript declaration files are not necessary for a CLI-only tool - No need to add
types
field to package.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if package.json has types field and if the CLI is meant to be programmatically consumed
cat package.json | jq '.types, .exports, .bin'
# Check if there are any TypeScript imports of this package in the codebase
rg -l "from ['\"](scaffolding-refarch|@[^/]+/scaffolding-refarch)['\"]" -g "*.ts"
Length of output: 181
Script:
#!/bin/bash
# Let's first find the package.json file as it might be in a subdirectory
fd package.json
# Let's also check if there are any .d.ts files being generated
fd "\.d\.ts$"
# Check the tsconfig.json content to see current declaration settings
cat refarch-cli/tsconfig.json
Length of output: 356
Script:
#!/bin/bash
# Let's check the package.json content to see if this is meant to be a CLI-only tool
cat refarch-cli/package.json
# Let's also check if there are any TypeScript files that export public APIs
rg "export" -g "*.ts" -l refarch-cli/src/
Length of output: 1094
refarch-cli/rollup.config.js (2)
1-4
: LGTM! All necessary Rollup plugins are imported.
The imports cover all required functionality for building a TypeScript-based CLI tool: TypeScript compilation, Node.js module resolution, CommonJS module conversion, and JSON file support.
7-12
: Verify the entry point file exists.
The configuration looks good and follows modern Node.js practices using ESM format. However, let's verify the entry point file exists.
✅ Verification successful
Let me check the actual content of the entry point file to ensure it's properly set up as a CLI entry point.
Entry point file is properly configured
The entry point file exists and is correctly set up as a CLI application with:
- Proper shebang for Node.js execution (
#!/usr/bin/env node
) - CLI-related imports from
@inquirer/prompts
- Interactive CLI implementation with proper command handling
The rollup configuration with ESM format and .mjs
extension aligns perfectly with this implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the entry point file exists and contains CLI-related code
# Check if index.ts exists
if [ -f "refarch-cli/index.ts" ]; then
echo "Entry point exists"
# Look for CLI-related imports/code
rg -l "commander|yargs|cli|prompt" "refarch-cli/index.ts"
else
echo "Entry point missing"
fi
Length of output: 159
Script:
#!/bin/bash
# Check the content of index.ts to verify CLI setup
cat refarch-cli/index.ts
Length of output: 8130
.gitignore (1)
35-36
: LGTM! The Node.js section is appropriately added.
The addition of node_modules
to .gitignore
is correct and follows the standard practice for Node.js projects.
refarch-cli/eslint.config.js (2)
1-5
: LGTM! Good setup with TypeScript checking
The file starts with proper TypeScript checking enabled and correctly imports the necessary ESLint modules for the flat config format.
27-31
: 🛠️ Refactor suggestion
Verify if console statements should be completely disabled
Since this is a CLI tool, completely disabling console statements might be too restrictive. Consider allowing certain console methods like info
or error
for user feedback.
rules: {
- "no-console": "error",
+ "no-console": ["error", { allow: ["info", "error", "warn"] }],
"no-unused-vars": "off",
"@typescript-eslint/no-unused-vars": ["error"],
},
✅ Verification successful
The no-console rule should remain as "error"
After examining the codebase, this CLI tool uses the @inquirer/prompts
package for user interaction instead of console statements. The tool performs file operations and user prompts without needing console logging. The strict no-console rule is appropriate here as it enforces the use of proper CLI interaction libraries rather than direct console output.
- The tool uses
select
,input
, andconfirm
from@inquirer/prompts
for all user interactions - File operations use native Node.js APIs without requiring console logging
- No debugging needs were identified that would require console statements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the codebase uses console statements for user feedback
rg "console\." --type ts
Length of output: 26
Script:
#!/bin/bash
# Let's try a broader search for console statements in all file types
rg "console\." -l
# Also check for any logging utilities or custom logger implementations
rg -i "log\(" -l
Length of output: 38
Script:
#!/bin/bash
# Let's check if there are any source files in the repository
fd -t f -e ts -e js -e tsx -e jsx
# Also check the package.json to understand the project type
cat package.json 2>/dev/null || echo "No package.json found"
Length of output: 186
Script:
#!/bin/bash
# Let's examine the content of index.ts to understand how the CLI tool handles output
cat refarch-cli/index.ts
Length of output: 8130
refarch-cli/package.json (2)
14-18
: Review dependency versions and security
The core-js package has newer versions available. Let's verify the latest versions and any security advisories.
19-36
: 🛠️ Refactor suggestion
Fix version constraint and verify dependencies
The eslint-plugin-tsdoc
uses a caret version (^0.3.0
) while other dependencies use fixed versions. Consider using a fixed version for consistency.
Apply this diff:
- "eslint-plugin-tsdoc": "^0.3.0",
+ "eslint-plugin-tsdoc": "0.3.0",
Let's verify if any packages are deprecated or have known issues:
✅ Verification successful
Update eslint-plugin-tsdoc and fix version constraint
The eslint-plugin-tsdoc
package:
- Has a newer stable version (0.4.0) released recently
- Shows no deprecation notices
- Uses inconsistent version constraint (^0.3.0) compared to other dependencies
Apply this diff:
- "eslint-plugin-tsdoc": "^0.3.0",
+ "eslint-plugin-tsdoc": "0.4.0",
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Install dependencies in a temporary directory and check for warnings
TEMP_DIR=$(mktemp -d)
cd "$TEMP_DIR"
echo '{"name":"temp","version":"1.0.0"}' > package.json
npm install --package-lock-only $(cat /workspace/refarch-cli/package.json | jq -r '.devDependencies | to_entries | map("\(.key)@\(.value)") | .[]') 2>&1 | grep -i "warn"
Length of output: 345
Script:
#!/bin/bash
# Let's verify if the package.json exists and check its content
fd -t f "package.json" --exec cat {}
Length of output: 1003
Script:
#!/bin/bash
# Check npm registry for eslint-plugin-tsdoc package info
npm view eslint-plugin-tsdoc time --json | jq -r 'to_entries | .[] | select(.key != "modified" and .key != "created") | "\(.key): \(.value)"' | sort -V | tail -n 5
# Check for any deprecation notices
npm view eslint-plugin-tsdoc deprecated
Length of output: 370
@@ -0,0 +1 @@ | |||
"@muenchen/prettier-codeformat" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix invalid .prettierrc.json format
The current format is invalid. When extending a shared Prettier configuration, it should be specified using proper JSON syntax.
Apply this diff to fix the configuration:
-"@muenchen/prettier-codeformat"
+{
+ "extends": "@muenchen/prettier-codeformat"
+}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
"@muenchen/prettier-codeformat" | |
{ | |
"extends": "@muenchen/prettier-codeformat" | |
} |
output: { | ||
dir: "dist", | ||
format: "esm", | ||
entryFileNames: "[name].mjs", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Add exports field to package.json for proper ESM package configuration
The package.json correctly specifies "type": "module"
for ESM compatibility, but lacks the exports field which is recommended for ESM packages. Add an exports configuration that matches your Rollup output:
{
"exports": {
".": "./dist/index.mjs"
}
}
This ensures proper module resolution and maintains compatibility with both ESM and CommonJS consumers.
🔗 Analysis chain
Verify npm package distribution configuration.
Since this is being built as an npm package for scaffolding, consider the following architectural points:
- Ensure
package.json
has the correct"type": "module"
field for ESM compatibility - Verify the package exports configuration matches this output structure
- Consider adding a
preserveModules: true
option if you need to maintain the module structure for better tree-shaking
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package.json configuration
# Check package.json configuration
if [ -f "refarch-cli/package.json" ]; then
echo "Package type configuration:"
jq '.type' "refarch-cli/package.json"
echo "Package exports configuration:"
jq '.exports' "refarch-cli/package.json"
fi
Length of output: 262
parserOptions: { | ||
projectService: true, | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the TypeScript parser configuration
The projectService
option is incorrect. To enable project-wide type checking, use project: true
instead.
parserOptions: {
- projectService: true,
+ project: true,
},
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
parserOptions: { | |
projectService: true, | |
}, | |
parserOptions: { | |
project: true, | |
}, |
{ | ||
"name": "scaffolding-refarch", | ||
"main": "dist/index.mjs", | ||
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add required package.json fields
The following required fields are missing:
version
: Required for npm packageslicense
: Required for npm packagesrepository
: Important for linking to source codedescription
: Should describe the package's purpose
Apply this diff to add the missing fields:
{
"name": "scaffolding-refarch",
+ "version": "0.1.0",
"main": "dist/index.mjs",
"type": "module",
+ "description": "CLI tool for scaffolding reference architecture projects",
+ "license": "MIT",
+ "repository": {
+ "type": "git",
+ "url": "https://github.com/owner/repo.git"
+ },
Also applies to: 13-13
"bin": { | ||
"scaffold": "dist/index.mjs" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
File has shebang but needs executable permissions
The entry point already has the correct shebang (#!/usr/bin/env node
), but the file permissions are not set for execution (-rw-r--r--
). You need to:
- Add a build step to set executable permissions:
"scripts": {
"build": "... && chmod +x dist/index.mjs"
}
- Document npm link permission requirements in README as suggested.
🔗 Analysis chain
Ensure proper CLI setup
The binary configuration needs additional setup:
- Add shebang to index.ts
- Set executable permissions in the build process
- Document the npm link permission requirements in README
Let's verify if the entry point has the correct shebang:
Consider documenting the following in your README:
- Running
npm link
requires admin privileges - Alternative: Use
npx
for running without installation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if index.ts has the correct shebang
rg -l '^#!.*node' src/
Length of output: 69
Script:
#!/bin/bash
# First, let's find the source files
fd -t f "index" --full-path
# Then check for shebang in any index files
fd -t f "index" --exec rg -l '^#!.*node' {}
Length of output: 117
Script:
#!/bin/bash
# Let's check the actual content of the index.ts to verify the shebang
cat refarch-cli/index.ts | head -n 5
# Also check if the file has executable permissions
ls -l refarch-cli/index.ts
Length of output: 331
const pass = value.match(/([a-z-])+/g); | ||
return pass | ||
? true | ||
: "ArtifactId name not valid (should match expression '^de\\.muenchen\\.[a-z0-9]+(\\.[a-z0-9]+)*$'): "; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix inconsistency between validation and error message for artifactId
The validation regex for artifactId
at line 70 is /([a-z-])+
but the error message refers to a different pattern. Ensure that the error message matches the regex used in validation.
Apply this diff to correct the error message:
const pass = value.match(/([a-z-])+/g);
return pass
? true
- : "ArtifactId name not valid (should match expression '^de\\.muenchen\\.[a-z0-9]+(\\.[a-z0-9]+)*$'): ";
+ : "ArtifactId name not valid (should contain only lowercase letters and hyphens)";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const pass = value.match(/([a-z-])+/g); | |
return pass | |
? true | |
: "ArtifactId name not valid (should match expression '^de\\.muenchen\\.[a-z0-9]+(\\.[a-z0-9]+)*$'): "; | |
const pass = value.match(/([a-z-])+/g); | |
return pass | |
? true | |
: "ArtifactId name not valid (should contain only lowercase letters and hyphens)"; |
await projectConfiguration(); | ||
} else { | ||
await generateDefaultCliForDocsAndStack(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid recursion in projectConfiguration
to prevent potential stack overflow
The recursive call to projectConfiguration
at line 44 may lead to a stack overflow if the user goes through many iterations. Consider refactoring the function to use a loop instead.
Apply this diff to refactor the code:
async function projectConfiguration() {
+ let result: string;
+ do {
result = await select({
message: "Select the project you want to generate and press enter",
choices: [
{ name: applications.FRONTEND, value: applications.FRONTEND },
{ name: applications.BACKEND, value: applications.BACKEND },
{ name: applications.EAI, value: applications.EAI },
{ name: EXIT, value: EXIT },
new Separator(),
],
});
switch (result) {
case applications.EAI:
await generateJavaInteractiveCli(applications.EAI);
break;
case applications.BACKEND:
await generateJavaInteractiveCli(applications.BACKEND);
break;
case applications.FRONTEND:
await generateFrontendInteractiveCli();
break;
}
- if (result != EXIT) {
- await projectConfiguration();
- } else {
- await generateDefaultCliForDocsAndStack();
- }
+ } while (result != EXIT);
+ await generateDefaultCliForDocsAndStack();
}
Committable suggestion skipped: line range outside the PR's diff.
Description
npm link scaffolding-refarch
-> run it withnode .\node_modules\scaffolding-refarch
-> scaffolding should start inside the CLIReference
Issues #88
Summary by CodeRabbit
New Features
Configuration Updates
.gitignore
to streamline ignored files and directories..npmrc
configuration to modify lockfile behavior..prettierignore
to exclude specific directories from formatting.package.json
file to establish project dependencies and scripts.