fix: create dojo contract manifest path #356
Conversation
WalkthroughThe changes in this pull request focus on modifying the Changes
Possibly related PRs
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 (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/create-dojo/src/commands/start.ts(1 hunks)
🔇 Additional comments (2)
packages/create-dojo/src/commands/start.ts (2)
124-126: 🛠️ Refactor suggestion
Enhance error handling with specific error messages
The current error handling could be more informative to help users troubleshoot issues.
Consider this improvement:
} catch (error) {
- console.warn(`Warning: Could not update dojoConfig.ts: ${error}`);
+ if (error.code === 'ENOENT') {
+ console.error(`Error: dojoConfig.ts not found in ${clientPath}`);
+ } else if (error.code === 'EACCES') {
+ console.error(`Error: Permission denied accessing dojoConfig.ts`);
+ } else {
+ console.error(`Error updating dojoConfig.ts: ${error.message}`);
+ }
+ throw new Error('Failed to configure project. Please check the errors above.');
}Likely invalid or redundant comment.
Line range hint 123-127: Verify manifest path existence and improve error handling
The current implementation assumes the manifest path exists and continues silently on errors. This could lead to runtime issues.
Let's verify the manifest path assumptions:
| @@ -120,7 +120,7 @@ async function rewriteDojoConfigFile(clientPath: string) { | |||
| // Update relative imports to account for new directory structure | |||
| content = content.replace( | |||
| /from ['"]\.{0,2}\/.*manifest(?:_dev)?\.json['"]/g, | |||
| 'from "../contract/target/dev/manifest.json"' | |||
| 'from "../contract/manifest_dev.json"' | |||
There was a problem hiding this comment.
Consider making the manifest path configurable
The hardcoded path "../contract/manifest_dev.json" reduces flexibility. Different projects might have varying directory structures or environment-specific manifest files.
Consider:
- Making the path configurable through project settings
- Restoring the regex pattern for flexibility
- Supporting environment-specific paths
- 'from "../contract/manifest_dev.json"'
+ // Read from project config or fall back to default
+ `from "${config.manifestPath || '../contract/manifest_dev.json'}"`Committable suggestion skipped: line range outside the PR's diff.
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
Bug Fixes
Chores