feat: update dojo version #375
Conversation
WalkthroughThis pull request introduces comprehensive updates across multiple Dojo framework packages, focusing on enhancing type safety, improving package dependencies, and refactoring type definitions. The changes span several example projects and core packages, with significant modifications to how directions, themes, and entity interactions are handled. The updates primarily involve transitioning from traditional enums to more flexible Changes
Sequence DiagramsequenceDiagram
participant Client
participant DojoFramework
participant StarknetCore
participant ContractsGen
Client->>ContractsGen: Invoke action with CairoCustomEnum
ContractsGen->>StarknetCore: Convert to DojoCall
StarknetCore->>DojoFramework: Execute transaction
DojoFramework-->>StarknetCore: Return result
StarknetCore-->>ContractsGen: Confirm transaction
ContractsGen-->>Client: Provide transaction response
Possibly related PRs
Poem
Finishing Touches
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: 7
🔭 Outside diff range comments (2)
examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (1)
Line range hint
31-33: Handle asynchronous initialization ofpabefore useThe
paarray is populated asynchronously usingpredeployedAccounts(...).then(...), but it's used synchronously in theconnectorsprop. This could result inpabeing empty whenStarknetProvideris rendered, leading to missing connectors.Consider modifying the code to handle the asynchronous initialization properly. One approach is to use
useEffectanduseStatehooks to managepa:+import { useEffect, useState } from "react"; let pa: PredeployedAccountsConnector[] = []; -predeployedAccounts({ - rpc: env.VITE_CONTROLLER_RPC, - id: "katana", - name: "Katana", -}).then((p) => (pa = p)); + +export default function StarknetProvider({ children }: PropsWithChildren) { + const [pa, setPa] = useState<PredeployedAccountsConnector[]>([]); + + useEffect(() => { + predeployedAccounts({ + rpc: env.VITE_CONTROLLER_RPC, + id: "katana", + name: "Katana", + }).then((p) => setPa(p)); + }, []); + const provider = jsonRpcProvider({ rpc: (chain: Chain) => ({ nodeUrl: getRpcUrl() }), }); return ( <StarknetConfig chains={[mainnet]} provider={provider} - // @ts-expect-error this is ok connectors={[cartridge, ...pa]} explorer={voyager} autoConnect > {children} </StarknetConfig> ); }Also applies to: 52-52
examples/example-vite-kitchen-sink/src/components/sidebar.tsx (1)
Line range hint
170-174: Replace @ts-expect-error with proper typing.Instead of suppressing the TypeScript error, consider extending the connector type to include the icon property.
Create a new type definition file (e.g.,
types/connector.d.ts):import { Connector } from '@starknet-react/core'; declare module '@starknet-react/core' { interface Connector { icon: { dark: string; light: string; }; } }
🧹 Nitpick comments (13)
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (3)
55-61: Consider improving type safety by addressing ts-expect-error comments.The code uses multiple
@ts-expect-errorcomments which indicate potential type system issues. Consider:
- Adding proper type definitions for the theme value mapping
- Using type guards instead of type assertions
Example improvement:
type ThemeKey = keyof typeof AvailableThemeClassMap; type ThemeValue = typeof AvailableThemeClassMap[ThemeKey]; function isValidTheme(theme: unknown): theme is ThemeKey { return typeof theme === 'string' && theme in AvailableThemeClassMap; }Also applies to: 124-128
70-74: Consider extracting the magic number into a named constant.The hardcoded value
9999999fortheme_keyshould be defined as a named constant to improve maintainability and clarity.const THEME_KEY_DEFAULT = 9999999;
Line range hint
109-133: Consider extracting repeated theme unwrapping logic into a utility function.The theme unwrapping logic with null checks is duplicated. This could be extracted into a reusable utility function to improve maintainability and reduce code duplication.
function unwrapTheme(entity: ParsedEntity<SchemaType> | undefined): AvailableTheme { if (!entity || entity.models.onchain_dash?.Theme?.value === undefined) { return AvailableTheme.Light; } const theme = entity.models?.onchain_dash?.Theme?.value.unwrap(); return AvailableTheme[theme]; }examples/example-vite-react-sdk/src/typescript/models.gen.ts (2)
3-8: Architectural change: Migration from enum to CairoCustomEnumThe transition from a TypeScript enum to CairoCustomEnum aligns better with Cairo's type system and provides stronger type safety for cross-contract interactions.
Also applies to: 67-73
93-111: Reduce code duplication in CairoCustomEnum initializationThe same CairoCustomEnum initialization pattern is repeated multiple times. Consider extracting it to a helper function.
+const createDirectionEnum = () => new CairoCustomEnum({ + Left: "", + Right: undefined, + Up: undefined, + Down: undefined, +}); DirectionsAvailable: { fieldOrder: ["player", "directions"], player: "", - directions: [ - new CairoCustomEnum({ - Left: "", - Right: undefined, - Up: undefined, - Down: undefined, - }), - ], + directions: [createDirectionEnum()], },Also applies to: 143-157
examples/example-vite-react-sql/src/hooks/useSystemCalls.ts (1)
68-68: Remove ts-expect-error comments with proper type assertionsInstead of suppressing TypeScript errors, consider adding proper type guards or assertions.
-// @ts-expect-error this is literrally the condition above calm down typescript +if (draft.entities[entityId.toString()]?.models?.dojo_starter?.Moves as Moves) { -// @ts-expect-error inner enum is not hydrated there +const lastDirection = entity?.models?.dojo_starter?.Moves?.last_direction; +const result = lastDirection?.isSome() && + (lastDirection as CairoOption<DirectionEnum>)?.Some === direction.activeVariant();Also applies to: 83-85
examples/example-vite-react-sql/src/typescript/models.gen.ts (1)
93-100: Consider extracting repeated direction enum initialization.The same direction enum initialization is repeated multiple times in the schema. This could be DRYer.
+const DEFAULT_DIRECTION = new CairoCustomEnum({ + Left: "", + Right: undefined, + Up: undefined, + Down: undefined, +}); export const schema: SchemaType = { dojo_starter: { DirectionsAvailable: { fieldOrder: ["player", "directions"], player: "", - directions: [ - new CairoCustomEnum({ - Left: "", - Right: undefined, - Up: undefined, - Down: undefined, - }), - ], + directions: [DEFAULT_DIRECTION], }, // ... similar changes for other occurrencesAlso applies to: 104-111, 143-148, 152-157
examples/example-vite-react-sdk/src/App.tsx (1)
175-177: Consider using a constant for direction initialization.The direction initialization could be simplified by using a map or predefined constants.
+const DIRECTIONS = { + UP: new CairoCustomEnum({ Up: "()" }), + LEFT: new CairoCustomEnum({ Left: "()" }), + RIGHT: new CairoCustomEnum({ Right: "()" }), + DOWN: new CairoCustomEnum({ Down: "()" }), +} as const; {[ { - direction: new CairoCustomEnum({ - Up: "()", - }), + direction: DIRECTIONS.UP, label: "↑", col: "col-start-2", }, // ... similar changes for other directionsAlso applies to: 182-184, 189-191, 196-198
examples/example-vite-kitchen-sink/src/components/caller-counter.tsx (3)
36-43: Remove unnecessary emptycallbackindb.getEntitiescallThe
callbackparameter in thedb.getEntitiescall is an empty function and appears to be unnecessary. If thecallbackis optional, consider omitting it to simplify the code.Apply this diff to remove the unused
callback:const entity = await db.getEntities({ query: new QueryBuilder<SchemaType>() .namespace("onchain_dash", (n) => n.entity("CallerCounter", (e) => e.eq("caller", addAddressPadding(address)) ) ) .build(), - callback: () => {}, });
45-46: Check for emptyentityarray before callingpop()Calling
pop()on an empty array returnsundefined. While you checkif (!counter)afterpop(), it's safer to verify that theentityarray has elements before callingpop()to prevent unnecessary operations.Apply this diff to check the array length before popping:
-if (!counter) { +if (entity.length === 0) { return 0; } +const counter = entity.pop() as ParsedEntity<SchemaType>;
76-77: Ensuredataarray is not empty before callingpop()in subscription callbackIn the
subscribeToEntityUpdatescallback, ensure thatdatais not empty before callingpop(). This prevents potential errors whendatais an empty array.Apply this diff to add a length check:
if (data) { + if (data.length === 0) { + return; + } const entity = data.pop() as ParsedEntity<SchemaType>; if (!entity) { return; }examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (1)
52-52: Avoid suppressing TypeScript errors with@ts-expect-errorSuppressing TypeScript errors hides potential issues that could lead to runtime errors. Instead, address the underlying type mismatch for the
connectorsprop.Investigate the types of
cartridgeand...pato ensure they conform to the expectedConnectortype from@starknet-react/core. You may need to adjust the types or create a custom connector that matches the expected interface.examples/example-vite-kitchen-sink/src/typescript/models.gen.ts (1)
69-74: Theme type changes look good but need documentation.The transition from enum to string mapping provides more flexibility. Consider adding JSDoc comments to explain the theme structure and usage.
+/** + * Represents available themes in the application. + * Each theme is mapped to a string value that corresponds to its Cairo representation. + */ export type AvailableTheme = { Light: string; Dark: string; Dojo: string; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
.changeset/dull-jeans-reply.md(1 hunks)examples/example-vite-kitchen-sink/src/app/page.tsx(0 hunks)examples/example-vite-kitchen-sink/src/components/caller-counter.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/chat.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/global-counter.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/sidebar.tsx(1 hunks)examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx(2 hunks)examples/example-vite-kitchen-sink/src/components/theme-switch.tsx(6 hunks)examples/example-vite-kitchen-sink/src/dojo/models.ts(0 hunks)examples/example-vite-kitchen-sink/src/dojo/provider.tsx(1 hunks)examples/example-vite-kitchen-sink/src/main.tsx(1 hunks)examples/example-vite-kitchen-sink/src/typescript/contracts.gen.ts(4 hunks)examples/example-vite-kitchen-sink/src/typescript/models.gen.ts(2 hunks)examples/example-vite-react-sdk/src/App.tsx(4 hunks)examples/example-vite-react-sdk/src/historical-events.tsx(1 hunks)examples/example-vite-react-sdk/src/typescript/contracts.gen.ts(3 hunks)examples/example-vite-react-sdk/src/typescript/models.gen.ts(4 hunks)examples/example-vite-react-sql/src/components/playground/action.tsx(2 hunks)examples/example-vite-react-sql/src/hooks/useSystemCalls.ts(3 hunks)examples/example-vite-react-sql/src/typescript/contracts.gen.ts(3 hunks)examples/example-vite-react-sql/src/typescript/models.gen.ts(4 hunks)package.json(1 hunks)packages/core/package.json(1 hunks)worlds/dojo-starter(1 hunks)worlds/onchain-dash(1 hunks)
💤 Files with no reviewable changes (2)
- examples/example-vite-kitchen-sink/src/app/page.tsx
- examples/example-vite-kitchen-sink/src/dojo/models.ts
✅ Files skipped from review due to trivial changes (2)
- worlds/dojo-starter
- worlds/onchain-dash
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (24)
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (2)
4-6: LGTM! Clean implementation of theme enumeration.The new imports and theme-related type definitions are well-structured. The enum implementation with its corresponding value mappings provides good type safety and maintainability.
Also applies to: 25-29, 35-39
Line range hint
156-199: LGTM! Well-structured UI implementation.The dropdown menu implementation is clean and user-friendly, with proper keyboard shortcuts and loading state handling.
examples/example-vite-kitchen-sink/src/dojo/provider.tsx (1)
14-14: Great type safety improvement!Changing from
typeof setupWorldtoReturnType<typeof setupWorld>correctly types the actions property with the actual return type of the setup function rather than the function type itself.examples/example-vite-react-sdk/src/typescript/contracts.gen.ts (3)
1-2: LGTM! Improved type imports.The addition of
DojoCallandCairoCustomEnumimports enhances type safety.
17-17: Consistent type update in action handler.The parameter type update in
actions_movematches the calldata builder's signature.
5-7: Verify consistent usage of CairoCustomEnum across the codebase.The transition from
models.DirectiontoCairoCustomEnumand explicitDojoCallreturn types improves type safety. Let's verify this pattern is consistently applied across the codebase.Also applies to: 31-31
✅ Verification successful
Type safety and consistency verified across codebase
The transition to
CairoCustomEnumis consistently implemented:
- All
DirectionEnumtypes are properly defined asCairoCustomEnum- Type safety is maintained in all interfaces and method signatures
- No legacy Direction enum implementations found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining Direction enum usage rg -l "Direction.*enum" # Search for CairoCustomEnum usage patterns rg "CairoCustomEnum.*Direction|Direction.*CairoCustomEnum"Length of output: 559
Script:
#!/bin/bash # Check Direction enum definitions in models files rg -A 5 "export.*Direction.*enum" examples/example-vite-*/src/**/models.gen.ts # Find direct Direction enum usages rg "Direction(?!.*CairoCustomEnum)" examples/example-vite-*/src/**/*.ts # Check for type definitions and usage patterns ast-grep --pattern 'type $_ = Direction'Length of output: 2745
Script:
#!/bin/bash # Check Direction definitions and implementations rg -A 3 "Direction" examples/example-vite-*/src/**/models.gen.ts # Check usage in contracts rg -A 3 "Direction" examples/example-vite-*/src/**/contracts.gen.ts # Look for type declarations rg "type.*Direction" examples/example-vite-*/src/**/*.tsLength of output: 13370
examples/example-vite-react-sql/src/typescript/contracts.gen.ts (1)
1-2: LGTM! Consistent type updates across examples.The changes mirror those in the React SDK example, maintaining consistency in:
- Import of
DojoCallandCairoCustomEnum- Parameter type updates to
CairoCustomEnum- Explicit
DojoCallreturn typesAlso applies to: 5-7, 17-17, 31-31
examples/example-vite-react-sql/src/components/playground/action.tsx (2)
3-3: LGTM! Updated imports for new type system.Correctly removed the Direction enum import and added CairoCustomEnum.
Also applies to: 8-8
40-42: Verify CairoCustomEnum instantiation pattern.The direction buttons consistently use
new CairoCustomEnum({ Direction: "()" })pattern. Let's verify this is the correct instantiation pattern across the codebase.Also applies to: 48-50, 56-58, 64-66
✅ Verification successful
CairoCustomEnum instantiation pattern is correct and consistent
The pattern
new CairoCustomEnum({ Direction: "()" })is consistently used across the codebase for direction enums, and matches the general pattern used for other CairoCustomEnum instantiations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for CairoCustomEnum instantiation patterns rg "new CairoCustomEnum.*\{.*\}" # Look for any documentation or test files that might show the correct usage fd -e md -e test.ts -e test.tsx | xargs rg "CairoCustomEnum"Length of output: 1134
examples/example-vite-react-sdk/src/typescript/models.gen.ts (1)
15-15: LGTM: Consistent type updates across interfacesThe DirectionEnum type is consistently applied across all relevant interfaces, maintaining type safety and ensuring compatibility with the Cairo backend.
Also applies to: 20-20, 27-27, 34-34, 58-58, 63-63
examples/example-vite-react-sql/src/hooks/useSystemCalls.ts (1)
6-11: LGTM: Proper handling of CairoCustomEnum in move functionThe move function correctly implements the new type system with proper imports and CairoOption usage.
Also applies to: 61-61, 71-72
examples/example-vite-kitchen-sink/src/typescript/contracts.gen.ts (1)
1-2: LGTM: Proper implementation of contract actions with CairoCustomEnumThe changes correctly implement the new type system for contract actions, with proper imports and type handling.
Also applies to: 5-11, 15-24
examples/example-vite-react-sql/src/typescript/models.gen.ts (2)
3-8: LGTM! Imports are properly updated for the new type system.The imports from
starknetnow include all necessary types for the migration toCairoCustomEnum.
67-73: Verify type compatibility with Cairo contract.The transition from enum to
CairoCustomEnumlooks correct, but we should ensure the type definition matches the Cairo contract's enum definition.examples/example-vite-kitchen-sink/src/main.tsx (2)
15-15: Imports updated to useSchemaTypeThe import statement has been correctly updated to use
SchemaTypeandschemafrom"./typescript/models.gen". This aligns with the updated schema definitions.
Line range hint
18-31: Updatedinitfunction to useSchemaTypeChanging
init<OnchainDashSchemaType>toinit<SchemaType>ensures consistency with the new schema types. The initialization parameters appear correct.examples/example-vite-kitchen-sink/src/components/global-counter.tsx (2)
34-40: Query builder implementation looks good.The new query builder implementation provides better type safety and structure compared to the previous object-based query.
Line range hint
15-23: Transaction hook update looks good, but verify contract address.The switch to
useSendTransactionis appropriate, but ensure the contract address indojoConfig.manifest.contracts[0].addressis the correct one for the increment operation.✅ Verification successful
Contract address usage is correct and consistent
The
dojoConfig.manifest.contracts[0].addressis the correct contract address as it's consistently used across components for all counter-related operations, including the global counter increment.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check contract address configuration cat dojoConfig.ts | grep -A 5 "contracts:"Length of output: 88
Script:
#!/bin/bash # Find potential dojo config files fd -t f -e ts -e js dojo fd -t f -e ts -e js config # Search for contract address patterns in the codebase rg -l "contracts.*address" --type ts --type js # Search for manifest configurations rg -l "manifest.*contracts" --type ts --type jsLength of output: 4036
Script:
#!/bin/bash # Check kitchen-sink's dojoConfig.ts content cat examples/example-vite-kitchen-sink/dojoConfig.ts # Check contract usage in provider rg -A 5 "manifest.*contracts" examples/example-vite-kitchen-sink/src/components/starknet-provider.tsxLength of output: 723
examples/example-vite-kitchen-sink/src/typescript/models.gen.ts (1)
136-141: Verify theme initialization consistency.The theme initialization in both
ThemeandThemeValuelooks correct, but ensure the undefined values forDarkandDojoare intentional.Also applies to: 149-154
✅ Verification successful
The undefined values for Dark and Dojo themes are intentional and correct.
The initialization follows the standard Cairo enum pattern where only one variant can be active at a time, with Light being the default theme. This is consistent with other CairoCustomEnum implementations throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other theme initializations in the codebase rg -A 2 "new CairoCustomEnum\(\{[^}]*Light:" .Length of output: 48
Script:
#!/bin/bash # Search for theme-related code and enums with broader patterns echo "=== Searching for theme-related enums ===" rg -i "enum.*theme|theme.*enum" -A 3 echo -e "\n=== Searching for Dark/Dojo theme references ===" rg -i "Dark.*theme|Dojo.*theme" -A 2 echo -e "\n=== Searching for CairoCustomEnum usage ===" rg "CairoCustomEnum" -A 3Length of output: 27326
examples/example-vite-kitchen-sink/src/components/chat.tsx (1)
77-82:⚠️ Potential issueReplace type assertion with proper type guard.
The
@ts-expect-errorcomment masks a potential runtime issue. Consider implementing a proper type guard.-// @ts-expect-error a & b are not undefined as they are filtered out with `filer(Boolean)` -sort((a: Message, b: Message): number => +sort((a, b): number => { + if (!a || !b) return 0; + return ( parseInt(a.timestamp.toString(), 16) < parseInt(b.timestamp.toString(), 16) ? -1 : 1 + ); +})Likely invalid or redundant comment.
.changeset/dull-jeans-reply.md (1)
1-13: Verify patch version compatibility.All packages are marked for patch version updates. Given the changes include new features (array conversion, new parameters), consider if any of these changes warrant a minor version bump instead of a patch.
Run this script to analyze the changes:
✅ Verification successful
Patch version updates are appropriate.
The changes consist of backward-compatible bug fixes and minor enhancements that don't modify the public API contract. This aligns with semantic versioning guidelines for patch releases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for breaking changes or significant feature additions # Look for new exports, parameters, or type changes rg -p "export" --type ts rg -p "interface" --type ts rg -p "type" --type ts # Check for removed or renamed functions/types git diff origin/main | grep -E "^[-+].*export"Length of output: 66697
package.json (1)
14-23: LGTM! Development dependencies updated appropriately.The development dependency updates follow semantic versioning and maintain backward compatibility.
packages/core/package.json (2)
30-30: Consider implications of moving starknet to direct dependencies.Moving
starknetfrompeerDependenciestodependenciescould lead to version conflicts if multiple packages in the dependency tree bundle their own version of starknet. Consider keeping it as a peer dependency if it's a primary framework dependency that should be managed by the consuming application.
29-29: Update zod to latest stable version.The current zod version (3.23.8) is outdated. Consider updating to the latest stable version for security fixes and improvements.
Run this script to check the latest version and changelog:
| {/* @ts-expect-error type is ok here */} | ||
| <div>Direction: {direction}</div> |
There was a problem hiding this comment.
Fix type mismatch instead of suppressing TypeScript error
Rather than using @ts-expect-error, consider properly typing the direction value. This type error is likely due to the migration to CairoCustomEnum.
-{/* @ts-expect-error type is ok here */}
-<div>Direction: {direction}</div>
+<div>Direction: {direction?.activeVariant()}</div>📝 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.
| {/* @ts-expect-error type is ok here */} | |
| <div>Direction: {direction}</div> | |
| <div>Direction: {direction?.activeVariant()}</div> |
| {/* @ts-expect-error we have an option here so type is ok */} | ||
| {lastDirection} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-expect-error by adding proper type assertions.
Similar to the previous occurrence, the TypeScript error should be handled properly instead of being suppressed.
-/* @ts-expect-error we have an option here so type is ok */
+const typedLastDirection = lastDirection as DirectionEnum | "N/A";
{lastDirection}Committable suggestion skipped: line range outside the PR's diff.
| ].map(({ direction, label, col }, idx) => ( | ||
| <button | ||
| className={`${col} h-12 w-12 bg-gray-600 rounded-full shadow-md active:shadow-inner active:bg-gray-500 focus:outline-none text-2xl font-bold text-gray-200`} | ||
| key={direction} | ||
| key={idx} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using direction value as key instead of index.
Using array indices as React keys is not recommended as it can lead to rendering issues if the array items are reordered.
-].map(({ direction, label, col }, idx) => (
+].map(({ direction, label, col }) => (
<button
className={`${col} h-12 w-12 bg-gray-600 rounded-full shadow-md active:shadow-inner active:bg-gray-500 focus:outline-none text-2xl font-bold text-gray-200`}
- key={idx}
+ key={Object.keys(direction)[0]}
onClick={async () => {Committable suggestion skipped: line range outside the PR's diff.
| {/* @ts-expect-error we have an option here so type is ok */} | ||
| {moves && moves.last_direction.isSome() | ||
| ? moves.last_direction.unwrap() | ||
| : ""} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove @ts-expect-error by adding proper type assertions.
Instead of suppressing TypeScript errors, consider adding proper type assertions or handling the Option type correctly.
-/* @ts-expect-error we have an option here so type is ok */
+const lastDirection = moves?.last_direction as CairoOption<DirectionEnum>;
{moves && moves.last_direction.isSome()
? moves.last_direction.unwrap()
: ""}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 164-164: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
| query: new QueryBuilder<SchemaType>() | ||
| .namespace("onchain_dash", (n) => | ||
| n.entity("GlobalCounter", (e) => | ||
| e.eq("global_counter_key", 9999999) | ||
| ) | ||
| ) | ||
| .build(), |
There was a problem hiding this comment.
Similar type assertion issue in subscription handler.
The same type assertion issue exists in the subscription handler.
-const entity = data.pop() as ParsedEntity<SchemaType>;
+const entity = data.pop();
if (!entity) {
return;
}
+const typedEntity = entity as ParsedEntity<SchemaType>;Also applies to: 72-72
| callback: ({ data, error }) => {}, | ||
| }); | ||
|
|
||
| const counter = entity.pop(); | ||
| const counter = entity.pop() as ParsedEntity<SchemaType>; |
There was a problem hiding this comment.
Type assertion needs null check.
The type assertion might fail if entity.pop() returns undefined.
-const counter = entity.pop() as ParsedEntity<SchemaType>;
+const counter = entity.pop();
if (!counter) {
return 0;
}
+const typedCounter = counter as ParsedEntity<SchemaType>;📝 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 counter = entity.pop() as ParsedEntity<SchemaType>; | |
| const counter = entity.pop(); | |
| if (!counter) { | |
| return 0; | |
| } | |
| const typedCounter = counter as ParsedEntity<SchemaType>; |
| setMessages((prevMessages) => [ | ||
| ...prevMessages, | ||
| msg as MessageItem, | ||
| ]); |
There was a problem hiding this comment.
Unsafe type assertion in state update.
The type assertion to MessageItem could lead to runtime errors if the message structure doesn't match.
-msg as MessageItem,
+{
+ content: msg.content,
+ identity: msg.identity,
+ timestamp: parseInt(msg.timestamp.toString(), 16)
+} as MessageItem,Committable suggestion skipped: line range outside the PR's diff.
MartianGreed
force-pushed
the
feat/update-dojo-version
branch
from
69b73b4 to
84dd776
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (1)
Fix type system issues by implementing proper type mapping
The type errors occur because the code attempts to use string values from the blockchain to index a numeric enum. Instead of suppressing these errors, implement one of these solutions:
- Add a type-safe mapping:
const themeStringToEnum: Record<string, AvailableTheme> = { "Light": AvailableTheme.Light, "Dark": AvailableTheme.Dark, "Dojo": AvailableTheme.Dojo };
- Or change
AvailableThemeto a string enum:enum AvailableTheme { Light = "Light", Dark = "Dark", Dojo = "Dojo" }Locations needing fixes:
- Line 87-88: Use mapping instead of direct indexing
- Lines 124-127: Use proper type assertion or mapping
- Lines 131-132: Use proper type assertion or mapping
🔗 Analysis chain
Line range hint
87-88: Address type system issues instead of suppressing them.Multiple
@ts-expect-errorcomments suggest underlying type system issues that should be addressed rather than suppressed. Consider:
- Adding proper type assertions
- Fixing the type definitions
- Using type guards
Let's verify the type issues:
Also applies to: 124-127, 131-132
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for type definitions that might need updating rg -A 5 "interface.*Theme|type.*Theme"Length of output: 8604
Script:
#!/bin/bash # Get the full context of the theme-switch component cat "examples/example-vite-kitchen-sink/src/components/theme-switch.tsx"Length of output: 7369
🧹 Nitpick comments (8)
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (2)
70-74: Document or extract the magic number.The hard-coded theme key
9999999should be documented or extracted into a named constant to explain its significance.+// Theme key for global theme settings +const GLOBAL_THEME_KEY = 9999999; + const entity = await db.getEntities({ query: new QueryBuilder<SchemaType>() .namespace("onchain_dash", (n) => - n.entity("Theme", (e) => e.eq("theme_key", 9999999)) + n.entity("Theme", (e) => e.eq("theme_key", GLOBAL_THEME_KEY)) ) .build(),
Line range hint
109-133: Improve error handling and simplify nested optional chaining.The current implementation has several areas for improvement:
- Error handling in subscription callback throws directly
- Complex nested optional chaining could be simplified
Consider this refactoring:
callback: ({ data, error }) => { + if (error) { + console.error('Subscription error:', error); + setIsLoading(false); + return AvailableTheme.Light; + } if (data) { const entity = data.pop() as ParsedEntity<SchemaType>; - if (!entity) { - return AvailableTheme.Light; - } - if ( - entity.models.onchain_dash?.Theme?.value === - undefined - ) { + const themeValue = entity?.models?.onchain_dash?.Theme?.value; + if (!themeValue) { return AvailableTheme.Light; } - const theme = - entity.models?.onchain_dash?.Theme?.value.unwrap(); + const theme = themeValue.unwrap(); const at = AvailableTheme[theme]; setTheme({ current: AvailableThemeClassMap[at], next: AvailableThemeClassMap[at], }); setIsLoading(false); - return AvailableTheme[ - entity.models.onchain_dash.Theme.value.unwrap() - ]; + return AvailableTheme[theme]; } - if (error) { - throw error; - } + return AvailableTheme.Light; },examples/example-vite-react-sql/src/hooks/useSystemCalls.ts (2)
68-72: Improve the clarity and professionalism of the@ts-expect-errorcomment.The current comment contains a typo and informal language. Consider rephrasing it for clarity and to maintain a professional tone.
Suggested change:
- // @ts-expect-error this is literrally the condition above calm down typescript + // @ts-expect-error: Condition above ensures this property exists
83-85: Clarify the@ts-expect-errorcomment for better understanding.The comment could be more descriptive to explain why the TypeScript error is expected and can be safely ignored.
Suggested change:
- // @ts-expect-error inner enum is not hydrated there + // @ts-expect-error: The inner enum may not be fully hydrated hereexamples/example-vite-kitchen-sink/src/typescript/contracts.gen.ts (1)
Line range hint
54-86: Consider refactoring to reduce code duplication in action functions.The action functions share similar structures for building calldata and executing actions. Refactoring to abstract common logic can enhance maintainability and reduce redundancy.
Suggested approach:
Create a generic function for executing actions:
async function executeAction( snAccount: Account | AccountInterface, dojoCall: DojoCall, domain: string = "onchain_dash" ) { try { return await provider.execute(snAccount, dojoCall, domain); } catch (error) { console.error(error); throw error; } }Update action functions to use the generic executor:
const actions_changeTheme = async ( snAccount: Account | AccountInterface, value: CairoCustomEnum ) => { return executeAction(snAccount, build_actions_changeTheme_calldata(value)); }; const actions_incrementCallerCounter = async ( snAccount: Account | AccountInterface ) => { return executeAction(snAccount, build_actions_incrementCallerCounter_calldata()); }; const actions_incrementGlobalCounter = async ( snAccount: Account | AccountInterface ) => { return executeAction(snAccount, build_actions_incrementGlobalCounter_calldata()); };Ensure the
actionsobject in the return statement reflects these changes.return { actions: { changeTheme: actions_changeTheme, buildChangeThemeCalldata: build_actions_changeTheme_calldata, incrementCallerCounter: actions_incrementCallerCounter, buildIncrementCallerCounterCalldata: build_actions_incrementCallerCounter_calldata, incrementGlobalCounter: actions_incrementGlobalCounter, buildIncrementGlobalCounterCalldata: build_actions_incrementGlobalCounter_calldata, }, };This refactor reduces code duplication and centralizes error handling, improving the code's readability and maintainability.
examples/example-vite-react-sdk/src/typescript/models.gen.ts (1)
67-73: Well-structured migration from enum to CairoCustomEnumThe transition to
CairoCustomEnumimproves type safety and Cairo compatibility. The initialization pattern is consistent across all instances.Consider creating a utility function to initialize direction enums to reduce code duplication:
function createDirectionEnum(defaultDirection: keyof Direction = 'Left'): CairoCustomEnum { return new CairoCustomEnum({ Left: defaultDirection === 'Left' ? '' : undefined, Right: defaultDirection === 'Right' ? '' : undefined, Up: defaultDirection === 'Up' ? '' : undefined, Down: defaultDirection === 'Down' ? '' : undefined, }); }Also applies to: 93-100, 104-111, 143-148, 152-157
examples/example-vite-kitchen-sink/src/typescript/models.gen.ts (1)
69-74: Simplify nested CairoCustomEnum initializationThe nested CairoCustomEnum structure for themes could be simplified for better maintainability.
Consider creating a utility function for theme initialization:
function createThemeEnum(defaultTheme: keyof AvailableTheme = 'Light'): CairoCustomEnum { return new CairoCustomEnum({ Predefined: new CairoCustomEnum({ Light: defaultTheme === 'Light' ? '' : undefined, Dark: defaultTheme === 'Dark' ? '' : undefined, Dojo: defaultTheme === 'Dojo' ? '' : undefined, }), Custom: undefined, }); }Also applies to: 136-141, 149-154
examples/example-vite-react-sdk/src/App.tsx (1)
175-177: Extract direction definitions to improve maintainability.Consider extracting the direction definitions to a constant or utility function to make the code more maintainable and reduce repetition.
+const DIRECTIONS = { + UP: new CairoCustomEnum({ Up: "()" }), + LEFT: new CairoCustomEnum({ Left: "()" }), + RIGHT: new CairoCustomEnum({ Right: "()" }), + DOWN: new CairoCustomEnum({ Down: "()" }), +} as const; [ { - direction: new CairoCustomEnum({ - Up: "()", - }), + direction: DIRECTIONS.UP, label: "↑", col: "col-start-2", }, { - direction: new CairoCustomEnum({ - Left: "()", - }), + direction: DIRECTIONS.LEFT, label: "←", col: "col-start-1", }, // ... similar changes for Right and Down ]Also applies to: 182-184, 189-191, 196-198
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (26)
.changeset/dull-jeans-reply.md(1 hunks).github/workflows/ci.yaml(1 hunks)examples/example-vite-kitchen-sink/src/app/page.tsx(0 hunks)examples/example-vite-kitchen-sink/src/components/caller-counter.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/chat.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/global-counter.tsx(3 hunks)examples/example-vite-kitchen-sink/src/components/sidebar.tsx(1 hunks)examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx(2 hunks)examples/example-vite-kitchen-sink/src/components/theme-switch.tsx(6 hunks)examples/example-vite-kitchen-sink/src/dojo/models.ts(0 hunks)examples/example-vite-kitchen-sink/src/dojo/provider.tsx(1 hunks)examples/example-vite-kitchen-sink/src/main.tsx(1 hunks)examples/example-vite-kitchen-sink/src/typescript/contracts.gen.ts(4 hunks)examples/example-vite-kitchen-sink/src/typescript/models.gen.ts(2 hunks)examples/example-vite-react-sdk/src/App.tsx(4 hunks)examples/example-vite-react-sdk/src/historical-events.tsx(1 hunks)examples/example-vite-react-sdk/src/typescript/contracts.gen.ts(3 hunks)examples/example-vite-react-sdk/src/typescript/models.gen.ts(4 hunks)examples/example-vite-react-sql/src/components/playground/action.tsx(2 hunks)examples/example-vite-react-sql/src/hooks/useSystemCalls.ts(3 hunks)examples/example-vite-react-sql/src/typescript/contracts.gen.ts(3 hunks)examples/example-vite-react-sql/src/typescript/models.gen.ts(4 hunks)package.json(1 hunks)packages/core/package.json(1 hunks)worlds/dojo-starter(1 hunks)worlds/onchain-dash(1 hunks)
💤 Files with no reviewable changes (2)
- examples/example-vite-kitchen-sink/src/app/page.tsx
- examples/example-vite-kitchen-sink/src/dojo/models.ts
🚧 Files skipped from review as they are similar to previous changes (13)
- examples/example-vite-react-sdk/src/historical-events.tsx
- examples/example-vite-kitchen-sink/src/components/sidebar.tsx
- examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx
- worlds/dojo-starter
- examples/example-vite-kitchen-sink/src/dojo/provider.tsx
- .changeset/dull-jeans-reply.md
- examples/example-vite-react-sdk/src/typescript/contracts.gen.ts
- worlds/onchain-dash
- examples/example-vite-react-sql/src/components/playground/action.tsx
- examples/example-vite-react-sql/src/typescript/contracts.gen.ts
- packages/core/package.json
- examples/example-vite-kitchen-sink/src/components/chat.tsx
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (19)
.github/workflows/ci.yaml (1)
23-23: Verify dojoup version and consider using environment variables for paths.The version update looks reasonable, but let's verify it's the latest stable version.
Consider refactoring the hardcoded dojo binary path to use an environment variable:
- - run: /home/runner/.config/.dojo/bin/dojoup -v v1.0.10 + - name: Set Dojo path + run: echo "DOJO_PATH=/home/runner/.config/.dojo/bin" >> $GITHUB_ENV + - run: ${{ env.DOJO_PATH }}/dojoup -v v1.0.10✅ Verification successful
✓ Dojoup version v1.0.10 verified as latest stable release
The version update is correct and matches the latest stable release. No issues were found related to this version. The path refactoring suggestion remains valid but is not critical for functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check latest dojoup version and any related issues # Fetch latest release version from GitHub API gh api repos/dojoengine/dojo/releases/latest --jq .tag_name # Check for any issues mentioning v1.0.10 gh api search/issues -X GET -f q="repo:dojoengine/dojo is:issue v1.0.10" --jq '.items[] | {title: .title, state: .state, url: .html_url}'Length of output: 209
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (1)
4-6: LGTM! Well-structured theme management implementation.The new imports and enum-based theme management provide good type safety and maintainability. The mapping objects effectively bridge between enum values and their string representations.
Also applies to: 25-39
examples/example-vite-react-sql/src/hooks/useSystemCalls.ts (2)
6-11: Imports updated to support new types.The addition of
CairoCustomEnum,CairoOption, andCairoOptionVariantfrom thestarknetlibrary aligns with the updatedmovefunction parameters and enhances type handling.
61-61: Updated function signature formoveto useCairoCustomEnum.Changing the
directionparameter toCairoCustomEnumreflects the transition to more flexible enum types and ensures consistency across the codebase.examples/example-vite-kitchen-sink/src/main.tsx (2)
15-15: Updated imports to reflect new schema definitions.Importing
SchemaTypefrom"./typescript/models.gen"replaces the previousOnchainDashSchemaType, aligning with the updated schema generation process.
18-18: Initialize database with the newSchemaType.Using
SchemaTypein theinitfunction ensures that the database client is initialized with the correct schema, maintaining type safety and consistency.examples/example-vite-kitchen-sink/src/typescript/contracts.gen.ts (4)
1-2: Include necessary imports for updated contract interactions.The addition of
DojoProvider,DojoCall,Account,AccountInterface, andCairoCustomEnumimports is appropriate for the updated action functions and calldata builders.
5-11: Definebuild_actions_changeTheme_calldatawith correct parameters and types.The
build_actions_changeTheme_calldatafunction correctly constructs the calldata for thechange_themeentry point usingCairoCustomEnum.
Line range hint
15-28: Implementactions_changeThemefunction appropriately.The
actions_changeThemefunction correctly executes thechange_themeaction using the provided calldata and handles errors effectively.
Line range hint
31-39: Ensure consistency in action calldata builders and executors.The
build_actions_incrementCallerCounter_calldataandactions_incrementCallerCounterfunctions are well-defined and consistent with the new structure.examples/example-vite-kitchen-sink/src/components/global-counter.tsx (3)
72-72: Same type assertion issue as aboveThe unsafe type assertion pattern is repeated here.
44-44:⚠️ Potential issueFix unsafe type assertion
The type assertion
as ParsedEntity<SchemaType>without a prior null check could lead to runtime errors.Apply this safer pattern:
-const counter = entity.pop() as ParsedEntity<SchemaType>; +const counter = entity.pop(); if (!counter) { return 0; } +const typedCounter = counter as ParsedEntity<SchemaType>;Likely invalid or redundant comment.
Line range hint
15-22: LGTM: Hook migration from useContractWrite to useSendTransactionThe migration to
useSendTransactionaligns with the latest Starknet React practices.Run this script to verify consistent hook usage across components:
✅ Verification successful
✅ Hook Migration Verification: Complete and Consistent
The migration from
useContractWritetouseSendTransactionhas been completed across the codebase, with consistent implementation in both components that require transaction functionality.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining useContractWrite usage that needs migration rg "useContractWrite" --type typescriptLength of output: 74
Script:
#!/bin/bash # Search for both hooks in TypeScript files echo "Checking for useContractWrite usage:" rg "useContractWrite" -g "*.ts" -g "*.tsx" -C 2 echo -e "\nChecking for useSendTransaction usage:" rg "useSendTransaction" -g "*.ts" -g "*.tsx" -C 2Length of output: 2601
examples/example-vite-kitchen-sink/src/components/caller-counter.tsx (1)
Line range hint
17-24: Changes consistent with global-counter.tsxThe modifications follow the same pattern as global-counter.tsx, including hook migration and query builder usage.
Also applies to: 34-45, 63-73
examples/example-vite-react-sql/src/typescript/models.gen.ts (1)
67-73: Changes mirror the SDK variantThe modifications are identical to those in the SDK variant of models.gen.ts.
examples/example-vite-react-sdk/src/App.tsx (4)
9-9: LGTM! Import changes align with the CairoCustomEnum transition.The import changes correctly reflect the transition from the Direction enum to CairoCustomEnum.
Also applies to: 11-11
163-164: Remove @ts-expect-error by adding proper type assertions.Instead of suppressing TypeScript errors, consider adding proper type assertions or handling the Option type correctly.
-/* @ts-expect-error we have an option here so type is ok */ +const typedDirection = moves?.last_direction as CairoOption<CairoCustomEnum>; {moves && moves.last_direction.isSome() ? moves.last_direction.unwrap() : ""}🧰 Tools
🪛 Biome (1.9.4)
[error] 164-164: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
283-284: Remove @ts-expect-error by adding proper type assertions.Similar to the previous occurrence, the TypeScript error should be handled properly instead of being suppressed.
-/* @ts-expect-error we have an option here so type is ok */ +const typedLastDirection = lastDirection as CairoCustomEnum | "N/A"; {lastDirection}
202-205: Consider using direction value as key instead of index.Using array indices as React keys is not recommended as it can lead to rendering issues if the array items are reordered.
-].map(({ direction, label, col }, idx) => ( +].map(({ direction, label, col }) => ( <button className={`${col} h-12 w-12 bg-gray-600 rounded-full shadow-md active:shadow-inner active:bg-gray-500 focus:outline-none text-2xl font-bold text-gray-200`} - key={idx} + key={Object.keys(direction)[0]} onClick={async () => {
| account!, | ||
| new CairoCustomEnum({ | ||
| Predefined: new CairoCustomEnum({ | ||
| [AvailableThemeEnumValues[theme]]: "()", | ||
| }), | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety check for undefined account.
The non-null assertion (account!) could lead to runtime errors if the account is undefined. Consider adding a guard clause.
const handleChangeTheme = useCallback(
async (theme: AvailableTheme) => {
+ if (!account) {
+ console.error('Account not connected');
+ return;
+ }
setIsLoading(true);
actions?.actions.changeTheme(
- account!,
+ account,
new CairoCustomEnum({
Predefined: new CairoCustomEnum({
[AvailableThemeEnumValues[theme]]: "()",
}),
})
);
},
[actions, account]
);📝 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.
| account!, | |
| new CairoCustomEnum({ | |
| Predefined: new CairoCustomEnum({ | |
| [AvailableThemeEnumValues[theme]]: "()", | |
| }), | |
| }) | |
| const handleChangeTheme = useCallback( | |
| async (theme: AvailableTheme) => { | |
| if (!account) { | |
| console.error('Account not connected'); | |
| return; | |
| } | |
| setIsLoading(true); | |
| actions?.actions.changeTheme( | |
| account, | |
| new CairoCustomEnum({ | |
| Predefined: new CairoCustomEnum({ | |
| [AvailableThemeEnumValues[theme]]: "()", | |
| }), | |
| }) | |
| ); | |
| }, | |
| [actions, account] | |
| ); |
Summary by CodeRabbit
Package Updates
Type System Improvements
CairoCustomEnumtypes.Framework Modifications
useContractWritetouseSendTransaction.Dependency Management
starknetfrom peer dependencies to direct dependencies.package.json.