feat: new readme #286
feat: new readme #286
Conversation
WalkthroughThis pull request introduces significant modifications to 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: 2
🧹 Outside diff range and nitpick comments (5)
readme.md (3)
1-15: Header looks great, consider adding alt text for improved accessibility.The header section effectively presents the Dojo project with its logo and social media links. The new heading clearly emphasizes Dojo's purpose in simplifying provable and onchain application development.
Consider adding alt text to the logo image for improved accessibility:
- <img src="./media/dojo-mark-full-dark.svg" height="128"> + <img src="./media/dojo-mark-full-dark.svg" height="128" alt="Dojo Logo">🧰 Tools
🪛 Markdownlint
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
17-36: Quick start guide looks good, minor capitalization fix needed.The introduction and quick start section provide clear guidance for users. The link to the Dojo book is helpful for newcomers, and the quick start guide offers concise instructions.
Please capitalize "React" in the quick start instructions:
- Start a new react app with (You will need [dojo](https://github.com/dojoengine/dojo) installed): + Start a new React app with (You will need [dojo](https://github.com/dojoengine/dojo) installed):🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ... It offers a streamlined way to explore Dojo from end to end, providing a comprehens...(AI_HYDRA_LEO_MISSING_A)
Line range hint
57-93: Packages and contributing sections are informative, minor typo correction needed.The packages section provides a concise overview of Dojo.js modularity, and the contributing section offers clear instructions for building packages and installing dependencies.
Please correct the typo in the packages section:
- Dojo.js is modulrised into small packages to keep it light. Choose all or one when working. + Dojo.js is modularized into small packages to keep it light. Choose all or one when working.🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ... It offers a streamlined way to explore Dojo from end to end, providing a comprehens...(AI_HYDRA_LEO_MISSING_A)
🪛 Markdownlint
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
packages/create-dojo/src/commands/start.ts (2)
144-164: Consider adding unit tests for the new template selection logicAdding unit tests for the new
--templateoption and the associated logic ensures that the feature works as expected and helps prevent future regressions.
Line range hint
12-25: Duplicate entries in thetemplatesarrayThe
templatesarray contains duplicate entries with the samevalue"example-vite-react-sdk"but different descriptions. This could cause confusion when selecting templates and may lead to unintended behavior.Consider removing or renaming one of the duplicate entries to ensure each template has a unique
value. Here’s how you can fix this:const templates = [ { value: "example-vite-react-sdk", description: "React app using Dojo SDK", }, { 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-vue-app-recs", description: "Basic vite app using RECS" }, ];If both templates are intended to be different, ensure they have unique
valueidentifiers.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
media/dojo-mark-full-dark.svgis excluded by!**/*.svgpackages/create-dojo/bin/index.js.mapis excluded by!**/*.map
📒 Files selected for processing (4)
- packages/create-dojo/bin/index.d.ts (0 hunks)
- packages/create-dojo/bin/index.js (0 hunks)
- packages/create-dojo/src/commands/start.ts (3 hunks)
- readme.md (1 hunks)
💤 Files with no reviewable changes (2)
- packages/create-dojo/bin/index.d.ts
- packages/create-dojo/bin/index.js
🧰 Additional context used
🪛 LanguageTool
readme.md
[grammar] ~21-~21: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ... It offers a streamlined way to explore Dojo from end to end, providing a comprehens...(AI_HYDRA_LEO_MISSING_A)
🪛 Markdownlint
readme.md
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
🔇 Additional comments (3)
readme.md (2)
38-55: Detailed setup instructions are clear and well-organized.The step-by-step instructions for setting up the Dojo environment are excellent. Breaking down the process into separate terminal steps makes it easy for users to follow along. The note about running each step in a separate terminal is a helpful reminder.
Line range hint
95-138: Debugging and examples sections provide valuable guidance.The debugging section offers helpful instructions for WSL users who might encounter issues. The examples section provides clear, step-by-step instructions for running examples, maintaining consistency with the earlier setup instructions by using a terminal-based approach.
🧰 Tools
🪛 LanguageTool
[grammar] ~21-~21: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[uncategorized] ~27-~27: Possible missing article found.
Context: ... It offers a streamlined way to explore Dojo from end to end, providing a comprehens...(AI_HYDRA_LEO_MISSING_A)
🪛 Markdownlint
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
5-5: null
Images should have alternate text (alt text)(MD045, no-alt-text)
packages/create-dojo/src/commands/start.ts (1)
140-140: Ensure option short flag-tdoes not conflict with other optionsThe new option
-t, --template <template>introduces a short flag-t. Verify that this short flag does not conflict with existing short flags in the command interface to prevent unexpected behavior.
| @@ -1,3 +1,5 @@ | |||
| #!/usr/bin/env node | |||
There was a problem hiding this comment.
Avoid adding a shebang line in a TypeScript file
Adding a shebang line (#!/usr/bin/env node) to a TypeScript file is unnecessary and may cause issues during compilation or execution. Since TypeScript files are transpiled to JavaScript before execution, the shebang should be placed in the compiled JavaScript output if needed.
To fix this, remove the shebang line from the TypeScript file:
-#!/usr/bin/env node📝 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.
| #!/usr/bin/env node |
| console.log(`Cloning dojo-starter repository...`); | ||
| const gitCloneResult = spawn.sync( | ||
| "git", | ||
| [ | ||
| "clone", | ||
| "https://github.com/dojoengine/dojo-starter.git", | ||
| dojoStarterPath, | ||
| ], | ||
| { stdio: "inherit" } | ||
| ); | ||
|
|
||
| if (gitCloneResult.status !== 0) { | ||
| throw new Error(`Failed to clone dojo-starter repository.`); | ||
| } | ||
|
|
There was a problem hiding this comment.
Duplicate cloning of dojo-starter repository
The code clones the dojo-starter repository twice: once using git clone and again using degit. This duplication may cause unnecessary processing and potential conflicts.
To resolve this, remove one of the cloning methods. If you prefer using git clone, remove the existing degit cloning code. Here's how you can remove the redundant degit cloning:
- // Clone dojo-starter
- console.log(`Downloading dojo-starter...`);
- spawn.sync("npx", ["degit", `dojoengine/dojo-starter`, dojoStarterPath], {
- stdio: "inherit",
- });Alternatively, if you prefer using degit, remove the newly added git clone code in lines 59-73.
Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
examples/example-vite-react-sdk/src/main.tsx (1)
34-36: Approve the simplified setup, but consider error handling and performance.The direct usage of
await setupBurnerManager(dojoConfig)in the JSX simplifies the code structure. This approach is valid in React 18+ for async components.However, consider the following suggestions:
- Error Handling: Implement a try-catch block to handle potential errors from
setupBurnerManager.- Performance: To avoid unnecessary re-renders, consider memoizing the result of
setupBurnerManager.Here's a suggested refactor to address these points:
import { useMemo } from 'react'; async function main() { // ... (previous code) let burnerManager; try { burnerManager = await setupBurnerManager(dojoConfig); } catch (error) { console.error("Failed to setup burner manager:", error); // Handle the error appropriately, e.g., show an error message to the user } const MemoizedApp = useMemo(() => <App db={db} />, [db]); createRoot(document.getElementById("root")!).render( <StrictMode> <DojoContextProvider burnerManager={burnerManager}> {MemoizedApp} </DojoContextProvider> </StrictMode> ); }This refactor separates the async operation from the render, handles potential errors, and memoizes the App component to optimize performance.
readme.md (5)
1-13: Great header and project introduction!The header section looks good, providing essential links and badges. The new heading effectively communicates Dojo's purpose.
Minor suggestion: On line 28 (outside this section), capitalize "React" as it's a proper noun.
🧰 Tools
🪛 Markdownlint
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
26-62: Well-structured quick start guide!The 5-minute quick start guide is clear, concise, and easy to follow. The separation of steps and use of code blocks for commands is excellent.
For consistency, consider using the same heading level for all main sections. For example, change "#### Then" on line 36 to "### Then" to match other section headings.
🧰 Tools
🪛 LanguageTool
[grammar] ~28-~28: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
🪛 Markdownlint
36-36: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
64-123: Informative Dojo SDK section with great code example!The SDK overview and usage example are well-explained and provide a good starting point for developers.
Add a language specifier to the code block on line 68 for better syntax highlighting. For example:
-``` +```bash pnpm i @dojoengine/sdk<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint</summary><blockquote> 68-68: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </blockquote></details> </details> --- `147-161`: **Great examples section with clear instructions!** The All Examples section provides clear instructions for running examples locally and a comprehensive list of available demo projects. Consider adding more variety to the example descriptions. Currently, most of them are described as "A React application using Vite and the Dojo SDK". You could highlight unique features of each example to differentiate them. --- `163-283`: **Excellent contributing section with comprehensive instructions!** The Contributing section provides detailed, well-structured guidance for setting up the development environment, building packages, running examples, and debugging. This will be very helpful for potential contributors. For consistency in formatting, consider using the same heading level for all subsections within the Contributing section. For example, change "Setting up the Development Environment" on line 167 to "### Setting up the Development Environment" to match other subsection headings. <details> <summary>🧰 Tools</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> [style] ~245-~245: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing. Context: ...name>` with the name of the example you want to run (e.g., `example-vite-react-sdk`). ... (REP_WANT_TO_VB) </blockquote></details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Files that changed from the base of the PR and between 9cbee9f5ea85bbe650bf25917fe3348faeb0f6ff and 872235c224af5971497901c3e8b3412d44eca629. </details> <details> <summary>📒 Files selected for processing (6)</summary> * examples/example-vite-react-sdk/src/bindings.ts (2 hunks) * examples/example-vite-react-sdk/src/main.tsx (2 hunks) * packages/create-burner/src/manager/index.ts (1 hunks) * packages/create-burner/src/manager/setupBurnerManager.ts (1 hunks) * packages/create-dojo/src/commands/start.ts (4 hunks) * readme.md (1 hunks) </details> <details> <summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary> * packages/create-dojo/src/commands/start.ts </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 LanguageTool</summary><blockquote> <details> <summary>readme.md</summary><blockquote> [grammar] ~28-~28: “React” is a proper noun and needs to be capitalized. Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://... (A_GOOGLE) --- [style] ~245-~245: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing. Context: ...name>` with the name of the example you want to run (e.g., `example-vite-react-sdk`). ... (REP_WANT_TO_VB) </blockquote></details> </blockquote></details> <details> <summary>🪛 Markdownlint</summary><blockquote> <details> <summary>readme.md</summary><blockquote> 36-36: Expected: h3; Actual: h4 Heading levels should only increment by one level at a time (MD001, heading-increment) --- 68-68: null Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 2-2: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 3-3: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 4-4: null Images should have alternate text (alt text) (MD045, no-alt-text) --- 23-23: null Link fragments should be valid (MD051, link-fragments) </blockquote></details> </blockquote></details> </details> <details> <summary>🔇 Additional comments (11)</summary><blockquote> <details> <summary>packages/create-burner/src/manager/index.ts (1)</summary><blockquote> `4-4`: **LGTM: New export statement added correctly.** The new export statement for `setupBurnerManager` is consistent with the existing code style and correctly makes the function accessible for import from this module. </blockquote></details> <details> <summary>packages/create-burner/src/manager/setupBurnerManager.ts (3)</summary><blockquote> Line range hint `1-31`: **Overall, the changes improve code clarity and organization.** The main changes in this file are: 1. Updated import statement for `BurnerManager` 2. Renamed function from `setupBurner` to `setupBurnerManager` These changes improve code clarity and better reflect the function's purpose. The core logic of the function remains unchanged and appropriate. However, please ensure that the verification steps mentioned in the previous comments are carried out to maintain consistency across the codebase and prevent any potential issues. --- `5-5`: **Function name change improves clarity but verify usage.** The function name change from `setupBurner` to `setupBurnerManager` better reflects its purpose. This is a good improvement in terms of code clarity and maintainability. However, this change might affect other parts of the codebase that use this function. Please run the following script to check for any occurrences of the old function name: ```shell #!/bin/bash # Description: Check for occurrences of the old function name 'setupBurner' echo "Checking for occurrences of 'setupBurner':" rg --type typescript 'setupBurner'If any occurrences are found, they should be updated to use the new function name
setupBurnerManager.
3-3: Verify the import change and its consistency.The import for
BurnerManagerhas been changed from a package import to a relative import. While this change works, it's important to ensure that:
- This change is consistent with the project's import style guidelines.
- It doesn't introduce any circular dependencies.
- Other files that may be importing
BurnerManagerare updated accordingly.To check for potential issues, run the following script:
examples/example-vite-react-sdk/src/main.tsx (2)
Line range hint
1-45: Summary of changes and next stepsThe changes in this file are part of a larger refactor to use a standardized burner manager setup from
@dojoengine/create-burner. This is a positive move towards better maintainability and consistency.Key points:
- The import has been updated to use the standardized package.
- The setup of the burner manager has been simplified in the JSX.
Next steps:
- Verify that all usages of the old
setupBurnerhave been updated across the codebase.- Consider implementing the suggested error handling and performance optimizations.
- Update any documentation or comments that might reference the old setup.
11-11: Approve the updated import, but verify its impact.The change from a local
setupBurnertosetupBurnerManagerfrom@dojoengine/create-burneris a good move towards using standardized packages. This can improve maintainability and consistency.To ensure this change doesn't break other parts of the codebase, please run the following script:
If the script finds any remaining usage of
setupBurner, those instances should be updated to usesetupBurnerManagerfrom@dojoengine/create-burner.✅ Verification successful
To ensure there are no remaining usages of
setupBurner, please run the following updated script:shell
#!/bin/bashDescription: Check for any remaining usage of the old setupBurner import
Test: Search for any remaining imports of setupBurner
echo "Checking for any remaining imports of setupBurner:"
rg "import.setupBurner.from.['"]./setupBurner['"]" --glob ".ts" --glob "*.tsx" || echo "No remaining imports found."Test: Search for any usage of setupBurner function
echo "Checking for any usage of setupBurner function:"
rg "setupBurner(" --glob ".ts" --glob ".tsx" || echo "No usage of setupBurner function found."
To accurately verify the removal of
setupBurner, please run the following updated script:
To accurately verify the removal of
setupBurner, please run the following updated script:
Verified: No remaining usage of
setupBurner.The import and usage of
setupBurnerhave been successfully updated tosetupBurnerManagerfrom@dojoengine/create-burner. No further action is required regarding this change.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of the old setupBurner import # Test: Search for any remaining imports of setupBurner echo "Checking for any remaining imports of setupBurner:" rg --type typescript --type tsx "import.*setupBurner.*from.*['\"]./setupBurner" || echo "No remaining imports found." # Test: Search for any usage of setupBurner function echo "Checking for any usage of setupBurner function:" rg --type typescript --type tsx "setupBurner\(" || echo "No usage of setupBurner function found."Length of output: 573
Script:
Length of output: 98
Script:
#!/bin/bash # Description: Check for any remaining usage of the old setupBurner import # Test: Search for any remaining imports of setupBurner echo "Checking for any remaining imports of setupBurner:" rg "import\s+.*setupBurner\s+from\s+['\"]\.\/setupBurner['\"]" --glob "*.ts" --glob "*.tsx" || echo "No remaining imports found." # Test: Search for any usage of setupBurner function echo "Checking for any usage of setupBurner function:" rg "setupBurner\s*\(" --glob "*.ts" --glob "*.tsx" || echo "No usage of setupBurner function found."Length of output: 680
Script:
#!/bin/bash # Description: Check for any remaining usage of the old setupBurner import # Test: Search for any remaining imports of setupBurner echo "Checking for any remaining imports of setupBurner:" rg 'import\s+.*setupBurner\s+from\s+["@'"'"']/setupBurner["'"'"']' --glob '*.ts' --glob '*.tsx' || echo "No remaining imports found." # Test: Search for any usage of setupBurner function echo "Checking for any usage of setupBurner function:" rg 'setupBurner\s*\(' --glob '*.ts' --glob '*.tsx' || echo "No usage of setupBurner function found."Length of output: 518
examples/example-vite-react-sdk/src/bindings.ts (2)
42-46: Approve the newModelStringsenum, but clarification needed.The addition of the
ModelStringsenum is a good way to centralize these string constants. However, its purpose and usage in the broader context of the application are not immediately clear.Could you please clarify:
- How and where this enum will be used?
- Why the "eternum-" prefix is necessary?
Consider adding a comment above the enum to explain its role in the codebase. For example:
/** * ModelStrings enum provides unique identifiers for the main model types. * These are used in [explain where/how they are used] to [explain purpose]. */ enum ModelStrings { // ... (existing code) }This will help other developers understand the purpose and importance of this enum.
70-77: LGTM: Export statement correctly updated.The export statement has been properly updated to include the new
ModelStringsenum. This ensures that the new type is available for use in other parts of the application, maintaining consistency with the rest of the exported types.readme.md (3)
15-24: Excellent introduction and table of contents!The brief introduction effectively explains Dojo.js's purpose, and the link to the Dojo book is helpful for newcomers. The addition of a table of contents greatly improves document navigation.
🧰 Tools
🪛 Markdownlint
23-23: null
Link fragments should be valid(MD051, link-fragments)
126-146: Comprehensive package list with clear descriptions!The All Packages section provides a great overview of the Dojo.js project structure. The brief descriptions for each package are informative and help users understand the purpose of each module.
1-283: Excellent improvements to the README file!The restructuring and content updates have significantly enhanced the clarity, organization, and usefulness of the documentation. The new sections, such as the quick start guide, package list, and contributing guidelines, provide valuable information for users and potential contributors.
While there are a few minor formatting and consistency issues, they do not detract from the overall quality of the document. These small improvements can be easily addressed in a follow-up commit.
Great job on making the Dojo.js project more accessible and user-friendly!
🧰 Tools
🪛 LanguageTool
[grammar] ~28-~28: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[style] ~245-~245: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...name>with the name of the example you want to run (e.g.,example-vite-react-sdk`). ...(REP_WANT_TO_VB)
🪛 Markdownlint
36-36: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
23-23: null
Link fragments should be valid(MD051, link-fragments)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (1)
- package.json (1 hunks)
🔇 Additional comments (1)
package.json (1)
14-14: Approve changes in devDependenciesThe changes in the
devDependenciessection look good:
- Updating
huskyto the latest minor version (^9.1.6) is a good practice for keeping development tools up-to-date.- Moving
@commitlint/cli,@commitlint/config-conventional, and@ianvs/prettier-plugin-sort-importsfromdependenciestodevDependenciesis correct. These are development tools and should not be included in the production build of your package.These changes improve the accuracy of your dependency management and potentially reduce the size of your production package.
Also applies to: 20-23
| "husky": "^9.1.6", | ||
| "lerna": "^8.1.5", | ||
| "prettier": "^3.3.3", | ||
| "tsup": "^8.1.0", | ||
| "typedoc": "^0.26.7", | ||
| "typedoc-material-theme": "^1.1.0", | ||
| "typedoc-plugin-coverage": "^3.3.0" | ||
| "typedoc-plugin-coverage": "^3.3.0", | ||
| "@commitlint/cli": "^18.4.4", | ||
| "@commitlint/config-conventional": "^18.4.4", | ||
| "@ianvs/prettier-plugin-sort-imports": "^4.3.1" |
There was a problem hiding this comment.
💡 Codebase verification
React is still in use within the project
The react package remains imported and utilized across multiple files in the codebase, including JSX and TSX files. This indicates that React is still actively used despite its removal from package.json.
Please address the following:
-
If React removal was intentional:
- Remove all React-related code and components from the project.
- Ensure that no React dependencies remain in the codebase.
- Update documentation to reflect the removal of React.
-
If React removal was unintentional:
- Add
reactback to the appropriate section (dependenciesordevDependencies) inpackage.json. - Ensure all necessary React packages are correctly listed and versioned.
- Add
🔗 Analysis chain
Verify the removal of React dependency
The react package seems to have been removed entirely from both dependencies and devDependencies. This is a significant change that wasn't mentioned in the PR objectives.
Please confirm if this removal was intentional. If it was:
- Ensure that the project no longer uses React in any way.
- Update any documentation or README files to reflect this change.
- Remove any React-related code or components from the project.
If this removal was unintentional, please add React back to the appropriate section (dependencies or devDependencies) based on how it's used in the project.
To help verify the usage of React in the project, you can run the following script:
This script will help identify if React is still being used in the project, which can guide the decision on whether to keep or remove the React dependency.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for React usage in the project
# Search for React imports or usage in JavaScript and TypeScript files
echo "Searching for React usage:"
rg --type js --type ts 'import.*React|from.*react|React\.' -g '!node_modules'
# Check if there are any JSX files in the project
echo -e "\nChecking for JSX files:"
fd -e jsx -e tsx
Length of output: 28503
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
examples/example-vite-react-sdk/src/useModel.tsx (2)
5-15: LGTM: Well-typed function signature with clear JSDoc.The function signature is well-designed, using TypeScript generics and template literal types for strong type safety. The JSDoc provides clear documentation for the function's purpose and parameters.
Consider adding a
@templatetag in the JSDoc to document the generic type parametersNandM. This can improve IDE intellisense and make the documentation more complete. For example:/** * Custom hook to retrieve a specific model for a given entityId within a specified namespace. * * @template N - The namespace type (keyof Schema) * @template M - The model type (keyof Schema[N] & string) * @param entityId - The ID of the entity. * @param model - The model to retrieve, specified as a string in the format "namespace-modelName". * @returns The model structure if found, otherwise undefined. */
16-27: LGTM: Efficient implementation with good error handling.The function body is well-implemented, using
useMemofor performance optimization and optional chaining for safe property access. The error handling is appropriate, returningundefinedwhen the model is not found.Consider improving type safety for the
model.split("-")operation. The current type assertionas [N, M]assumes themodelstring always contains a hyphen and splits into exactly two parts. To make this safer, you could add a runtime check:const parts = model.split("-"); if (parts.length !== 2) { throw new Error(`Invalid model format: ${model}. Expected "namespace-modelName".`); } const [namespace, modelName] = parts as [N, M];This ensures that the
modelstring is in the correct format and provides a helpful error message if it's not.examples/example-vite-react-sdk/src/App.tsx (1)
113-114: Approve refactoring to useModel hook with a minor suggestion.The refactoring from
useMemotouseModelformovesandpositionis a good improvement. It centralizes model retrieval, likely enhancing maintainability and potentially improving performance.For consistency, consider destructuring the return value of
useModel:const { remaining } = useModel(entityId, Models.Moves); const { vec } = useModel(entityId, Models.Position);This would allow you to directly use
remainingandvecin your JSX, potentially simplifying the code further.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- examples/example-vite-react-sdk/src/App.tsx (2 hunks)
- examples/example-vite-react-sdk/src/bindings.ts (2 hunks)
- examples/example-vite-react-sdk/src/useModel.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/example-vite-react-sdk/src/bindings.ts
🔇 Additional comments (3)
examples/example-vite-react-sdk/src/useModel.tsx (1)
1-3: LGTM: Import statements are appropriate.The import statements are concise and import only the necessary dependencies for the hook's functionality.
examples/example-vite-react-sdk/src/App.tsx (2)
3-3: Approve import changes for enhanced model management.The addition of
Modelsfrom "./bindings.ts" anduseModelfrom "./useModel.tsx" indicates a shift towards a more structured and reusable approach for handling model state. These changes align well with the subsequent modifications in the component's logic.Also applies to: 8-8
Line range hint
1-314: Approve overall changes: Improved model management without functional alterations.The modifications in this PR enhance the code structure by introducing a more centralized approach to model management through the
useModelhook. These changes align with React best practices and likely improve maintainability without altering the core functionality of the component. The localized nature of the changes minimizes the risk of unintended side effects.Great job on improving the code quality while maintaining its functionality!
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/sdk/package.json (1)
4-4: Approve description update with a minor suggestion.The updated description effectively conveys the purpose and benefit of the SDK. It aligns well with the PR objective of updating project information.
Consider adding a comma for better readability:
- "description": "Dojo SDK: Build onchain and provable apps faster", + "description": "Dojo SDK: Build onchain and provable apps faster,",
📜 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 (5)
- examples/example-vite-react-sdk/src/main.tsx (2 hunks)
- packages/sdk/package.json (3 hunks)
- packages/sdk/src/convertQuerytoClause.ts (1 hunks)
- packages/sdk/src/index.ts (1 hunks)
- packages/sdk/src/types.ts (2 hunks)
🔇 Additional comments (15)
examples/example-vite-react-sdk/src/main.tsx (2)
23-25: Replace "WORLD_NAME" placeholder and approve chainId update.The domain configuration has been updated with more specific values:
The
nameproperty now uses "WORLD_NAME". This appears to be a placeholder and should be replaced with the actual world name for this application.The
chainIdhas been updated to "KATANA", which is more specific and likely correct for your environment.Please ensure "WORLD_NAME" is replaced with the correct world name before merging. You can use the following script to check for any remaining "WORLD_NAME" placeholders:
#!/bin/bash # Description: Check for any remaining "WORLD_NAME" placeholders # Test: Search for "WORLD_NAME" in TypeScript and TSX files rg --type typescript --type tsx '"WORLD_NAME"'
11-11: Approve import change and verify its impact.The update from a local
setupBurnertosetupBurnerManagerfrom@dojoengine/create-burneris a good move towards using standardized packages. This change likely provides more robust and maintained functionality.To ensure this change doesn't break existing functionality, please run the following script:
✅ Verification successful
Verified: No remaining usage of
setupBurnerfound andsetupBurnerManageris used consistently.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of the old setupBurner function # Test: Search for any remaining usage of setupBurner rg --type typescript --type tsx 'setupBurner' # Test: Verify that setupBurnerManager is now used consistently rg --type typescript --type tsx 'setupBurnerManager'Length of output: 177
Script:
#!/bin/bash # Description: Verify usage of setupBurner and setupBurnerManager functions # Test: Search for any remaining usage of setupBurner rg --type ts --type tsx 'setupBurner' # Test: Verify that setupBurnerManager is now used consistently rg --type ts --type tsx 'setupBurnerManager'Length of output: 138
Script:
#!/bin/bash # Description: Verify usage of setupBurner and setupBurnerManager functions # Test: Search for any remaining usage of setupBurner in .ts and .tsx files rg 'setupBurner' --glob '**/*.ts' --glob '**/*.tsx' # Test: Verify that setupBurnerManager is now used consistently in .ts and .tsx files rg 'setupBurnerManager' --glob '**/*.ts' --glob '**/*.tsx'Length of output: 1008
packages/sdk/package.json (4)
43-43: Approve dependencies changes and request clarification on HTTP handling.The addition of
@dojoengine/coreas a workspace dependency is a good move towards a more modular architecture. The removal ofaxiosand the shift ofvite-plugin-wasmto devDependencies are also appropriate changes.Questions for clarification:
- With the removal of
axios, what is the new strategy for handling HTTP requests in the SDK?- Are there any migration steps required for users of the SDK who might have been relying on
axiosbeing bundled?To ensure that axios is no longer used in the codebase, please run the following script:
#!/bin/bash # Description: Verify removal of axios usage # Test: Search for any remaining axios imports or usage rg --type js 'axios'
Line range hint
1-65: Summary of changes and documentation update request.The changes to
package.jsonindicate a significant update to the @dojoengine/sdk package, including:
- Updated description
- New WebAssembly integration
- Dependency changes, including the addition of @dojoengine/core
- Increased minimum Node.js version requirement
To ensure consistency across the project:
- Please review and update the README.md file to reflect these changes, especially the new description and Node.js requirement.
- Update the CHANGELOG.md to document the breaking change in Node.js version and any new features or removals.
- Consider updating any API documentation or guides to reflect the removal of axios and the addition of WebAssembly support.
To ensure documentation is up-to-date, please run the following script:
#!/bin/bash # Description: Check for recent updates in documentation files # Test: List recently modified documentation files fd -e md -e mdx --changed-within 7d
63-63:⚠️ Potential issueApprove Node.js version update but flag as a breaking change.
Updating the minimum Node.js version to 18.0.0 is a significant change that may leverage newer features and improvements. However, this is a breaking change that needs to be clearly communicated to users.
Action items:
- Update the CHANGELOG.md to prominently mention this breaking change.
- Consider adding a note in the README.md about the new Node.js version requirement.
- Update any CI/CD configurations to use Node.js 18 or later.
To ensure that the project is compatible with Node.js 18, please run the following script:
✅ Verification successful
Node.js version update verified.
The usage of the
fetchAPI inexamples/example-nodejs-bot/src/queries/getMoves.tsconfirms the need for Node.js version 18.0.0 or higher.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify usage of Node.js 18+ features # Test: Search for usage of Node.js 18+ features like fetch API rg --type js 'fetch\(' -A 5Length of output: 864
Script:
#!/bin/bash # Description: Correctly verify usage of Node.js 18+ features like fetch API # Test: Search for usage of fetch API in JavaScript and TypeScript files rg --type js --type ts 'fetch\('Length of output: 147
34-35: Approve devDependencies changes and request WASM integration details.The addition of
vite-plugin-wasmsuggests the integration of WebAssembly modules. This could potentially improve performance or enable new features.Could you provide more details on:
- The specific use cases for WebAssembly in this project?
- Any performance benchmarks or improvements expected from this integration?
To ensure the WASM plugin is properly configured, please run the following script:
✅ Verification successful
Vite-plugin-wasm configuration verified.
The
vite-plugin-wasmis properly configured and utilized across multiple configuration files in the project. All necessary integrations appear to be correctly set up.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Vite configuration for WASM plugin # Test: Search for vite-plugin-wasm configuration in vite.config.js or similar files rg --type js 'vite-plugin-wasm' -A 5Length of output: 36
Script:
#!/bin/bash # Description: Search for 'vite-plugin-wasm' usage across all files in the repository rg 'vite-plugin-wasm'Length of output: 2302
packages/sdk/src/convertQuerytoClause.ts (5)
4-5: Approve changes in import statementsAfter verification, the changes in the import statements appear to be correct:
- The removal of
LogicalOperatoris justified as it's not used directly in this file.- The removal of
convertQueryToKeysClauseimport is correct, as the file now usesconvertQueryToEntityKeyClausesinstead.These changes help to clean up unused imports and reflect the current usage in the file.
Line range hint
13-45: Main functionconvertQueryToClauseremains consistentThe
convertQueryToClausefunction has not been modified in this change. It continues to correctly process the query object and generate Torii clauses. The function's logic remains consistent with its purpose and is not affected by the changes in the import statements.
Line range hint
47-301: Helper functions remain consistent and functionalThe helper functions
processModels,buildWhereClause,convertToPrimitive, andconvertOperatorhave not been modified in this change. They continue to correctly support the mainconvertQueryToClausefunction. The logic of these functions remains consistent with their purposes and is not affected by the changes in the import statements.
Line range hint
1-301: Summary of changes in convertQuerytoClause.tsThe changes in this file are focused on cleaning up the import statements:
- Removed the unused
LogicalOperatorimport.- Replaced
convertQueryToKeysClausewithconvertQueryToEntityKeyClauses.These changes improve code cleanliness without affecting the functionality of the file. The main
convertQueryToClausefunction and its helper functions remain unchanged and continue to work as expected.Overall, these changes are positive and maintain the integrity of the code.
4-5: Verify the impact of removed importsThe import statements have been modified:
LogicalOperatorhas been removed from the imports.- The import of
convertQueryToKeysClausehas been removed.Please ensure that these removals don't negatively impact the functionality of the code.
Let's verify the usage of these removed imports:
packages/sdk/src/index.ts (3)
2-2: Imports from 'starknet' added correctlyThe necessary types
Account,Signature,StarknetDomain, andTypedDataare imported from "starknet" and are used appropriately in the code.
8-8: Imports from './types' updated appropriatelyThe import statement includes
SchemaType,SDK,SDKConfig, andUnionOfModelDatafrom "./types", which are used correctly throughout the module.
Line range hint
145-166: Ensure robust handling of the signature returned byaccount.signMessageThere is a potential issue with how the
signatureis processed. Theaccount.signMessage(data)method may return different formats depending on the implementation. It might return an array of strings or an object withrandsproperties. To avoid runtime errors, ensure that all possible formats are handled correctly.Run the following script to verify the possible formats of
signaturereturned bysignMessage:Consider updating the signature handling code as follows:
const signature: Signature = await account.signMessage(data); // Stringify typed data for publishing const dataString = JSON.stringify(data); // Publish the signed message await client.publishMessage( dataString, - Array.isArray(signature) - ? signature - : [signature.r.toString(), signature.s.toString()] + Array.isArray(signature) ? signature : Object.values(signature).map((value) => value.toString()) );This change ensures that all properties of the signature object are converted to strings and included in an array, accommodating different signature formats.
✅ Verification successful
Signature Handling is Robustified Successfully
The verification confirms that
signMessagereturns different formats in the codebase. The proposed changes to handle both array and object formats are correctly implemented.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the return type and structure of 'signature' from 'signMessage' methods. # Test: Search for 'signMessage' method implementations. Expect: Identify the return types. rg --type ts 'signMessage' -A 5Length of output: 1058
packages/sdk/src/types.ts (1)
338-353: Well-definedSDKConfiginterfaceThe addition of the
SDKConfiginterface provides a clear and organized way to configure the SDK. The propertiesclientanddomainare appropriately typed and documented, enhancing maintainability and readability.
| <DojoContextProvider | ||
| burnerManager={await setupBurnerManager(dojoConfig)} | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor burner manager setup to improve code structure.
While the direct integration of setupBurnerManager into the component props simplifies the code, using await inside JSX is not recommended and may lead to unexpected behavior.
Consider refactoring this part to separate the asynchronous operation from the JSX:
async function main() {
// ... (previous code)
+ const burnerManager = await setupBurnerManager(dojoConfig);
createRoot(document.getElementById("root")!).render(
<StrictMode>
<DojoContextProvider
- burnerManager={await setupBurnerManager(dojoConfig)}
+ burnerManager={burnerManager}
>
<App db={db} />
</DojoContextProvider>
</StrictMode>
);
}This change will improve code readability and prevent potential issues with using await in JSX.
📝 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.
| <DojoContextProvider | |
| burnerManager={await setupBurnerManager(dojoConfig)} | |
| > | |
| async function main() { | |
| // ... (previous code) | |
| const burnerManager = await setupBurnerManager(dojoConfig); | |
| createRoot(document.getElementById("root")!).render( | |
| <StrictMode> | |
| <DojoContextProvider | |
| burnerManager={burnerManager} | |
| > | |
| <App db={db} /> | |
| </DojoContextProvider> | |
| </StrictMode> | |
| ); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (7)
packages/sdk/readme.md (5)
5-10: LGTM: Clear presentation of key featuresThe new "Key Features" section effectively highlights the main strengths of the SDK. The list is concise and relevant to potential users.
Consider clarifying the term "torii" in the last bullet point. If it's a specific component or service, it might be helpful to briefly explain it or provide a link to more information.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. ## Understand Entities and Models - ...(VB_TO_NN_DT)
18-44: LGTM: Helpful example of entity subscriptionThe example effectively demonstrates how to subscribe to specific model states using the SDK. The code is clear and well-commented.
Consider adding a brief explanation of what the
$isoperator does in the query, as it might not be immediately obvious to new users.
46-62: LGTM: Clear installation and setup instructionsThe Getting Started section provides concise installation instructions and helpful guidance on generating TypeScript types for enhanced type safety.
In line 54, consider rephrasing to "To take advantage of this type safety, you will need dojo installed:" for better grammatical structure.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~54-~54: Possible missing preposition found.
Context: ...schema: To take advantage of this type safety (You will need [dojo](https://github.co...(AI_HYDRA_LEO_MISSING_OF)
64-113: LGTM: Improved usage example with detailed schemaThe updated Usage section provides a more comprehensive example of how to initialize and use the SDK. The new schema structure and initialization process are well-explained.
Consider adding a brief comment explaining the purpose of each configuration option in the
initfunction (e.g., whatrpcUrl,toriiUrl, etc. are used for). This would help new users understand the configuration better.
Line range hint
116-194: LGTM: Comprehensive query explanation and examplesThe new Query Explanation section and the accompanying examples provide a clear and detailed guide on how to use different query types in the SDK. The examples are practical and well-commented.
To improve formatting:
- Adjust the indentation of unordered lists to be consistent (2 spaces instead of 4) in lines 126, 128, 141, and 142.
- Consider using a different formatting for the numbered list in the Query Explanation section to avoid Markdownlint warnings.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. ## Understand Entities and Models - ...(VB_TO_NN_DT)
[uncategorized] ~54-~54: Possible missing preposition found.
Context: ...schema: To take advantage of this type safety (You will need [dojo](https://github.co...(AI_HYDRA_LEO_MISSING_OF)
[uncategorized] ~120-~120: Loose punctuation mark.
Context: ... Engine: 1.SubscriptionQueryType: Used for real-time subscriptions to ent...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~121-~121: Loose punctuation mark.
Context: ...ty and event updates. 2.QueryType: Used for fetching entities and event me...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~123-~123: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...perties. The key difference lies in the operators supported within thewhereclause: -...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~125-~125: Loose punctuation mark.
Context: ...clause: -SubscriptionQueryType: - Supports only the$isoperato...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~127-~127: Loose punctuation mark.
Context: ...r for exact matches. -QueryType: - Supports a variety of operators...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~128-~128: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: .... -QueryType: - Supports a variety of operators including$eq,$neq, `$gt...(A_VARIETY_OF)
🪛 Markdownlint
126-126: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
128-128: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
141-141: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
142-142: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
packages/sdk/src/types.ts (2)
343-358: LGTM! Consider enhancing the documentation.The addition of the
SDKConfiginterface is a good improvement, providing a clear structure for SDK configuration. It aligns well with the existing SDK structure and enhances its configurability.Consider adding a brief example in the documentation to illustrate how to use this interface when initializing the SDK. This would help developers quickly understand how to properly configure the SDK.
Example:
/** * @example * const sdkConfig: SDKConfig = { * client: { * url: 'https://api.example.com', * // other Torii client configurations * }, * domain: { * name: 'My Dojo App', * version: '1', * chainId: '0x1', * }, * }; */
Remaining references to Burner-related types found.
The shell script execution revealed that several references to the removed Burner-related types still exist in the codebase, particularly within example projects and the
create-burnerpackage. This incomplete removal may lead to inconsistencies and potential issues in the SDK.Affected areas:
examples/example-vite-react-threejs-recs/src/dojo/DojoContext.tsxexamples/example-vite-react-sdk/src/DojoContext.tsxexamples/example-vite-react-pwa-recs/src/dojo/DojoContext.tsxexamples/example-vite-react-app-recs/src/dojo/DojoContext.tsxpackages/create-burner/src/manager/predeployedManager.tspackages/create-burner/src/types/index.tspackages/create-burner/src/manager/burnerManager.tspackages/create-burner/test/utils/storage.test.tspackages/create-burner/test/manager/burnerManager.test.tspackages/create-burner/src/context/burnerProvider.tsxpackages/create-burner/src/hooks/useBurner.tspackages/create-burner/src/hooks/useBurnerManager.ts🔗 Analysis chain
Line range hint
1-358: Clarify the impact of removing Burner-related types.The removal of numerous types related to 'Burner' functionality (e.g.,
BurnerStorage,BurnerRecord,Burner, etc.) represents a significant change in the SDK's structure and capabilities. This change may impact existing implementations that rely on these types.Please address the following points:
- Confirm that this removal is intentional and aligns with the project's roadmap.
- Provide information on any replacement functionality or alternative approaches for managing accounts.
- Outline a migration path for users who may be relying on the removed Burner-related functionality.
- Update any documentation or guides that may reference these removed types.
To help verify the impact, you can run the following script to check for any remaining references to the removed types:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to removed Burner-related types echo "Searching for references to removed Burner-related types:" rg -i "burner(storage|record|manageroptions|account|createoptions|keys)|predeployed(storage|manageroptions|account)" --type tsLength of output: 11201
Script:
#!/bin/bash # Search for any remaining references to removed Burner-related types echo "Searching for references to removed Burner-related types:" rg -i 'burner(storage|record|manageroptions|account|createoptions|keys)|predeployed(storage|manageroptions|account)' --type tsLength of output: 6845
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/sdk/readme.md (3 hunks)
- packages/sdk/src/types.ts (1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/sdk/readme.md
[grammar] ~10-~10: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. ## Understand Entities and Models - ...(VB_TO_NN_DT)
[uncategorized] ~54-~54: Possible missing preposition found.
Context: ...schema: To take advantage of this type safety (You will need [dojo](https://github.co...(AI_HYDRA_LEO_MISSING_OF)
[uncategorized] ~120-~120: Loose punctuation mark.
Context: ... Engine: 1.SubscriptionQueryType: Used for real-time subscriptions to ent...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~121-~121: Loose punctuation mark.
Context: ...ty and event updates. 2.QueryType: Used for fetching entities and event me...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~123-~123: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...perties. The key difference lies in the operators supported within thewhereclause: -...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~125-~125: Loose punctuation mark.
Context: ...clause: -SubscriptionQueryType: - Supports only the$isoperato...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~127-~127: Loose punctuation mark.
Context: ...r for exact matches. -QueryType: - Supports a variety of operators...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~128-~128: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: .... -QueryType: - Supports a variety of operators including$eq,$neq, `$gt...(A_VARIETY_OF)
🪛 Markdownlint
packages/sdk/readme.md
126-126: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
128-128: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
141-141: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
142-142: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
🔇 Additional comments (4)
packages/sdk/readme.md (3)
1-3: LGTM: Improved title and introductionThe updated title and introduction provide a clearer focus on the SDK's core functionality of interacting with onchain state. The language is concise and effectively communicates the purpose of the SDK.
12-16: LGTM: Clear explanation of entities and modelsThis new section provides a concise yet informative explanation of how entities and models work in the Dojo SDK. It helps users understand the core concepts quickly.
Line range hint
1-194: Great job on the README update!The changes to this README file significantly improve the documentation for the Dojo SDK. The new structure, detailed explanations, and practical examples provide a comprehensive guide for users. The focus on type safety and the clear explanation of core concepts like entities and models are particularly valuable.
A few minor suggestions have been made throughout the review to further enhance clarity and formatting. Once these are addressed, this README will serve as an excellent introduction and reference for the Dojo SDK.
🧰 Tools
🪛 LanguageTool
[grammar] ~10-~10: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. ## Understand Entities and Models - ...(VB_TO_NN_DT)
[uncategorized] ~54-~54: Possible missing preposition found.
Context: ...schema: To take advantage of this type safety (You will need [dojo](https://github.co...(AI_HYDRA_LEO_MISSING_OF)
[uncategorized] ~120-~120: Loose punctuation mark.
Context: ... Engine: 1.SubscriptionQueryType: Used for real-time subscriptions to ent...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~121-~121: Loose punctuation mark.
Context: ...ty and event updates. 2.QueryType: Used for fetching entities and event me...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~123-~123: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...perties. The key difference lies in the operators supported within thewhereclause: -...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~125-~125: Loose punctuation mark.
Context: ...clause: -SubscriptionQueryType: - Supports only the$isoperato...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~127-~127: Loose punctuation mark.
Context: ...r for exact matches. -QueryType: - Supports a variety of operators...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~128-~128: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: .... -QueryType: - Supports a variety of operators including$eq,$neq, `$gt...(A_VARIETY_OF)
🪛 Markdownlint
126-126: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
128-128: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
141-141: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
142-142: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
packages/sdk/src/types.ts (1)
Line range hint
1-358: Summary of changes and potential impactThe changes in this file represent a significant shift in the SDK's structure:
- The addition of the
SDKConfiginterface enhances the SDK's configurability and provides a clear structure for initialization.- The removal of Burner-related types indicates a move away from built-in burner account management.
These changes suggest a refactoring effort to simplify the SDK and potentially shift certain responsibilities (like account management) to the user or other parts of the system.
To ensure a smooth transition:
- Update the SDK documentation to reflect these changes, particularly focusing on the new configuration approach and any changes in account management.
- Review and update any examples or tutorials that may be using the removed Burner-related functionality.
- Consider providing a migration guide for users updating from a previous version of the SDK.
Run the following script to check for any documentation files that might need updating:
#!/bin/bash # Search for documentation files that might reference old types or need updating echo "Searching for documentation files:" fd -e md -e mdx | xargs rg -i "burner|predeployed|sdkconfig"This will help identify documentation files that may need to be updated to reflect the recent changes.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
readme.md (6)
1-13: Improved header with concise description and additional links.The changes to the header enhance the README's appearance and provide more ways for users to engage with the project. The reduced image size and concise description effectively communicate Dojo's key benefits.
Consider adding alt text to the images for better accessibility. For example:
- <img src="./media/dojo-mark-full-dark.svg" height="48" align="center" /> + <img src="./media/dojo-mark-full-dark.svg" height="48" align="center" alt="Dojo logo" />🧰 Tools
🪛 Markdownlint
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
26-62: Improved and comprehensive quick start guide.The updated quick start guide provides clear, step-by-step instructions for setting up and running a Dojo project. The separation of commands into different terminals and the added explanations enhance user understanding.
For improved clarity, consider adding a note about the expected output or success indicators for each step. This could help users confirm they've completed each step correctly.
🧰 Tools
🪛 LanguageTool
[grammar] ~28-~28: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
🪛 Markdownlint
36-36: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
64-124: Comprehensive Dojo SDK overview with examples.The Dojo SDK section provides a clear introduction, installation instructions, and a basic usage example. The links to more complex examples are valuable for users who want to dive deeper.
To improve code readability, consider using template literals for multi-line strings in the JavaScript example:
-const db = - (await init) < - Schema > - ({ +const db = (await init)<Schema>({🧰 Tools
🪛 Markdownlint
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
127-147: Well-organized package list with descriptions.The All Packages section provides a comprehensive overview of the Dojo.js ecosystem, with brief descriptions of each package's purpose. The link to the SDK full documentation is particularly helpful.
For consistency, consider using the same format for all package links. Some links are relative (
./packages/create-burner/), while others are not (e.g.,[core](./packages/core/)). Standardizing these would improve maintainability.
148-162: Comprehensive list of examples with local setup instructions.The All Examples section provides a valuable resource for users to explore different implementations of Dojo.js. The command to spin up examples locally is particularly useful.
To improve clarity, consider adding a brief note about the prerequisites needed to run the examples (e.g., Node.js version, any global packages required).
164-284: Comprehensive contributing guidelines with detailed setup instructions.The Contributing section provides excellent guidance for potential contributors, including detailed steps for setting up the development environment, building packages, running examples, and debugging. This level of detail will greatly assist new contributors in getting started with the project.
Consider the following minor improvements:
- Add a note about the required Node.js version in the "Setting up the Development Environment" section.
- In the "Running The Examples" section, consider using consistent capitalization for terminal numbers (e.g., "Terminal 1" instead of "Terminal 1" and "Terminal 2").
- In the Contributing Guidelines list, add a comma after "if applicable" in point 3 for consistency with the other list items.
🧰 Tools
🪛 LanguageTool
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...name>with the name of the example you want to run (e.g.,example-vite-react-sdk`). ...(REP_WANT_TO_VB)
[uncategorized] ~280-~280: Possible missing comma found.
Context: ...s coding style. 3. Write tests for your changes if applicable. 4. Run the test suite to...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- readme.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
readme.md
[grammar] ~28-~28: “React” is a proper noun and needs to be capitalized.
Context: ...# Quick start in 5 minutes Start a new react app with (You will need [dojo](https://...(A_GOOGLE)
[style] ~246-~246: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...name>with the name of the example you want to run (e.g.,example-vite-react-sdk`). ...(REP_WANT_TO_VB)
[uncategorized] ~280-~280: Possible missing comma found.
Context: ...s coding style. 3. Write tests for your changes if applicable. 4. Run the test suite to...(AI_HYDRA_LEO_MISSING_COMMA)
🪛 Markdownlint
readme.md
36-36: Expected: h3; Actual: h4
Heading levels should only increment by one level at a time(MD001, heading-increment)
68-68: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
2-2: null
Images should have alternate text (alt text)(MD045, no-alt-text)
3-3: null
Images should have alternate text (alt text)(MD045, no-alt-text)
4-4: null
Images should have alternate text (alt text)(MD045, no-alt-text)
23-23: null
Link fragments should be valid(MD051, link-fragments)
🔇 Additional comments (1)
readme.md (1)
15-24: Well-structured introduction and table of contents.The addition of a concise introduction and a table of contents greatly improves the README's organization and usability. The link to the Dojo book is a valuable resource for newcomers.
🧰 Tools
🪛 Markdownlint
23-23: null
Link fragments should be valid(MD051, link-fragments)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
packages/sdk/readme.md (5)
1-19: Great improvements to the introduction and structure!The updated title, new introduction, and comprehensive table of contents significantly enhance the README's clarity and usability. These changes provide users with a clear understanding of the SDK's purpose and make it easier to navigate the documentation.
Consider adjusting the indentation of the table of contents for better readability. For example:
## Table of Contents - [Installation](#installation) - [Usage](#usage) - [Initializing the SDK](#initializing-the-sdk) - [Understanding Queries](#understanding-queries) - [Querying Entities](#querying-entities) - [Subscribing to Entity Changes](#subscribing-to-entity-changes) - [Sending Signed Messages](#sending-signed-messages) - [Using with Zustand](#using-with-zustand) - [Optimistic Client Rendering](#optimistic-client-rendering) - [Advanced Usage](#advanced-usage) - [Complex Queries](#complex-queries) - [Troubleshooting](#troubleshooting)🧰 Tools
🪛 Markdownlint
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
14-14: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
17-17: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
7-7: null
Link fragments should be valid(MD051, link-fragments)
20-27: Excellent addition of Key Features section!The new Key Features section provides a clear and concise overview of the SDK's main capabilities. This addition helps users quickly understand the benefits of using the Dojo SDK.
There's a minor grammatical issue in the "Signed messages" feature description. Consider revising it as follows:
- **Signed messages**: Sign off-chain state and send to torii. + **Signed messages**: Sign off-chain state and send to Torii.This change capitalizes "Torii" as it appears to be a proper noun in this context.
🧰 Tools
🪛 LanguageTool
[grammar] ~25-~25: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. - Automatic Zustand Support: Dro...(VB_TO_NN_DT)
29-33: Great addition of Entities and Models explanation!This new section provides a concise overview of how entities and models work in the Dojo SDK, which is crucial for users to understand the system's architecture.
To further enhance this section, consider adding a simple example to illustrate the concept. For instance:
Example: An entity "Player" might have multiple models such as "Position", "Inventory", and "Stats". When a subscription returns data for a Player entity, it includes the updated models, e.g., the new Position after movement.This addition would help users better grasp the relationship between entities and models.
63-132: Great updates to the Usage section!The Installation and Initializing the SDK sections provide clear and detailed instructions for setting up the Dojo SDK. The inclusion of TypeScript type generation steps is particularly helpful for ensuring type safety.
To help users who might be updating from a previous version, consider adding a note highlighting the changes to the schema structure. For example:
Note: The schema structure has been updated in this version. If you're upgrading from a previous version, you may need to adjust your schema definition to match the new format shown in the example below.This addition would help users quickly identify and adapt to the changes in the SDK initialization process.
135-220: Excellent explanations and examples for queries and subscriptions!The new Understanding Queries section provides a clear distinction between SubscriptionQueryType and QueryType, which is crucial for users to understand. The examples in the Querying Entities and Subscribing To Entity Changes sections are well-structured, include error handling, and demonstrate practical use cases.
To further enhance the Understanding Queries section, consider adding a brief explanation of when to use each query type. For example:
When to use each query type: - Use SubscriptionQueryType when you need real-time updates on entity changes. - Use QueryType when you need to fetch data once or perform more complex filtering operations.This addition would help users choose the appropriate query type for their specific use cases.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~139-~139: Loose punctuation mark.
Context: ... Engine: 1.SubscriptionQueryType: Used for real-time subscriptions to ent...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ...ty and event updates. 2.QueryType: Used for fetching entities and event me...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~144-~144: Loose punctuation mark.
Context: ...clause: -SubscriptionQueryType: - Supports only the$isoperato...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~146-~146: Loose punctuation mark.
Context: ...r for exact matches. -QueryType: - Supports a variety of operator...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~148-~148: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ... -QueryType: - Supports a variety of operators for more advanced filtering: ...(A_VARIETY_OF)
🪛 Markdownlint
145-145: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
148-148: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
159-159: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/sdk/readme.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/sdk/readme.md
[grammar] ~25-~25: The word ‘torii’ is not correct in this context. Use one of the suggestions or replace it with an appropriate verb.
Context: ...ssages**: Sign off-chain state and send to torii. - Automatic Zustand Support: Dro...(VB_TO_NN_DT)
[uncategorized] ~139-~139: Loose punctuation mark.
Context: ... Engine: 1.SubscriptionQueryType: Used for real-time subscriptions to ent...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~140-~140: Loose punctuation mark.
Context: ...ty and event updates. 2.QueryType: Used for fetching entities and event me...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~144-~144: Loose punctuation mark.
Context: ...clause: -SubscriptionQueryType: - Supports only the$isoperato...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~146-~146: Loose punctuation mark.
Context: ...r for exact matches. -QueryType: - Supports a variety of operator...(UNLIKELY_OPENING_PUNCTUATION)
[style] ~148-~148: The phrase “a variety of” may be wordy. To make your writing clearer, consider replacing it.
Context: ... -QueryType: - Supports a variety of operators for more advanced filtering: ...(A_VARIETY_OF)
🪛 Markdownlint
packages/sdk/readme.md
9-9: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
10-10: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
11-11: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
12-12: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
13-13: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
14-14: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
15-15: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
17-17: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
145-145: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
148-148: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
159-159: Expected: 2; Actual: 4
Unordered list indentation(MD007, ul-indent)
7-7: null
Link fragments should be valid(MD051, link-fragments)
🔇 Additional comments (1)
packages/sdk/readme.md (1)
35-61: Excellent example of subscribing to specific model states!This example effectively demonstrates how to use the SDK to subscribe to specific model states. The code is well-structured, includes error handling, and provides clear comments. This addition will be very helpful for users learning how to work with the Dojo SDK.
The inclusion of the unsubscribe method at the end is particularly useful, as it reminds users to properly manage their subscriptions.
|
|
||
| - `SubscriptionQueryType` only supports the `$is` operator for exact matches. | ||
| - `QueryType` supports additional operators like `$eq`, `$neq`, `$gt`, `$gte`, `$lt`, and `$lte` for more complex filtering. | ||
| // import this outside of components | ||
| export const useDojoStore = createDojoStore<Schema>(); | ||
|
|
||
| Example of a subscription query: | ||
| ... | ||
|
|
||
| ```typescript | ||
| { | ||
| Player: { | ||
| position: [{ $: { where: { x: { $is: 5 } } } }]; | ||
| // Using in your app | ||
| const state = useDojoStore((state) => state); | ||
| const entities = useDojoStore((state) => state.entities); | ||
|
|
||
| ... | ||
|
|
||
| // Adding to a callback | ||
| const subscription = await sdk.subscribeEntityQuery( | ||
| { | ||
| world: { | ||
| item: { | ||
| $: { | ||
| where: { | ||
| type: { $is: "sword" }, | ||
| durability: { $is: 5 }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| (response) => { | ||
| if (response.error) { | ||
| console.error("Error setting up entity sync:", response.error); | ||
| } else if (response.data && response.data[0].entityId !== "0x0") { | ||
| // You just need to do this! | ||
| state.setEntities(response.data); | ||
| } | ||
| } | ||
| } | ||
| ); | ||
| ``` | ||
|
|
||
| Example of a fetch query: | ||
| ## Optimistic Client Rendering | ||
|
|
||
| <!-- TODO --> | ||
|
|
||
| # Advanced Usage | ||
|
|
||
| Create complex 'AND' with 'OR' statements to narrow in on what you want to fetch. | ||
|
|
||
| ## Complex Queries | ||
|
|
||
| ```typescript | ||
| { | ||
| Player: { | ||
| health: [{ $: { where: { value: { $gte: 50, $lt: 100 } } } }]; | ||
| const entities = await sdk.getEntities( | ||
| { | ||
| world: { | ||
| player: { | ||
| $: { | ||
| where: { | ||
| AND: [ | ||
| { score: { $gt: 100 } }, | ||
| { | ||
| OR: [ | ||
| { name: { $eq: "Alice" } }, | ||
| { name: { $eq: "Bob" } }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| }, | ||
| }, | ||
| item: { | ||
| $: { | ||
| where: { | ||
| AND: [{ durability: { $lt: 50 } }], | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| (response) => { | ||
| if (response.data) { | ||
| console.log("Fetched entities:", response.data); | ||
| } else if (response.error) { | ||
| console.error("Fetch error:", response.error); | ||
| } | ||
| } | ||
| } | ||
| ); | ||
| ``` | ||
|
|
||
| These query structures allow you to efficiently filter and retrieve the data you need from the Dojo Engine. | ||
| ## Troubleshooting |
There was a problem hiding this comment.
Good additions to advanced topics and integration!
The Sending Signed Messages and Using With Zustand sections provide valuable information and examples for users who need these advanced features. The Advanced Usage section with complex query examples is particularly useful for power users.
There are a few areas that need attention:
-
The Optimistic Client Rendering section is currently empty (marked as TODO). This should be completed to provide users with information on this important feature.
-
The Troubleshooting section is empty. Consider adding common issues and their solutions to help users resolve problems they might encounter.
Would you like assistance in drafting content for the Optimistic Client Rendering section and some common troubleshooting scenarios?
Summary by CodeRabbit
New Features
Bug Fixes
Documentation