feat: nextjs #328
feat: nextjs #328
Conversation
WalkthroughThis pull request introduces several new files and configurations for a Next.js project, particularly in 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: 17
🧹 Outside diff range and nitpick comments (22)
examples/example-nextjs-react-sdk/postcss.config.mjs (1)
1-6: Consider adding autoprefixer plugin.While the basic Tailwind CSS setup is correct, it's recommended to include autoprefixer as well, which is a standard practice in Next.js projects to ensure better browser compatibility.
Here's the suggested configuration:
/** @type {import('postcss-load-config').Config} */ const config = { plugins: { tailwindcss: {}, + autoprefixer: {}, }, };examples/example-nextjs-react-sdk/src/app/dojoConfig.ts (1)
5-7: Consider environment-specific configuration and type safety.The current setup has several potential improvements:
- It's using a development manifest (
manifest_dev.json) without environment checks- No type validation for the manifest structure
- No error handling for invalid configurations
Consider:
- Adding environment-specific manifest selection
- Adding type definitions for the manifest
- Adding validation and error handling
Example enhancement:
import { z } from 'zod'; // Define manifest schema const ManifestSchema = z.object({ // add your manifest structure here }); // Type-safe manifest type Manifest = z.infer<typeof ManifestSchema>; // Environment-aware configuration const getManifest = (): Manifest => { const manifest = process.env.NODE_ENV === 'production' ? require('@config/manifest.json') : require('@config/manifest_dev.json'); return ManifestSchema.parse(manifest); }; export const dojoConfig = createDojoConfig({ manifest: getManifest(), });examples/example-nextjs-react-sdk/next.config.ts (1)
4-11: Consider enhancing WebAssembly configuration for production readiness.While the basic WASM setup is correct, consider these production-ready enhancements:
- Add WASM caching strategy
- Configure WASM loading fallbacks
- Add error boundaries for WASM loading failures
Example enhancement:
webpack: (config) => { config.experiments = { ...config.experiments, asyncWebAssembly: true, topLevelAwait: true, }; + // Optimize WASM loading + config.output = { + ...config.output, + webassemblyModuleFilename: 'static/wasm/[modulehash].wasm' + }; return config; },examples/example-nextjs-react-sdk/src/app/useDojo.tsx (1)
4-11: Consider enhancing the error message for better debugging.While the error handling is well-placed and follows React hooks best practices, the error message could be more specific about the setup requirements.
Consider this enhancement:
- "The `useDojo` hook must be used within a `DojoProvider`" + "The `useDojo` hook must be used within a `DojoProvider`. Ensure you have wrapped your component tree with <DojoProvider> at a higher level."examples/example-nextjs-react-sdk/src/app/layout.tsx (3)
1-14: Consider optimizing font configuration for better maintainability and performance.
- Move font files to a centralized
public/fontsdirectory for better asset management- Add font loading strategy for performance optimization
const geistSans = localFont({ - src: "./fonts/GeistVF.woff", + src: "../../../public/fonts/GeistVF.woff", variable: "--font-geist-sans", weight: "100 900", + display: "swap", + preload: true, });
16-19: Update metadata with project-specific information.The current metadata contains default values from create-next-app. Please update them to reflect your Dojo.js example application.
export const metadata: Metadata = { - title: "Create Next App", - description: "Generated by create next app", + title: "Dojo.js Next.js Example", + description: "Example application showcasing Dojo.js integration with Next.js", };
21-35: Enhance accessibility and SEO with additional meta tags and attributes.The layout component could benefit from additional accessibility and SEO optimizations.
export default function RootLayout({ children, }: Readonly<{ children: React.ReactNode; }>) { return ( - <html lang="en"> + <html lang="en" suppressHydrationWarning> + <head> + <meta name="viewport" content="width=device-width, initial-scale=1" /> + <meta name="theme-color" content="#000000" /> + </head> <body - className={`${geistSans.variable} ${geistMono.variable} antialiased`} + className={`${geistSans.variable} ${geistMono.variable} antialiased min-h-screen`} > {children} </body> </html> ); }examples/example-nextjs-react-sdk/src/app/useModel.tsx (3)
1-2: Consider moving store initialization to a separate file.The
useDojoStoreimport fromApp.tsxcould lead to circular dependencies. Consider moving the store initialization and its hooks to a dedicated file (e.g.,store.ts) to maintain a cleaner architecture.
4-10: Enhance documentation with usage examples and type details.While the documentation is clear, it would be more helpful with:
- A usage example showing how to call the hook
- More details about the expected format of
entityId- Examples of valid namespace-model combinations
Example addition:
/** * Custom hook to retrieve a specific model for a given entityId within a specified namespace. * * @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. + * + * @example + * // Get player position model + * const position = useModel('player123', 'position-coordinates'); + * if (position) { + * const { x, y } = position; + * } */
28-28: Consider using named export instead of default export.Named exports are generally preferred in TypeScript projects as they:
- Make refactoring easier
- Provide better autocomplete support
- Are more explicit in their usage
-export default useModel; +export { useModel };examples/example-nextjs-react-sdk/src/app/page.tsx (1)
33-35: Consider enhancing the layout structure.The current layout is very basic and might not provide the best user experience.
Consider adding more semantic HTML and better structure:
-<div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> - <h1>Hello World</h1> -</div> +<main className="grid grid-rows-[auto_1fr_auto] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> + <header className="w-full text-center"> + <h1 className="text-3xl font-bold">Hello World</h1> + </header> + <section className="w-full"> + {/* Add your main content here */} + </section> + <footer className="w-full text-center"> + {/* Add your footer content here */} + </footer> +</main>examples/example-nextjs-react-sdk/src/app/bindings.ts (3)
1-7: Consider enhancing type safety and documentation.The
Movesinterface could benefit from:
- Using a more specific type for
player(e.g.,EntityIDor similar)- Adding JSDoc comments to explain the purpose of fields, especially
remaining- Consider moving
fieldOrderto a separate metadata interface since it's not movement-related data+/** Represents the movement state of a player in the game */ interface Moves { - fieldOrder: string[]; - player: string; + player: EntityID; // Assuming you create a type alias for this remaining: number; last_direction: Direction; can_move: boolean; }
21-27: Consider using numeric enum values.The
Directionenum uses string values for what appears to be sequential numbers. Using numeric values would be more efficient and conventional for this use case.enum Direction { - None = "0", - Left = "1", - Right = "2", - Up = "3", - Down = "4", + None = 0, + Left = 1, + Right = 2, + Up = 3, + Down = 4, }
29-32: Consider making Vec2 immutable and adding utility methods.The
Vec2interface could be enhanced with readonly properties to prevent accidental mutations and utility methods for common vector operations.-interface Vec2 { - x: number; - y: number; +interface Vec2 { + readonly x: number; + readonly y: number; } + +// Consider adding utility functions +const Vec2 = { + add: (a: Vec2, b: Vec2): Vec2 => ({ x: a.x + b.x, y: a.y + b.y }), + subtract: (a: Vec2, b: Vec2): Vec2 => ({ x: a.x - b.x, y: a.y - b.y }), + magnitude: (v: Vec2): number => Math.sqrt(v.x * v.x + v.y * v.y), +};examples/example-nextjs-react-sdk/src/app/DojoContext.tsx (2)
1-16: Add JSDoc documentation for the interface and consider strengthening types.The
DojoContextTypeinterface represents core functionality but lacks documentation. Consider adding JSDoc comments to explain the purpose of each property, especiallymasterAccountwhich handles sensitive blockchain operations.+/** + * Core Dojo context interface that provides access to blockchain accounts and client. + */ interface DojoContextType { + /** Master account for blockchain operations */ masterAccount: Account; + /** Generated client for contract interactions */ client: ReturnType<typeof client>; + /** Current burner account for user operations */ account: BurnerAccount; }
1-64: Consider adding configuration validation and environment checks.As this is a Next.js application, consider adding:
- Runtime configuration validation using a schema validation library like Zod
- Environment-specific configuration handling
- Error boundaries for graceful fallbacks in production
This will improve the reliability and maintainability of the Dojo integration.
examples/example-nextjs-react-sdk/src/app/useSystemCalls.ts (2)
26-26: Extract magic number into a named constant.The value
100should be defined as a constant with a meaningful name to improve maintainability.+const INITIAL_MOVES_COUNT = 100; const spawn = async () => { // ... - const remainingMoves = 100; + const remainingMoves = INITIAL_MOVES_COUNT;
30-35: Simplify nested property access in optimistic update.The deep nesting makes the code harder to read and maintain. Consider extracting the path check into a helper function.
+const getMovesModel = (draft: any, entityId: string) => + draft.entities[entityId]?.models?.dojo_starter?.Moves; state.applyOptimisticUpdate(transactionId, (draft) => { - if (draft.entities[entityId]?.models?.dojo_starter?.Moves) { - draft.entities[entityId].models.dojo_starter.Moves.remaining = - remainingMoves; + const moves = getMovesModel(draft, entityId); + if (moves) { + moves.remaining = remainingMoves; } });examples/example-nextjs-react-sdk/src/app/contracts.gen.ts (1)
9-13: Consider parameterizing the contract name.The contract name "actions" is hardcoded. Consider making it configurable through the client function parameters for better flexibility and reusability.
-export function client(provider: DojoProvider) { +export function client(provider: DojoProvider, contractName: string = "actions") { function actions() { - const contract_name = "actions"; + const contract_name = contractName;readme.md (1)
49-49: Add explanation for the build step.While the command is correct, it would be helpful to explain why both
buildandmigratesteps are necessary. This helps users understand the process better.Consider adding a comment like this:
- sozo build && sozo migrate + # Build the contracts and migrate them to the local network + sozo build && sozo migrateexamples/example-nextjs-react-sdk/src/app/App.tsx (2)
194-194: Simplify expression using optional chainingThe expression
{moves && moves.last_direction}at line 194 can be simplified using optional chaining to improve readability.Apply this diff:
- {moves && moves.last_direction} + {moves?.last_direction}🧰 Tools
🪛 Biome
[error] 194-194: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
159-162: Avoid variable shadowing inmapfunctionIn the
mapfunction at lines 159-162, the parameteraccountshadows the outeraccountvariable. This can lead to confusion. Consider renaming the parameter to avoid variable shadowing.Apply this diff:
- {account?.list().map((account, index) => ( - <option value={account.address} key={index}> - {account.address} + {account?.list().map((acct, index) => ( + <option value={acct.address} key={index}> + {acct.address}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (9)
examples/example-nextjs-react-sdk/public/file.svgis excluded by!**/*.svgexamples/example-nextjs-react-sdk/public/globe.svgis excluded by!**/*.svgexamples/example-nextjs-react-sdk/public/next.svgis excluded by!**/*.svgexamples/example-nextjs-react-sdk/public/vercel.svgis excluded by!**/*.svgexamples/example-nextjs-react-sdk/public/window.svgis excluded by!**/*.svgexamples/example-nextjs-react-sdk/src/app/favicon.icois excluded by!**/*.icoexamples/example-nextjs-react-sdk/src/app/fonts/GeistMonoVF.woffis excluded by!**/*.woffexamples/example-nextjs-react-sdk/src/app/fonts/GeistVF.woffis excluded by!**/*.woffpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (21)
.prettierignore(1 hunks)examples/example-nextjs-react-sdk/.eslintrc.json(1 hunks)examples/example-nextjs-react-sdk/.gitignore(1 hunks)examples/example-nextjs-react-sdk/README.md(1 hunks)examples/example-nextjs-react-sdk/next.config.ts(1 hunks)examples/example-nextjs-react-sdk/package.json(1 hunks)examples/example-nextjs-react-sdk/postcss.config.mjs(1 hunks)examples/example-nextjs-react-sdk/src/app/App.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/DojoContext.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/bindings.ts(1 hunks)examples/example-nextjs-react-sdk/src/app/contracts.gen.ts(1 hunks)examples/example-nextjs-react-sdk/src/app/dojoConfig.ts(1 hunks)examples/example-nextjs-react-sdk/src/app/globals.css(1 hunks)examples/example-nextjs-react-sdk/src/app/layout.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/page.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/useDojo.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/useModel.tsx(1 hunks)examples/example-nextjs-react-sdk/src/app/useSystemCalls.ts(1 hunks)examples/example-nextjs-react-sdk/tailwind.config.ts(1 hunks)examples/example-nextjs-react-sdk/tsconfig.json(1 hunks)readme.md(1 hunks)
✅ Files skipped from review due to trivial changes (8)
- .prettierignore
- examples/example-nextjs-react-sdk/.eslintrc.json
- examples/example-nextjs-react-sdk/.gitignore
- examples/example-nextjs-react-sdk/README.md
- examples/example-nextjs-react-sdk/package.json
- examples/example-nextjs-react-sdk/src/app/globals.css
- examples/example-nextjs-react-sdk/tailwind.config.ts
- examples/example-nextjs-react-sdk/tsconfig.json
🧰 Additional context used
🪛 Biome
examples/example-nextjs-react-sdk/src/app/App.tsx
[error] 194-194: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (14)
examples/example-nextjs-react-sdk/postcss.config.mjs (1)
2-6: Verify Tailwind CSS configuration.
The empty Tailwind configuration here implies that the main Tailwind settings should be in a separate file.
Let's verify the Tailwind configuration:
✅ Verification successful
Tailwind CSS configuration is properly set up
The empty configuration in postcss.config.mjs is correct as the main Tailwind settings are properly defined in tailwind.config.ts. The configuration includes:
- Content paths for all relevant source files
- Theme extensions with color variables
- Empty plugins array (which is fine if no plugins are needed)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Tailwind config file and its content
# Expected: Should find a tailwind.config.ts/js file with proper configuration
# Look for Tailwind config file
echo "Searching for Tailwind config file..."
fd -t f "tailwind.config.(ts|js)" "examples/example-nextjs-react-sdk"
# If found, check its content
echo "Checking Tailwind configuration content..."
rg -A 10 "content|theme|plugins" "examples/example-nextjs-react-sdk/tailwind.config.*"
Length of output: 483
Script:
#!/bin/bash
# Let's try to read the content of the tailwind config file directly
cat "examples/example-nextjs-react-sdk/tailwind.config.ts"
Length of output: 523
examples/example-nextjs-react-sdk/next.config.ts (2)
1-14: LGTM! The basic configuration for WebAssembly support is correct.
The configuration properly enables WebAssembly support through Webpack experimental features while maintaining type safety with TypeScript.
3-12: Verify Next.js version compatibility.
Since this PR targets Next.js 15 (beta), ensure that the Webpack configuration remains compatible.
✅ Verification successful
Let me verify the webpack configuration compatibility with Next.js 15 by checking the official documentation and examples.
The Webpack configuration is compatible with Next.js 15.0.2
The current Webpack configuration enabling asyncWebAssembly and topLevelAwait is fully supported in Next.js 15, especially given the presence of WASM-related dependencies (@dojoengine/torii-wasm) in the project. These Webpack experiments are standard features in Next.js 15's Webpack 5 configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check package.json for Next.js version and any related WASM dependencies
echo "Checking Next.js version and WASM-related dependencies:"
cat examples/example-nextjs-react-sdk/package.json | grep -A 5 -B 5 '"next":'
Length of output: 638
Script:
#!/bin/bash
# Check if there are any other Next.js config files or webpack configurations in the repository
echo "Searching for Next.js config files:"
fd "next.config" --type f
echo -e "\nSearching for webpack configurations in other Next.js examples:"
fd "webpack" --type f --exec grep -l "webpack" {} \;
echo -e "\nChecking package dependencies for webpack-related packages:"
cat examples/example-nextjs-react-sdk/package.json | grep -A 10 -B 10 '"dependencies":'
Length of output: 1238
examples/example-nextjs-react-sdk/src/app/useDojo.tsx (1)
1-2: LGTM! Clean and specific imports.
The imports are well-structured, using named imports and appropriate relative paths.
examples/example-nextjs-react-sdk/src/app/layout.tsx (1)
1-35: Address WASM configuration requirements for Next.js.
Since the PR mentions WASM-related fixes are needed, ensure you:
- Configure WASM support in next.config.js
- Handle WASM loading appropriately on the client side
examples/example-nextjs-react-sdk/src/app/useModel.tsx (1)
1-2: Verify store schema and hook usage patterns.
Let's verify the integration with the Dojo store and consistent usage patterns:
✅ Verification successful
Let me gather more information about the store integration by checking the implementation and usage of useDojoStore.
Let me check one more aspect to verify the complete store integration pattern.
Store integration is properly implemented and type-safe
The integration is well-structured with:
- Correct Schema type definition mapping to store structure
- Type-safe useModel hook implementation with proper generic constraints
- Consistent usage pattern across components using Models enum
- Proper store selector pattern in useDojoStore usage
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Schema type definition
echo "Checking Schema type definition:"
rg -A 10 "type\s+Schema\s*=" examples/example-nextjs-react-sdk/src/
# Check hook usage patterns
echo -e "\nChecking useModel hook usage:"
rg "useModel\(" examples/example-nextjs-react-sdk/src/
Length of output: 1337
Script:
#!/bin/bash
# Check useDojoStore implementation and usage
echo "Checking useDojoStore implementation and usage:"
rg -A 5 "useDojoStore" examples/example-nextjs-react-sdk/src/
# Check Position model definition
echo -e "\nChecking Position model definition:"
rg -A 5 "type\s+Position\s*=" examples/example-nextjs-react-sdk/src/
# Check DirectionsAvailable and Moves types
echo -e "\nChecking other model types:"
rg -A 5 "type\s+(DirectionsAvailable|Moves)\s*=" examples/example-nextjs-react-sdk/src/
Length of output: 3543
Script:
#!/bin/bash
# Check createDojoStore implementation and the complete useModel hook
echo "Checking createDojoStore and useModel implementation:"
cat examples/example-nextjs-react-sdk/src/app/useModel.tsx
# Check the Models enum usage
echo -e "\nChecking Models enum usage:"
rg -A 3 "Models\." examples/example-nextjs-react-sdk/src/
Length of output: 1666
examples/example-nextjs-react-sdk/src/app/page.tsx (1)
1-2: Verify WebAssembly configuration for Next.js.
Since this PR mentions WASM fixes, ensure that Next.js is properly configured to handle WebAssembly modules.
examples/example-nextjs-react-sdk/src/app/bindings.ts (1)
70-71: LGTM! Clean export organization.
The exports are well-organized, separating types from values as per TypeScript best practices.
examples/example-nextjs-react-sdk/src/app/DojoContext.tsx (1)
55-58:
Review the account fallback logic for security implications.
The current implementation falls back to using the masterAccount when burnerManagerData.account is not available. This could potentially expose the master account unintentionally.
Consider implementing a more secure fallback mechanism:
- account: {
- ...burnerManagerData,
- account: burnerManagerData.account || masterAccount,
- },
+ account: {
+ ...burnerManagerData,
+ account: burnerManagerData.account ||
+ (() => {
+ console.warn('No burner account available, creating a new one');
+ return burnerManager.createBurner();
+ })(),
+ },examples/example-nextjs-react-sdk/src/app/useSystemCalls.ts (2)
1-12: LGTM: Clean hook setup with proper imports and state management.
The hook follows React patterns and correctly retrieves necessary dependencies.
59-62: LGTM: Clean hook interface.
The hook exports a clear and focused API with the spawn function.
examples/example-nextjs-react-sdk/src/app/contracts.gen.ts (3)
7-7: LGTM! Type definition is well-structured.
The IClient type definition correctly uses TypeScript's ReturnType utility type to infer the interface from the client function.
54-63: LGTM! Return structure is clean and well-organized.
The return statements properly expose the actions object with its methods.
1-6: Address the regeneration comment.
The comment on line 3 suggests that the file needs to be regenerated using sozo build --typescript. This should be addressed before finalizing the PR.
Let's check if this is the latest generated version:
| @@ -0,0 +1,7 @@ | |||
| import { createDojoConfig } from "@dojoengine/core"; | |||
|
|
|||
| import manifest from "../../../../worlds/dojo-starter/manifest_dev.json"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider improving the manifest import path.
The deep relative import path (../../../../) is fragile and could break if the directory structure changes. Consider:
- Moving the manifest files closer to the application code
- Using path aliases in your TypeScript configuration
- Creating a dedicated config directory within your Next.js app
Example using path aliases:
- import manifest from "../../../../worlds/dojo-starter/manifest_dev.json";
+ import manifest from "@config/manifest_dev.json";Then configure the path in tsconfig.json:
{
"compilerOptions": {
"paths": {
"@config/*": ["../../worlds/dojo-starter/*"]
}
}
}| const { account, ...setup } = context; | ||
|
|
||
| return { | ||
| setup, | ||
| account, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript types for better type safety and documentation.
While the destructuring and return structure are clean, adding TypeScript types would improve type safety and provide better documentation.
Consider adding types like this:
+interface DojoContextType {
+ account: Account; // Replace with actual account type
+ setup: SetupConfig; // Replace with actual setup config type
+}
-export const useDojo = () => {
+export const useDojo = (): DojoContextType => {
const context = useContext(DojoContext);
if (!context) {
throw new Error(
"The `useDojo` hook must be used within a `DojoProvider`"
);
}
- const { account, ...setup } = context;
+ const { account, ...setup }: DojoContextType = context;
return {
setup,
account,
};
};Committable suggestion skipped: line range outside the PR's diff.
| function useModel<N extends keyof Schema, M extends keyof Schema[N] & string>( | ||
| entityId: string, | ||
| model: `${N}-${M}` | ||
| ): Schema[N][M] | undefined { | ||
| const [namespace, modelName] = model.split("-") as [N, M]; | ||
|
|
||
| // Select only the specific model data for the given entityId | ||
| const modelData = useDojoStore( | ||
| (state) => | ||
| state.entities[entityId]?.models?.[namespace]?.[modelName] as | ||
| | Schema[N][M] | ||
| | undefined | ||
| ); | ||
|
|
||
| return modelData; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add input validation and error handling.
The current implementation assumes the input format is always correct. Consider adding validation to handle edge cases:
function useModel<N extends keyof Schema, M extends keyof Schema[N] & string>(
entityId: string,
model: `${N}-${M}`
): Schema[N][M] | undefined {
+ if (!entityId?.trim()) {
+ throw new Error('entityId is required');
+ }
+
const [namespace, modelName] = model.split("-") as [N, M];
+
+ if (!namespace || !modelName) {
+ throw new Error('Invalid model format. Expected "namespace-modelName"');
+ }
// Select only the specific model data for the given entityId
const modelData = useDojoStore(Also, consider adding memoization to prevent unnecessary re-renders:
function useModel<N extends keyof Schema, M extends keyof Schema[N] & string>(
entityId: string,
model: `${N}-${M}`
): Schema[N][M] | undefined {
const [namespace, modelName] = model.split("-") as [N, M];
// Select only the specific model data for the given entityId
- const modelData = useDojoStore(
+ const modelData = useDojoStore(useCallback(
(state) =>
state.entities[entityId]?.models?.[namespace]?.[modelName] as
| Schema[N][M]
- | undefined
+ | undefined,
+ [entityId, namespace, modelName])
);
return modelData;
}Committable suggestion skipped: line range outside the PR's diff.
| const sdk = await init<Schema>( | ||
| { | ||
| client: { | ||
| rpcUrl: dojoConfig.rpcUrl, | ||
| toriiUrl: dojoConfig.toriiUrl, | ||
| relayUrl: dojoConfig.relayUrl, | ||
| worldAddress: dojoConfig.manifest.world.address, | ||
| }, | ||
| domain: { | ||
| name: "WORLD_NAME", | ||
| version: "1.0", | ||
| chainId: "KATANA", | ||
| revision: "1", | ||
| }, | ||
| }, | ||
| schema | ||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Move SDK initialization into a proper initialization function.
The top-level await usage could cause issues with Next.js client-side rendering. Consider moving this into a proper initialization function with error handling.
Here's a suggested implementation:
-const sdk = await init<Schema>(
+async function initializeDojo() {
+ try {
+ return await init<Schema>(
{
client: {
rpcUrl: dojoConfig.rpcUrl,
toriiUrl: dojoConfig.toriiUrl,
relayUrl: dojoConfig.relayUrl,
worldAddress: dojoConfig.manifest.world.address,
},
domain: {
- name: "WORLD_NAME",
+ name: process.env.NEXT_PUBLIC_WORLD_NAME || "WORLD_NAME",
version: "1.0",
chainId: "KATANA",
revision: "1",
},
},
schema
-);
+ );
+ } catch (error) {
+ console.error("Failed to initialize Dojo SDK:", error);
+ throw error;
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sdk = await init<Schema>( | |
| { | |
| client: { | |
| rpcUrl: dojoConfig.rpcUrl, | |
| toriiUrl: dojoConfig.toriiUrl, | |
| relayUrl: dojoConfig.relayUrl, | |
| worldAddress: dojoConfig.manifest.world.address, | |
| }, | |
| domain: { | |
| name: "WORLD_NAME", | |
| version: "1.0", | |
| chainId: "KATANA", | |
| revision: "1", | |
| }, | |
| }, | |
| schema | |
| ); | |
| async function initializeDojo() { | |
| try { | |
| return await init<Schema>( | |
| { | |
| client: { | |
| rpcUrl: dojoConfig.rpcUrl, | |
| toriiUrl: dojoConfig.toriiUrl, | |
| relayUrl: dojoConfig.relayUrl, | |
| worldAddress: dojoConfig.manifest.world.address, | |
| }, | |
| domain: { | |
| name: process.env.NEXT_PUBLIC_WORLD_NAME || "WORLD_NAME", | |
| version: "1.0", | |
| chainId: "KATANA", | |
| revision: "1", | |
| }, | |
| }, | |
| schema | |
| ); | |
| } catch (error) { | |
| console.error("Failed to initialize Dojo SDK:", error); | |
| throw error; | |
| } | |
| } |
| export default async function Home() { | ||
| return ( | ||
| <DojoContextProvider | ||
| burnerManager={await setupBurnerManager(dojoConfig)} | ||
| > | ||
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | ||
| <h1>Hello World</h1> | ||
| </div> | ||
| </DojoContextProvider> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add proper error handling and loading states.
The async component lacks error handling and loading states, which could lead to poor user experience.
Consider implementing this improved version:
-export default async function Home() {
+export default function Home() {
+ const [isLoading, setIsLoading] = useState(true);
+ const [error, setError] = useState<Error | null>(null);
+ const [burnerManager, setBurnerManager] = useState<BurnerManager | null>(null);
+
+ useEffect(() => {
+ async function setup() {
+ try {
+ const manager = await setupBurnerManager(dojoConfig);
+ setBurnerManager(manager);
+ } catch (err) {
+ setError(err instanceof Error ? err : new Error('Failed to setup burner'));
+ } finally {
+ setIsLoading(false);
+ }
+ }
+ setup();
+ }, []);
+
+ if (isLoading) return <div>Loading...</div>;
+ if (error) return <div>Error: {error.message}</div>;
+ if (!burnerManager) return <div>Failed to initialize</div>;
return (
<DojoContextProvider
- burnerManager={await setupBurnerManager(dojoConfig)}
+ burnerManager={burnerManager}
>
<div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]">
<h1>Hello World</h1>
</div>
</DojoContextProvider>
);
}📝 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.
| export default async function Home() { | |
| return ( | |
| <DojoContextProvider | |
| burnerManager={await setupBurnerManager(dojoConfig)} | |
| > | |
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | |
| <h1>Hello World</h1> | |
| </div> | |
| </DojoContextProvider> | |
| ); | |
| } | |
| export default function Home() { | |
| const [isLoading, setIsLoading] = useState(true); | |
| const [error, setError] = useState<Error | null>(null); | |
| const [burnerManager, setBurnerManager] = useState<BurnerManager | null>(null); | |
| useEffect(() => { | |
| async function setup() { | |
| try { | |
| const manager = await setupBurnerManager(dojoConfig); | |
| setBurnerManager(manager); | |
| } catch (err) { | |
| setError(err instanceof Error ? err : new Error('Failed to setup burner')); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| } | |
| setup(); | |
| }, []); | |
| if (isLoading) return <div>Loading...</div>; | |
| if (error) return <div>Error: {error.message}</div>; | |
| if (!burnerManager) return <div>Failed to initialize</div>; | |
| return ( | |
| <DojoContextProvider | |
| burnerManager={burnerManager} | |
| > | |
| <div className="grid grid-rows-[20px_1fr_20px] items-center justify-items-center min-h-screen p-8 pb-20 gap-16 sm:p-20 font-[family-name:var(--font-geist-sans)]"> | |
| <h1>Hello World</h1> | |
| </div> | |
| </DojoContextProvider> | |
| ); | |
| } |
| // Call the `spawn` system with the specified Account and calldata | ||
| const spawn = async (props: { account: Account }) => { | ||
| try { | ||
| return await provider.execute( | ||
| props.account, | ||
| { | ||
| contractName: contract_name, | ||
| entrypoint: "spawn", | ||
| calldata: [], | ||
| }, | ||
| "dojo_starter" | ||
| ); | ||
| } catch (error) { | ||
| console.error("Error executing spawn:", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Multiple improvements needed for the spawn function.
- The empty calldata array might need validation or documentation to explain why it's empty
- The error logging is too generic and could include more context
- The system name "dojo_starter" is hardcoded
const spawn = async (props: { account: Account }) => {
try {
return await provider.execute(
props.account,
{
contractName: contract_name,
entrypoint: "spawn",
calldata: [],
},
- "dojo_starter"
+ systemName
);
} catch (error) {
- console.error("Error executing spawn:", error);
+ console.error(`Error executing spawn for account ${props.account.address}:`, error);
throw error;
}
};Committable suggestion skipped: line range outside the PR's diff.
| // Call the `move` system with the specified Account and calldata | ||
| const move = async (props: { account: Account; direction: any }) => { | ||
| try { | ||
| return await provider.execute( | ||
| props.account, | ||
| { | ||
| contractName: contract_name, | ||
| entrypoint: "move", | ||
| calldata: [ | ||
| ["None", "Left", "Right", "Up", "Down"].indexOf( | ||
| props.direction.type | ||
| ), | ||
| ], | ||
| }, | ||
| "dojo_starter" | ||
| ); | ||
| } catch (error) { | ||
| console.error("Error executing spawn:", error); | ||
| throw error; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Critical improvements needed for the move function.
Several issues need to be addressed:
- Using 'any' type for direction is unsafe
- Direction validation using indexOf is fragile
- Error message incorrectly refers to "spawn" instead of "move"
- Direction types should be an enum
+enum Direction {
+ None = 0,
+ Left = 1,
+ Right = 2,
+ Up = 3,
+ Down = 4
+}
+
-const move = async (props: { account: Account; direction: any }) => {
+const move = async (props: { account: Account; direction: { type: keyof typeof Direction } }) => {
try {
return await provider.execute(
props.account,
{
contractName: contract_name,
entrypoint: "move",
- calldata: [
- ["None", "Left", "Right", "Up", "Down"].indexOf(
- props.direction.type
- ),
- ],
+ calldata: [Direction[props.direction.type]],
},
"dojo_starter"
);
} catch (error) {
- console.error("Error executing spawn:", error);
+ console.error(`Error executing move for account ${props.account.address} in direction ${props.direction.type}:`, error);
throw error;
}
};📝 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.
| // Call the `move` system with the specified Account and calldata | |
| const move = async (props: { account: Account; direction: any }) => { | |
| try { | |
| return await provider.execute( | |
| props.account, | |
| { | |
| contractName: contract_name, | |
| entrypoint: "move", | |
| calldata: [ | |
| ["None", "Left", "Right", "Up", "Down"].indexOf( | |
| props.direction.type | |
| ), | |
| ], | |
| }, | |
| "dojo_starter" | |
| ); | |
| } catch (error) { | |
| console.error("Error executing spawn:", error); | |
| throw error; | |
| } | |
| }; | |
| enum Direction { | |
| None = 0, | |
| Left = 1, | |
| Right = 2, | |
| Up = 3, | |
| Down = 4 | |
| } | |
| // Call the `move` system with the specified Account and calldata | |
| const move = async (props: { account: Account; direction: { type: keyof typeof Direction } }) => { | |
| try { | |
| return await provider.execute( | |
| props.account, | |
| { | |
| contractName: contract_name, | |
| entrypoint: "move", | |
| calldata: [Direction[props.direction.type]], | |
| }, | |
| "dojo_starter" | |
| ); | |
| } catch (error) { | |
| console.error(`Error executing move for account ${props.account.address} in direction ${props.direction.type}:`, error); | |
| throw error; | |
| } | |
| }; |
|
|
||
| # Start Torii indexer - world address can be found in the print out of migrate | ||
| torii --world <WORLD_ADDRESS> --allowed-origins "*" | ||
| torii --world 0x00e2ea9b5dd9804d13903edf712998943b7d5d606c139dd0f13eeb8f5b84da8d --allowed-origins "*" |
There was a problem hiding this comment.
Replace hardcoded world address with clearer instructions.
Using a specific world address in the documentation can be misleading as this address will vary for each deployment. The comment above correctly mentions that the address comes from the migrate output, but the example could be clearer.
Consider this revision:
- torii --world 0x00e2ea9b5dd9804d13903edf712998943b7d5d606c139dd0f13eeb8f5b84da8d --allowed-origins "*"
+ # Replace <WORLD_ADDRESS> with the address from your migrate output
+ torii --world <WORLD_ADDRESS> --allowed-origins "*"📝 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.
| torii --world 0x00e2ea9b5dd9804d13903edf712998943b7d5d606c139dd0f13eeb8f5b84da8d --allowed-origins "*" | |
| # Replace <WORLD_ADDRESS> with the address from your migrate output | |
| torii --world <WORLD_ADDRESS> --allowed-origins "*" |
| $is: addAddressPadding( | ||
| account.account.address | ||
| ), |
There was a problem hiding this comment.
Ensure account.account.address is defined before usage
At lines 39-41, 50-52, and 99-100, account.account.address is accessed without checking if account or account.account are defined. This may lead to runtime errors if account is undefined. Consider using optional chaining to safely access account.account.address.
Apply this diff to prevent potential runtime errors:
At lines 39-41:
- $is: addAddressPadding(
- account.account.address
- ),
+ $is: account?.account?.address
+ ? addAddressPadding(account.account.address)
+ : "",At lines 50-52:
- $is: addAddressPadding(
- account.account.address
- ),
+ $is: account?.account?.address
+ ? addAddressPadding(account.account.address)
+ : "",At lines 99-100:
- account.account.address
+ account?.account?.address ?? ""Also applies to: 50-52, 99-100
| const { spawn } = useSystemCalls(); | ||
|
|
||
| const entityId = useMemo( | ||
| () => getEntityIdFromKeys([BigInt(account?.account.address)]), |
There was a problem hiding this comment.
Handle potential undefined account.account.address when using BigInt
At line 24, BigInt(account?.account.address) could throw a TypeError if account.account.address is undefined, because BigInt(undefined) is invalid. To prevent this, ensure that account.account.address is defined before calling BigInt.
Apply this diff to fix the issue:
- () => getEntityIdFromKeys([BigInt(account?.account.address)]),
+ () => {
+ const address = account?.account?.address;
+ return address ? getEntityIdFromKeys([BigInt(address)]) : null;
+ },📝 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.
| () => getEntityIdFromKeys([BigInt(account?.account.address)]), | |
| () => { | |
| const address = account?.account?.address; | |
| return address ? getEntityIdFromKeys([BigInt(address)]) : null; | |
| }, |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation