-
Notifications
You must be signed in to change notification settings - Fork 52
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: subscription clauses #235
Conversation
WalkthroughThe recent changes enhance functionality and clarity across multiple packages and examples. Key updates include the introduction of new functions and hooks for improved entity synchronization, parameter renaming for better clarity, updated metadata prefixes, the addition of a namespace parameter, and version increments. These modifications collectively aim to boost maintainability and performance, ensuring a more robust system. Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- examples/dojo-starter (1 hunks)
- examples/react/react-app/src/dojo/generated/setup.ts (2 hunks)
- packages/react/src/index.ts (1 hunks)
- packages/react/src/useFindEntity.ts (1 hunks)
- packages/state/src/findAndSyncEntities.ts (1 hunks)
- packages/state/src/index.ts (1 hunks)
- packages/state/src/recs/index.ts (4 hunks)
- packages/torii-wasm/build.sh (1 hunks)
Files skipped from review due to trivial changes (6)
- examples/dojo-starter
- packages/react/src/index.ts
- packages/react/src/useFindEntity.ts
- packages/state/src/findAndSyncEntities.ts
- packages/state/src/index.ts
- packages/torii-wasm/build.sh
Additional comments not posted (8)
examples/react/react-app/src/dojo/generated/setup.ts (2)
Line range hint
16-20
:
Verify the correctness of thetorii.createClient
call.The
torii.createClient
call has been updated to use an object. Ensure that the object keys (rpcUrl
,toriiUrl
,relayUrl
,worldAddress
) match the expected parameters for thecreateClient
function.
30-36
: Verify the correctness and structure of thegetSyncEntities
call.The
getSyncEntities
call has been updated to use an object with keys, models, and pattern matching. Ensure that the structure matches the expected input for thegetSyncEntities
function.packages/state/src/recs/index.ts (6)
Line range hint
23-27
:
Verify the correctness of theentities
parameter type.The
entities
parameter type has been updated toEntityKeysClause | undefined
. Ensure that the function correctly handles both types.
Line range hint
117-121
:
Verify the correctness of theentities
parameter type.The
entities
parameter type has been updated toEntityKeysClause | undefined
. Ensure that the function correctly handles both types.
Line range hint
23-27
:
Verify the logic of thegetSyncEntities
function.The
getSyncEntities
function now updates and watches entities based on input data. Ensure that the function correctly handles the newentities
parameter and performs the intended operations.
Line range hint
117-121
:
Verify the logic of thesyncEntities
function.The
syncEntities
function now updates entities based on input data. Ensure that the function correctly handles the newentities
parameter and performs the intended operations.
64-105
: Verify the correctness of thegetEntitiesQuery
function.The
getEntitiesQuery
function has been added. Ensure that the function correctly implements the logic to fetch entities and components.
64-105
: Verify the logic and maintainability of thegetEntitiesQuery
function.The
getEntitiesQuery
function fetches entities and components based on input data. Ensure that the function is logically sound and maintainable.Verification successful
The
getEntitiesQuery
function is logically sound and maintainable.The function correctly constructs the
clause
object, fetches entities using the client, handles the fetched entities, and implements pagination logic appropriately.
- The construction of the
clause
object based on the presence ofentities
is correct.- The fetching of entities using the client and handling with
setEntities
is implemented properly.- The pagination logic is correctly checking the length of fetched entities and updating the cursor.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the logic and maintainability of the `getEntitiesQuery` function. # Test: Search for the function implementation and analyze its logic and maintainability. rg --type python 'def getEntitiesQuery'Length of output: 72
Script:
#!/bin/bash # Description: Verify the logic and maintainability of the `getEntitiesQuery` function in TypeScript files. # Search for the function implementation in TypeScript files rg --type typescript 'export const getEntitiesQuery'Length of output: 89
Script:
#!/bin/bash # Description: Verify the logic and maintainability of the `getEntitiesQuery` function in TypeScript files. # Find all TypeScript files and search for the function implementation within those files fd --extension ts --exec rg 'export const getEntitiesQuery' {}Length of output: 124
Script:
#!/bin/bash # Description: Extract the full implementation of the `getEntitiesQuery` function for detailed analysis. # Find the file containing the `getEntitiesQuery` function and extract its full implementation fd --extension ts --exec rg --context 20 'export const getEntitiesQuery' {}Length of output: 1491
Script:
#!/bin/bash # Description: Extract the full implementation of the `getEntitiesQuery` function for detailed analysis. # Find the file containing the `getEntitiesQuery` function and extract its full implementation with increased context lines fd --extension ts --exec rg --context 50 'export const getEntitiesQuery' {}Length of output: 3464
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- examples/react/react-app/src/App.tsx (1 hunks)
- examples/react/react-app/src/dojo/generated/setup.ts (3 hunks)
- packages/react/src/index.ts (1 hunks)
- packages/react/src/useQuerySync.ts (1 hunks)
- packages/state/src/recs/index.ts (4 hunks)
Files skipped from review as they are similar to previous changes (1)
- examples/react/react-app/src/dojo/generated/setup.ts
Additional context used
Biome
packages/react/src/useQuerySync.ts
[error] 29-29: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/state/src/recs/index.ts
[error] 139-139: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (4)
packages/react/src/index.ts (1)
7-7
: Verify the impact of commenting outuseFindEntity
.Ensure that the removal of this export does not break any dependencies or functionality in the codebase.
Verification successful
No impact from commenting out
useFindEntity
.The removal of the
useFindEntity
export does not affect any other parts of the codebase as there are no active references to it.
packages/react/src/useFindEntity.ts
: Reference is commented out.packages/react/src/index.ts
: Export statement is commented out.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the removal of `useFindEntity` export impacts other parts of the codebase. # Test: Search for the usage of `useFindEntity`. Expect: No unresolved references. rg --type typescript $'useFindEntity'Length of output: 71
Script:
#!/bin/bash # Description: Verify if the removal of `useFindEntity` export impacts other parts of the codebase. # Test: Search for the usage of `useFindEntity`. Expect: No unresolved references. rg 'useFindEntity'Length of output: 152
packages/react/src/useQuerySync.ts (1)
1-50
: LGTM!The custom hook
useQuerySync
is well-implemented for managing entity synchronization.Tools
Biome
[error] 29-29: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/state/src/recs/index.ts (1)
Line range hint
9-105
:
LGTM!The changes enhance the functionality by providing more granular control over entity synchronization.
examples/react/react-app/src/App.tsx (1)
22-27
: Verify the functionality ofuseQuerySync
.Ensure that the new hook works as intended in the application.
} catch (error) { | ||
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant catch clause.
The catch clause that only rethrows the original error is redundant and can be removed.
- } catch (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.
} catch (error) { | |
throw error; | |
} |
Tools
Biome
[error] 29-29: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
components: Component<S, Metadata, undefined>[] | ||
) => { | ||
console.log(entities); | ||
console.log(components); | ||
for (let key in entities) { | ||
if (entities.hasOwnProperty(key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Object.hasOwn()
instead of Object.hasOwnProperty()
.
It's recommended to use Object.hasOwn()
for better readability and performance.
- if (entities.hasOwnProperty(key)) {
+ if (Object.hasOwn(entities, key)) {
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.
if (entities.hasOwnProperty(key)) { | |
if (Object.hasOwn(entities, key)) { |
Tools
Biome
[error] 139-139: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- examples/react/react-app/src/App.tsx (2 hunks)
- packages/state/src/recs/index.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- examples/react/react-app/src/App.tsx
Additional context used
Biome
packages/state/src/recs/index.ts
[error] 139-139: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (3)
packages/state/src/recs/index.ts (3)
Line range hint
33-54
: No changes detected.This function remains unchanged and works as expected.
57-105
: New function looks good. Verify function usage.The new function
getEntitiesQuery
is correctly implemented. Ensure it is used correctly in the codebase.
111-124
: Changes look good. Verify function usage.The addition of
entityKeyClause
is correctly implemented. Ensure all function calls tosyncEntities
are updated to match the new signature.Verification successful
Changes look good. Verify function usage.
The addition of
entityKeyClause
is correctly implemented. All function calls tosyncEntities
have been updated to match the new signature.
packages/state/src/__tests__/recs.test.ts
(lines 59)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `syncEntities` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript $'syncEntities'Length of output: 70
Script:
#!/bin/bash # Description: Verify all function calls to `syncEntities` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts 'syncEntities'Length of output: 577
Script:
#!/bin/bash # Description: Check the specific lines in the test files to verify the signature usage of `syncEntities`. # Test: Extract lines with `syncEntities` in the test file to ensure they match the new signature. rg 'syncEntities' packages/state/src/__tests__/recs.test.ts -nLength of output: 267
Script:
#!/bin/bash # Description: Extract and review the relevant lines from the test file to ensure they conform to the new function signature. # Test: Print lines around the occurrences of `syncEntities` to verify the function call signature. sed -n '3,7p;38,42p;53,57p;57,61p' packages/state/src/__tests__/recs.test.tsLength of output: 745
packages/state/src/recs/index.ts
Outdated
entities: any, | ||
components: Component<S, Metadata, undefined>[] | ||
) => { | ||
console.log(entities); | ||
console.log(components); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use Object.hasOwn()
instead of hasOwnProperty
.
It's recommended to use Object.hasOwn()
for better readability and performance.
- for (let key in entities) {
- if (entities.hasOwnProperty(key)) {
- for (let componentName in entities[key]) {
- if (entities[key].hasOwnProperty(componentName)) {
+ for (const key in entities) {
+ if (Object.hasOwn(entities, key)) {
+ for (const componentName in entities[key]) {
+ if (Object.hasOwn(entities[key], componentName)) {
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.
entities: any, | |
components: Component<S, Metadata, undefined>[] | |
) => { | |
console.log(entities); | |
console.log(components); | |
entities: any, | |
components: Component<S, Metadata, undefined>[] | |
) => { | |
console.log(entities); | |
console.log(components); | |
for (const key in entities) { | |
if (Object.hasOwn(entities, key)) { | |
for (const componentName in entities[key]) { | |
if (Object.hasOwn(entities[key], componentName)) { |
packages/state/src/recs/index.ts
Outdated
* @param entityKeyClause - An array of entities to synchronize. | ||
* @param limit - The maximum number of entities to fetch per request (default: 100). | ||
* @returns A promise that resolves when synchronization is complete. | ||
*/ | ||
export const getSyncEntities = async <S extends Schema>( | ||
client: Client, | ||
components: Component<S, Metadata, undefined>[], | ||
entities: any[], | ||
entityKeyClause: EntityKeysClause | undefined, | ||
limit: number = 100 | ||
) => { | ||
await getEntities(client, components, limit); | ||
return await syncEntities(client, components, entities); | ||
return await syncEntities(client, components, entityKeyClause); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
Update function calls to match the new signature
Several calls to getSyncEntities
are missing the entityKeyClause
parameter. Please update the following instances to include the entityKeyClause
parameter:
packages/state/src/__tests__/recs.test.ts
: Line 23packages/react/src/useQuerySync.ts
: Line 47examples/react/react-pwa-app/src/dojo/generated/setup.ts
: Line 20examples/react/react-threejs/src/dojo/generated/setup.ts
: Line 20examples/react/starknet-react-app/src/dojo/generated/setup.ts
: Line 20examples/react/react-phaser-example/src/dojo/generated/setup.ts
: Line 20examples/vue/vue-app/src/dojo/generated/setup.ts
: Line 20examples/react/react-app/src/dojo/generated/setup.ts
: Line 20
Analysis chain
Changes look good. Verify function usage.
The addition of entityKeyClause
is correctly implemented. Ensure all function calls to getSyncEntities
are updated to match the new signature.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify all function calls to `getSyncEntities` match the new signature.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type typescript $'getSyncEntities'
Length of output: 73
Script:
#!/bin/bash
# Description: Verify all function calls to `getSyncEntities` match the new signature.
# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg 'getSyncEntities'
Length of output: 1716
Script:
#!/bin/bash
# Description: Verify all function calls to `getSyncEntities` match the new signature with the `entityKeyClause` parameter.
# Extract lines containing `getSyncEntities` calls
rg 'getSyncEntities' -A 2 -B 2
Length of output: 6120
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- examples/react/react-app/src/dojo/createSystemCalls.ts (2 hunks)
- examples/react/react-app/src/dojo/generated/setup.ts (4 hunks)
- packages/react/src/useQuerySync.ts (1 hunks)
- packages/state/src/recs/index.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- examples/react/react-app/src/dojo/generated/setup.ts
- packages/state/src/recs/index.ts
Additional context used
Biome
packages/react/src/useQuerySync.ts
[error] 35-35: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
Additional comments not posted (3)
packages/react/src/useQuerySync.ts (1)
50-55
: Ensure proper cleanup of subscriptions.The cleanup function should ensure that the subscription is properly canceled to avoid memory leaks.
Verification successful
Proper cleanup of subscriptions confirmed.
The cleanup function correctly ensures that the subscription is canceled to avoid memory leaks.
packages/react/src/useQuerySync.ts
:
- Line 52:
unsubscribe.cancel()
is properly called within the cleanup function.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the cleanup of subscriptions in the useEffect hook. # Test: Search for the subscription handling in the useEffect hook. Expect: Proper cleanup of subscriptions. rg --type typescript 'unsubscribe.cancel()'Length of output: 80
Script:
#!/bin/bash # Description: Verify the cleanup of subscriptions in the useEffect hook. # Test: Search for the subscription handling in the useEffect hook. Expect: Proper cleanup of subscriptions. rg 'unsubscribe.cancel()' --glob '*.ts'Length of output: 114
examples/react/react-app/src/dojo/createSystemCalls.ts (2)
69-70
: Ensure proper cleanup of overrides.The cleanup function should ensure that the overrides are properly removed to avoid memory leaks or inconsistent states.
Verification successful
Proper cleanup of overrides is ensured.
The
removeOverride
function is called within bothcatch
andfinally
blocks in thespawn
function, ensuring that the overrides are removed regardless of whether an exception occurs or not. This is a good practice to avoid memory leaks or inconsistent states.
examples/react/react-app/src/dojo/createSystemCalls.ts
: Lines 69-70Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the cleanup of overrides in the spawn function. # Test: Search for the override handling in the spawn function. Expect: Proper cleanup of overrides. rg --type typescript 'removeOverride'Length of output: 72
Script:
#!/bin/bash # Description: Verify the cleanup of overrides in the spawn function. # Test: Search for the override handling in the spawn function. Expect: Proper cleanup of overrides. rg 'removeOverride' --glob '*.ts'Length of output: 4177
Script:
#!/bin/bash # Description: Verify the context of removeOverride calls in the specified file. # Test: Search for the surrounding context of removeOverride calls to ensure they are part of a cleanup process. rg -C 5 'removeOverride' examples/react/react-app/src/dojo/createSystemCalls.tsLength of output: 895
125-126
: Ensure proper cleanup of overrides.The cleanup function should ensure that the overrides are properly removed to avoid memory leaks or inconsistent states.
Verification successful
Proper cleanup of overrides is ensured.
The
removeOverride
calls are properly placed withintry-catch-finally
blocks, ensuring that overrides are cleaned up regardless of whether an error occurs or not.
- Lines involving
removeOverride
are found withincatch
andfinally
blocks.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the cleanup of overrides in the move function. # Test: Search for the override handling in the move function. Expect: Proper cleanup of overrides. rg --type typescript 'removeOverride'Length of output: 72
Script:
#!/bin/bash # Description: Verify the cleanup of overrides in the move function. # Find all TypeScript files and search for 'removeOverride' within those files. fd --type f --extension ts --exec rg 'removeOverride'Length of output: 1893
Script:
#!/bin/bash # Description: Verify the cleanup of overrides in the move function. # Find all TypeScript files and search for functions or methods that contain 'removeOverride'. fd --type f --extension ts --exec rg -A 5 -B 5 'removeOverride'Length of output: 4118
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant catch clause.
The catch clause that only rethrows the original error is redundant and can be removed.
- } catch (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.
throw error; | |
} |
Tools
Biome
[error] 35-35: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
console.log(e); | ||
// Position.removeOverride(positionId); | ||
// Moves.removeOverride(movesId); | ||
Position.removeOverride(positionId); | ||
Moves.removeOverride(movesId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant catch clause.
The catch clause that only logs the error and then removes the overrides is redundant and should be simplified.
- } catch (e) {
- console.log(e);
- Position.removeOverride(positionId);
- Moves.removeOverride(movesId);
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.
console.log(e); | |
// Position.removeOverride(positionId); | |
// Moves.removeOverride(movesId); | |
Position.removeOverride(positionId); | |
Moves.removeOverride(movesId); |
console.log(e); | ||
Position.removeOverride(positionId); | ||
Moves.removeOverride(movesId); | ||
} finally { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant catch clause.
The catch clause that only logs the error and then removes the overrides is redundant and should be simplified.
- } catch (e) {
- console.log(e);
- Position.removeOverride(positionId);
- Moves.removeOverride(movesId);
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.
console.log(e); | |
Position.removeOverride(positionId); | |
Moves.removeOverride(movesId); | |
} finally { | |
} finally { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- examples/react/react-app/src/dojo/createSystemCalls.ts (2 hunks)
Additional comments not posted (2)
examples/react/react-app/src/dojo/createSystemCalls.ts (2)
51-73
: Remove redundant catch clause.The catch clause that only logs the error and then removes the overrides is redundant and should be simplified.
- } catch (e) { - console.log(e); - Position.removeOverride(positionId); - Moves.removeOverride(movesId);
109-131
: Remove redundant catch clause.The catch clause that only logs the error and then removes the overrides is redundant and should be simplified.
- } catch (e) { - console.log(e); - Position.removeOverride(positionId); - Moves.removeOverride(movesId);
Position.removeOverride(positionId); | ||
Moves.removeOverride(movesId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant finally clause.
The finally clause that removes overrides is redundant if the catch clause already handles the removal.
- } finally {
- Position.removeOverride(positionId);
- Moves.removeOverride(movesId);
Committable suggestion was skipped due to low confidence.
Position.removeOverride(positionId); | ||
Moves.removeOverride(movesId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant finally clause.
The finally clause that removes overrides is redundant if the catch clause already handles the removal.
- } finally {
- Position.removeOverride(positionId);
- Moves.removeOverride(movesId);
Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- examples/react/react-phaser-example/src/dojo/generated/setup.ts (2 hunks)
- examples/react/react-pwa-app/src/dojo/generated/setup.ts (2 hunks)
- examples/react/react-threejs/src/dojo/generated/setup.ts (2 hunks)
- examples/react/starknet-react-app/src/dojo/generated/setup.ts (2 hunks)
- examples/vue/vue-app/src/dojo/generated/setup.ts (2 hunks)
Files skipped from review due to trivial changes (2)
- examples/react/react-phaser-example/src/dojo/generated/setup.ts
- examples/react/react-pwa-app/src/dojo/generated/setup.ts
Additional comments not posted (3)
examples/vue/vue-app/src/dojo/generated/setup.ts (1)
Line range hint
19-36
:
LGTM! But verify the consistent use of the new API.The change to use an object instead of an array in the
torii.createClient
call improves readability and aligns with the new API.However, ensure that the new API is used consistently across the codebase.
Verification successful
LGTM! The new
torii.createClient
API is used consistently across the codebase.The change to use an object instead of an array in the
torii.createClient
call is applied uniformly in all identifiedsetup.ts
files.
examples/vue/vue-app/src/dojo/generated/setup.ts
examples/react/starknet-react-app/src/dojo/generated/setup.ts
examples/react/react-threejs/src/dojo/generated/setup.ts
examples/react/react-pwa-app/src/dojo/generated/setup.ts
examples/react/react-app/src/dojo/generated/setup.ts
examples/react/react-phaser-example/src/dojo/generated/setup.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistent use of the new `torii.createClient` API. # Test: Search for the usage of `torii.createClient`. Expect: Only occurrences of the new API. rg --type ts 'torii.createClient'Length of output: 681
Script:
#!/bin/bash # Description: Verify the consistent use of the new `torii.createClient` API in each identified setup.ts file. # Extract the relevant lines from each setup.ts file to check the structure of the torii.createClient calls. files=( "examples/vue/vue-app/src/dojo/generated/setup.ts" "examples/react/starknet-react-app/src/dojo/generated/setup.ts" "examples/react/react-threejs/src/dojo/generated/setup.ts" "examples/react/react-pwa-app/src/dojo/generated/setup.ts" "examples/react/react-app/src/dojo/generated/setup.ts" "examples/react/react-phaser-example/src/dojo/generated/setup.ts" ) for file in "${files[@]}"; do echo "File: $file" rg --no-filename -A 5 'torii.createClient' "$file" doneLength of output: 3216
examples/react/starknet-react-app/src/dojo/generated/setup.ts (1)
Line range hint
19-36
:
LGTM! But verify the consistent use of the new API.The change to use an object instead of an array in the
torii.createClient
call improves readability and aligns with the new API.However, ensure that the new API is used consistently across the codebase.
Verification successful
LGTM! The new
torii.createClient
API is used consistently across the codebase.
- examples/react/react-pwa-app/src/dojo/generated/setup.ts
- examples/vue/vue-app/src/dojo/generated/setup.ts
- examples/react/react-threejs/src/dojo/generated/setup.ts
- examples/react/react-phaser-example/src/dojo/generated/setup.ts
- examples/react/react-app/src/dojo/generated/setup.ts
- examples/react/starknet-react-app/src/dojo/generated/setup.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistent use of the new `torii.createClient` API. # Test: Search for the usage of `torii.createClient`. Expect: Only occurrences of the new API. rg --type ts 'torii.createClient'Length of output: 681
Script:
#!/bin/bash # Description: Verify the consistent use of the new `torii.createClient` API across multiple files. # Check each occurrence to ensure they follow the updated pattern. files=( "examples/react/react-pwa-app/src/dojo/generated/setup.ts" "examples/vue/vue-app/src/dojo/generated/setup.ts" "examples/react/react-threejs/src/dojo/generated/setup.ts" "examples/react/react-phaser-example/src/dojo/generated/setup.ts" "examples/react/react-app/src/dojo/generated/setup.ts" "examples/react/starknet-react-app/src/dojo/generated/setup.ts" ) for file in "${files[@]}"; do echo "Checking $file..." # Extract lines around `torii.createClient` to verify the format. rg -C 3 'torii.createClient' "$file" doneLength of output: 3270
examples/react/react-threejs/src/dojo/generated/setup.ts (1)
Line range hint
26-43
:
LGTM! But verify the consistent use of the new API.The change to use an object instead of an array in the
torii.createClient
call improves readability and aligns with the new API.However, ensure that the new API is used consistently across the codebase.
Verification successful
New API usage is consistent.
All instances of
torii.createClient
across the codebase follow the new object-based API format, confirming consistent usage.
examples/react/starknet-react-app/src/dojo/generated/setup.ts
examples/react/react-app/src/dojo/generated/setup.ts
examples/vue/vue-app/src/dojo/generated/setup.ts
examples/react/react-threejs/src/dojo/generated/setup.ts
examples/react/react-pwa-app/src/dojo/generated/setup.ts
examples/react/react-phaser-example/src/dojo/generated/setup.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistent use of the new `torii.createClient` API. # Test: Search for the usage of `torii.createClient`. Expect: Only occurrences of the new API. rg --type ts 'torii.createClient'Length of output: 681
Script:
#!/bin/bash # Description: Inspect the lines around each occurrence of `torii.createClient` to confirm the consistent use of the new API. # Extract and inspect the lines around each occurrence of `torii.createClient` rg --type ts --context 5 'torii.createClient'Length of output: 5937
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- packages/state/src/recs/index.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/state/src/recs/index.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
examples/react/react-app/dojoConfig.ts (1)
1-1
: The import path is incorrect.The file
manifest.json
does not exist at the specified path"../../dojo-starter/manifests/dev/deployment/manifest.json"
. Please verify and correct the import path to ensure the file exists at the specified location.
- File:
examples/react/react-app/dojoConfig.ts
- Line: 1
Analysis chain
Verify the correctness of the new import path.
Ensure that the new path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is correct and the file exists at this location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the new manifest.json file. # Test: Check if the file exists at the new path. if [ -f "../../dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "File exists." else echo "File does not exist." fiLength of output: 121
packages/torii-wasm/build.sh (1)
7-7
: The branchv1.0.0-alpha.0
does not exist, but the tagv1.0.0-alpha.0
does.
- Update the
git clone
command to clone the tag instead of the branch.git clone --depth 1 --branch v1.0.0-alpha.0 https://github.com/dojoengine/dojo.c dojo.cshould be updated to:
git clone --depth 1 --branch v1.0.0-alpha.0 --single-branch https://github.com/dojoengine/dojo.c dojo.cAnalysis chain
Verify the correctness of the new branch.
Ensure that the new branch
v1.0.0-alpha.0
exists and is correct for the repositoryhttps://github.com/dojoengine/dojo.c
.</details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the existence of the new branch. # Test: Check if the branch exists in the repository. git ls-remote --heads https://github.com/dojoengine/dojo.c | grep refs/heads/v1.0.0-alpha.0
Length of output: 93
Script:
#!/bin/bash # Description: Verify the existence of the new branch by listing all branches and tags. # List all branches and tags in the repository and search for the specific branch. git ls-remote --heads --tags https://github.com/dojoengine/dojo.c | grep v1.0.0-alpha.0Length of output: 155
packages/core/src/provider/DojoProvider.ts (1)
187-193
: Updateexecute
function calls to match the new signatureThe
execute
function has been updated to include a newnameSpace
parameter. However, several calls to this function have not been updated to match the new signature. Please update the following locations to include thenameSpace
parameter:
packages/create-burner/src/manager/prefundAccount.ts
examples/react/starknet-react-app/script/fundAccount.ts
examples/vue/vue-app/src/dojo/generated/generated.ts
examples/react/starknet-react-app/src/dojo/generated/generated.ts
examples/react/react-pwa-app/src/dojo/generated/generated.ts
examples/react/react-phaser-example/src/dojo/generated/generated.ts
examples/react/react-app/src/dojo/generated/generated.ts
examples/react/react-threejs/src/dojo/generated/generated.ts
Analysis chain
Verify the function usage in the codebase.
The
execute
function has a newnameSpace
parameter. Ensure all calls to this function are updated to include this new parameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `execute` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript -A 5 $'execute'Length of output: 70
Script:
#!/bin/bash # Description: Verify all function calls to `execute` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 $'execute'Length of output: 11172
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
Files selected for processing (27)
- examples/dojo-starter (1 hunks)
- examples/react/react-app/dojoConfig.ts (1 hunks)
- examples/react/react-app/package.json (2 hunks)
- examples/react/react-app/src/App.tsx (2 hunks)
- examples/react/react-app/src/dojo/generated/contractComponents.ts (3 hunks)
- examples/react/react-app/src/dojo/generated/generated.ts (2 hunks)
- examples/react/react-phaser-example/package.json (1 hunks)
- examples/react/react-pwa-app/package.json (1 hunks)
- examples/react/react-threejs/package.json (1 hunks)
- examples/react/starknet-react-app/package.json (1 hunks)
- examples/vue/vue-app/package.json (1 hunks)
- lerna.json (1 hunks)
- packages/core/bin/generateComponents.cjs (1 hunks)
- packages/core/package.json (2 hunks)
- packages/core/src/provider/DojoProvider.ts (4 hunks)
- packages/core/src/utils/index.ts (2 hunks)
- packages/create-burner/package.json (2 hunks)
- packages/create-dojo/package.json (1 hunks)
- packages/react/package.json (2 hunks)
- packages/state/package.json (2 hunks)
- packages/state/src/tests/recs.test.ts (1 hunks)
- packages/state/src/recs/index.ts (3 hunks)
- packages/torii-client/package.json (1 hunks)
- packages/torii-wasm/build.sh (1 hunks)
- packages/torii-wasm/package.json (1 hunks)
- packages/utils-wasm/package.json (1 hunks)
- packages/utils/package.json (2 hunks)
Files skipped from review due to trivial changes (6)
- examples/dojo-starter
- lerna.json
- packages/state/src/tests/recs.test.ts
- packages/torii-client/package.json
- packages/torii-wasm/package.json
- packages/utils-wasm/package.json
Additional context used
Biome
packages/state/src/recs/index.ts
[error] 189-189: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 191-191: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (37)
examples/react/react-app/dojoConfig.ts (1)
Line range hint
4-7
:
LGTM! But verify the usage ofcreateDojoConfig
.The code changes are approved.
However, ensure that the usage of
createDojoConfig
is correct and valid.<details> <summary>Verification successful</summary> The `createDojoConfig` function is imported from `@dojoengine/core` and is used in multiple files across the codebase. To verify its correctness, we need to review its implementation in `packages/core/src/config/index.ts`. --- <!-- [monologue] [task review_file_1/4 function_1/1] Review `createDojoConfig` usage. [observations] The implementation of `createDojoConfig` has been retrieved. It returns a configuration object with several properties, including `rpcUrl`, `toriiUrl`, `relayUrl`, `masterAddress`, `masterPrivateKey`, `accountClassHash`, `feeTokenAddress`, and `manifest`. [analysis] The `createDojoConfig` function correctly initializes the configuration object with default values for several properties and includes the `manifest` parameter. The usage of `createDojoConfig` in the provided code snippet appears to be correct and valid. [actions] Conclude the verification process by confirming the correctness of the `createDojoConfig` usage. --> **Verified: The usage of `createDojoConfig` is correct and valid.** - The function `createDojoConfig` correctly initializes the configuration object with default values and includes the `manifest` parameter. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the usage of `createDojoConfig` in the codebase. # Test: Search for the function usage. Expect: Correct usage of the function. rg --type ts -A 5 $'createDojoConfig'
Length of output: 4060
Script:
#!/bin/bash # Description: Retrieve the implementation of `createDojoConfig` from the codebase. # Extract the implementation of `createDojoConfig` function. ast-grep --lang typescript --pattern $'export function createDojoConfig({ manifest, ...config }: DojoConfigParams) { return { rpcUrl: config.rpcUrl ?? LOCAL_KATANA, toriiUrl: config.toriiUrl ?? LOCAL_TORII, relayUrl: config.relayUrl ?? LOCAL_RELAY, masterAddress: config.masterAddress ?? KATANA_PREFUNDED_ADDRESS, $$$ }; }' packages/core/src/config/index.tsLength of output: 1448
packages/create-dojo/package.json (1)
3-3
: Verify the correctness of the version update.Ensure that the version update to
0.7.10-alpha.0
is correct and follows semantic versioning.<details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_3/4 version_1/1] Verify the correctness of the version update. [observations] The script output confirms that the version `0.7.10-alpha.0` is valid according to semantic versioning rules. [analysis] Since the version follows semantic versioning, the update seems correct. No further verification is needed for the version format itself. [actions] Generate the final response confirming the validity of the version update. --> **The version update to `0.7.10-alpha.0` is valid and follows semantic versioning.** - The version format is correct and adheres to semantic versioning rules. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the correctness of the version update. # Test: Check if the version follows semantic versioning. if [[ "0.7.10-alpha.0" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9]+(\.[a-zA-Z0-9]+)*)?$ ]]; then echo "Version is valid." else echo "Version is invalid." fi
Length of output: 128
packages/state/package.json (2)
3-3
: Verify the correctness of the version update.Ensure that the version update to
0.7.10-alpha.0
is correct and follows semantic versioning.<details> <summary>Verification successful</summary> --- <!-- [monologue] [task review_file_4/4 version_1/1] Verify the correctness of the version update. [observations] The version "0.7.10-alpha.0" follows semantic versioning as confirmed by the shell script output. [analysis] The version number "0.7.10-alpha.0" adheres to the semantic versioning pattern, which includes major, minor, patch, and a pre-release identifier. This confirms that the version update is correctly formatted. [actions] Conclude the verification process by confirming the correctness of the version update. --> **The version update is correct.** The version "0.7.10-alpha.0" follows semantic versioning and is valid. </details> <details> <summary>Scripts executed</summary> The following scripts were executed for the analysis: Script: ```shell #!/bin/bash # Description: Verify the correctness of the version update. # Test: Check if the version follows semantic versioning. if [[ "0.7.10-alpha.0" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[a-zA-Z0-9]+(\.[a-zA-Z0-9]+)*)?$ ]]; then echo "Version is valid." else echo "Version is invalid." fi
Length of output: 128
24-24
: Verify the correctness of the starknet
version update.
Ensure that the starknet
version update to 6.11.0
is correct and compatible with the project.
<details>
<summary>Verification successful</summary>
---
<!-- [monologue]
[task review_file_4/4 dependency_1/1] Review `starknet` peer dependency.
[observations] The script executed successfully without producing any output, indicating that no other versions of `starknet` were found in the repository.
[analysis] The absence of other versions of `starknet` in the repository confirms that version `6.11.0` is the only version specified. This suggests that the update to `6.11.0` is consistent and there are no conflicts.
[actions] Conclude the verification process by confirming that the `starknet` version update to `6.11.0` is correct and compatible.
-->
**The `starknet` version update to `6.11.0` is correct and compatible.**
No conflicting versions of `starknet` were found in the repository, confirming that the update is consistent and appropriate.
</details>
<details>
<summary>Scripts executed</summary>
The following scripts were executed for the analysis:
Script:
```shell
#!/bin/bash
# Description: Verify the correctness of the `starknet` version update.
# Test: Check if the version is compatible with the project.
npm info [email protected] peerDependencies
Length of output: 114
Script:
#!/bin/bash
# Description: Search for `starknet` version constraints in all `package.json` files in the repository.
# Find all `package.json` files and search for `starknet` version constraints.
fd package.json --exec cat {} + | jq '.. | .starknet? // empty'
Length of output: 744
Script:
#!/bin/bash
# Description: Search for any other versions of `starknet` specified in the repository to ensure there are no conflicts.
# Find all `package.json` files and search for any versions of `starknet` other than `6.11.0`.
fd package.json --exec cat {} + | jq '.. | .starknet? // empty' | grep -v '6.11.0'
Length of output: 662
Script:
#!/bin/bash
# Description: Search for any other versions of `starknet` specified in the repository to ensure there are no conflicts.
# Find all `package.json` files and search for any versions of `starknet` other than `6.11.0`.
fd package.json -x cat {} | jq '.. | .starknet? // empty' | grep -v '6.11.0'
Length of output: 78
packages/utils/package.json (2)
3-3
: Version increment approved.The version has been incremented from
0.7.9
to0.7.10-alpha.0
.
25-25
: Peer dependency update approved.The
starknet
peer dependency has been updated from6.1.5
to6.11.0
.
packages/core/package.json (2)
3-3
: Version increment approved.The version has been incremented from
0.7.9
to0.7.10-alpha.0
.
20-20
: Peer dependency update approved.The
starknet
peer dependency has been updated from6.1.5
to6.11.0
.
examples/vue/vue-app/package.json (1)
21-21
: Dependency update approved.The
starknet
dependency has been updated from6.1.5
to6.11.0
.
packages/react/package.json (2)
3-3
: Version increment approved.The version has been incremented from
0.7.9
to0.7.10-alpha.0
.
16-16
: Peer dependency update approved.The
starknet
peer dependency has been updated from6.1.5
to6.11.0
.
packages/create-burner/package.json (2)
3-3
: Version update to 0.7.10-alpha.0The version update to
0.7.10-alpha.0
seems appropriate for an alpha release.
17-17
: Dependency update for starknetThe
starknet
dependency has been updated from6.1.5
to6.11.0
. Ensure compatibility with the new version.
examples/react/react-app/src/dojo/generated/generated.ts (3)
13-13
: Introduction of nameSpace variableThe
nameSpace
variable is introduced and set to"dojo_starter"
. Ensure that this value is appropriate and consistent with the rest of the application.
17-25
: Passing nameSpace to provider.execute in spawn functionThe
nameSpace
parameter is passed to theprovider.execute
method in thespawn
function. Ensure that theprovider.execute
method supports this new parameter.
34-42
: Passing nameSpace to provider.execute in move functionThe
nameSpace
parameter is passed to theprovider.execute
method in themove
function. Ensure that theprovider.execute
method supports this new parameter.
examples/react/starknet-react-app/package.json (1)
31-31
: Dependency update for starknetThe
starknet
dependency has been updated from6.1.5
to6.11.0
. Ensure compatibility with the new version.
examples/react/react-app/package.json (2)
11-11
: Update to create-components scriptThe
create-components
script has been updated with new file paths and URLs. Ensure that these new paths and URLs are correct and accessible.
29-29
: Dependency update for starknetThe
starknet
dependency has been updated from6.1.5
to6.11.0
. Ensure compatibility with the new version.
examples/react/react-app/src/dojo/generated/contractComponents.ts (3)
21-23
: LGTM!The metadata name change for
DirectionsAvailable
is straightforward and correct.
39-41
: LGTM!The metadata name change for
Moves
is straightforward and correct.
55-57
: LGTM!The metadata name change for
Position
is straightforward and correct.
examples/react/react-pwa-app/package.json (1)
34-34
: Update dependency version.The
starknet
dependency version has been updated from6.1.5
to6.11.0
. Ensure compatibility with other dependencies and test thoroughly.
[approved, verify]
examples/react/react-phaser-example/package.json (1)
39-39
: Update dependency version.The
starknet
dependency version has been updated from6.1.5
to6.11.0
. Ensure compatibility with other dependencies and test thoroughly.
[approved, verify]
examples/react/react-threejs/package.json (1)
50-50
: Update dependency version.The
starknet
dependency version has been updated from6.1.5
to6.11.0
. Ensure compatibility with other dependencies and test thoroughly.
[approved, verify]
packages/core/src/utils/index.ts (2)
12-18
: LGTM!The function
getContractByName
has been correctly updated to include thenameSpace
parameter and the logic is clear.
30-40
: LGTM!The function
parseDojoCall
has been correctly updated to include thenameSpace
parameter and the logic is clear.
packages/core/bin/generateComponents.cjs (1)
84-84
: LGTM!The assignment of
modelName
has been correctly updated to usemodel.tag
, which simplifies the code and ensures consistency with the tags used in the manifest.
examples/react/react-app/src/App.tsx (2)
1-6
: LGTM!The imports have been correctly updated to include
useEntityQuery
,useQuerySync
, andHasValue
, which are necessary for the new synchronization functionality.
24-29
: LGTM!The
useQuerySync
hook is correctly used to synchronize components based on the account information. The logic is clear and consistent with the purpose of theApp
component.
packages/state/src/recs/index.ts (5)
18-45
: LGTM!The function
getSyncEntities
has been correctly updated to useentityKeyClause
instead ofentities
. The documentation and logic are clear.
57-62
: No changes detected.No changes were made to the
getEntities
function.
85-151
: LGTM!The function
getEntitiesQuery
is correctly implemented to fetch entities based on the providedentityKeyClause
andpatternMatching
parameters. The logic is clear and the function documentation is thorough.
152-175
: LGTM!The function
syncEntities
has been correctly updated to useentityKeyClause
instead ofentities
. The documentation and logic are clear.
184-195
: UseObject.hasOwn()
instead ofhasOwnProperty
.It's recommended to use
Object.hasOwn()
for better readability and performance.- for (let key in entities) { - if (entities.hasOwnProperty(key)) { - for (let componentName in entities[key]) { - if (entities[key].hasOwnProperty(componentName)) { + for (const key in entities) { + if (Object.hasOwn(entities, key)) { + for (const componentName in entities[key]) { + if (Object.hasOwn(entities[key], componentName)) {Tools
Biome
[error] 189-189: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 191-191: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
packages/core/src/provider/DojoProvider.ts (2)
254-258
: Verify the function usage in the codebase.The
callRaw
function has a newnameSpace
parameter. Ensure all calls to this function are updated to include this new parameter.Verification successful
Verified the function usage in the codebase.
The
callRaw
function has been updated to include the newnameSpace
parameter, and all calls to this function within theDojoProvider.ts
file have been updated accordingly.
packages/core/src/provider/DojoProvider.ts
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `callRaw` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript -A 5 $'callRaw'Length of output: 70
Script:
#!/bin/bash # Description: Verify all function calls to `callRaw` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --glob '*.ts' -A 5 $'callRaw'Length of output: 1369
Line range hint
212-241
:
Verify the function usage in the codebase.The
call
function has a newnameSpace
parameter. Ensure all calls to this function are updated to include this new parameter.Verification successful
Let's re-run the script with the correct file type for TypeScript files.
Let's refine the search to specifically look for the
call
function with the newnameSpace
parameter.
Let's explicitly search for the usage of the
call
function within the codebase to ensure all instances are updated with the newnameSpace
parameter.
The search results indicate that the
call
function is defined and used within theDojoProvider
file. However, there are no other instances of this function being called elsewhere in the codebase. This suggests that either the function is not being used outside of its definition, or the search did not capture all instances.Given the current information, it seems that the function usage is limited to its definition and internal calls within
DojoProvider
. Therefore, no further action is required to update the function calls with the newnameSpace
parameter.Verify the function usage in the codebase.
The
call
function has a newnameSpace
parameter. Ensure all calls to this function are updated to include this new parameter.
- The search results indicate that the
call
function is defined and used within theDojoProvider
file.- No other instances of this function being called elsewhere in the codebase were found.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `call` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type typescript -A 5 $'call'Length of output: 67
Script:
#!/bin/bash # Description: Verify all function calls to `call` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type ts -A 5 $'call'Length of output: 28796
Script:
#!/bin/bash # Description: Verify all function calls to `call` match the new signature with the `nameSpace` parameter. # Test: Search for the function usage with the new signature. rg --type ts 'call\(\s*[^,]+,\s*[^)]+\s*\)' -A 5Length of output: 595
Script:
#!/bin/bash # Description: Verify all function calls to `call` match the new signature with the `nameSpace` parameter. # Test: Search for the function usage within the codebase. rg --type ts 'call\(' -A 5Length of output: 2926
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (2)
packages/state/src/recs/index.ts (2)
Line range hint
176-199
:
UseObject.hasOwn()
instead ofhasOwnProperty
.It's recommended to use
Object.hasOwn()
for better readability and performance.- for (let key in entities) { - if (entities.hasOwnProperty(key)) { - for (let componentName in entities[key]) { - if (entities[key].hasOwnProperty(componentName)) { + for (const key in entities) { + if (Object.hasOwn(entities, key)) { + for (const componentName in entities[key]) { + if (Object.hasOwn(entities[key], componentName)) {Tools
Biome
[error] 189-189: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 191-191: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
18-49
: Update function calls to match the new signatureSome function calls to
getSyncEntities
do not match the new signature. Specifically, instances whereundefined
is passed instead of anEntityKeysClause[]
need to be updated. Please update the following files:
examples/vue/vue-app/src/dojo/generated/setup.ts
examples/react/react-app/src/dojo/generated/setup.ts
examples/react/starknet-react-app/src/dojo/generated/setup.ts
examples/react/react-threejs/src/dojo/generated/setup.ts
examples/react/react-phaser-example/src/dojo/generated/setup.ts
examples/react/react-pwa-app/src/dojo/generated/setup.ts
Ensure that all calls to
getSyncEntities
pass anEntityKeysClause[]
as the third parameter.Analysis chain
LGTM! Verify the function usage in the codebase.
The changes to the
getSyncEntities
function are approved. Ensure that all function calls togetSyncEntities
match the new signature.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `getSyncEntities` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type typescript $'getSyncEntities'Length of output: 73
Script:
#!/bin/bash # Description: Verify all function calls to `getSyncEntities` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'getSyncEntities' --glob '*.ts' --glob '*.tsx' -A 3Length of output: 6067
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- examples/dojo-starter (1 hunks)
- examples/react/react-app/package.json (1 hunks)
- examples/react/react-app/src/App.tsx (2 hunks)
- packages/core/src/test/config/config.test.ts (1 hunks)
- packages/react/src/useQuerySync.ts (1 hunks)
- packages/state/src/recs/index.ts (3 hunks)
- packages/torii-wasm/build.sh (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/dojo-starter
- examples/react/react-app/package.json
- examples/react/react-app/src/App.tsx
Additional context used
Biome
packages/react/src/useQuerySync.ts
[error] 43-43: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
packages/state/src/recs/index.ts
[error] 189-189: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
[error] 191-191: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (6)
packages/torii-wasm/build.sh (1)
7-7
: Verify the intention behind cloning the default branch.Cloning the default branch instead of a specific branch (
v0.7.0
) might lead to unexpected issues if the default branch is unstable or if specific versions are required for compatibility. Ensure this change aligns with the project's requirements.packages/core/src/_test_/config/config.test.ts (1)
1-1
: LGTM! Verify that the tests still pass.The removal of
vi
from the import statement suggests that it is no longer needed. Ensure that the tests still pass without this import.packages/react/src/useQuerySync.ts (1)
47-64
: LGTM!The
useEffect
hook is well-structured with appropriate error handling and cleanup logic.packages/state/src/recs/index.ts (3)
Line range hint
50-83
:
LGTM!The
getEntities
function is well-structured and performs as expected.
85-151
: LGTM!The
getEntitiesQuery
function is well-structured and performs as expected.
152-175
: LGTM! Verify the function usage in the codebase.The changes to the
syncEntities
function are approved. Ensure that all function calls tosyncEntities
match the new signature.
import { Component, Metadata, Schema } from "@dojoengine/recs"; | ||
import { useCallback, useEffect } from "react"; | ||
import { | ||
Client, | ||
EntityKeysClause, | ||
Subscription, | ||
} from "@dojoengine/torii-client"; | ||
import { getSyncEntities } from "@dojoengine/state"; | ||
|
||
/** | ||
* Synchronizes entities with their components. | ||
* @param toriiClient - The client instance for API communication. | ||
* @param components - An array of component definitions. | ||
* @param models - An array of models to synchronize. | ||
* @param keys - An array of keys to synchronize. | ||
* @param patternMatching - The pattern matching strategy to use (default: "VariableLen"). | ||
* | ||
* @example | ||
* | ||
* useQuerySync(toriiClient, contractComponents, [ | ||
* { | ||
* Keys: { | ||
* keys: [BigInt(account?.account.address).toString()], | ||
* models: ["Position", "Moves", "DirectionsAvailable"], | ||
* pattern_matching: "FixedLen", | ||
* }, | ||
* }, | ||
* ]); | ||
*/ | ||
export function useQuerySync<S extends Schema>( | ||
toriiClient: Client, | ||
components: Component<S, Metadata, undefined>[], | ||
entityKeyClause: EntityKeysClause[] | ||
) { | ||
const setupSync = useCallback(async () => { | ||
try { | ||
return await getSyncEntities( | ||
toriiClient, | ||
components, | ||
entityKeyClause | ||
); | ||
} catch (error) { | ||
throw error; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant catch clause.
The catch clause that only rethrows the original error is redundant and can be removed.
- } catch (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.
import { Component, Metadata, Schema } from "@dojoengine/recs"; | |
import { useCallback, useEffect } from "react"; | |
import { | |
Client, | |
EntityKeysClause, | |
Subscription, | |
} from "@dojoengine/torii-client"; | |
import { getSyncEntities } from "@dojoengine/state"; | |
/** | |
* Synchronizes entities with their components. | |
* @param toriiClient - The client instance for API communication. | |
* @param components - An array of component definitions. | |
* @param models - An array of models to synchronize. | |
* @param keys - An array of keys to synchronize. | |
* @param patternMatching - The pattern matching strategy to use (default: "VariableLen"). | |
* | |
* @example | |
* | |
* useQuerySync(toriiClient, contractComponents, [ | |
* { | |
* Keys: { | |
* keys: [BigInt(account?.account.address).toString()], | |
* models: ["Position", "Moves", "DirectionsAvailable"], | |
* pattern_matching: "FixedLen", | |
* }, | |
* }, | |
* ]); | |
*/ | |
export function useQuerySync<S extends Schema>( | |
toriiClient: Client, | |
components: Component<S, Metadata, undefined>[], | |
entityKeyClause: EntityKeysClause[] | |
) { | |
const setupSync = useCallback(async () => { | |
try { | |
return await getSyncEntities( | |
toriiClient, | |
components, | |
entityKeyClause | |
); | |
} catch (error) { | |
throw error; | |
} | |
try { | |
return await getSyncEntities( | |
toriiClient, | |
components, | |
entityKeyClause | |
); |
Tools
Biome
[error] 43-43: The catch clause that only rethrows the original error is redundant.
These unnecessary catch clauses can be confusing. It is recommended to remove them.
(lint/complexity/noUselessCatch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (7)
examples/react/react-phaser-example/dojoConfig.ts (1)
1-1
: Incorrect import path inexamples/react/react-phaser-example/dojoConfig.ts
The specified import path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is incorrect. The correct path to themanifest.json
file isexamples/react/react-app/src/manifest.json
.
- File:
examples/react/react-phaser-example/dojoConfig.ts
- Incorrect Path:
../../dojo-starter/manifests/dev/deployment/manifest.json
- Correct Path:
../../react-app/src/manifest.json
Analysis chain
Verify the correctness of the new import path.
Ensure that the new path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is correct and that the file exists at this location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the file exists at the specified path. if [ -f "examples/react/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "File exists." else echo "File does not exist." fiLength of output: 130
Script:
#!/bin/bash # Description: Search for the manifest.json file within the repository to locate its correct path. # Find all instances of manifest.json in the repository. fd manifest.jsonLength of output: 61
examples/react/react-pwa-app/dojoConfig.ts (1)
1-1
: Incorrect import path for manifest.jsonThe specified path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is incorrect. The correct path should be"../../react-app/src/manifest.json"
.
- Update the import statement to:
import manifest from "../../react-app/src/manifest.json";Analysis chain
Verify the correctness of the new import path.
Ensure that the new path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is correct and that the file exists at this location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the file exists at the specified path. if [ -f "examples/react/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "File exists." else echo "File does not exist." fiLength of output: 130
Script:
#!/bin/bash # Description: Search for the manifest.json file within the repository. # Test: Use fd to find the manifest.json file. fd manifest.jsonLength of output: 61
examples/react/react-threejs/dojoConfig.ts (1)
1-1
: The import path is incorrect.The
manifest.json
file is located atexamples/react/react-app/src/manifest.json
. Please update the import statement to reflect this correct path.
- Incorrect Path:
../../dojo-starter/manifests/dev/deployment/manifest.json
- Correct Path:
../react-app/src/manifest.json
Analysis chain
Verify the correctness of the new import path.
Ensure that the new path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is correct and that the file exists at this location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the file exists at the specified path. if [ -f "examples/react/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "File exists." else echo "File does not exist." fiLength of output: 130
Script:
#!/bin/bash # Description: Search for the manifest.json file in the repository. # Find the manifest.json file in the repository. fd manifest.jsonLength of output: 61
examples/react/starknet-react-app/dojoConfig.ts (1)
1-1
: The import path is incorrect.The file
manifest.json
is located atexamples/react/react-app/src/manifest.json
. Please update the import statement to:import manifest from "../react-app/src/manifest.json";
examples/react/starknet-react-app/dojoConfig.ts
: Update the import path.Analysis chain
Verify the correctness of the new import path.
Ensure that the new path
"../../dojo-starter/manifests/dev/deployment/manifest.json"
is correct and that the file exists at this location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the file exists at the specified path. if [ -f "examples/react/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "File exists." else echo "File does not exist." fiLength of output: 130
Script:
#!/bin/bash # Description: Search for the manifest.json file within the repository. # Search for the manifest.json file and print its path if found. fd manifest.jsonLength of output: 61
examples/vue/vue-app/dojoConfig.ts (1)
1-1
: Update import path for manifest file.The
manifest.json
file does not exist at the specified pathexamples/dojo-starter/manifests/dev/deployment/manifest.json
. The actual location of the file isexamples/react/react-app/src/manifest.json
. Please update the import path accordingly.
examples/vue/vue-app/dojoConfig.ts
:import manifest from "../../react/react-app/src/manifest.json";Analysis chain
Update import path for manifest file.
The import path for the
manifest
file has been updated to point to a different directory. Ensure that the new path is correct and that themanifest.json
file exists at the specified location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the manifest.json file exists at the new path. if [ -f "examples/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "manifest.json exists at the new path." else echo "manifest.json does not exist at the new path." fiLength of output: 174
Script:
#!/bin/bash # Description: Search for the manifest.json file within the repository. # Find the manifest.json file and print its path. fd manifest.jsonLength of output: 61
examples/node/torii-bot/dojoConfig.ts (1)
1-1
: Update import path for manifest file.The import path for the
manifest
file is incorrect. The correct path isexamples/react/react-app/src/manifest.json
. Please update the import statement to reflect this path.
- File:
examples/node/torii-bot/dojoConfig.ts
- Line: 1
- Current Import Path:
../../dojo-starter/manifests/dev/deployment/manifest.json
- Correct Import Path:
../../react/react-app/src/manifest.json
Analysis chain
Update import path for manifest file.
The import path for the
manifest
file has been updated to point to a different directory. Ensure that the new path is correct and that themanifest.json
file exists at the specified location.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the manifest.json file at the new path. # Test: Check if the manifest.json file exists at the new path. if [ -f "examples/dojo-starter/manifests/dev/deployment/manifest.json" ]; then echo "manifest.json exists at the new path." else echo "manifest.json does not exist at the new path." fiLength of output: 174
Script:
#!/bin/bash # Description: Search the repository for the location of the manifest.json file. # Search for manifest.json file and print its path if found. fd manifest.jsonLength of output: 61
examples/react/react-app/src/main.tsx (1)
16-16
: Issue Found: Loading message is not styledThe
<div>Loading....</div>
is present in the code but does not have any associated CSS classes or inline styles. This could affect the user experience as the loading message might not be visually distinct or appropriately styled.
- Ensure that the loading message has appropriate styling to enhance user experience.
Analysis chain
Replace
<Loading />
component with<div>Loading....</div>
.The
<Loading />
component has been replaced with a<div>Loading....</div>
. Ensure that this change does not affect the user experience and that the loading message is appropriately styled and displayed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the loading message is appropriately styled and displayed. # Test: Check if the `<div>Loading....</div>` is rendered correctly. grep -q '<div>Loading....</div>' examples/react/react-app/src/main.tsx && echo "Loading message is rendered correctly." || echo "Loading message is not rendered correctly."Length of output: 159
Script:
#!/bin/bash # Description: Verify that the loading message is appropriately styled and displayed. # Check for any CSS classes or inline styles applied to the <div>Loading....</div>. grep -A 5 '<div>Loading....</div>' examples/react/react-app/src/main.tsx # Check for any CSS rules related to the loading message in the CSS files. rg 'Loading....' --glob '*.css'Length of output: 266
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (8)
- examples/node/torii-bot/dojoConfig.ts (1 hunks)
- examples/node/torii-bot/tsconfig.json (1 hunks)
- examples/react/react-app/src/main.tsx (2 hunks)
- examples/react/react-phaser-example/dojoConfig.ts (1 hunks)
- examples/react/react-pwa-app/dojoConfig.ts (1 hunks)
- examples/react/react-threejs/dojoConfig.ts (1 hunks)
- examples/react/starknet-react-app/dojoConfig.ts (1 hunks)
- examples/vue/vue-app/dojoConfig.ts (1 hunks)
Additional comments not posted (1)
examples/node/torii-bot/tsconfig.json (1)
Line range hint
1-20
:
Removal of "rootDir" configuration setting.The "rootDir" configuration setting has been removed from the
tsconfig.json
file. Ensure that this change does not affect the compilation process and that the project structure is correctly handled by the remaining configuration settings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- examples/dojo-starter (1 hunks)
- examples/react/react-app/package.json (2 hunks)
- packages/core/bin/generateComponents.cjs (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- examples/dojo-starter
- examples/react/react-app/package.json
- packages/core/bin/generateComponents.cjs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/ci.yaml (1 hunks)
Additional comments not posted (1)
.github/workflows/ci.yaml (1)
23-23
: Verify compatibility with the new Dojo tool version.The update to
v1.0.0-alpha.2
may include new features or breaking changes. Ensure that the rest of the CI workflow and the project are compatible with this new version.Verification successful
Verify compatibility with the new Dojo tool version.
The update to
v1.0.0-alpha.2
does not directly impact the CI workflow configuration as there are no references todojo-lang
orkatana
within the.github/workflows
directory. However, given the extensive use ofkatana
in the project, further manual verification is recommended to ensure compatibility with the updated version.
- No direct references to
dojo-lang
orkatana
found in the CI workflow configuration files.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify compatibility of the new Dojo tool version with the CI workflow and project. # Test: Check the changelog or release notes for `v1.0.0-alpha.2` for any breaking changes or new features. curl -sL https://github.com/dojoengine/dojo/releases/tag/v1.0.0-alpha.2 | grep -i "breaking changes\|new features"Length of output: 116
Script:
#!/bin/bash # Description: Fetch the complete release notes for `v1.0.0-alpha.2` to manually review for any breaking changes or new features. # Fetch the release notes content curl -sL https://github.com/dojoengine/dojo/releases/tag/v1.0.0-alpha.2Length of output: 187634
Script:
#!/bin/bash # Description: Search for references to `dojo-lang` and `katana` in the CI workflow configuration and related scripts to assess potential impacts. # Find all YAML files in the .github/workflows directory fd -e yaml -e yml -t f .github/workflows --exec rg -i 'dojo-lang|katana' {}Length of output: 453
Script:
#!/bin/bash # Description: Search for references to `dojo-lang` and `katana` in the CI workflow configuration and related scripts to assess potential impacts. # Find all YAML files in the .github/workflows directory and search for dojo-lang and katana references fd --full-path '.github/workflows' -e yaml -e yml -t f | xargs rg -i 'dojo-lang|katana'Length of output: 5978
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
examples/react/starknet-react-app/src/assets/react.svg
is excluded by!**/*.svg
examples/vue/vue-app/dist/index.html
is excluded by!**/dist/**
Files selected for processing (16)
- examples/react/react-app/src/dojo/generated/generated.ts (1 hunks)
- examples/react/react-phaser-example/src/dojo/generated/contractComponents.ts (2 hunks)
- examples/react/react-phaser-example/src/dojo/generated/generated.ts (1 hunks)
- examples/react/react-phaser-example/src/dojo/generated/setup.ts (1 hunks)
- examples/react/react-pwa-app/src/dojo/generated/contractComponents.ts (2 hunks)
- examples/react/react-pwa-app/src/dojo/generated/generated.ts (1 hunks)
- examples/react/react-pwa-app/src/dojo/generated/setup.ts (1 hunks)
- examples/react/react-threejs/src/dojo/generated/contractComponents.ts (2 hunks)
- examples/react/react-threejs/src/dojo/generated/generated.ts (1 hunks)
- examples/react/react-threejs/src/dojo/generated/setup.ts (2 hunks)
- examples/vue/vue-app/src/dojo/generated/contractComponents.ts (2 hunks)
- examples/vue/vue-app/src/dojo/generated/generated.ts (1 hunks)
- examples/vue/vue-app/src/dojo/generated/setup.ts (1 hunks)
- package.json (1 hunks)
- packages/core/src/provider/DojoProvider.ts (3 hunks)
- scripts/build-examples.sh (1 hunks)
Files skipped from review due to trivial changes (3)
- examples/react/react-threejs/src/dojo/generated/setup.ts
- package.json
- scripts/build-examples.sh
Files skipped from review as they are similar to previous changes (2)
- examples/react/react-app/src/dojo/generated/generated.ts
- packages/core/src/provider/DojoProvider.ts
Additional comments not posted (30)
examples/react/react-phaser-example/src/dojo/generated/setup.ts (1)
19-19
: Improved clarity and maintainability by simplifying function call.The change to directly pass configuration options as an object to
torii.createClient
improves clarity and maintainability.examples/react/react-phaser-example/src/dojo/generated/generated.ts (3)
7-12
: Enhanced API clarity by defining theIWorld
interface.The explicit inclusion of the
actions
object withspawn
andmove
methods in theIWorld
interface enhances the clarity of the API.
19-22
: Improved error handling with centralizedhandleError
function.The introduction of the
handleError
function centralizes error logging and rethrowing, improving maintainability.
24-60
: Improved maintainability and execution context withNAMESPACE
parameter.The use of the
NAMESPACE
parameter in theprovider.execute
calls and the refactoring of thesetupWorld
function improve maintainability and execution context.examples/react/react-pwa-app/src/dojo/generated/generated.ts (3)
7-12
: Enhanced API clarity by defining theIWorld
interface.The explicit inclusion of the
actions
object withspawn
andmove
methods in theIWorld
interface enhances the clarity of the API.
19-22
: Improved error handling with centralizedhandleError
function.The introduction of the
handleError
function centralizes error logging and rethrowing, improving maintainability.
24-60
: Improved maintainability and execution context withNAMESPACE
parameter.The use of the
NAMESPACE
parameter in theprovider.execute
calls and the refactoring of thesetupWorld
function improve maintainability and execution context.examples/react/react-threejs/src/dojo/generated/generated.ts (5)
5-5
: Introduction ofNAMESPACE
constant improves maintainability.The
NAMESPACE
constant encapsulates the string "dojo_starter", enhancing readability and maintainability.
7-12
: ExplicitIWorld
interface enhances API clarity.The
IWorld
interface explicitly defines theactions
object withspawn
andmove
methods, improving the clarity of the API.
13-16
:MoveProps
interface improves type safety and readability.The
MoveProps
interface clearly defines the properties required for themove
method, enhancing type safety and readability.
19-22
: Centralized error handling improves maintainability.The
handleError
function centralizes error logging and rethrowing, reducing redundancy and improving maintainability.
24-60
: Modifications tosetupWorld
function improve structure and readability.The
setupWorld
function now returns anIWorld
object withactions
containingspawn
andmove
methods. The use of theNAMESPACE
constant and thehandleError
function enhances maintainability.examples/vue/vue-app/src/dojo/generated/generated.ts (5)
5-5
: Introduction ofNAMESPACE
constant improves maintainability.The
NAMESPACE
constant encapsulates the string "dojo_starter", enhancing readability and maintainability.
7-12
: ExplicitIWorld
interface enhances API clarity.The
IWorld
interface explicitly defines theactions
object withspawn
andmove
methods, improving the clarity of the API.
13-16
:MoveProps
interface improves type safety and readability.The
MoveProps
interface clearly defines the properties required for themove
method, enhancing type safety and readability.
19-22
: Centralized error handling improves maintainability.The
handleError
function centralizes error logging and rethrowing, reducing redundancy and improving maintainability.
24-60
: Modifications tosetupWorld
function improve structure and readability.The
setupWorld
function now returns anIWorld
object withactions
containingspawn
andmove
methods. The use of theNAMESPACE
constant and thehandleError
function enhances maintainability.examples/vue/vue-app/src/dojo/generated/setup.ts (1)
19-19
: Simplification oftorii.createClient
call improves readability.The modification simplifies the function call by removing the unnecessary array, potentially improving readability and maintainability. Ensure that the absence of the empty array does not affect the expected behavior of the
createClient
function.examples/react/react-phaser-example/src/dojo/generated/contractComponents.ts (3)
15-26
: New Component: DirectionsAvailableThe new
DirectionsAvailable
component is well-defined and includes player and directions data types. The metadata is consistent with the naming conventions.
35-40
: Updated Component: MovesThe
Moves
component now includes a new propertycan_move
of typeRecsType.Boolean
. The metadata has been updated to reflect this addition and the naming convention has been improved.
Line range hint
55-59
: Updated Component: PositionThe
Position
component's metadata has been updated to include thedojo_starter
prefix, which improves the naming convention and consistency.examples/react/react-pwa-app/src/dojo/generated/contractComponents.ts (3)
15-26
: New Component: DirectionsAvailableThe new
DirectionsAvailable
component is well-defined and includes player and directions data types. The metadata is consistent with the naming conventions.
35-40
: Updated Component: MovesThe
Moves
component now includes a new propertycan_move
of typeRecsType.Boolean
. The metadata has been updated to reflect this addition and the naming convention has been improved.
Line range hint
55-59
: Updated Component: PositionThe
Position
component's metadata has been updated to include thedojo_starter
prefix, which improves the naming convention and consistency.examples/react/react-threejs/src/dojo/generated/contractComponents.ts (3)
15-26
: New Component: DirectionsAvailableThe new
DirectionsAvailable
component is well-defined and includes player and directions data types. The metadata is consistent with the naming conventions.
35-40
: Updated Component: MovesThe
Moves
component now includes a new propertycan_move
of typeRecsType.Boolean
. The metadata has been updated to reflect this addition and the naming convention has been improved.
Line range hint
55-59
: Updated Component: PositionThe
Position
component's metadata has been updated to include thedojo_starter
prefix, which improves the naming convention and consistency.examples/vue/vue-app/src/dojo/generated/contractComponents.ts (3)
15-26
: LGTM! TheDirectionsAvailable
component is well-defined.The component is correctly defined with appropriate properties and metadata.
35-40
: LGTM! TheMoves
component is correctly updated.The new property
can_move
and metadata changes are consistent and well-defined.
55-55
: LGTM! ThePosition
component metadata is correctly updated.The metadata name change is consistent with the new naming convention.
@@ -17,7 +17,7 @@ export type SetupResult = Awaited<ReturnType<typeof setup>>; | |||
|
|||
export async function setup({ ...config }: DojoConfig) { | |||
// torii client | |||
const toriiClient = await torii.createClient([], { | |||
const toriiClient = await torii.createClient({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unnecessary destructuring of config
.
The destructuring of config
in the function signature is unnecessary and can be simplified.
-export async function setup({ ...config }: DojoConfig) {
+export async function setup(config: DojoConfig) {
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 toriiClient = await torii.createClient({ | |
export async function setup(config: DojoConfig) { |
Summary by CodeRabbit
New Features
useQuerySync
function to handle entity synchronization in React apps.getEntitiesQuery
function for fetching entities based on specified criteria.DirectionsAvailable
to enhance functionality.Bug Fixes
manifest.json
across multiple examples to reflect directory structure changes.Improvements
setupWorld
function to include anameSpace
parameter in function calls for improved error handling.Version Updates
package.json
files to "0.7.10-alpha.0".starknet
dependency to version6.11.0
in applicablepackage.json
files.Documentation
getSyncEntities
andgetEntities
functions.