feat: fixed issue with example-vite-react-sdk #343
Conversation
WalkthroughThe pull request introduces modifications to enhance the Dojo framework's configuration and functionality. Key changes include the addition of new properties to the Changes
Assessment against linked issues
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 2
🧹 Outside diff range and nitpick comments (7)
examples/example-vite-react-sdk/dojoConfig.ts (1)
5-12: Add JSDoc comments to document configuration parametersTo enhance the Quick Start Guide experience and prevent confusion, consider adding documentation for each configuration parameter explaining its purpose and expected format.
Here's a suggested improvement:
+/** + * Dojo configuration for the example React SDK + * @property {string} rpcUrl - The HTTP URL for the RPC endpoint (e.g., "http://localhost:5050/") + * @property {string} toriiUrl - The WebSocket URL for real-time updates (e.g., "http://localhost:8080") + * @property {string} masterAddress - The master account's public address for authentication + * @property {string} masterPrivateKey - The master account's private key (use env vars in production) + */ export const dojoConfig = createDojoConfig({ manifest, rpcUrl: "http://localhost:5050/", toriiUrl: "http://localhost:8080", masterAddress: "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec", masterPrivateKey: "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912", });examples/example-vite-react-sdk/src/useSystemCalls.ts (4)
44-44: Consider making the timeout configurableThe timeout value is hardcoded to 10000ms. Consider making this configurable through environment variables or the dojoConfig to allow flexibility across different environments.
- 10000 + process.env.ENTITY_CHANGE_TIMEOUT || 10000
31-45: Add JSDoc comments to improve code maintainabilityThe AI summary indicates that documentation comments were removed. Consider adding JSDoc comments to explain the purpose of the spawn function, its parameters, and the significance of the timeout value.
+ /** + * Spawns a new entity with initial moves and handles optimistic updates. + * @throws {Error} If the spawn action fails or entity state doesn't update within timeout + * @remarks Waits up to 10 seconds for entity state confirmation + */ const spawn = async () => {
31-45: Consider potential race conditions in optimistic updatesThe optimistic update pattern could be susceptible to race conditions if multiple spawn actions are triggered in quick succession. Consider:
- Adding a mutex/lock mechanism
- Tracking pending transactions
- Implementing a queue system for sequential processing
Would you like me to provide a more detailed implementation for handling concurrent spawn actions?
34-39: Enhance error logging for debuggingWhile logging has been added, the error context could be more detailed. Consider including:
- Entity ID in log messages
- Transaction ID for tracing
- Timing information
- console.log("Waiting for entity change..."); + console.log(`Waiting for entity change... (EntityId: ${entityId}, TxId: ${transactionId})`); - console.log("Entity state:", entity); + console.log(`Entity state at ${Date.now()}: `, { entityId, entity });packages/create-dojo/src/commands/start.ts (2)
Line range hint
2-39: Remove duplicate template entry for "example-vite-react-sdk"The template list contains two entries for "example-vite-react-sdk" with different descriptions:
- "React app using Dojo SDK"
- "Basic react app using the sdk"
This duplication could cause confusion during project initialization.
Apply this diff to remove the duplicate entry:
const templates = [ { value: "example-vite-react-sdk", description: "React app using Dojo SDK", }, { value: "example-vite-kitchen-sink", description: "Vite app with a variety of Dojo features", }, { value: "example-vanillajs-phaser-recs", description: "Vanilla JS/Phaser app using Dojo RECS", }, { value: "example-vite-react-phaser-recs", description: "React/Phaser app using Dojo RECS", }, { value: "example-vite-react-pwa-recs", description: "React Progressive Web App using Dojo RECS", }, { value: "example-vite-react-threejs-recs", description: "React Three.js app using Dojo RECS", }, - { - value: "example-vite-react-sdk", - description: "Basic react app using the sdk", - }, { value: "example-vite-svelte-recs", description: "Basic svelte app using Dojo RECS", }, { value: "example-vue-app-recs", description: "Basic vite app using RECS" }, ];
Line range hint
127-129: Enhance error handling with troubleshooting guidanceThe current error handling for
dojoConfig.tsupdates only logs a warning. Given that the PR objectives focus on fixing configuration issues, consider enhancing the error message with troubleshooting steps.Apply this diff to improve error handling:
} catch (error) { - console.warn(`Warning: Could not update dojoConfig.ts: ${error}`); + console.warn( + `Warning: Could not update dojoConfig.ts: ${error}\n` + + 'Troubleshooting steps:\n' + + '1. Ensure the dojoConfig.ts file exists in your project\n' + + '2. Verify you have write permissions to the file\n' + + '3. Check if the manifest import statement follows the expected format\n' + + 'For more help, visit: https://book.dojoengine.org/framework/configuration' + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
examples/example-vite-react-sdk/dojoConfig.ts(1 hunks)examples/example-vite-react-sdk/src/useSystemCalls.ts(1 hunks)packages/create-dojo/src/commands/start.ts(1 hunks)
🔇 Additional comments (2)
examples/example-vite-react-sdk/dojoConfig.ts (1)
7-12: Configuration looks complete and addresses the white screen issue.
The added configuration parameters (rpcUrl, toriiUrl, masterAddress, masterPrivateKey) provide all the necessary settings that were previously missing and causing the white screen error mentioned in issue #342.
packages/create-dojo/src/commands/start.ts (1)
123-123: Verify the manifest file location in the contract directory
The path has been simplified from target/dev/manifest.json to manifest_dev.json. While this change aligns with the PR's objective to fix configuration issues, we should verify that this is the correct location where the manifest file will be generated during the contract build process.
Let's verify the manifest file location and any related build configurations:
✅ Verification successful
Based on the search results, I can see that the manifest file path manifest_dev.json is consistently used across multiple example projects in their dojoConfig.ts files:
examples/example-vite-svelte-recs/dojoConfig.tsexamples/example-vite-react-sdk/dojoConfig.tsexamples/example-vite-react-pwa-recs/dojoConfig.tsexamples/example-vite-react-app-recs/dojoConfig.tsexamples/example-nodejs-bot/dojoConfig.ts- And others...
Let's verify the manifest generation process to ensure this is the correct path:
Path change to manifest_dev.json is correct and consistent with the codebase
The change to use manifest_dev.json in the relative path "../contract/manifest_dev.json" is correct and verified for the following reasons:
- The manifest file is consistently imported as
manifest_dev.jsonacross all example projects in their dojoConfig.ts files. - The manifest is generated through the
sozo buildcommand, as seen in various README files and build instructions. - The path rewrite in
start.tsis part of a pattern that standardizes manifest imports across all Dojo projects.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for manifest file references and build configurations
# Search for manifest file references in build scripts or configurations
echo "Searching for manifest file references..."
rg -g '!node_modules' -g '!dist' "manifest.*\.json"
# Check if there are any build scripts that generate the manifest
echo "Searching for manifest generation in build scripts..."
rg -g '*.{ts,js,sh,json}' -g '!node_modules' -g '!dist' "manifest.*\.json.*generate|create|build"
Length of output: 35013
Script:
#!/bin/bash
# Check for manifest generation in build scripts and configurations
echo "Checking contract build scripts..."
rg -g '!node_modules' -g '!dist' "sozo build|scarb build" -A 2 -B 2
echo "Checking manifest generation patterns..."
rg -g '!node_modules' -g '!dist' "manifest.*dev.*json" -A 2 -B 2
Length of output: 8187
| masterPrivateKey: | ||
| "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912", |
There was a problem hiding this comment.
Security: Avoid hardcoding credentials
While these credentials are for development/example purposes, it's important to note that in a real deployment:
- Private keys should never be hardcoded in source code
- Use environment variables or secure secret management instead
Consider adding a warning comment above these values to prevent users from accidentally using them in production.
Here's a suggested improvement:
masterAddress:
"0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec",
+ // WARNING: Do not use this private key in production!
+ // Use environment variables instead, e.g.: process.env.MASTER_PRIVATE_KEY
masterPrivateKey:
"0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912",📝 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.
| masterPrivateKey: | |
| "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912", | |
| masterAddress: | |
| "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec", | |
| // WARNING: Do not use this private key in production! | |
| // Use environment variables instead, e.g.: process.env.MASTER_PRIVATE_KEY | |
| masterPrivateKey: | |
| "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912", |
There was a problem hiding this comment.
@okhaimie-dev, thanks for fixing the issue!
(_/)
(•ᴗ•)
/💐✨
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
| await state.waitForEntityChange( | ||
| entityId, | ||
| (entity) => { | ||
| console.log("Entity state:", entity); | ||
| return ( | ||
| entity?.models?.dojo_starter?.Moves?.remaining === | ||
| remainingMoves | ||
| ); | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Based on the analysis of the codebase, I can now provide a final response:
Add defensive entity state validation
The current validation can be improved based on patterns found in the codebase:
- Add null check for entity existence (
if (!entity) return false) - Add explicit model structure validation (
if (!entity.models?.dojo_starter?.Moves) return false) - Consider logging validation failures for debugging
Example locations requiring similar validations:
examples/example-vite-react-sdk/src/useSystemCalls.ts:40packages/sdk/src/__tests__/state.test.ts
🔗 Analysis chain
Verify entity state validation logic
The entity state validation could be more robust. Consider adding additional checks:
- Verify that the entity exists
- Validate the model structure
- Check for unexpected states
Let me search for more context around entity state validations.
Let me check for any existing model validation patterns or error handling in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other entity state validations in the codebase
ast-grep --pattern 'entity?.models?.dojo_starter?.Moves'
Length of output: 190
Script:
#!/bin/bash
# Search for entity state validations and related patterns
ast-grep --pattern 'waitForEntityChange'
# Also search for any entity state checks
rg "entity\?" -A 3
Length of output: 1974
Script:
#!/bin/bash
# Search for model validation patterns
rg "models\." -A 3
# Search for error handling related to entity state
rg "if.*entity" -A 3
Length of output: 24121
| rpcUrl: "http://localhost:5050/", | ||
| toriiUrl: "http://localhost:8080", | ||
| masterAddress: | ||
| "0x127fd5f1fe78a71f8bcd1fec63e3fe2f0486b6ecd5c86a0466c3a21fa5cfcec", | ||
| // WARNING: Do not use this private key in production! | ||
| // Use environment variables instead, e.g.: process.env.MASTER_PRIVATE_KEY | ||
| masterPrivateKey: | ||
| "0xc5b2fcab997346f3ea1c00b002ecf6f382c5f9c9659a3894eb783c5320f912", |
There was a problem hiding this comment.
I think those lines are not required are they ?
There was a problem hiding this comment.
I always get a whitescreen without adding those configurations.
These values are used in the DojoContext, so I think it is required. You can try it recreate it on your end to verify. A lot of people have complained about this in the past also, and this resolved their issue
There was a problem hiding this comment.
I had no issues on my end, just be sure that everything is deployed.
here are the commands that I ran :
katana --dev --dev.no-fee --http.cors_origins '*'in dojo-starter directory - be sure that world is deployed on local katana :
sozo migrateafter command ran successfully you should have world address output in stdio
torii -w 0x06171ed98331e849d6084bf2b3e3186a7ddf35574dd68cab4691053ee8ab69d7 --http.cors_origins '*' Other than that, pr is ok, but as those values are default and private keys should be pulled out of dojo config, we shouldn't write them down manually
Let me know if you want to make this clearer
There was a problem hiding this comment.
I have no issue as well thanks to the commands above, but once I start using the UI (after creating a burner wallet and selecting it from the dropdown), any commands return the following error:
App.tsx:225 Uncaught (in promise) LibraryError: RPC: starknet_estimateFee with params {
"request": [
{
"type": "INVOKE",
"sender_address": "0x513866925d3db1da7971a8083107a3da616d444b26d1dd5f3e36675d1d55762",
"calldata": [
"0x1",
"0x1aac0d0befd489010ebd11092e4c3df787ad01287d9b8af337507e54cbd0b41",
"0x239e4c8fbd11b680d7214cfc26d1780d5c099453f0832beb15fd040aebd4ebb",
"0x1",
"0x3"
],
"version": "0x100000000000000000000000000000001",
"signature": [],
"nonce": "0x1",
"max_fee": "0x0"
}
],
"block_id": "pending",
"simulation_flags": [
"SKIP_VALIDATE"
]
}
41: Transaction execution error: {"execution_error":"Transaction reverted: Transaction execution has failed:\n0: Error in the called contract (contract address: 0x0513866925d3db1da7971a8083107a3da616d444b26d1dd5f3e36675d1d55762, class hash: 0x07dc7899aa655b0aae51eadff6d801a58e97dd99cf4666ee59e704249e51adf2, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):\nError at pc=0:4211:\nCairo traceback (most recent call last):\nUnknown location (pc=0:290)\nUnknown location (pc=0:2951)\n\n1: Error in the called contract (contract address: 0x01aac0d0befd489010ebd11092e4c3df787ad01287d9b8af337507e54cbd0b41, class hash: 0x0000000000000000000000000000000000000000000000000000000000000000, selector: 0x0239e4c8fbd11b680d7214cfc26d1780d5c099453f0832beb15fd040aebd4ebb):\nRequested contract address 0x01aac0d0befd489010ebd11092e4c3df787ad01287d9b8af337507e54cbd0b41 is not deployed.\n","transaction_index":0}
at RpcChannel2.errorHandler (chunk-MQOJJ254.js?v=e82f18f5:17768:13)
at RpcChannel2.fetchEndpoint (chunk-MQOJJ254.js?v=e82f18f5:17785:12)
at async Account.getSuggestedFee (chunk-MQOJJ254.js?v=e82f18f5:20313:23)
at async Account.getUniversalSuggestedFee (chunk-MQOJJ254.js?v=e82f18f5:20302:35)
at async Account.execute (chunk-MQOJJ254.js?v=e82f18f5:20067:22)
at async DojoProvider.execute (chunk-X4NSLGG4.js?v=e82f18f5:4234:14)
at async Object.move (contracts.gen.ts:35:24)
at async onClick (App.tsx:221:41)
Is there anything else needed to run the template?
Closes #342
Introduced changes
Checklist
Summary by CodeRabbit
dojoConfigwith new properties for improved configuration.useSystemCallsfor better logging and increased timeout for entity state updates.dojoConfigfile.