feat: deprecate QueryBuilder -- Default to ToriiQueryBuilder #392
Conversation
WalkthroughThis PR updates several Dojo Engine packages with minor version bumps and introduces a new default query mechanism using Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant SDK
participant Database
Component->>SDK: Call subscribeEntityQuery(new ToriiQueryBuilder() with KeysClause)
SDK->>Database: Process query with includeHashedKeys() and clause details
Database-->>SDK: Return initial data and subscription handle
SDK->>Component: Invoke callback with initial data update
Component->>Component: Update state with latest entity values
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 (
|
MartianGreed
force-pushed
the
feat/deprecate-qb
branch
from
10ccefc to
d8aa0b6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
examples/example-vite-react-sdk/src/historical-events.tsx (1)
15-50: Add proper subscription cleanupWhile there's error handling for the subscription, the useEffect should include a cleanup function to prevent memory leaks.
Add a cleanup function:
useEffect(() => { async function subscribeHistoricalEvent(account: AccountInterface) { // ... existing code ... } if (account) { subscribeHistoricalEvent(account); } + return () => { + if (subscription) { + subscription.free(); + } + }; }, [account, setEvents, sdk, subscription]);
🧹 Nitpick comments (19)
packages/sdk/src/queryBuilder.ts (3)
7-9: Enhance deprecation notices with migration details.While the deprecation notices correctly mark the classes as deprecated, they could be more helpful to users by including:
- Migration timeline
- Link to migration guide
- Code example showing how to use
ClauseBuilderExample enhancement:
/** - * @deprecated use ClauseBuilder instead + * @deprecated Since version X.Y.Z. Will be removed in version A.B.C. + * Use ClauseBuilder instead. See migration guide: <link> + * Example: + * ```typescript + * // Old way + * const query = new QueryBuilder() + * .namespace("game", ns => + * ns.entity("player", e => e.is("health", 100)) + * ); + * + * // New way using ClauseBuilder + * const query = new ClauseBuilder() + * // ... equivalent code + * ``` */Also applies to: 58-60, 87-89
10-10: Strengthen deprecation implementation.Consider enhancing the deprecation with:
- Runtime warnings using
console.warn- TypeScript's
@deprecateddecorator (when enabled)- Deprecated export marker
+ /** @deprecated */ export class QueryBuilder<T extends SchemaType> { + constructor() { + console.warn( + 'Warning: QueryBuilder is deprecated. Use ClauseBuilder instead. ' + + 'See migration guide: <link>' + ); + this.namespaces = new Map<string, Namespace<T>>(); + }
1-205: Consider providing migration utilities.Given the complexity of the query building logic and type constraints, consider:
- Creating a migration script to automatically update codebases
- Providing a compatibility layer for gradual migration
- Adding type definitions that help TypeScript users identify deprecated usage
Would you like me to:
- Generate a codemod script for automated migration?
- Create a compatibility wrapper that supports both old and new APIs?
- Add type utilities for migration assistance?
examples/example-vite-react-sdk/src/historical-events.tsx (1)
78-79: Address type safety issue instead of suppressing itThe
@ts-expect-errorcomment suggests underlying type issues that should be properly addressed rather than suppressed.Consider:
- Adding proper type definitions for the direction field
- Using type guards to ensure type safety
- Documenting why the type assertion is necessary if it cannot be avoided
examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (2)
10-15: Consider handling async initialization state.The async initialization of predeployed accounts could lead to race conditions if the accounts aren't ready when the provider attempts to use them. Consider adding a loading state or ensuring accounts are initialized before rendering the provider.
let pa: PredeployedAccountsConnector[] = []; +const [isInitialized, setIsInitialized] = useState(false); + +useEffect(() => { predeployedAccounts({ rpc: env.VITE_CONTROLLER_RPC, id: "katana", name: "Katana", -}).then((p) => (pa = p)); +}).then((p) => { + pa = p; + setIsInitialized(true); +}); +}, []); + +if (!isInitialized) { + return <div>Loading accounts...</div>; +}
26-26: Add type safety for connectors array spread.The spread operator on
pacould be unsafe ifpais undefined. Consider adding a type guard or default empty array.-connectors={[...pa]} +connectors={[...(pa || [])]}examples/example-vite-kitchen-sink/src/components/global-counter.tsx (1)
38-49: Consider externalizing hard-coded values.
The literal "9999999" within the KeysClause call might be better maintained as a constant or configuration to avoid confusion in production.packages/sdk/src/__example__/index.ts (2)
80-93: Replace placeholder URLs with environment-specific settings.
While using placeholders is useful for examples, consider injecting real environment variables in production to avoid accidental commits of sensitive data.
143-151: Nested AndComposeClauses are logical and flexible.
Consider potential performance implications if the dataset is large; indexing or server-side optimizations may be necessary.examples/example-vite-kitchen-sink/src/components/chat.tsx (2)
71-80: Confirm intentional use of [undefined] in the KeysClause.
If the intention is to handle all entity keys, clarify this in a comment or consider a more explicit approach.
100-110: Efficient chaining of map, filter, and sort.
This is a concise way to transform fetched messages. For large datasets, consider server-side paging or incremental loading to maintain performance.examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (2)
78-86: Consider moving hardcoded '9999999' into a config.
Currently, the call to KeysClause uses “9999999” as a key. Future maintainers might not know its significance. Extracting it into a named constant or referencing it from config clarifies its purpose.const [initialTheme, sub] = await db.subscribeEntityQuery({ query: new ToriiQueryBuilder() .withClause( KeysClause( - ["onchain_dash-Theme"], - ["9999999"], + [THEME_MODEL_ID], + [DEFAULT_KEY], "FixedLen" ).build() ) .includeHashedKeys(),
139-139: Check for subscription object consistency.
Using “sub === null” is fine, but code readability might improve by consistently checking “!sub.” If the subscription is undefined for any reason, the condition might not catch it.- if (db && sub === null) { + if (db && !sub) { subscribeToEntityUpdates(db).then().catch(console.error); }examples/example-vite-react-sdk/src/App.tsx (1)
72-73: Safe logging in callback.
The error handling properly logs to console. If required, consider providing user-facing feedback or hooking this into a monitoring system.packages/sdk/src/index.ts (1)
42-43: Signature updated for init function.
Removing the schema parameter streamlines configuration. Double-check that your docs mention the new usage pattern without referencing a separate schema parameter.packages/sdk/src/types.ts (1)
399-403: Optional logger configuration.withLogger provides a convenient debugging toggle. If usage is purely for developer mode, consider a more explicit naming (e.g., enableLogging) but not mandatory.
examples/example-vite-react-sql/src/hooks/usePlayerActions.ts (1)
24-34: Enhance error handling in subscription callback.While the transition to ToriiQueryBuilder is correct, consider improving error handling:
- The error is logged but not propagated
- The state update continues even after an error
callback: ({ error, data }) => { if (error) { console.error("Error setting up entity sync:", error); + return; } else if (Also applies to: 45-45
examples/example-vite-kitchen-sink/src/components/caller-counter.tsx (1)
46-56: Refactor repeated count parsing logic.The count parsing logic is duplicated between the subscription callback and initial entity handling.
+const parseCount = (count: unknown) => { + if (!count) return 0; + return parseInt(count.toString(), 16); +}; const [initialEntities, sub] = await db.subscribeEntityQuery({ query: new ToriiQueryBuilder() .withClause( KeysClause( ["onchain_dash-CallerCounter"], [addAddressPadding(address)], "FixedLen" ).build() ) .includeHashedKeys(), callback: ({ data, error }) => { if (error) { + console.error("Subscription error:", error); + return; } if (data) { const entity = data.pop() as ParsedEntity<SchemaType>; if (!entity) return; const count = entity.models.onchain_dash?.CallerCounter?.counter; - if (undefined === count) return 0; setIsLoading(false); - setCount(parseInt(count.toString(), 16)); + setCount(parseCount(count)); } - if (error) { - throw error; - } }, }); setSub(sub); -const count = initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter; -if (!count) { - setCount(0); -} else { - setCount(parseInt(count.toString(), 16)); -} +setCount(parseCount(initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter));Also applies to: 84-90
readme.md (1)
85-88: Client initialization syntax: Remove explicit schema if unnecessary.
The example still shows "(await init) < Schema > ({ ... });". If the intention is to simplify and remove the explicit schema parameter (as indicated in the PR summary), consider updating this snippet to "const db = (await init)({ ... });". This change will align the documentation with the new default behavior.Do you want me to propose a revised diff for this snippet?
📜 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 (28)
.changeset/tidy-toys-compare.md(1 hunks)examples/example-vite-experimental-sdk/tsconfig.json(1 hunks)examples/example-vite-kitchen-sink/package.json(1 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/starknet-provider.tsx(2 hunks)examples/example-vite-kitchen-sink/src/components/theme-switch.tsx(4 hunks)examples/example-vite-kitchen-sink/src/main.tsx(1 hunks)examples/example-vite-kitchen-sink/tsconfig.json(1 hunks)examples/example-vite-kitchen-sink/vite.config.ts(1 hunks)examples/example-vite-react-sdk/src/App.tsx(3 hunks)examples/example-vite-react-sdk/src/historical-events.tsx(3 hunks)examples/example-vite-react-sdk/src/main.tsx(2 hunks)examples/example-vite-react-sql/src/dojo-sdk-provider.tsx(0 hunks)examples/example-vite-react-sql/src/hooks/usePlayerActions.ts(3 hunks)examples/example-vite-react-sql/src/main.tsx(2 hunks)examples/example-vite-react-sql/tsconfig.app.tsbuildinfo(1 hunks)packages/sdk/src/__example__/index.ts(4 hunks)packages/sdk/src/__tests__/convertClauseToEntityKeysClause.test.ts(1 hunks)packages/sdk/src/__tests__/queryBuilder.test.ts(1 hunks)packages/sdk/src/convertClauseToEntityKeysClause.ts(1 hunks)packages/sdk/src/experimental/index.ts(1 hunks)packages/sdk/src/index.ts(3 hunks)packages/sdk/src/parseHistoricalEvents.ts(0 hunks)packages/sdk/src/queryBuilder.ts(3 hunks)packages/sdk/src/types.ts(4 hunks)readme.md(11 hunks)
💤 Files with no reviewable changes (2)
- packages/sdk/src/parseHistoricalEvents.ts
- examples/example-vite-react-sql/src/dojo-sdk-provider.tsx
✅ Files skipped from review due to trivial changes (4)
- packages/sdk/src/tests/queryBuilder.test.ts
- packages/sdk/src/experimental/index.ts
- packages/sdk/src/tests/convertClauseToEntityKeysClause.test.ts
- .changeset/tidy-toys-compare.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (68)
examples/example-vite-kitchen-sink/vite.config.ts (3)
3-3: LGTM! Import updates align with best practices.The change from
vite-preset-reactto@vitejs/plugin-reactand using the node:path module are good improvements.Also applies to: 7-7
12-19: LGTM! Build configuration ensures modern output.The build configuration targeting "esnext" with ESM output format is appropriate for modern web applications and aligns well with the project's direction.
20-24: LGTM! Improved path resolution configuration.The alias configuration using path.resolve is cleaner and more maintainable than the previous array format.
examples/example-vite-experimental-sdk/tsconfig.json (1)
21-21: LGTM! Simplified TypeScript compilation scope.Limiting compilation to just the "src" directory is a good practice that helps maintain clear boundaries in the project structure.
examples/example-vite-kitchen-sink/tsconfig.json (3)
3-6: LGTM! Modern TypeScript configuration.The target and module settings are appropriate for modern web development, and the lib includes necessary DOM types.
14-19: LGTM! Enhanced type safety and code quality checks.The addition of React JSX support and strict type checking with comprehensive linting rules will help maintain code quality.
20-23: LGTM! Clear path resolution configuration.The baseUrl and paths configuration provides clear module resolution rules that align with the Vite configuration.
examples/example-vite-react-sql/tsconfig.app.tsbuildinfo (1)
1-1: Verify the removal of dojo-sdk-provider.tsx and its replacement.The removal of
dojo-sdk-provider.tsxfrom the TypeScript build configuration aligns with deprecating the old query builder. However, we should ensure that any new files implementing ToriiQueryBuilder functionality are properly included in the build.Let's verify the changes and look for potential new files:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for new ToriiQueryBuilder-related files and verify dojo-sdk-provider removal # Check if dojo-sdk-provider.tsx still exists echo "Checking if dojo-sdk-provider.tsx still exists:" fd "dojo-sdk-provider.tsx" src/ # Look for new ToriiQueryBuilder-related files echo -e "\nLooking for new ToriiQueryBuilder files:" rg -l "ToriiQueryBuilder" src/Length of output: 401
Let's try a repository-wide search without assuming a "src/" directory. We'll run a script that searches the entire repository for both "dojo-sdk-provider.tsx" and any references to "ToriiQueryBuilder".
🏁 Script executed:
#!/bin/bash # Search for dojo-sdk-provider.tsx anywhere in the repository echo "Searching for dojo-sdk-provider.tsx across the repository:" fd dojo-sdk-provider.tsx . # Search for any files referencing ToriiQueryBuilder echo -e "\nSearching for references to ToriiQueryBuilder across the repository:" rg -l "ToriiQueryBuilder" .Length of output: 1492
Review Verification: Removal Confirmed and ToriiQueryBuilder Present
- The search did not locate any occurrence of dojo-sdk-provider.tsx, confirming its removal.
- Multiple references to ToriiQueryBuilder exist (e.g., in packages/sdk/src/toriiQueryBuilder.ts and corresponding test files), indicating that the new implementation is in place.
examples/example-vite-react-sdk/src/historical-events.tsx (3)
1-7: LGTM: Import changes align with ToriiQueryBuilder migrationThe new imports correctly introduce the required components for the ToriiQueryBuilder implementation.
9-13: LGTM: Clean component and state setupThe state management is well-structured with proper typing for events and subscription handling.
19-25: Verify the empty array parameter in KeysClauseThe empty array as the first parameter of KeysClause seems suspicious. Please verify if this is intentional or if specific keys should be provided.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for other KeysClause usage patterns in the codebase rg "KeysClause\(" -A 3Length of output: 5449
Empty KeysClause Keys Array is Intentional
After scanning the codebase, we found that all other usage sites of KeysClause (e.g., in App.tsx, theme-switch.tsx, chat.tsx, etc.) pass in non-empty arrays of keys. The historical-events.tsx file is the sole exception where an empty array is used for the first parameter. This suggests that for historical events—likely due to the nature of the query filtering—no static keys are required. The empty array appears to be an intentional design decision rather than an oversight.
examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (1)
2-3: 🌐 Web query:What are the recommended chain configurations for local Starknet development with Katana?💡 Result:
For local Starknet development with Katana, these configurations are recommended based on current practices: ## Core Node Configuration Start Katana with these parameters for basic local development: ``` katana --accounts 3 --seed 0 --gas-price 250 ``` This creates: - 3 pre-funded developer accounts[1][8] - Deterministic address generation via seed[1][4] - Fixed gas pricing at 250 wei[1] ## Environment Setup Essential environment variables in `.env`: ```bash export STARKNET_ACCOUNT=katana-0 # Default funded account[2][8] export STARKNET_RPC=http://0.0.0.0:5050 # Local node endpoint[2][8] export STARKNET_KEYSTORE=~/.katana/keystore.json # Key storage[1] ``` ## Contract Development Configuration 1. **Scarb Project Setup** (`Scarb.toml`): ```toml [dependencies] starknet = ">=2.3.0" [[target.starknet-contract]] ``` 2. **Build Process**: ```bash scarb build # Compiles Cairo contracts[2][8] ``` ## Deployment Workflow 1. **Declaration**: ```bash starkli declare target/dev/contract.sierra.json \ --compiler-version=2.1.0 \ --rpc $STARKNET_RPC \ --account $STARKNET_ACCOUNT[1][4] ``` 2. **Deployment**: ```bash starkli deploy <CLASS_HASH> \ --salt 0x1234 \ --account $STARKNET_ACCOUNT[5][8] ``` ## Advanced Configurations For cross-chain testing with Ethereum: ```bash katana --messaging anvil.messaging.json # Enables L1<>L2 messaging[5] ``` Requires parallel Anvil Ethereum node setup[5]. ## Recommended Tool Versions - Scarb: ≥2.6.3[5] - Starkli: Dojo-maintained versions[5][8] - Cairo: 2.x series[1][4] These configurations balance simplicity with production-like environment simulation while maintaining deterministic behavior for testing[1][4][8]. Citations: - 1: https://livesoftwaredeveloper.com/articles/8/smart-contract-development-with-starknet-cairo-language-with-katana-local-node - 2: https://github.com/dojoengine/book/blob/main/docs/pages/tutorial/katana-starkli-scarb/main.md - 3: https://book.dojoengine.org/toolchain/katana/messaging - 4: https://docs.starknet.io/quick-start/devnet/ - 5: https://github.com/glihm/starknet-messaging-dev - 6: https://livesoftwaredeveloper.com/articles/9/how-to-set-up-starknet-devnet-and-frontend-for-smart-contract-development - 7: https://chainide.gitbook.io/chainide-english-1/ethereum-ide-1/15.-starknet-ide/2.-how-to-use-starknet-ide - 8: https://book.starknet.io/zh-cn/ch02-01-introduction-starkli-scarb-katana.html - 9: https://hackmd.io/@sambarnes/By7kitOCt - 10: https://book.dojoengine.org/toolchain/katana/interactexamples/example-vite-kitchen-sink/src/components/global-counter.tsx (2)
4-9: Acknowledgment of newly introduced imports.
These newly imported modules align well with the subscription-based logic below.
78-84: Logic for handling absent or hex-based counter values is clear.
The code correctly defaults to 0 and parses the remainder as a hex integer.packages/sdk/src/__example__/index.ts (2)
4-10: Appropriate reorganization of imports.
Leveraging these specific clauses and builders improves the consistency of query-building patterns throughout the SDK.
118-123: Subscription query looks well-formed.
Including hashed keys ensures compatibility with Torii's subscription mechanism.examples/example-vite-kitchen-sink/src/components/chat.tsx (1)
10-15: Introduction of Torii-centric imports is consistent.
These utilities support the enhanced subscription approach.examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (3)
1-1: No issues found for the newly imported React hooks.
This line looks good, and the import suits this component’s usage of state, effects, and callbacks.
5-10: Imports are aligned with the new Torii-based querying approach.
All new imports (KeysClause, ParsedEntity, SDK, ToriiQueryBuilder) are relevant to the updated subscription logic. Ensure that any unused imports, if introduced in future updates, are pruned to maintain a clean import list.
54-54: Good catch removing unused destructured properties from the account.
Previously, you might have included additional fields like address. Now that only “account” is needed, the destructuring is consistent and concise.examples/example-vite-react-sdk/src/App.tsx (4)
2-2: Imports now reflect the Torii-based KeysClause approach.
Switching from older QueryBuilder to ToriiQueryBuilder and KeysClause is consistent with the new subscription model throughout the codebase.
6-6: ModelsMapping usage is consistent with new type references.
This helps keep the code aligned with the updated schema references. Ensure that references to ModelsMapping remain valid if the schema evolves.
58-69: Efficient subscription initialization with immediate data assignment.
The code destructures “[initialData, subscription]” and updates the state with “initialData” before setting up the subscription callback. This approach avoids a potential race condition. Ensure that “initialData” is properly validated or checked before storing in state, especially if the array might be empty.
92-92: Dependency array includes updated 'state'.
Ensuring the effect triggers when 'state' changes is important, but be aware of potential over-triggering if 'state' is updated frequently. If performance issues arise, you might consider more granular effect dependencies.packages/sdk/src/index.ts (6)
4-16: Consolidation of types and utilities.
Replacing multiple function imports (e.g., getEntities, getEventMessages) with parse-based utilities is a clean design that centralizes parsing logic. This helps reduce duplication but requires consistent usage across the codebase.
51-53: Accurate TypeScript doc comments.
You are using TSDoc-style comments with type parameters (SubscribeParams<T, false>). Looks good, but ensure that the comments remain consistent if you refactor.
54-66: Subscription logic properly enforces hashed key inclusion.
The new check ensures that developers don't accidentally subscribe without specifying entity IDs. This is a crucial safeguard.Also applies to: 67-91
106-158: Robust error handling in subscribeEventQuery.
By throwing errors if hashed keys aren’t included, the code encourages best practice for event subscriptions. The catch block logs and rethrows the error in callback form, which is consistent with the entity subscription approach.
162-164: Updated getEntities signature is consistent with the new query building approach.
The parseEntities call ensures a standardized result, removing the need to handle raw query data in usage sites.
172-190: Unified path for getEventMessages with optional historical data.
Converging historical vs. current event logic under one method is a clean approach. The dynamic cast to ToriiResponse<T, Historical> can be a bit tricky to maintain, so keep an eye on type safety.packages/sdk/src/types.ts (13)
5-5: New import for ToriiQueryBuilder.The import from "./toriiQueryBuilder" appears correct. Ensure that any references to the deprecated QueryBuilder are replaced throughout the code.
261-265: Good introduction of SubscribeResponse type.This tuple-based response structure clearly captures both the returned data and the subscription handle. Make sure to handle the possibility of errors or undefined data gracefully in related subscripting code paths.
281-282: Updated doc comment for subscribeEntityQuery.The doc comment now correctly references SubscribeResponse<T, false>. Nice job keeping documentation consistent.
293-294: Documentation improvement for Historical usage.Including the Historical parameter in the doc comment clarifies the query’s optional retrospective scope.
295-300: Refreshed method signature for subscribeEventQuery.The new signature supports flexible historical data queries. Confirm any invocation sites are updated to pass the correct Historical parameter.
305-306: Doc comment refined for getEntities.The reference to GetParams<T, false> now aligns accurately with the new type definitions.
307-308: Return type updated to ToriiResponse<T, false>.This unifies the response type for getEntities. Ensure existing code expecting a different structure is migrated.
315-316: Historical documentation consistency.Including the Historical parameter in getEventMessages doc comment clarifies retroactive event retrieval.
317-318: getEventMessages method signature.The method now supports both real-time and historical event messages. Good approach to unify these use cases, but confirm no breakage in existing usage.
Also applies to: 319-320
411-423: Revamped SubscribeParams structure.• The use of ToriiQueryBuilder ensures consistent query building.
• The inclusion of a historical flag is useful for toggling retrospective data retrieval.
• The typed callback with ToriiResponse<T, Historical> clarifies expected data shape.Overall: Looks well-organized and user-friendly.
426-433: Refactored GetParams interface.• Replacing older query references with ToriiQueryBuilder streamlines the approach.
• The optional historical parameter aligns with the rest of the subscription logic.Good work ensuring consistency across both subscription and get operations.
285-286: Verify downstream calls for updated subscribeEntityQuery signature.The signature changed to accept SubscribeParams<T, false> and return SubscribeResponse<T, false>. Ensure all internal or external callers follow this new type contract.
✅ Verification successful
🏁 Script executed:
#!/bin/bash ast-grep --pattern $'subscribeEntityQuery($_)'Length of output: 47
I'm going to gather more evidence of how and where subscribeEntityQuery is called. I'll run a script to search for its usage in the codebase using rg with some context for better clarity.
🏁 Script executed:
#!/bin/bash rg "subscribeEntityQuery\(" --context 3Length of output: 5624
Downstream Usage of subscribeEntityQuery Matches the New Contract
All inspected call sites (from examples and SDK usage) are using subscribeEntityQuery consistent with the updated signature (accepting SubscribeParams<T, false> and returning SubscribeResponse<T, false>). No violations or mismatches were found based on the current static analysis.
254-260: Validate double array for the historical case.When Historical is true, ToriiResponse<T, true> becomes an array of arrays, since StandardizedQueryResult is itself an array. Verify this design choice is intentional and works with the rest of the codebase.
Run the following script to locate all references to the ToriiResponse type, confirming no conflicting expectations:
✅ Verification successful
🏁 Script executed:
#!/bin/bash ast-grep --pattern $'ToriiResponse<$_, $_>'Length of output: 44
🏁 Script executed:
#!/bin/bash echo "Searching for the definition of StandardizedQueryResult..." rg "export type StandardizedQueryResult" -n . echo "Searching for any references to StandardizedQueryResult in the codebase..." rg "StandardizedQueryResult" -n . echo "Searching for all usage references of ToriiResponse..." rg "ToriiResponse<" -n .Length of output: 5370
Double Array for Historical ToriiResponse is Intentional
The shell script search confirms that StandardizedQueryResult is defined as an array and that ToriiResponse is used consistently across the codebase. When Historical is true, ToriiResponse<T, true> indeed becomes an array of StandardizedQueryResult (i.e. a double array), and this design is reflected in functions like getEventMessages and index.ts. There’s no evidence of conflicting expectations, so the design choice appears intentional.
packages/sdk/src/convertClauseToEntityKeysClause.ts (2)
2-2: Switched from StandardizedQueryResult to ToriiResponse import.This aligns with the broader shift to ToriiResponse in the SDK. Ensure all references or destructuring rely on ToriiResponse’s data shape.
6-6: Updated initialData to ToriiResponse<T, false>.Because ToriiResponse<T, false> is an array of entities by default, verify that mapping over initialData still produces the correct hashed keys for your logic.
examples/example-vite-kitchen-sink/src/main.tsx (2)
13-13: Removed direct schema import.The new SDK initialization approach no longer requires the schema argument. Make sure any references to the old schema import have been removed for clarity.
18-31: Initialization call streamlined by removing the schema parameter.• The rest of the configuration block is preserved, simplifying client usage.
• Confirm that any features previously relying on the schema are now updated to use the new query builder approach.Overall, this change reduces complexity and potential confusion. Good cleanup.
examples/example-vite-react-sdk/src/main.tsx (1)
9-9: LGTM! SDK initialization updated to use type-safe approach.The changes correctly implement the transition to ToriiQueryBuilder by:
- Using SchemaType generic for type safety
- Removing the schema parameter from init
Also applies to: 23-36
examples/example-vite-react-sql/src/main.tsx (1)
11-11: LGTM! Consistent SDK initialization pattern.The changes mirror those in
example-vite-react-sdk/src/main.tsx, maintaining consistency across examples.Also applies to: 34-47
examples/example-vite-react-sql/src/hooks/usePlayerActions.ts (1)
3-3: LGTM! Updated imports for new query mechanism.Correctly imports ToriiQueryBuilder and KeysClause for the new query mechanism.
Also applies to: 6-6
examples/example-vite-kitchen-sink/src/components/caller-counter.tsx (1)
3-8: LGTM! Updated imports for ToriiQueryBuilder.Correctly imports required components for the new query mechanism.
examples/example-vite-kitchen-sink/package.json (10)
14-14: Add @dojoengine/sdk dependency as the new default.
The new dependency on "@dojoengine/sdk" (set to "workspace:*") is now included as part of the transition away from the legacy QueryBuilder. Ensure that all related code uses the updated API from this package.
17-24: Update Radix UI package versions.
The versions for several "@radix-ui" packages have been bumped (e.g., react-dialog from "^1.1.1" to "^1.1.6", react-dropdown-menu from "^2.1.1" to "^2.1.6", etc.). These updates are consistent and help modernize the dependency set.
29-29: Bump class-variance-authority to "^0.7.1".
The updated version here appears to be in line with the overall dependency modernization effort. Verify that no breaking changes have been introduced by this minor version bump.
31-32: Upgrade dotenv and jiti versions.
The new versions for "dotenv" and "jiti" (now "^16.4.7" and "^1.21.7", respectively) ensure that the project stays current with security and performance improvements.
37-37: Upgrade react-hook-form to "^7.54.2".
This change appears consistent with the overall upgrade of dependencies. Make sure the API changes (if any) in react-hook-form are handled in the consuming code.
39-39: Bump tailwind-merge version.
Updating "tailwind-merge" to "^2.6.0" is a straightforward dependency upgrade; just be cautious of any minor differences in behavior.
42-42: Upgrade zod to "^3.24.1".
The updated version of zod is applied correctly. As always, check if any validation schemas or error messages are affected by the upgrade.
45-48: Update dev dependency versions for TypeScript and React.
The changes in "@types/node", "@types/react", "@types/react-dom", and "@vitejs/plugin-react" contribute to a more modern development setup. Ensure that these versions are compatible with your code base and the new SDK changes.
51-54: Modernize build tools by updating postcss, tailwindcss, typescript, and vite.
The updated versions in these tools (e.g., "tailwindcss" to "^3.4.17", "typescript" to "^5.7.3", and "vite" to "^6.1.0") should yield performance and maintenance improvements. Be sure to run a full build after upgrading to catch any incompatibilities in the configuration.
57-57: Upgrade vite-plugin-wasm to "^3.4.1".
This update is consistent with the other development dependency bumps. Confirm that any usage of WebAssembly remains fully functional with the upgraded plugin.readme.md (7)
21-22: Add Official Documentation link in the Table of Contents.
The addition of the [Official documentation] link provides users with quick access to the latest SDK docs. This improves discoverability and clarity in onboarding new users.
44-45: Update Katana node start command.
The command change from using disabled fee flags to the new "--dev --http.api dev,starknet --dev.no-fee --http.cors_origins '*'" format reflects updates in the backend configuration. Verify that the new flags work correctly in all target environments.
54-57: Revise Torii indexer commands.
Updating the Torii indexer start command (now using "--http.cors_origins" instead of "--allowed-origins") and adding a new command for historical events makes the command structure clearer and more consistent.
82-83: Update Dojo SDK client initialization import.
The new import statement "import { init } from '@dojoengine/sdk';" is necessary for the updated initialization process. Ensure that any references in other documentation are updated accordingly.
106-117: Adopt the new ToriiQueryBuilder for querying entities.
The updated query snippet using "new ToriiQueryBuilder()" along with the chained clause (via MemberClause.build()) reflects the deprecation of the legacy QueryBuilder. Confirm that "MemberClause" is correctly imported or defined in this context.
194-197: Update build watch command for Terminal 3.
The revised command "pnpm run build --watch" is clear and helpful as a development convenience. Ensure that users have the necessary context if any build configuration changes accompany this update.
275-278: Simplify Dojo creation command.
The change to "npx @dojoengine/create-dojo" for creating a new Dojo project is consistent with the updated developer experience. This simplification should reduce friction when onboarding new users.
| }, | ||
| historical: true, | ||
| }); | ||
| setEvents(e as unknown as ParsedEntity<SchemaType>[]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid unsafe type casting
The double type casting through unknown is a code smell and could hide type-related issues.
Consider using proper type guards or adjusting the SDK types:
-setEvents(e as unknown as ParsedEntity<SchemaType>[]);
+setEvents(Array.isArray(e) ? e.filter((item): item is ParsedEntity<SchemaType> => {
+ return item !== null && typeof item === 'object';
+}) : []);📝 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.
| setEvents(e as unknown as ParsedEntity<SchemaType>[]); | |
| setEvents(Array.isArray(e) ? e.filter((item): item is ParsedEntity<SchemaType> => { | |
| return item !== null && typeof item === 'object'; | |
| }) : []); |
| const provider = jsonRpcProvider({ | ||
| rpc: (chain: Chain) => ({ nodeUrl: getRpcUrl() }), | ||
| rpc: () => ({ nodeUrl: getRpcUrl() }), | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for RPC configuration.
The RPC configuration should include error handling in case getRpcUrl() fails or returns an invalid URL.
const provider = jsonRpcProvider({
- rpc: () => ({ nodeUrl: getRpcUrl() }),
+ rpc: () => {
+ const url = getRpcUrl();
+ if (!url) {
+ throw new Error('Invalid RPC URL configuration');
+ }
+ return { nodeUrl: url };
+ },
});📝 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 provider = jsonRpcProvider({ | |
| rpc: (chain: Chain) => ({ nodeUrl: getRpcUrl() }), | |
| rpc: () => ({ nodeUrl: getRpcUrl() }), | |
| }); | |
| const provider = jsonRpcProvider({ | |
| rpc: () => { | |
| const url = getRpcUrl(); | |
| if (!url) { | |
| throw new Error('Invalid RPC URL configuration'); | |
| } | |
| return { nodeUrl: url }; | |
| }, | |
| }); |
| const theme = | ||
| initialTheme[0].models?.onchain_dash?.Theme?.value?.unwrap(); | ||
| setEntityId(initialTheme[0].entityId); | ||
| let th = null; | ||
| if (undefined === theme) { | ||
| th = AvailableTheme.Light; | ||
| } else { | ||
| th = AvailableTheme[theme]; | ||
| } | ||
|
|
||
| setTheme({ | ||
| // @ts-expect-error this is ok | ||
| current: AvailableThemeClassMap[th] as string, | ||
| // @ts-expect-error this is ok | ||
| next: AvailableThemeClassMap[th] as string, | ||
| }); |
There was a problem hiding this comment.
Guard against potential out-of-bounds error on initialTheme[0].
If “initialTheme” is empty, accessing “initialTheme[0]” could lead to a runtime error. Ensure that the array has elements before indexing.
+ if (!initialTheme.length) {
+ // handle empty scenario: perhaps default to Light or skip
+ return;
+ }
const theme = initialTheme[0].models?.onchain_dash?.Theme?.value?.unwrap();
setEntityId(initialTheme[0].entityId);
let th = null;
...📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const theme = | |
| initialTheme[0].models?.onchain_dash?.Theme?.value?.unwrap(); | |
| setEntityId(initialTheme[0].entityId); | |
| let th = null; | |
| if (undefined === theme) { | |
| th = AvailableTheme.Light; | |
| } else { | |
| th = AvailableTheme[theme]; | |
| } | |
| setTheme({ | |
| // @ts-expect-error this is ok | |
| current: AvailableThemeClassMap[th] as string, | |
| // @ts-expect-error this is ok | |
| next: AvailableThemeClassMap[th] as string, | |
| }); | |
| if (!initialTheme.length) { | |
| // handle empty scenario: perhaps default to Light or skip | |
| return; | |
| } | |
| const theme = | |
| initialTheme[0].models?.onchain_dash?.Theme?.value?.unwrap(); | |
| setEntityId(initialTheme[0].entityId); | |
| let th = null; | |
| if (undefined === theme) { | |
| th = AvailableTheme.Light; | |
| } else { | |
| th = AvailableTheme[theme]; | |
| } | |
| setTheme({ | |
| // @ts-expect-error this is ok | |
| current: AvailableThemeClassMap[th] as string, | |
| // @ts-expect-error this is ok | |
| next: AvailableThemeClassMap[th] as string, | |
| }); |
MartianGreed
force-pushed
the
feat/deprecate-qb
branch
from
d8aa0b6 to
27d0342
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (2)
examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (1)
18-20:⚠️ Potential issueAdd error handling for RPC configuration.
The RPC configuration should include error handling in case
getRpcUrl()fails or returns an invalid URL.const provider = jsonRpcProvider({ - rpc: () => ({ nodeUrl: getRpcUrl() }), + rpc: () => { + const url = getRpcUrl(); + if (!url) { + throw new Error('Invalid RPC URL configuration'); + } + return { nodeUrl: url }; + }, });examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (1)
122-137:⚠️ Potential issueGuard against potential out-of-bounds error on initialTheme[0].
If "initialTheme" is empty, accessing "initialTheme[0]" could lead to a runtime error. Ensure that the array has elements before indexing.
Apply this diff to add the necessary check:
+ if (!initialTheme.length) { + setTheme({ + current: AvailableThemeClassMap[AvailableTheme.Light], + next: AvailableThemeClassMap[AvailableTheme.Light], + }); + return; + } const theme = initialTheme[0].models?.onchain_dash?.Theme?.value?.unwrap(); setEntityId(initialTheme[0].entityId); let th = null; if (undefined === theme) { th = AvailableTheme.Light; } else { th = AvailableTheme[theme]; } setTheme({ // @ts-expect-error this is ok current: AvailableThemeClassMap[th] as string, // @ts-expect-error this is ok next: AvailableThemeClassMap[th] as string, });
🧹 Nitpick comments (7)
examples/example-vite-react-sql/src/hooks/usePlayerActions.ts (1)
24-33: Consider adding error handling for the subscription setup.While the query construction is correct, there's no error handling for the subscription setup itself.
const [entities, subscription] = await sdk.subscribeEntityQuery({ + }).catch(error => { + console.error("Failed to setup subscription:", error); + return [[], { cancel: () => {} }]; });examples/example-vite-react-sdk/src/historical-events.tsx (1)
27-32: Improve error handling in the callback.The callback could benefit from more detailed error logging and recovery mechanisms.
if (error) { - console.error(error); + console.error("Historical event subscription error:", error); + // Consider retrying the subscription after a delay + setTimeout(() => subscribeHistoricalEvent(account), 5000); }examples/example-vite-kitchen-sink/src/components/global-counter.tsx (1)
49-74: Consider simplifying the nested conditional logic.The callback contains multiple nested conditionals that could be simplified using early returns.
callback: ({ data, error }) => { + if (error) throw error; + if (!data) return; + const entity = data.pop() as ParsedEntity<SchemaType>; - if (!entity) { - return; - } - if ( - entity.models.onchain_dash?.GlobalCounter - ?.counter === undefined - ) { - return; - } + if (!entity || entity.models.onchain_dash?.GlobalCounter?.counter === undefined) return; const count = entity.models.onchain_dash?.GlobalCounter?.counter; - if (undefined === count) { - return 0; - } + if (count === undefined) return; const value = parseInt(count.toString(), 16); setCount(value); setIsLoading(false); - return; }packages/sdk/src/__example__/index.ts (1)
117-139: Add retry logic for subscription errors.The subscription error handling could benefit from retry logic with exponential backoff.
callback: (resp) => { if (resp.error) { console.error( "Error querying todos and goals:", resp.error.message ); + // Add retry logic with exponential backoff + setTimeout(() => { + console.log("Retrying subscription..."); + // Re-subscribe logic here + }, 5000); return; }packages/sdk/src/types.ts (2)
292-293: Fix typo in JSDoc comment.The word "Whether" is misspelled as "Wether" in the JSDoc comment.
- * @template Historical - Wether to include historical events or not. + * @template Historical - Whether to include historical events or not.
399-403: Fix typo in JSDoc comment.The word "Whether" is misspelled as "Wether" and "subscription" is misspelled as "subscdription" in the JSDoc comment.
- * Wether to include logger in queries and subscdription. + * Whether to include logger in queries and subscription.packages/sdk/src/index.ts (1)
54-92: Enhance error message for better clarity.The error message for missing entity IDs could be more descriptive to help users understand the requirement better.
- "For subscription, you need to include entity ids" + "Entity IDs are required for subscriptions. Please include them in the query using the Keys clause."
📜 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 (32)
.changeset/tidy-toys-compare.md(1 hunks)examples/example-vite-experimental-sdk/tsconfig.json(1 hunks)examples/example-vite-kitchen-sink/package.json(1 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/starknet-provider.tsx(2 hunks)examples/example-vite-kitchen-sink/src/components/theme-switch.tsx(4 hunks)examples/example-vite-kitchen-sink/src/main.tsx(1 hunks)examples/example-vite-kitchen-sink/tsconfig.json(1 hunks)examples/example-vite-kitchen-sink/vite.config.ts(1 hunks)examples/example-vite-react-sdk/src/App.tsx(3 hunks)examples/example-vite-react-sdk/src/historical-events.tsx(3 hunks)examples/example-vite-react-sdk/src/main.tsx(2 hunks)examples/example-vite-react-sql/src/dojo-sdk-provider.tsx(0 hunks)examples/example-vite-react-sql/src/hooks/usePlayerActions.ts(3 hunks)examples/example-vite-react-sql/src/main.tsx(2 hunks)examples/example-vite-react-sql/tsconfig.app.tsbuildinfo(1 hunks)packages/sdk/src/__example__/index.ts(4 hunks)packages/sdk/src/__tests__/convertClauseToEntityKeysClause.test.ts(1 hunks)packages/sdk/src/__tests__/queryBuilder.test.ts(1 hunks)packages/sdk/src/convertClauseToEntityKeysClause.ts(1 hunks)packages/sdk/src/experimental/index.ts(1 hunks)packages/sdk/src/getEntities.ts(0 hunks)packages/sdk/src/getEventMessages.ts(0 hunks)packages/sdk/src/index.ts(3 hunks)packages/sdk/src/parseHistoricalEvents.ts(0 hunks)packages/sdk/src/queryBuilder.ts(3 hunks)packages/sdk/src/subscribeEntityQuery.ts(0 hunks)packages/sdk/src/subscribeEventQuery.ts(0 hunks)packages/sdk/src/types.ts(4 hunks)readme.md(11 hunks)
💤 Files with no reviewable changes (6)
- packages/sdk/src/parseHistoricalEvents.ts
- packages/sdk/src/subscribeEntityQuery.ts
- packages/sdk/src/subscribeEventQuery.ts
- packages/sdk/src/getEventMessages.ts
- packages/sdk/src/getEntities.ts
- examples/example-vite-react-sql/src/dojo-sdk-provider.tsx
🚧 Files skipped from review as they are similar to previous changes (14)
- packages/sdk/src/tests/queryBuilder.test.ts
- packages/sdk/src/experimental/index.ts
- examples/example-vite-experimental-sdk/tsconfig.json
- examples/example-vite-react-sql/tsconfig.app.tsbuildinfo
- examples/example-vite-react-sdk/src/main.tsx
- packages/sdk/src/tests/convertClauseToEntityKeysClause.test.ts
- packages/sdk/src/queryBuilder.ts
- examples/example-vite-react-sql/src/main.tsx
- .changeset/tidy-toys-compare.md
- examples/example-vite-kitchen-sink/src/main.tsx
- packages/sdk/src/convertClauseToEntityKeysClause.ts
- examples/example-vite-kitchen-sink/vite.config.ts
- readme.md
- examples/example-vite-kitchen-sink/package.json
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (21)
examples/example-vite-kitchen-sink/tsconfig.json (6)
3-6: Modern TS Targeting and Module Configuration Update.
The updated "target" (ES2020) and "lib" array now ensure that the compiler leverages modern JavaScript features while explicitly including support for DOM and iterable interfaces. The addition of "useDefineForClassFields": true promotes a more predictable class field initialization. Make sure these choices align with your supported runtime environments.
8-9: Bundler Mode Comment Clarity.
The comment "/* Bundler mode */" provides useful context for developers to understand why "moduleResolution" is set to "bundler". It clarifies the intended build configuration and aids readability.
10-10: Allow Importing TS Extensions.
Introducing "allowImportingTsExtensions": true makes it easier to handle imports with TypeScript-specific extensions, which can improve module resolution clarity when mixing JS and TS files.
12-14: Enhanced Module Detection and JSX Transformation.
The new "moduleDetection": "force" ensures modules are correctly identified during compilation. Setting "noEmit" to true avoids accidental file generation during type-checking, and "jsx": "react-jsx" efficiently enables the new JSX transform for React projects. Verify that these options integrate seamlessly with your build and development workflows.
15-20: Strict Linting and Module Resolution Enhancements.
The reintroduction of "strict" alongside "noUnusedLocals", "noUnusedParameters", and "noFallthroughCasesInSwitch" reinforces code quality through enhanced linting. Also, specifying "baseUrl": "." standardizes module resolution across the project. These changes promote robust standards but ensure that existing code adheres to the stricter checks.
25-25: Narrowed File Inclusion Scope.
Limiting the "include" option to ["src"] helps avoid processing unnecessary files, thereby possibly improving compilation performance. Confirm that all relevant TypeScript files reside within the "src" directory.examples/example-vite-kitchen-sink/src/components/starknet-provider.tsx (1)
2-3: LGTM!The imports are correctly structured and use appropriate modules from the Starknet React ecosystem.
examples/example-vite-react-sql/src/hooks/usePlayerActions.ts (2)
3-3: LGTM! Imports are correctly updated for the new query builder.The imports have been properly updated to include the new
ToriiQueryBuilder,KeysClause, and generatedModelsMapping.Also applies to: 6-6
45-45: 🏁 Script executed:#!/bin/bash # Search for similar patterns in the codebase to verify if this is a common pattern rg -A 5 "setEntities.*subscription"Length of output: 37
examples/example-vite-react-sdk/src/historical-events.tsx (2)
36-36: Avoid unsafe type casting.The double type casting through
unknownis a code smell and could hide type-related issues.
18-25: 🏁 Script executed:#!/bin/bash # Search for other KeysClause usages to verify the pattern rg -A 3 "KeysClause\(\[.*\],"Length of output: 31
examples/example-vite-kitchen-sink/src/components/global-counter.tsx (1)
78-84: LGTM! Initial count handling is correct.The initial count handling properly checks for undefined values and parses the hex string.
packages/sdk/src/__example__/index.ts (1)
143-151: LGTM! Complex query composition is well structured.The nested
AndComposeClausestructure is clear and maintainable.examples/example-vite-kitchen-sink/src/components/caller-counter.tsx (1)
46-56: LGTM! Clean implementation of ToriiQueryBuilder.The subscription query is well-structured with proper error handling and type safety.
examples/example-vite-kitchen-sink/src/components/chat.tsx (1)
71-81: LGTM! Well-structured subscription query.The subscription query is properly configured with KeysClause and includes hashed keys.
examples/example-vite-kitchen-sink/src/components/theme-switch.tsx (1)
78-87: LGTM! Clean implementation of ToriiQueryBuilder.The subscription query is well-structured with proper error handling and type safety.
examples/example-vite-react-sdk/src/App.tsx (1)
58-69: LGTM! Well-structured subscription query with proper error handling.The subscription query is properly configured with KeysClause and includes hashed keys. The error handling is comprehensive.
packages/sdk/src/types.ts (2)
254-265: LGTM! Well-structured type definitions for handling historical events.The conditional types
ToriiResponseandSubscribeResponseprovide excellent type safety by distinguishing between historical and non-historical responses.
411-434: LGTM! Well-structured interfaces with improved type safety.The updated interfaces properly handle historical events and align with the transition to ToriiQueryBuilder.
packages/sdk/src/index.ts (2)
99-158: Address @ts-expect-error comment.The code contains a technical debt marker that should be addressed to ensure type safety.
Could you provide more context about the type mismatch that's causing the need for
@ts-expect-erroron line 128? This would help in fixing the underlying type issue.
165-190: LGTM! Clean and consistent implementations.The getEntities and getEventMessages implementations are well-structured and properly handle historical events while maintaining consistency with the new type system.
| }); | ||
|
|
||
| return ( | ||
| <StarknetConfig | ||
| chains={[mainnet]} | ||
| provider={provider} | ||
| // @ts-expect-error this is ok | ||
| connectors={[cartridge, ...pa]} | ||
| connectors={[...pa]} |
There was a problem hiding this comment.
Handle potential race condition with predeployed accounts.
The pa array is populated asynchronously, but it's used directly in the StarknetConfig without waiting for the Promise to resolve. This could lead to an empty connectors array if the component renders before the accounts are loaded.
Consider using React state to handle the async initialization:
export default function StarknetProvider({ children }: PropsWithChildren) {
+ const [accounts, setAccounts] = useState<PredeployedAccountsConnector[]>([]);
+
+ useEffect(() => {
+ predeployedAccounts({
+ rpc: env.VITE_CONTROLLER_RPC,
+ id: "katana",
+ name: "Katana",
+ }).then(setAccounts);
+ }, []);
+
const provider = jsonRpcProvider({
rpc: () => ({ nodeUrl: getRpcUrl() }),
});
return (
<StarknetConfig
chains={[mainnet]}
provider={provider}
- connectors={[...pa]}
+ connectors={accounts}
explorer={voyager}
autoConnect
>📝 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.
| connectors={[...pa]} | |
| import { useState, useEffect, PropsWithChildren } from 'react'; | |
| import { jsonRpcProvider } from 'some-provider-package'; // adjust the import as needed | |
| import { StarknetConfig } from 'some-starknet-package'; // adjust the import as needed | |
| import { mainnet, voyager } from 'some-chain-config'; // adjust the import as needed | |
| import { getRpcUrl } from 'some-getRpcUrl-module'; // adjust the import as needed | |
| import { predeployedAccounts, PredeployedAccountsConnector } from 'some-accounts-module'; // adjust the import as needed | |
| import { env } from 'some-env-module'; // adjust the import as needed | |
| export default function StarknetProvider({ children }: PropsWithChildren) { | |
| const [accounts, setAccounts] = useState<PredeployedAccountsConnector[]>([]); | |
| useEffect(() => { | |
| predeployedAccounts({ | |
| rpc: env.VITE_CONTROLLER_RPC, | |
| id: "katana", | |
| name: "Katana", | |
| }).then(setAccounts); | |
| }, []); | |
| const provider = jsonRpcProvider({ | |
| rpc: () => ({ nodeUrl: getRpcUrl() }), | |
| }); | |
| return ( | |
| <StarknetConfig | |
| chains={[mainnet]} | |
| provider={provider} | |
| connectors={accounts} | |
| explorer={voyager} | |
| autoConnect | |
| > | |
| {children} | |
| </StarknetConfig> | |
| ); | |
| } |
| query: new ToriiQueryBuilder() | ||
| .withClause( | ||
| KeysClause( | ||
| ["onchain_dash-GlobalCounter"], | ||
| ["9999999"], | ||
| "VariableLen" | ||
| ).build() | ||
| ) | ||
| .build(), | ||
| .includeHashedKeys(), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the hardcoded values in KeysClause.
The hardcoded values "onchain_dash-GlobalCounter" and "9999999" should be constants or configuration values.
🏁 Script executed:
#!/bin/bash
# Search for other occurrences of these hardcoded values
rg "9999999|onchain_dash-GlobalCounter"Length of output: 812
Hardcoded Value Extraction Recommended
The search confirms that the literal values "onchain_dash-GlobalCounter" and "9999999" are used in multiple parts of the codebase:
- In the auto‑generated models file (models.gen.ts)
- In the global-counter.tsx and theme-switch.tsx components
- In test files (parseEntities.test.ts)
Given their repeated usage, it would be beneficial to extract these as constants or configuration values to improve maintainability and consistency.
| const db = await init<MockSchemaType>({ | ||
| client: { | ||
| rpcUrl: "your-rpc-url", | ||
| toriiUrl: "your-torii-url", | ||
| relayUrl: "your-relay-url", | ||
| worldAddress: "your-world-address", | ||
| }, | ||
| schema | ||
| ); | ||
| domain: { | ||
| name: "Example", | ||
| version: "1.0", | ||
| chainId: "your-chain-id", | ||
| revision: "1", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using environment variables for configuration.
The hardcoded URLs and addresses should be replaced with environment variables.
const db = await init<MockSchemaType>({
client: {
- rpcUrl: "your-rpc-url",
- toriiUrl: "your-torii-url",
- relayUrl: "your-relay-url",
- worldAddress: "your-world-address",
+ rpcUrl: process.env.RPC_URL,
+ toriiUrl: process.env.TORII_URL,
+ relayUrl: process.env.RELAY_URL,
+ worldAddress: process.env.WORLD_ADDRESS,
},📝 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 db = await init<MockSchemaType>({ | |
| client: { | |
| rpcUrl: "your-rpc-url", | |
| toriiUrl: "your-torii-url", | |
| relayUrl: "your-relay-url", | |
| worldAddress: "your-world-address", | |
| }, | |
| schema | |
| ); | |
| domain: { | |
| name: "Example", | |
| version: "1.0", | |
| chainId: "your-chain-id", | |
| revision: "1", | |
| }, | |
| }); | |
| const db = await init<MockSchemaType>({ | |
| client: { | |
| rpcUrl: process.env.RPC_URL, | |
| toriiUrl: process.env.TORII_URL, | |
| relayUrl: process.env.RELAY_URL, | |
| worldAddress: process.env.WORLD_ADDRESS, | |
| }, | |
| domain: { | |
| name: "Example", | |
| version: "1.0", | |
| chainId: "your-chain-id", | |
| revision: "1", | |
| }, | |
| }); |
| const count = | ||
| initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter; | ||
| if (!count) { | ||
| setCount(0); | ||
| } else { | ||
| setCount(parseInt(count.toString(), 16)); | ||
| } |
There was a problem hiding this comment.
Add null check for initialEntities[0].
The code directly accesses initialEntities[0] without checking if the array is empty, which could lead to runtime errors.
Apply this diff to add the necessary check:
+ if (!initialEntities.length) {
+ setCount(0);
+ return;
+ }
const count =
initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter;
if (!count) {
setCount(0);
} else {
setCount(parseInt(count.toString(), 16));
}📝 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 count = | |
| initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter; | |
| if (!count) { | |
| setCount(0); | |
| } else { | |
| setCount(parseInt(count.toString(), 16)); | |
| } | |
| if (!initialEntities.length) { | |
| setCount(0); | |
| return; | |
| } | |
| const count = | |
| initialEntities[0]?.models.onchain_dash?.CallerCounter?.counter; | |
| if (!count) { | |
| setCount(0); | |
| } else { | |
| setCount(parseInt(count.toString(), 16)); | |
| } |
| setMessages( | ||
| initialMessages | ||
| .map((e) => e.models.onchain_dash.Message as MessageItem) | ||
| .filter(Boolean) | ||
| .sort((a: Message, b: Message): number => | ||
| parseInt(a.timestamp.toString(), 16) < | ||
| parseInt(b.timestamp.toString(), 16) | ||
| ? -1 | ||
| : 1 | ||
| ) | ||
| ); |
There was a problem hiding this comment.
Add null check for initialMessages.
The code directly processes initialMessages without checking if it's empty, which could lead to runtime errors.
Apply this diff to add the necessary check:
+ if (!initialMessages.length) {
+ setMessages([]);
+ return;
+ }
setMessages(
initialMessages
.map((e) => e.models.onchain_dash.Message as MessageItem)
.filter(Boolean)
.sort((a: Message, b: Message): number =>
parseInt(a.timestamp.toString(), 16) <
parseInt(b.timestamp.toString(), 16)
? -1
: 1
)
);| } | ||
| }, | ||
| }); | ||
|
|
||
| state.setEntities(initialData); |
There was a problem hiding this comment.
Add null check for initialData.
The code directly sets initialData without checking if it's empty or null, which could lead to unexpected state.
Apply this diff to add the necessary check:
+ if (!initialData?.length) {
+ state.setEntities([]);
+ return;
+ }
state.setEntities(initialData);📝 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.
| state.setEntities(initialData); | |
| if (!initialData?.length) { | |
| state.setEntities([]); | |
| return; | |
| } | |
| state.setEntities(initialData); |
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
ToriiQueryBuilderfor improved data fetching and subscription handling.