-
Notifications
You must be signed in to change notification settings - Fork 54
chore: update dojo.c version #432
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
Conversation
WalkthroughThis update introduces Node.js support for the Changes
Sequence Diagram(s)sequenceDiagram
participant UserScript as Node.js Worker (main.ts)
participant SDK as Node SDK (init)
participant ToriiClient as @dojoengine/torii-wasm/node
participant Callback as User Callback
UserScript->>SDK: init(config)
SDK->>ToriiClient: createClient(config)
SDK->>ToriiClient: subscribeEntityQuery(query, callback)
ToriiClient->>SDK: initialEntities
SDK->>UserScript: return initialEntities
ToriiClient-->>SDK: entityUpdate
SDK->>Callback: parseEntities and invoke callback
UserScript->>SDK: (on SIGTERM/SIGINT) free subscription
Possibly related PRs
Poem
🪧 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 (
|
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
🧹 Nitpick comments (2)
packages/sdk/src/react/hooks/token.ts (2)
28-28
: Add null check for tokens.items to prevent potential runtime errors.The code now extracts the
items
property from the tokens response, which reflects a change in the API response structure. However, there's no validation to ensure thattokens.items
exists.- setTokens(tokens.items); + setTokens(tokens?.items || []);
49-49
: Add null check for tokenBalances.items to prevent potential runtime errors.Similar to the tokens setter, this code now extracts the
items
property from the tokenBalances response without validation.- setTokenBalances(tokenBalances.items); + setTokenBalances(tokenBalances?.items || []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/sdk/src/react/hooks/token.ts
(3 hunks)packages/torii-wasm/dojo.c
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/sdk/src/react/hooks/token.ts (1)
packages/sdk/src/types.ts (1)
GetTokenRequest
(261-264)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (4)
packages/sdk/src/react/hooks/token.ts (3)
17-17
: Type simplification for tokens state.The state type has been changed from a custom
Tokens
type to a standard array ofToken
objects, which simplifies the type structure and makes the code more intuitive.
19-19
: Type simplification for tokenBalances state.Similar to the tokens state change, the tokenBalances state now uses a standard array of
TokenBalance
objects instead of a customTokenBalances
type.
15-76
:❓ Verification inconclusive
Verify compatibility with other components using the useTokens hook.
The changes to the state types and response handling in this hook might affect components that consume it. Consider verifying that all consumers of this hook are updated to handle the new data structure properly.
🏁 Script executed:
#!/bin/bash # Search for components using the useTokens hook echo "Searching for components using useTokens hook..." rg "useTokens\(" --type ts --type tsxLength of output: 174
Below is an updated review comment. Since the initial search yielded an error (due to the unrecognized TSX file type), it’s recommended that you re-run a search without file type restrictions to manually verify which components, if any, consume the useTokens hook and ensure they adapt to its new result structure.
ACTION: Verify Compatibility of useTokens Consumers
• Confirm that components using the useTokens hook (in TS, TSX, or other files) are updated to handle the revised state types and response data.
• Pay special attention to the updated handling in the fetchTokens and fetchTokenBalances functions inpackages/sdk/src/react/hooks/token.ts
(lines 15–76).Please re-run the following updated search if necessary:
Manually verify the results to ensure that all consumers correctly interpret the updated return values.
🏁 Script executed:
#!/bin/bash echo "Searching for components using useTokens hook (all files)..." rg "useTokens\("Length of output: 515
ACTION: Verify Consumers of the Updated useTokens Hook
The search identified one consumer in the repository:
- examples/example-vite-token-balance/src/components/token-balance.tsx
Since the hook now returns an updated structure—with fields
tokens
,balances
,getBalance
, andtoDecimal
—please verify that this component (and any other consumers you might have) is updated accordingly to use the new state types and data response. Manual confirmation is recommended to ensure compatibility and avoid runtime issues.🧰 Tools
🪛 Biome (1.9.4)
[error] 52-52: This hook does not specify all of its dependencies: fetchTokenBalances
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
[error] 52-52: This hook does not specify all of its dependencies: fetchTokens
This dependency is not specified in the hook dependency list.
(lint/correctness/useExhaustiveDependencies)
packages/torii-wasm/dojo.c (1)
1-1
: Subproject Commit UpdateThe commit reference has been updated to
8c6a98132ef5fdc3a67ce56284d850138d1a89c9
. This appears to be a straightforward dependency version bump. Please verify that the new commit points to a stable version of the subproject and that all integrations relying on it have been validated.
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: 9
🔭 Outside diff range comments (1)
packages/sdk/src/__tests__/parseEntities.test.ts (1)
6-6
: 🛠️ Refactor suggestionUse type-only import for SchemaType
SchemaType
is only used as a generic type parameter inparseEntities<SchemaType>
, so it should be imported as a type:-import { SchemaType } from "./models.gen.ts"; +import type { SchemaType } from "./models.gen.ts";[lint/style/useImportType]
🧰 Tools
🪛 Biome (1.9.4)
[error] 6-6: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
♻️ Duplicate comments (1)
packages/sdk/src/node/index.ts (1)
136-163
: 🛠️ Refactor suggestionSame
any
issue in event callback
Apply the same fix as above to the event‑query subscription to silence lint warnings and prevent un‑typed leakage.🧰 Tools
🪛 Biome (1.9.4)
[error] 136-136: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🧹 Nitpick comments (15)
.changeset/smart-panthers-attend.md (1)
15-15
: Fix spelling of Node.js
The commit message should use the official “Node.js” capitalization:-feat: torii-wasm nodejs support +feat: torii-wasm Node.js support🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support(NODE_JS)
packages/sdk/src/web/state/zustand.ts (1)
172-173
: Consider addressing the @ts-expect-error TODOThe type error is currently being suppressed with a
@ts-expect-error
comment. This works as a temporary solution but should be addressed with proper typing.Consider implementing a proper type for the recursive
deepMerge
function:- // @ts-expect-error TODO: change to better type - result[key] = deepMerge(target[key], source[key]); + result[key] = deepMerge<T>( + target[key] as MergedModels<T>, + source[key] as Partial<MergedModels<T>> + );packages/sdk/tsup.node.ts (1)
1-13
: Good Node.js build configurationThe new Node.js build configuration properly extends the base config and sets appropriate Node-specific settings. This enables separate builds for Node.js and web environments, which is essential for a cross-platform SDK.
Consider using a type-only import for the Options type to optimize the build:
-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/tsup.web.ts (1)
1-16
: Well-structured web build configurationThe web build configuration properly extends the base config and defines multiple entry points for different modules. This modular approach allows consumers to import only what they need, potentially reducing bundle sizes.
Consider using a type-only import for the Options type to optimize the build:
-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/tsconfig.json (1)
22-22
: Broader include patternChanging from
"src/**/*.ts"
to just"src"
broadens the included files to all files in the src directory, including potential non-TypeScript files. This is fine if intentional, but verify that only appropriate files are included.If you only want to include TypeScript files specifically, consider:
- "include": ["src"], + "include": ["src/**/*.ts", "src/**/*.tsx", "src/**/*.d.ts"],examples/example-node-worker/main.ts (1)
13-20
: Query structure is correct but could benefit from more clarityThe query builder and clause look well-structured, but the purpose of
[undefined]
in line 17 isn't immediately clear.Consider adding a comment explaining the purpose of passing
[undefined]
as the second parameter toKeysClause
. This would improve code readability for future developers.packages/sdk/src/internal/convertToMemberValue.ts (1)
16-19
: Function signature updated with additional parameterThe
convertToPrimitive
function now accepts ashortStringToFelt
parameter instead of using a direct import, which improves flexibility.Consider adding a JSDoc comment for the new
shortStringToFelt
parameter to document its purpose.packages/sdk/src/web/clauseBuilder.ts (1)
146-152
: LeverageconvertToPrimitive
’s built‑in array handling to remove duplication
convertToPrimitive
already returns{ List: … }
when passed an array, so the extra wrapper is redundant and risks double‑nesting if the function implementation changes.-const memberValue: MemberValue = Array.isArray(value) - ? { - List: value.map((i) => - convertToPrimitive(i, cairoShortStringToFelt) - ), - } - : convertToPrimitive(value, cairoShortStringToFelt); +const memberValue: MemberValue = convertToPrimitive( + value, + cairoShortStringToFelt +);This trims 6 lines and guarantees behaviour stays in sync with
convertToPrimitive
.packages/sdk/src/node/clauseBuilder.ts (2)
146-152
: Remove redundantList
constructionMirror the simplification suggested for the web builder:
-const memberValue: MemberValue = Array.isArray(value) - ? { - List: value.map((i) => - convertToPrimitive(i, cairoShortStringToFelt) - ), - } - : convertToPrimitive(value, cairoShortStringToFelt); +const memberValue: MemberValue = convertToPrimitive( + value, + cairoShortStringToFelt +);
19-24
: Replaceany
withunknown
to tighten type safety
Record<string, any>
disables many type‑checking rules and is flagged by Biome.
Unless the API really requires arbitrary values, preferRecord<string, unknown>
:-type ModelPath<T, K extends keyof T> = K extends string - ? T[K] extends Record<string, any> +type ModelPath<T, K extends keyof T> = K extends string + ? T[K] extends Record<string, unknown> ... -class CompositeBuilder<T extends Record<string, Record<string, any>>> { +class CompositeBuilder<T extends Record<string, Record<string, unknown>>> {This keeps the same flexibility while preventing accidental
any
leakage.Also applies to: 183-186
🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/node/index.ts (5)
33-37
: Avoid hard‑coded default endpoints – pull from env / config instead
Hard‑codingtoriiUrl
andrelayUrl
makes it easy to ship a broken build when the endpoints move (e.g. prod vs. staging). Prefer reading fromprocess.env
with sensible fall‑backs so integrators can override at deploy time.export const defaultClientConfig: Partial<torii.ClientConfig> = { - toriiUrl: "http://localhost:8080", - relayUrl: "/ip4/127.0.0.1/tcp/9090", + toriiUrl: process.env.TORII_URL ?? "http://localhost:8080", + relayUrl: process.env.RELAY_URL ?? "/ip4/127.0.0.1/tcp/9090", };
55-67
:Object.hasOwn
may break on older Node LTS – prefer safe polyfill
Object.hasOwn
landed in Node ≥ 16.9. If you intend to publish a lib consumed by broader versions or by bundlers that down‑level, replace with the canonicalObject.prototype.hasOwnProperty.call(...)
.- !Object.hasOwn(q.clause, "Keys") + !Object.prototype.hasOwnProperty.call(q.clause, "Keys")
174-178
: Unnecessaryawait
causes an extra micro‑task
Since you immediately return the promise, remove theawait
. This applies to a few other wrappers (getTokens
,getTokenBalances
, etc.).- return await subscribeTokenBalance(client, request); + return subscribeTokenBalance(client, request);
285-286
: Docstring typo:Funtion
→Function
Minor spelling correction keeps generated docs clean.- * @param {Funtion} callback - JavaScript function to call on updates + * @param {Function} callback - JavaScript function to call on updates
307-311
: Remove redundantawait
inupdateTokenBalanceSubscription
Same micro‑task nit as above.- return await updateTokenBalanceSubscription(client, request); + return updateTokenBalanceSubscription(client, request);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.changeset/smart-panthers-attend.md
(1 hunks)examples/example-node-worker/.nvmrc
(1 hunks)examples/example-node-worker/dojoConfig.ts
(1 hunks)examples/example-node-worker/main.ts
(1 hunks)examples/example-node-worker/package.json
(1 hunks)packages/sdk/package.json
(3 hunks)packages/sdk/src/__example__/index.ts
(1 hunks)packages/sdk/src/__tests__/clauseBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/models.gen.ts
(1 hunks)packages/sdk/src/__tests__/parseEntities.test.ts
(1 hunks)packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
(1 hunks)packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/zustand.perf.test.ts
(1 hunks)packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
(1 hunks)packages/sdk/src/internal/convertToMemberValue.ts
(4 hunks)packages/sdk/src/internal/generateTypedData.ts
(1 hunks)packages/sdk/src/internal/parseEntities.ts
(1 hunks)packages/sdk/src/internal/parseHistoricalEvents.ts
(1 hunks)packages/sdk/src/internal/token.ts
(1 hunks)packages/sdk/src/internal/toriiQueryBuilder.ts
(1 hunks)packages/sdk/src/internal/types.ts
(1 hunks)packages/sdk/src/node/clauseBuilder.ts
(1 hunks)packages/sdk/src/node/index.ts
(1 hunks)packages/sdk/src/node/tsconfig.json
(1 hunks)packages/sdk/src/react/index.ts
(0 hunks)packages/sdk/src/web/clauseBuilder.ts
(2 hunks)packages/sdk/src/web/execute.ts
(1 hunks)packages/sdk/src/web/experimental/index.ts
(1 hunks)packages/sdk/src/web/index.ts
(4 hunks)packages/sdk/src/web/queryBuilder.ts
(1 hunks)packages/sdk/src/web/react/hooks/entities.ts
(1 hunks)packages/sdk/src/web/react/hooks/events.ts
(1 hunks)packages/sdk/src/web/react/hooks/hooks.ts
(1 hunks)packages/sdk/src/web/react/hooks/index.ts
(1 hunks)packages/sdk/src/web/react/hooks/state.ts
(1 hunks)packages/sdk/src/web/react/hooks/token.ts
(3 hunks)packages/sdk/src/web/react/index.ts
(1 hunks)packages/sdk/src/web/react/provider.tsx
(1 hunks)packages/sdk/src/web/sql/index.ts
(1 hunks)packages/sdk/src/web/state/index.ts
(1 hunks)packages/sdk/src/web/state/zustand.ts
(2 hunks)packages/sdk/tsconfig.json
(1 hunks)packages/sdk/tsup.config.ts
(0 hunks)packages/sdk/tsup.node.ts
(1 hunks)packages/sdk/tsup.web.ts
(1 hunks)packages/torii-wasm/dojo.c
(1 hunks)packages/torii-wasm/package.json
(2 hunks)
💤 Files with no reviewable changes (2)
- packages/sdk/src/react/index.ts
- packages/sdk/tsup.config.ts
✅ Files skipped from review due to trivial changes (26)
- packages/sdk/src/web/react/hooks/index.ts
- packages/sdk/src/web/react/hooks/state.ts
- packages/sdk/src/web/queryBuilder.ts
- packages/sdk/src/tests/models.gen.ts
- examples/example-node-worker/.nvmrc
- packages/sdk/src/web/execute.ts
- packages/sdk/src/web/react/provider.tsx
- packages/sdk/src/internal/generateTypedData.ts
- packages/sdk/src/tests/zustand.perf.test.ts
- packages/sdk/src/internal/token.ts
- packages/sdk/src/web/react/hooks/events.ts
- packages/sdk/src/example/index.ts
- packages/sdk/src/web/react/hooks/entities.ts
- packages/sdk/src/web/react/index.ts
- packages/sdk/src/internal/types.ts
- packages/sdk/src/web/sql/index.ts
- packages/sdk/src/internal/toriiQueryBuilder.ts
- packages/sdk/src/web/state/index.ts
- examples/example-node-worker/dojoConfig.ts
- packages/sdk/src/internal/parseHistoricalEvents.ts
- packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
- packages/sdk/src/web/react/hooks/hooks.ts
- examples/example-node-worker/package.json
- packages/sdk/src/web/experimental/index.ts
- packages/sdk/src/internal/parseEntities.ts
- packages/sdk/src/node/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/torii-wasm/dojo.c
🧰 Additional context used
🧬 Code Graph Analysis (4)
packages/sdk/src/web/react/hooks/token.ts (1)
packages/sdk/src/internal/types.ts (1)
GetTokenRequest
(259-262)
packages/sdk/src/web/clauseBuilder.ts (1)
packages/sdk/src/internal/convertToMemberValue.ts (1)
convertToPrimitive
(16-55)
packages/sdk/tsup.node.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
packages/sdk/tsup.web.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
🪛 Biome (1.9.4)
packages/sdk/src/web/clauseBuilder.ts
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/node/clauseBuilder.ts
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/node/index.ts
[error] 74-74: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 136-136: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/__tests__/clauseBuilder.test.ts
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseEntities.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/tsup.node.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/tsup.web.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
🪛 LanguageTool
.changeset/smart-panthers-attend.md
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support
(NODE_JS)
🔇 Additional comments (21)
packages/sdk/src/web/state/zustand.ts (3)
1-1
: Good use of type-only importsConverting
StateCreator
andStoreApi
to type-only imports helps reduce bundle size as these are only used for type checking.
3-9
: Well-structured immer importsThe immer imports are now better organized with type imports explicitly marked, improving build-time optimization. Good job consolidating the
enablePatches
import here.
12-13
: Proper type imports for internal modulesConverting
ParsedEntity
,SchemaType
, andGameState
to type-only imports is a good practice since they're only used for type checking.packages/sdk/tsconfig.json (3)
10-10
: Correct setting for output generationSetting
"noEmit": false
ensures that TypeScript will generate output files during compilation, which is necessary for the build process.
12-12
: Updated module resolution strategyChanging
"moduleResolution"
to"bundler"
aligns with modern ECMAScript module bundling practices and is appropriate for a package that uses bundlers like tsup.
19-20
:❓ Verification inconclusive
New TypeScript features enabled
Adding
"allowImportingTsExtensions": true
enables imports with explicit TypeScript file extensions, useful for development. However, ensure that your bundler properly resolves these imports in the final output.Verify that your bundler configuration correctly handles .ts extensions in imports:
🏁 Script executed:
#!/bin/bash # Check if there are any .ts imports in the final bundle find dist -type f -name "*.js" -exec grep -l "\.ts[\"']" {} \;Length of output: 109
Ensure bundler resolves TypeScript extensions
The
allowImportingTsExtensions
setting lets you import files with explicit.ts
extensions. To avoid stray.ts
imports in your distributed code, confirm that your bundler is set up to resolve.ts
files. For example:
- In vite.config.{js,ts}
ensureresolve.extensions
includes.ts
- In rollup.config.js
configure@rollup/plugin-node-resolve
withextensions: ['.ts', '.js', …]
- In webpack.config.js
add.ts
toresolve.extensions
Please build the project and inspect the output (e.g., in
dist/
) for any remaining.ts
import paths, or review your bundler configuration to confirm it handles.ts
extensions correctly.examples/example-node-worker/main.ts (4)
1-11
: SDK initialization looks goodThe setup properly imports from the new Node-specific module path and configures the SDK with appropriate parameters from the config file.
22-33
: Good error handling in the callback functionThe entity update callback properly checks for errors and handles empty entities, which is a good practice.
35-39
: Properly subscribes to entity updatesThe subscription is set up correctly with appropriate parameters, including
historical: false
to only receive new updates.
43-53
: Excellent resource cleanupThe code properly handles process termination signals (SIGTERM and SIGINT) and frees the subscription before exiting, which prevents resource leaks.
packages/sdk/src/web/index.ts (3)
1-1
: Updated import to use the new wasm packageImport source has been correctly changed from
"@dojoengine/torii-client"
to"@dojoengine/torii-wasm"
.
17-24
: Updated import paths to the new internal directory structureImport paths have been correctly updated to reference modules from the internal directory.
32-36
: Export paths aligned with new directory structureExport paths are correctly updated to match the new internal module organization.
packages/sdk/src/internal/convertToMemberValue.ts (3)
1-1
: Updated to type-only import from wasm packageThe import has been correctly updated to use type-only imports from
@dojoengine/torii-wasm/types
.
36-38
: Using passed-in function instead of direct importThe code now uses the provided function parameter instead of directly calling the imported function, which supports the architectural changes.
46-51
: Updated recursion to pass the conversion functionThe recursive calls now correctly pass the
shortStringToFelt
function, ensuring consistent conversion throughout the nested structure.packages/sdk/src/web/react/hooks/token.ts (3)
3-3
: Updated to use type imports from wasm packageThe imports have been correctly updated to use only types from the new
@dojoengine/torii-wasm
package.
13-13
: Added explicit type annotations for state variablesAdding explicit type annotations to the state variables improves type safety and readability.
Also applies to: 15-15
23-23
:✅ Verification successful
Updated to use the items property for token data
The code now correctly sets state using the
items
property of the returned data, reflecting the API changes in the new version.Let's verify the structure of the token response objects to ensure the
.items
property is consistent across the codebase:Also applies to: 43-43
🏁 Script executed:
#!/bin/bash # Check how token responses are handled throughout the codebase rg -A 3 -B 3 "token.*\.items" --glob "*.ts"Length of output: 1042
Items property usage verified
All occurrences of the
.items
property for token data—specifically in
packages/sdk/src/web/react/hooks/token.ts
atsetTokens(tokens.items)
packages/sdk/src/web/react/hooks/token.ts
atsetTokenBalances(tokenBalances.items)
—are consistent with the updated API response shape. No further changes required.
packages/sdk/package.json (1)
7-15
:main
now diverges from the exported entry – keep them consistent
"exports"
maps"."
to./dist/web/index.js
, but"main"
still
points to the olddist/index.js
. Tooling that still looks atmain
(e.g.
rollup, older bundlers) will resolve a non‑existent file after the new build
split.- "main": "dist/index.js", + "main": "./dist/web/index.js",Consider removing the lingering
@dojoengine/torii-client
dependency if the
migration totorii-wasm
is complete.packages/sdk/src/node/index.ts (1)
245-255
: Robust signature handling
account.signMessage
may return{ r, s }
(object) or[string,string]
(tuple), but the current fallback assumes the object fields areBigNumberish
with.toString()
. Ifr
ands
are already strings the coercion is harmless; if not, this silently stringifies non‑hex data. Consider normalising explicitly:- Array.isArray(signature) - ? signature - : [signature.r.toString(), signature.s.toString()] + Array.isArray(signature) + ? signature + : [signature.r, signature.s].map((v) => v.toString())
@@ -1,4 +1,4 @@ | |||
import * as torii from "@dojoengine/torii-client"; | |||
import * as torii from "@dojoengine/torii-wasm"; |
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.
🛠️ Refactor suggestion
Use type-only import for torii module
The import import * as torii from "@dojoengine/torii-wasm";
is only used for the torii.Entities
type and doesn’t need to bring in runtime code. Switch to a type-only import to avoid unnecessary module loading:
-import * as torii from "@dojoengine/torii-wasm";
+import type * as torii from "@dojoengine/torii-wasm";
[lint/style/useImportType]
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
@@ -1,4 +1,4 @@ | |||
import * as torii from "@dojoengine/torii-client"; | |||
import * as torii from "@dojoengine/torii-wasm"; |
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.
🛠️ Refactor suggestion
Use type-only import for torii module
The torii
import is only used to type incoming entities (torii.Entities
). Convert it to a type-only import:
-import * as torii from "@dojoengine/torii-wasm";
+import type * as torii from "@dojoengine/torii-wasm";
[lint/style/useImportType]
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
@@ -1,6 +1,6 @@ | |||
import { describe, expect, it } from "vitest"; | |||
import { ToriiQueryBuilder } from "../toriiQueryBuilder"; | |||
import { Clause, OrderBy } from "@dojoengine/torii-client"; | |||
import { Clause, OrderBy } from "@dojoengine/torii-wasm"; |
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.
🛠️ Refactor suggestion
Use type-only import for Clause and OrderBy
Clause
and OrderBy
are only used as types in this test. Change to a type-only import:
-import { Clause, OrderBy } from "@dojoengine/torii-wasm";
+import type { Clause, OrderBy } from "@dojoengine/torii-wasm";
[lint/style/useImportType]
🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
@@ -9,7 +9,7 @@ import { | |||
ComparisonOperator, | |||
LogicalOperator, | |||
PatternMatching, | |||
} from "@dojoengine/torii-client"; | |||
} from "@dojoengine/torii-wasm"; |
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.
🛠️ Refactor suggestion
Use type-only import for comparison and logical operators
The imports from @dojoengine/torii-wasm
(ComparisonOperator
, LogicalOperator
, PatternMatching
) are only used in type assertions. Convert them to a type-only import:
-import {
- ComparisonOperator,
- LogicalOperator,
- PatternMatching,
-} from "@dojoengine/torii-wasm";
+import type {
+ ComparisonOperator,
+ LogicalOperator,
+ PatternMatching,
+} from "@dojoengine/torii-wasm";
[lint/style/useImportType]
🧰 Tools
🪛 Biome (1.9.4)
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
import { | ||
Clause, | ||
ComparisonOperator, | ||
MemberValue, | ||
PatternMatching, | ||
} from "@dojoengine/torii-client"; | ||
cairoShortStringToFelt, | ||
} from "@dojoengine/torii-wasm"; |
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.
🛠️ Refactor suggestion
Use import type
for type‑only symbols to avoid pulling extra code at runtime
Clause
, ComparisonOperator
, MemberValue
, and PatternMatching
are used purely as types. Importing them as values keeps them in the emitted JS bundle and triggers Biome’s useImportType
warning. Split the import so that only cairoShortStringToFelt
is brought in as a value.
-import {
- Clause,
- ComparisonOperator,
- MemberValue,
- PatternMatching,
- cairoShortStringToFelt,
-} from "@dojoengine/torii-wasm";
+import { cairoShortStringToFelt } from "@dojoengine/torii-wasm";
+import type {
+ Clause,
+ ComparisonOperator,
+ MemberValue,
+ PatternMatching,
+} from "@dojoengine/torii-wasm";
This removes dead code and silences the linter.
📝 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 { | |
Clause, | |
ComparisonOperator, | |
MemberValue, | |
PatternMatching, | |
} from "@dojoengine/torii-client"; | |
cairoShortStringToFelt, | |
} from "@dojoengine/torii-wasm"; | |
import { cairoShortStringToFelt } from "@dojoengine/torii-wasm"; | |
import type { | |
Clause, | |
ComparisonOperator, | |
MemberValue, | |
PatternMatching, | |
} from "@dojoengine/torii-wasm"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
import { | ||
Clause, | ||
ComparisonOperator, | ||
MemberValue, | ||
PatternMatching, | ||
cairoShortStringToFelt, | ||
} from "@dojoengine/torii-wasm/node"; |
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.
🛠️ Refactor suggestion
Split runtime vs. type imports to cut bundle size
Same rationale as the web variant—only cairoShortStringToFelt
is required at runtime.
-import {
- Clause,
- ComparisonOperator,
- MemberValue,
- PatternMatching,
- cairoShortStringToFelt,
-} from "@dojoengine/torii-wasm/node";
+import { cairoShortStringToFelt } from "@dojoengine/torii-wasm/node";
+import type {
+ Clause,
+ ComparisonOperator,
+ MemberValue,
+ PatternMatching,
+} from "@dojoengine/torii-wasm/node";
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { | |
Clause, | |
ComparisonOperator, | |
MemberValue, | |
PatternMatching, | |
cairoShortStringToFelt, | |
} from "@dojoengine/torii-wasm/node"; | |
import { cairoShortStringToFelt } from "@dojoengine/torii-wasm/node"; | |
import type { | |
Clause, | |
ComparisonOperator, | |
MemberValue, | |
PatternMatching, | |
} from "@dojoengine/torii-wasm/node"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
"main": "./pkg/node/dojo_c.js", | ||
"type": "module", | ||
"scripts": { | ||
"build:web": "cd dojo.c && pnpx wasm-pack build --out-dir ../pkg/web --release", | ||
"build:node": "cd dojo.c && pnpx wasm-pack build --out-dir ../pkg/node --release --target nodejs", | ||
"build:types": "rm -rf pkg/types && mkdir -p pkg/types && cp pkg/node/dojo_c.d.ts pkg/types/dojo_c.d.ts", | ||
"build:wasm": "pnpm build:web && pnpm build:node" |
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.
Exports map serves browser code to Node — fix conditional exports
With "exports"
present, Node ignores main
.
Because "import"
/"require"
for "."
still point to the web build, a plain
import "@dojoengine/torii-wasm"
in Node will load the browser bundle and fail
to instantiate the WASM. Provide conditional exports instead:
- ".": {
- "types": "./pkg/web/dojo_c.d.ts",
- "import": "./pkg/web/dojo_c.js",
- "require": "./pkg/web/dojo_c.js"
- },
+ ".": {
+ "types": "./pkg/types/dojo_c.d.ts",
+ "browser": {
+ "import": "./pkg/web/dojo_c.js",
+ "require": "./pkg/web/dojo_c.js"
+ },
+ "import": "./pkg/node/dojo_c.js",
+ "require": "./pkg/node/dojo_c.js"
+ },
Also update the top‑level types
field and "files"
array to point at
pkg/types/dojo_c.d.ts
so consumers always get the correct declarations.
These changes let a single specifier resolve correctly in both runtimes.
Also applies to: 16-33
(entityId: string, entityData: any) => { | ||
try { | ||
if (callback) { | ||
const parsedData = parseEntities<T>({ | ||
[entityId]: entityData, | ||
}); | ||
callback({ | ||
data: parsedData, | ||
error: undefined, | ||
}); | ||
} | ||
} catch (error) { | ||
if (callback) { | ||
callback({ | ||
data: undefined, | ||
error: | ||
error instanceof Error | ||
? error | ||
: new Error(String(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.
Replace any
with a stricter type to satisfy Biome & improve safety
Static analysis rightfully flags the any
. You already know entityData
is the raw Torii entity payload, so type it accordingly (or at least unknown
) to retain TS’ guarantees.
- (entityId: string, entityData: any) => {
+ (
+ entityId: string,
+ entityData: Record<string, unknown> // or a dedicated ToriiEntityRaw type
+ ) => {
try {
if (callback) {
- const parsedData = parseEntities<T>({
- [entityId]: entityData,
+ const parsedData = parseEntities<T>({
+ [entityId]: entityData,
});
📝 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.
(entityId: string, entityData: any) => { | |
try { | |
if (callback) { | |
const parsedData = parseEntities<T>({ | |
[entityId]: entityData, | |
}); | |
callback({ | |
data: parsedData, | |
error: undefined, | |
}); | |
} | |
} catch (error) { | |
if (callback) { | |
callback({ | |
data: undefined, | |
error: | |
error instanceof Error | |
? error | |
: new Error(String(error)), | |
}); | |
} | |
} | |
} | |
), | |
]; | |
( | |
entityId: string, | |
entityData: Record<string, unknown> // or a dedicated ToriiEntityRaw type | |
) => { | |
try { | |
if (callback) { | |
const parsedData = parseEntities<T>({ | |
[entityId]: entityData, | |
}); | |
callback({ | |
data: parsedData, | |
error: undefined, | |
}); | |
} | |
} catch (error) { | |
if (callback) { | |
callback({ | |
data: undefined, | |
error: | |
error instanceof Error | |
? error | |
: new Error(String(error)), | |
}); | |
} | |
} | |
} |
🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
559f8ce
to
823945e
Compare
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
♻️ Duplicate comments (8)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts (1)
3-3
: Use type-only import for Clause and OrderByThe
Clause
andOrderBy
imports are only used as types in this test file and don't need to bring in runtime code.-import { Clause, OrderBy } from "@dojoengine/torii-wasm"; +import type { Clause, OrderBy } from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/parseHistoricalEvents.test.ts (1)
1-1
: Use type-only import for torii moduleThe
torii
import is only used as a type reference fortorii.Entities
and doesn't require runtime code.-import * as torii from "@dojoengine/torii-wasm"; +import type * as torii from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/parseEntities.test.ts (1)
1-1
: Use type-only import for torii moduleThe
torii
import is only used for type annotations and doesn't require runtime code.-import * as torii from "@dojoengine/torii-wasm"; +import type * as torii from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/clauseBuilder.test.ts (1)
8-12
: Use type-only import for comparison and logical operatorsThe imports from
@dojoengine/torii-wasm
(ComparisonOperator
,LogicalOperator
,PatternMatching
) are only used in type assertions. Convert them to a type-only import:-import { - ComparisonOperator, - LogicalOperator, - PatternMatching, -} from "@dojoengine/torii-wasm"; +import type { + ComparisonOperator, + LogicalOperator, + PatternMatching, +} from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/web/clauseBuilder.ts (1)
1-7
: Useimport type
for type‑only symbols to avoid pulling extra code at runtime
Clause
,ComparisonOperator
,MemberValue
, andPatternMatching
are used purely as types. Importing them as values keeps them in the emitted JS bundle and triggers Biome'suseImportType
warning. Split the import so that onlycairoShortStringToFelt
is brought in as a value.-import { - Clause, - ComparisonOperator, - MemberValue, - PatternMatching, - cairoShortStringToFelt, -} from "@dojoengine/torii-wasm"; +import { cairoShortStringToFelt } from "@dojoengine/torii-wasm"; +import type { + Clause, + ComparisonOperator, + MemberValue, + PatternMatching, +} from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/clauseBuilder.ts (1)
1-7
: Split runtime vs. type imports to reduce bundle sizeSeveral of these imports are only used as types and should be imported with
import type
for better tree-shaking.-import { - Clause, - ComparisonOperator, - MemberValue, - PatternMatching, - cairoShortStringToFelt, -} from "@dojoengine/torii-wasm/node"; +import { cairoShortStringToFelt } from "@dojoengine/torii-wasm/node"; +import type { + Clause, + ComparisonOperator, + MemberValue, + PatternMatching, +} from "@dojoengine/torii-wasm/node";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/index.ts (2)
74-74
: Replaceany
with a stricter type to satisfy Biome & improve safetyStatic analysis rightfully flags the
any
. You already knowentityData
is the raw Torii entity payload, so type it accordingly (or at leastunknown
) to retain TS' guarantees.- (entityId: string, entityData: any) => { + ( + entityId: string, + entityData: Record<string, unknown> // or a dedicated ToriiEntityRaw type + ) => { try { if (callback) { - const parsedData = parseEntities<T>({ - [entityId]: entityData, + const parsedData = parseEntities<T>({ + [entityId]: entityData, });🧰 Tools
🪛 Biome (1.9.4)
[error] 74-74: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
136-136
: 🛠️ Refactor suggestionReplace
any
with a stricter type in the event callbackSimilar to the entity callback, this event callback should use a more specific type than
any
for improved type safety.- (entityId: string, entityData: any) => { + (entityId: string, entityData: Record<string, unknown>) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 136-136: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🧹 Nitpick comments (7)
packages/sdk/tsup.node.ts (1)
1-13
: LGTM: Good Node.js build configuration setupThis configuration correctly sets up a Node.js-specific build using tsup. The separation of Node.js and web builds is a good practice for cross-platform support.
Consider using a type-only import for the Options type since it's only used for type casting:
-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
.changeset/smart-panthers-attend.md (1)
15-15
: Use official spelling "Node.js" instead of "nodejs"Follow the official spelling convention for Node.js with a dot and capitalized N.
-feat: torii-wasm nodejs support +feat: torii-wasm Node.js support🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support(NODE_JS)
packages/sdk/tsup.web.ts (1)
1-1
: Use type-only import for the Options typeThe
Options
type is only used for type assertions and not as a value. Convert it to a type-only import to avoid including it in the runtime bundle:-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/clauseBuilder.ts (3)
19-25
: Consider using a more specific type thanany
for the record valueThe
any
type disables TypeScript's type checking. Consider using a more specific type likeunknown
orRecord<string, unknown>
to maintain type safety.-type ModelPath<T, K extends keyof T> = K extends string - ? T[K] extends Record<string, any> +type ModelPath<T, K extends keyof T> = K extends string + ? T[K] extends Record<string, unknown> ? { [SubK in keyof T[K]]: `${K}-${SubK & string}`; }[keyof T[K]] : never : never;🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
109-110
: Consider removing or documenting the TS ignore commentThe
@ts-expect-error
comment should either be removed if possible or documented to explain why it's necessary. Empty clause initialization could potentially be handled without silencing the type checker.- constructor() { - // @ts-expect-error It's ok if it's not assignable here. - this.clause = {}; - } + constructor() { + // Initialize with an empty object that will be properly filled + // by subsequent method calls before build() is invoked + this.clause = {} as Clause; + }
183-183
: Replaceany
with a more specific type for record valuesUsing
any
in type declarations reduces type safety. Use a more specific type for better type guarantees.-class CompositeBuilder<T extends Record<string, Record<string, any>>> { +class CompositeBuilder<T extends Record<string, Record<string, unknown>>> {🧰 Tools
🪛 Biome (1.9.4)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/node/index.ts (1)
285-286
: Fix typo in JSDoc foronTokenBalanceUpdated
There's a typo in the JSDoc comment for the
onTokenBalanceUpdated
function.- * @param {Funtion} callback - JavaScript function to call on updates + * @param {Function} callback - JavaScript function to call on updates
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (47)
.changeset/smart-panthers-attend.md
(1 hunks)examples/example-node-worker/.nvmrc
(1 hunks)examples/example-node-worker/dojoConfig.ts
(1 hunks)examples/example-node-worker/main.ts
(1 hunks)examples/example-node-worker/package.json
(1 hunks)packages/sdk/package.json
(3 hunks)packages/sdk/src/__example__/index.ts
(1 hunks)packages/sdk/src/__tests__/clauseBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/models.gen.ts
(1 hunks)packages/sdk/src/__tests__/parseEntities.test.ts
(1 hunks)packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
(1 hunks)packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/zustand.perf.test.ts
(1 hunks)packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
(1 hunks)packages/sdk/src/internal/convertToMemberValue.ts
(4 hunks)packages/sdk/src/internal/generateTypedData.ts
(1 hunks)packages/sdk/src/internal/parseEntities.ts
(1 hunks)packages/sdk/src/internal/parseHistoricalEvents.ts
(1 hunks)packages/sdk/src/internal/token.ts
(1 hunks)packages/sdk/src/internal/toriiQueryBuilder.ts
(1 hunks)packages/sdk/src/internal/types.ts
(1 hunks)packages/sdk/src/node/clauseBuilder.ts
(1 hunks)packages/sdk/src/node/index.ts
(1 hunks)packages/sdk/src/node/tsconfig.json
(1 hunks)packages/sdk/src/react/index.ts
(0 hunks)packages/sdk/src/web/clauseBuilder.ts
(2 hunks)packages/sdk/src/web/execute.ts
(1 hunks)packages/sdk/src/web/experimental/index.ts
(1 hunks)packages/sdk/src/web/index.ts
(4 hunks)packages/sdk/src/web/queryBuilder.ts
(1 hunks)packages/sdk/src/web/react/hooks/entities.ts
(1 hunks)packages/sdk/src/web/react/hooks/events.ts
(1 hunks)packages/sdk/src/web/react/hooks/hooks.ts
(1 hunks)packages/sdk/src/web/react/hooks/index.ts
(1 hunks)packages/sdk/src/web/react/hooks/state.ts
(1 hunks)packages/sdk/src/web/react/hooks/token.ts
(3 hunks)packages/sdk/src/web/react/index.ts
(1 hunks)packages/sdk/src/web/react/provider.tsx
(1 hunks)packages/sdk/src/web/sql/index.ts
(1 hunks)packages/sdk/src/web/state/index.ts
(1 hunks)packages/sdk/src/web/state/zustand.ts
(2 hunks)packages/sdk/tsconfig.json
(1 hunks)packages/sdk/tsup.config.ts
(0 hunks)packages/sdk/tsup.node.ts
(1 hunks)packages/sdk/tsup.web.ts
(1 hunks)packages/torii-wasm/dojo.c
(1 hunks)packages/torii-wasm/package.json
(2 hunks)
💤 Files with no reviewable changes (2)
- packages/sdk/src/react/index.ts
- packages/sdk/tsup.config.ts
✅ Files skipped from review due to trivial changes (6)
- packages/sdk/src/tests/models.gen.ts
- packages/sdk/src/internal/token.ts
- examples/example-node-worker/package.json
- packages/sdk/src/internal/parseEntities.ts
- packages/sdk/src/web/state/index.ts
- packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
🚧 Files skipped from review as they are similar to previous changes (28)
- packages/sdk/src/web/react/hooks/state.ts
- packages/sdk/src/example/index.ts
- packages/sdk/src/web/react/hooks/index.ts
- packages/sdk/src/web/sql/index.ts
- packages/sdk/src/tests/zustand.perf.test.ts
- packages/sdk/src/web/react/hooks/entities.ts
- packages/sdk/src/web/react/hooks/events.ts
- examples/example-node-worker/.nvmrc
- packages/sdk/src/web/execute.ts
- packages/sdk/src/web/queryBuilder.ts
- packages/sdk/src/internal/generateTypedData.ts
- packages/torii-wasm/dojo.c
- examples/example-node-worker/dojoConfig.ts
- packages/sdk/tsconfig.json
- packages/sdk/src/web/state/zustand.ts
- packages/sdk/src/web/react/index.ts
- packages/sdk/src/web/react/provider.tsx
- examples/example-node-worker/main.ts
- packages/sdk/src/internal/types.ts
- packages/sdk/src/internal/convertToMemberValue.ts
- packages/sdk/src/web/index.ts
- packages/sdk/src/web/experimental/index.ts
- packages/sdk/src/web/react/hooks/hooks.ts
- packages/sdk/src/internal/toriiQueryBuilder.ts
- packages/sdk/src/node/tsconfig.json
- packages/sdk/src/web/react/hooks/token.ts
- packages/sdk/src/internal/parseHistoricalEvents.ts
- packages/torii-wasm/package.json
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/sdk/src/node/clauseBuilder.ts (4)
packages/sdk/src/web/clauseBuilder.ts (6)
KeysClause
(46-52)keys
(116-129)ClauseBuilder
(105-181)MemberClause
(64-78)AndComposeClause
(87-91)OrComposeClause
(99-103)packages/sdk/src/__tests__/models.gen.ts (1)
SchemaType
(87-99)packages/sdk/src/internal/types.ts (1)
SchemaType
(49-69)packages/sdk/src/internal/convertToMemberValue.ts (2)
MemberValueParam
(7-7)convertToPrimitive
(16-55)
packages/sdk/tsup.node.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
packages/sdk/tsup.web.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
🪛 Biome (1.9.4)
packages/sdk/src/node/clauseBuilder.ts
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/__tests__/clauseBuilder.test.ts
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseEntities.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/node/index.ts
[error] 74-74: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 136-136: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/web/clauseBuilder.ts
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/tsup.node.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/tsup.web.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
🪛 LanguageTool
.changeset/smart-panthers-attend.md
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (17)
packages/sdk/tsup.web.ts (1)
5-16
: Web-specific build configuration looks goodThe configuration properly separates web-specific entry points and sets the correct output directory structure. This aligns well with the Node.js support feature being added.
packages/sdk/src/web/clauseBuilder.ts (2)
147-152
: Correctly integrates cairoShortStringToFelt for value conversionThe code now properly passes the
cairoShortStringToFelt
function to theconvertToPrimitive
utility for both array values and individual values. This ensures consistent string handling across the SDK.
11-12
:✅ Verification successful
Verify explicit .ts extension imports are compatible with the build system
The imports now have explicit
.ts
extensions. While this works for TypeScript source files, confirm this approach is compatible with your bundling setup to avoid potential issues after bundling.
🏁 Script executed:
#!/bin/bash # Check if tsup configuration supports explicit .ts extensions # 1. Examine the tsconfig.json for moduleResolution settings if [ -f packages/sdk/tsconfig.json ]; then echo "TypeScript configuration:" grep -A 5 "moduleResolution" packages/sdk/tsconfig.json fi # 2. Check if there are any other files using this pattern echo "Other files using explicit .ts extensions in imports:" rg "from ['\"].*\.ts['\"]" --type ts packages/sdk/src -l | head -n 10Length of output: 915
Explicit .ts imports are safe under your bundler setup
Your
tsconfig.json
is already using"moduleResolution": "bundler"
and across
packages/sdk/src
we see explicit.ts
extensions used consistently. Bundlers like esbuild/tsup support resolving.ts
imports natively, so no changes are required here.packages/sdk/package.json (5)
9-9
: Setting"type": "module"
enables ES modules supportAdding the "type": "module" field sets the default format to ES modules, which aligns well with modern JavaScript practices and Node.js support. This is a fundamental change that impacts how imports/exports work throughout the package.
12-14
: Reorganized exports structure to support dual environmentsThe exports field has been comprehensively restructured to:
- Move all web-specific exports to
./dist/web/*
paths- Add Node.js-specific exports under the
./node
subpathThis dual-environment support allows consumers to import appropriate code for their target platform, which is essential for Node.js compatibility where browser-specific APIs aren't available.
Also applies to: 17-19, 22-24, 27-29, 32-34, 37-39, 41-44
48-48
: Modified build process to support separate Node.js and web outputsThe build process now uses two separate configurations to generate both Node.js and web-compatible outputs, which is necessary for proper environment-specific optimizations.
58-58
: Added DTS plugin dependencies for better TypeScript declarationsThe new dependencies related to TypeScript declaration files (
@anymud/bun-plugin-dts
andbun-plugin-dts
) will help generate proper type definitions for both Node.js and web environments.Also applies to: 62-62
83-83
: Added dependency on the WASM implementation of Torii clientThe SDK now depends on
@dojoengine/torii-wasm
, which is likely a WebAssembly-based implementation that enables cross-environment compatibility including Node.js support.packages/sdk/src/node/clauseBuilder.ts (5)
46-52
: Approve well-typedKeysClause
helper functionThe
KeysClause
function provides a convenient way to create key-based clauses with proper type safety. The defaultPatternMatching
value of "VariableLen" is a sensible choice.
64-78
: Approve strongly-typedMemberClause
helper functionThe
MemberClause
function leverages TypeScript's generics to ensure type safety when creating member comparisons. The complex type parameters ensure that only valid model paths, members, and values can be used.
87-103
: Approve logical composition helpersThe
AndComposeClause
andOrComposeClause
helper functions provide a clean way to compose multiple clauses with logical operators.
146-153
: Approve proper value conversion for array and primitive typesThe implementation correctly handles both array values and single values by using the
convertToPrimitive
helper function with the appropriate Cairo string converter.
204-243
: Logic for handling composite clauses looks thoroughThe build method correctly handles different scenarios:
- Empty clauses (throws an error)
- Only OR clauses
- Only AND clauses
- Both OR and AND clauses (nested structure)
This comprehensive handling ensures proper clause composition for complex queries.
packages/sdk/src/node/index.ts (4)
1-32
: Clean organization of imports and exportsThe imports are well structured, separating runtime and type imports appropriately. Exporting the internal query builder and the node-specific clause builder makes sense for extensibility.
33-45
: Default configuration with sensible defaultsThe default configuration points to standard localhost URLs, which is appropriate for development. The
init
function properly merges user-provided configuration with these defaults.
239-260
: Message signing implementation correctly handles signature formatsThe
sendMessage
function properly handles both array and object formats for signatures, which is important for compatibility with different Starknet account implementations. The error handling is also appropriate, preserving the original error for better debugging.
375-407
: Query conversion methods are well-typed and handle historical data appropriatelyThe
toriiQueryIntoHashedKeys
andtoriiEventMessagesQueryIntoHashedKeys
methods correctly convert query clauses to hashed key format for efficient subscriptions. The handling of historical vs. non-historical data is also properly typed with generics.
823945e
to
a4ed667
Compare
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
♻️ Duplicate comments (8)
packages/sdk/src/__tests__/parseEntities.test.ts (1)
1-1
: Use type-only import for torii moduleThe import
import * as torii from "@dojoengine/torii-wasm";
is only used for thetorii.Entities
type and doesn't need to bring in runtime code. Switch to a type-only import to avoid unnecessary module loading:-import * as torii from "@dojoengine/torii-wasm"; +import type * as torii from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/parseHistoricalEvents.test.ts (1)
1-1
: Use type-only import for torii moduleThe
torii
import is only used for type annotations in type castings (lines 12 and 16). Convert it to a type-only import to avoid unnecessary code in the bundle.-import * as torii from "@dojoengine/torii-wasm"; +import type * as torii from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts (1)
3-3
: Use type-only import for Clause and OrderBy
Clause
andOrderBy
are only used as types in this test. Change to a type-only import:-import { Clause, OrderBy } from "@dojoengine/torii-wasm"; +import type { Clause, OrderBy } from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/clauseBuilder.test.ts (1)
8-12
: Use type-only import for comparison and logical operatorsThe imports from
@dojoengine/torii-wasm
(ComparisonOperator
,LogicalOperator
,PatternMatching
) are only used in type assertions. Convert them to a type-only import:-import { - ComparisonOperator, - LogicalOperator, - PatternMatching, -} from "@dojoengine/torii-wasm"; +import type { + ComparisonOperator, + LogicalOperator, + PatternMatching, +} from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/web/clauseBuilder.ts (1)
1-7
: Useimport type
for type-only imports
Clause
,ComparisonOperator
,MemberValue
, andPatternMatching
are used only as types. Usingimport type
would optimize the bundle size by removing these imports during compilation.-import { - Clause, - ComparisonOperator, - MemberValue, - PatternMatching, - cairoShortStringToFelt, -} from "@dojoengine/torii-wasm"; +import { cairoShortStringToFelt } from "@dojoengine/torii-wasm"; +import type { + Clause, + ComparisonOperator, + MemberValue, + PatternMatching, +} from "@dojoengine/torii-wasm";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/clauseBuilder.ts (1)
1-7
: Useimport type
for type-only importsSimilar to the web version,
Clause
,ComparisonOperator
,MemberValue
, andPatternMatching
are used only as types. Usingimport type
would optimize the bundle size.-import { - Clause, - ComparisonOperator, - MemberValue, - PatternMatching, - cairoShortStringToFelt, -} from "@dojoengine/torii-wasm/node"; +import { cairoShortStringToFelt } from "@dojoengine/torii-wasm/node"; +import type { + Clause, + ComparisonOperator, + MemberValue, + PatternMatching, +} from "@dojoengine/torii-wasm/node";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/index.ts (2)
75-75
: Replaceany
with a stricter type to satisfy Biome & improve safetyThe static analysis tool rightfully flags the
any
type. You already knowentityData
is the raw Torii entity payload, so type it accordingly to retain TypeScript's guarantees.- (entityId: string, entityData: any) => { + (entityId: string, entityData: Record<string, unknown>) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 75-75: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
137-137
: 🛠️ Refactor suggestionReplace
any
with a stricter type for entity dataUsing
any
bypasses TypeScript's type checking, potentially leading to runtime errors. Define a more specific type forentityData
based on its expected structure.- (entityId: string, entityData: any) => { + (entityId: string, entityData: Record<string, unknown>) => {🧰 Tools
🪛 Biome (1.9.4)
[error] 137-137: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
🧹 Nitpick comments (13)
packages/sdk/src/__tests__/state.test.ts (1)
2-2
: Prefer usingimport type
for theParsedEntity
type.Since
ParsedEntity
is only used as a type, switching to a type-only import will prevent emitting unnecessary JavaScript at runtime and satisfy the Biome lint rule. For example:-import { ParsedEntity } from "../internal/types"; +import type { ParsedEntity } from "../internal/types";🧰 Tools
🪛 Biome (1.9.4)
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
.changeset/smart-panthers-attend.md (1)
15-15
: Use proper capitalization for Node.js.The official spelling of this platform is "Node.js" rather than "nodejs".
-feat: torii-wasm nodejs support +feat: torii-wasm Node.js support🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support(NODE_JS)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts (1)
4-4
: Use type-only import for SchemaType
SchemaType
is only used as an interface extension and should be imported as a type-only import.-import { SchemaType } from "../internal/types"; +import type { SchemaType } from "../internal/types";🧰 Tools
🪛 Biome (1.9.4)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/src/__tests__/clauseBuilder.test.ts (1)
13-13
: Use type-only import for SchemaType
SchemaType
is only used as an interface extension and should be imported as a type-only import.-import { SchemaType } from "../internal/types"; +import type { SchemaType } from "../internal/types";🧰 Tools
🪛 Biome (1.9.4)
[error] 13-13: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.(lint/style/useImportType)
packages/sdk/tsup.node.ts (1)
1-13
: Node.js build configuration looks good with a minor improvementThe configuration correctly extends the base tsup config while specifying Node.js-specific settings for format, output directory, tsconfig, and entry point. However, the Options import is only used as a type cast and should be a type-only import.
-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";The rest of the configuration is well structured and correctly sets up a Node.js-specific build process.
🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/tsup.web.ts (1)
1-1
: Useimport type
for the Options importSince
Options
is only used as a type in this file, consider using theimport type
syntax to ensure it's removed during compilation and doesn't contribute to the bundle size.-import { defineConfig, Options } from "tsup"; +import { defineConfig } from "tsup"; +import type { Options } from "tsup";🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/clauseBuilder.ts (2)
19-26
: Replaceany
with more specific typesThe
any
type inRecord<string, any>
disables type checking. Consider using a more specific type or a generic parameter if the exact type cannot be determined.-type ModelPath<T, K extends keyof T> = K extends string - ? T[K] extends Record<string, any> +type ModelPath<T, K extends keyof T> = K extends string + ? T[K] extends Record<string, unknown>🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
183-183
: Replaceany
with more specific typesSimilar to the earlier comment, consider replacing
any
with a more specific type here.-class CompositeBuilder<T extends Record<string, Record<string, any>>> { +class CompositeBuilder<T extends Record<string, Record<string, unknown>>> {🧰 Tools
🪛 Biome (1.9.4)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/node/index.ts (5)
34-37
: Consider making default URLs configurable via environment variablesThe hardcoded URLs in
defaultClientConfig
might not be suitable for all environments. Consider making these configurable via environment variables to support different deployment environments.export const defaultClientConfig: Partial<torii.ClientConfig> = { - toriiUrl: "http://localhost:8080", - relayUrl: "/ip4/127.0.0.1/tcp/9090", + toriiUrl: process.env.TORII_URL || "http://localhost:8080", + relayUrl: process.env.RELAY_URL || "/ip4/127.0.0.1/tcp/9090", };
205-206
: Simplify conditional expressionThe conditional expression can be simplified to directly use the provided
historical
value or fall back tofalse
.- const events = await client.getEventMessages( - q, - historical ? historical : false - ); + const events = await client.getEventMessages(q, !!historical);
286-286
: Fix typo in JSDoc parameter typeThere's a typo in the JSDoc for the callback parameter type.
- * @param {Funtion} callback - JavaScript function to call on updates + * @param {Function} callback - JavaScript function to call on updates
398-401
: Simplify conditional expressionThe conditional expression can be simplified, similar to the earlier instance.
- const events = await client.getEventMessages( - q, - historical ? historical : false - ); + const events = await client.getEventMessages(q, !!historical);
258-259
: Consider more specific error handlingThe current error handling logs the error and rethrows it without any modification. Consider adding more context to the error to make debugging easier.
- console.error("Failed to send message:", error); - throw error; + const enhancedError = error instanceof Error + ? new Error(`Failed to send message: ${error.message}`, { cause: error }) + : new Error(`Failed to send message: ${String(error)}`); + console.error(enhancedError); + throw enhancedError;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (52)
.changeset/smart-panthers-attend.md
(1 hunks)examples/example-node-worker/.nvmrc
(1 hunks)examples/example-node-worker/dojoConfig.ts
(1 hunks)examples/example-node-worker/main.ts
(1 hunks)examples/example-node-worker/package.json
(1 hunks)packages/sdk/package.json
(3 hunks)packages/sdk/src/__example__/index.ts
(1 hunks)packages/sdk/src/__tests__/clauseBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/convertClauseToEntityKeysClause.test.ts
(1 hunks)packages/sdk/src/__tests__/generateTypedData.test.ts
(1 hunks)packages/sdk/src/__tests__/models.gen.ts
(1 hunks)packages/sdk/src/__tests__/parseEntities.test.ts
(1 hunks)packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
(1 hunks)packages/sdk/src/__tests__/queryBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/state.test.ts
(1 hunks)packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
(1 hunks)packages/sdk/src/__tests__/zustand.perf.test.ts
(2 hunks)packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
(1 hunks)packages/sdk/src/internal/convertToMemberValue.ts
(4 hunks)packages/sdk/src/internal/generateTypedData.ts
(1 hunks)packages/sdk/src/internal/parseEntities.ts
(1 hunks)packages/sdk/src/internal/parseHistoricalEvents.ts
(1 hunks)packages/sdk/src/internal/token.ts
(1 hunks)packages/sdk/src/internal/toriiQueryBuilder.ts
(1 hunks)packages/sdk/src/internal/types.ts
(1 hunks)packages/sdk/src/node/clauseBuilder.ts
(1 hunks)packages/sdk/src/node/index.ts
(1 hunks)packages/sdk/src/node/tsconfig.json
(1 hunks)packages/sdk/src/react/index.ts
(0 hunks)packages/sdk/src/web/clauseBuilder.ts
(2 hunks)packages/sdk/src/web/execute.ts
(1 hunks)packages/sdk/src/web/experimental/index.ts
(1 hunks)packages/sdk/src/web/index.ts
(4 hunks)packages/sdk/src/web/queryBuilder.ts
(1 hunks)packages/sdk/src/web/react/hooks/entities.ts
(1 hunks)packages/sdk/src/web/react/hooks/events.ts
(1 hunks)packages/sdk/src/web/react/hooks/hooks.ts
(1 hunks)packages/sdk/src/web/react/hooks/index.ts
(1 hunks)packages/sdk/src/web/react/hooks/state.ts
(1 hunks)packages/sdk/src/web/react/hooks/token.ts
(3 hunks)packages/sdk/src/web/react/index.ts
(1 hunks)packages/sdk/src/web/react/provider.tsx
(1 hunks)packages/sdk/src/web/sql/index.ts
(1 hunks)packages/sdk/src/web/state/index.ts
(1 hunks)packages/sdk/src/web/state/zustand.ts
(1 hunks)packages/sdk/tsconfig.json
(1 hunks)packages/sdk/tsup.config.ts
(0 hunks)packages/sdk/tsup.node.ts
(1 hunks)packages/sdk/tsup.web.ts
(1 hunks)packages/sdk/typedoc.json
(1 hunks)packages/torii-wasm/dojo.c
(1 hunks)packages/torii-wasm/package.json
(2 hunks)
💤 Files with no reviewable changes (2)
- packages/sdk/src/react/index.ts
- packages/sdk/tsup.config.ts
✅ Files skipped from review due to trivial changes (9)
- packages/sdk/typedoc.json
- packages/sdk/src/tests/convertClauseToEntityKeysClause.test.ts
- packages/sdk/src/web/state/index.ts
- packages/sdk/src/tests/queryBuilder.test.ts
- packages/sdk/src/internal/convertClauseToEntityKeysClause.ts
- packages/sdk/src/internal/parseEntities.ts
- packages/sdk/src/internal/token.ts
- packages/sdk/src/tests/generateTypedData.test.ts
- packages/sdk/src/internal/parseHistoricalEvents.ts
🚧 Files skipped from review as they are similar to previous changes (28)
- packages/sdk/src/web/react/hooks/index.ts
- packages/sdk/src/web/react/hooks/state.ts
- examples/example-node-worker/.nvmrc
- packages/sdk/src/web/queryBuilder.ts
- packages/sdk/src/tests/models.gen.ts
- packages/sdk/src/example/index.ts
- packages/sdk/src/web/execute.ts
- examples/example-node-worker/dojoConfig.ts
- packages/sdk/src/web/sql/index.ts
- packages/sdk/src/internal/toriiQueryBuilder.ts
- packages/sdk/src/web/state/zustand.ts
- packages/sdk/src/internal/generateTypedData.ts
- packages/sdk/src/web/react/hooks/events.ts
- packages/sdk/src/web/react/hooks/entities.ts
- packages/sdk/src/web/react/index.ts
- packages/sdk/src/web/experimental/index.ts
- packages/sdk/src/web/react/hooks/hooks.ts
- examples/example-node-worker/package.json
- packages/sdk/src/web/react/provider.tsx
- packages/sdk/src/web/index.ts
- packages/sdk/tsconfig.json
- packages/torii-wasm/dojo.c
- packages/sdk/src/internal/convertToMemberValue.ts
- packages/sdk/src/node/tsconfig.json
- packages/sdk/src/web/react/hooks/token.ts
- examples/example-node-worker/main.ts
- packages/sdk/src/internal/types.ts
- packages/torii-wasm/package.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/sdk/tsup.node.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
packages/sdk/tsup.web.ts (1)
tsup.config.ts (1)
tsupConfig
(3-20)
🪛 Biome (1.9.4)
packages/sdk/src/node/clauseBuilder.ts
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/src/__tests__/clauseBuilder.test.ts
[error] 8-12: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
[error] 13-13: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseEntities.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/parseHistoricalEvents.test.ts
[error] 1-1: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/state.test.ts
[error] 2-2: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts
[error] 3-3: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
[error] 4-4: All these imports are only used as types.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Use import type.
(lint/style/useImportType)
packages/sdk/src/__tests__/zustand.perf.test.ts
[error] 131-132: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 131-131: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 131-131: The computed expression can be simplified without the use of a string literal.
Unsafe fix: Use a literal key instead.
(lint/complexity/useLiteralKeys)
packages/sdk/src/node/index.ts
[error] 75-75: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 137-137: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
packages/sdk/src/web/clauseBuilder.ts
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/tsup.node.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
packages/sdk/tsup.web.ts
[error] 1-1: Some named imports are only used as types.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.
(lint/style/useImportType)
🪛 LanguageTool
.changeset/smart-panthers-attend.md
[uncategorized] ~15-~15: The official spelling of this programming framework is “Node.js”.
Context: ...tils-wasm": patch --- feat: torii-wasm nodejs support
(NODE_JS)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: build
🔇 Additional comments (17)
packages/sdk/src/__tests__/state.test.ts (1)
1-1
: Import path updated for web-specific state.The
createDojoStore
import now correctly points to the web implementation (../web/state
), which aligns with the new web vs. node split. Ensure there are no remaining references to the old"../state"
path in other tests or modules.packages/sdk/src/__tests__/parseEntities.test.ts (1)
4-4
: Import path update looks goodThe updated import path for
parseEntities
correctly reflects the SDK's new internal structure, aligning with the broader restructuring of modules.packages/sdk/src/__tests__/zustand.perf.test.ts (2)
1-1
: Approve import update forcreateDojoStore
path.The import path has been correctly updated to the new
web/state
directory to align with the SDK restructuring.
2-2
: Approve type-only import for internal types.Switching to a type-only import of
ParsedEntity
from../internal/types
improves modularity and tree‑shaking.packages/sdk/src/__tests__/parseHistoricalEvents.test.ts (1)
4-7
: Import path update looks good.The import path change from "../parseHistoricalEvents" to "../internal/parseHistoricalEvents" correctly reflects the SDK's restructuring to support both web and Node.js environments.
.changeset/smart-panthers-attend.md (1)
1-13
: Changeset correctly specifies patches for all affected packages.All the necessary packages are included in this changeset with patch version bumps, which is appropriate for a feature addition without breaking changes.
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts (2)
2-2
: Import path update looks good.The import path change from "../toriiQueryBuilder" to "../internal/toriiQueryBuilder" correctly reflects the SDK's restructuring.
5-5
: Import path update looks good.The import path change from "../clauseBuilder" to "../web/clauseBuilder" correctly reflects the new web-specific module organization.
packages/sdk/src/__tests__/clauseBuilder.test.ts (1)
7-7
: Import path update looks good.The import path change to "../web/clauseBuilder" correctly reflects the new web-specific module organization.
packages/sdk/tsup.web.ts (1)
5-16
: LGTM! Good configuration for web buildsThe configuration nicely extends the base config while specifying web-specific output directory and entry points. This structure aligns well with the dual build system for web and Node.js environments.
packages/sdk/src/web/clauseBuilder.ts (1)
146-152
: Updated value conversion now correctly handles Cairo short stringsThe function now properly passes
cairoShortStringToFelt
to theconvertToPrimitive
function, ensuring consistent handling of Cairo short string values. This change is important for correct data conversion when working with the Torii wasm client.packages/sdk/package.json (4)
9-45
: Well-structured exports configuration for dual environment supportThe package.json changes effectively set up dual builds for web and Node.js environments. The ES module type declaration and properly structured exports ensure compatibility while enabling Node.js-specific functionality through the new
./node
export.
48-48
: Improved build process for multi-environment supportThe updated build script correctly runs both Node.js and web configurations sequentially, ensuring both environments are properly built.
58-63
: Added dependencies for enhanced build capabilitiesThe new dependencies support the dual build system and type declarations needed for the enhanced functionality.
83-83
: Added torii-wasm dependencyThe addition of
@dojoengine/torii-wasm
aligns with the codebase migration fromtorii-client
to the wasm-based implementation, supporting Node.js compatibility.packages/sdk/src/node/clauseBuilder.ts (1)
1-244
: Well-structured Node.js-specific implementationThis Node.js-specific clauseBuilder implementation maintains API consistency with the web version while targeting the Node.js environment. The approach of having separate implementations for different environments is clean and allows for environment-specific optimizations.
🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 183-183: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 1-7: Some named imports are only used as types.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
This import is only used as a type.
Importing the types with import type ensures that they are removed by the compilers and avoids loading unnecessary modules.
Safe fix: Add inline type keywords.(lint/style/useImportType)
packages/sdk/src/node/index.ts (1)
1-414
: Overall Node.js SDK implementation looks well-structuredThe Node.js SDK implementation is comprehensive and well-structured, providing all necessary functionality for interacting with the Torii client from a Node.js environment. The code demonstrates good separation of concerns and follows a consistent pattern across methods.
Some minor improvements could be made to TypeScript typing and error handling, but the overall implementation is solid.
🧰 Tools
🪛 Biome (1.9.4)
[error] 75-75: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
[error] 137-137: Unexpected any. Specify a different type.
any disables many type checking rules. Its use should be avoided.
(lint/suspicious/noExplicitAny)
draft.entities["entity500"].models.world! | ||
.item!.durability = 75; |
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 non-null assertions and simplify property access.
The use of !
for non-null assertions is forbidden by the lint rule noNonNullAssertion
, and accessing the entity by a string literal can be simplified. Consider flattening the chain into a single line and using optional chaining:
-draft.entities["entity500"].models.world!
- .item!.durability = 75;
+draft.entities.entity500.models.world?.item?.durability = 75;
This eliminates the non-null assertions and uses a literal key for improved safety and readability.
📝 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.
draft.entities["entity500"].models.world! | |
.item!.durability = 75; | |
draft.entities.entity500.models.world?.item?.durability = 75; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 131-132: Forbidden non-null assertion.
(lint/style/noNonNullAssertion)
[error] 131-131: Forbidden non-null assertion.
Unsafe fix: Replace with optional chain operator ?. This operator includes runtime checks, so it is safer than the compile-only non-null assertion operator
(lint/style/noNonNullAssertion)
[error] 131-131: The computed expression can be simplified without the use of a string literal.
Unsafe fix: Use a literal key instead.
(lint/complexity/useLiteralKeys)
Closes #
Introduced changes
Checklist
Summary by CodeRabbit
New Features
@dojoengine/torii-wasm
package.Bug Fixes
Refactor
Chores
@dojoengine/torii-wasm
package entry points and exports to prioritize Node.js builds and typings.Tests
Documentation