fix: use starknet.shortstring.encodeShortString instead of torii-wasm… #448
Conversation
WalkthroughThis change standardizes short string encoding across multiple Dojo Engine packages by switching from the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant ClauseBuilder (internal)
participant starknet.shortString
Caller->>ClauseBuilder (internal): Build clause with string value
ClauseBuilder (internal)->>starknet.shortString: encodeShortString(value)
starknet.shortString-->>ClauseBuilder (internal): Encoded value
ClauseBuilder (internal)-->>Caller: Return clause with encoded value
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/sdk/src/internal/clauseBuilder.ts (1)
149-152: VerifyencodeShortStringintegration inconvertToPrimitive
Ensure that passingshortString.encodeShortStringas the conversion callback preserves the expected behavior for plain string values and properly encodes shortstrings. Consider adding a unit test that exercises aMemberClausewith a short string value to confirm the encoded FELT output matches expectations..changeset/sharp-rocks-enjoy.md (1)
15-15: Remove commit-type prefix from summary
Changeset summaries are rendered as release notes—no need for afix:or other conventional commit tag. Consider rewriting to something like:
“Usestarknet.shortstring.encodeShortStringinstead of thetorii-wasmimplementation.”
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/sharp-rocks-enjoy.md(1 hunks)packages/sdk/src/__tests__/clauseBuilder.test.ts(1 hunks)packages/sdk/src/__tests__/toriiQueryBuilder.test.ts(1 hunks)packages/sdk/src/internal/clauseBuilder.ts(2 hunks)packages/sdk/src/node/index.ts(1 hunks)packages/sdk/src/web/clauseBuilder.ts(0 hunks)packages/sdk/src/web/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/sdk/src/web/clauseBuilder.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (5)
packages/sdk/src/__tests__/toriiQueryBuilder.test.ts (1)
5-5: Update import path for ClauseBuilder
The test now correctly references the relocatedclauseBuilderfrom the internal module.packages/sdk/src/node/index.ts (1)
32-32: Update export to use internal ClauseBuilder
Switching the export to the shared internal module aligns with the consolidation ofclauseBuilder.packages/sdk/src/internal/clauseBuilder.ts (1)
11-11: ImportshortStringfrom StarkNet
Pulling in the StarkNet-providedshortStringfor encoding is correct and replaces the previous WASM-based function.packages/sdk/src/web/index.ts (1)
35-35: Update export to use internal ClauseBuilder
Re-exporting from../internal/clauseBuildercorrectly reflects the removal of the web-specific implementation.packages/sdk/src/__tests__/clauseBuilder.test.ts (1)
7-12: Update imports to the internal clauseBuilder module
All runtime and type-only imports have been adjusted to point at the shared internal implementation. The use ofimport typefor purely type annotations is appropriate.
| "@dojoengine/sdk": patch | ||
| "@dojoengine/core": patch | ||
| "@dojoengine/create-burner": patch | ||
| "@dojoengine/create-dojo": patch | ||
| "@dojoengine/predeployed-connector": patch | ||
| "@dojoengine/react": patch | ||
| "@dojoengine/state": patch | ||
| "@dojoengine/torii-client": patch | ||
| "@dojoengine/torii-wasm": patch | ||
| "@dojoengine/utils": patch | ||
| "@dojoengine/utils-wasm": patch |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify package bump scope
Ensure you’re only bumping versions for packages that actually include code changes. Unnecessary version bumps across unrelated packages (e.g., @dojoengine/core, @dojoengine/react, etc.) can lead to noise and unintended releases.
Run the following script to list which packages have real file changes and confirm alignment with this changeset:
Compare the output to your list and prune any packages that didn’t change.
🏁 Script executed:
#!/bin/bash
# Identify packages with changes since main
base=$(git merge-base HEAD origin/main)
git diff --name-only $base HEAD \
| grep '^packages/' \
| cut -d'/' -f2 \
| sort -uLength of output: 199
Prune unchanged packages from this changeset
Only @dojoengine/sdk has file changes since main. Please remove all other package bumps from .changeset/sharp-rocks-enjoy.md.
- File:
.changeset/sharp-rocks-enjoy.md(lines 2–12) - Remove entries for unaffected packages
Suggested diff:
@@ .changeset/sharp-rocks-enjoy.md
- "@dojoengine/core": patch
- "@dojoengine/create-burner": patch
- "@dojoengine/create-dojo": patch
- "@dojoengine/predeployed-connector": patch
- "@dojoengine/react": patch
- "@dojoengine/state": patch
- "@dojoengine/torii-client": patch
- "@dojoengine/torii-wasm": patch
- "@dojoengine/utils": patch
- "@dojoengine/utils-wasm": patchLeave only:
"@dojoengine/sdk": patch🤖 Prompt for AI Agents
In the file .changeset/sharp-rocks-enjoy.md between lines 2 and 12, the
changeset currently lists version bumps for multiple packages, but only
@dojoengine/sdk has actual code changes. Remove all other package entries from
this changeset so that only "@dojoengine/sdk": patch remains, ensuring the
changeset accurately reflects the packages with real modifications.
… equivalent
Closes #
Introduced changes
Checklist
Summary by CodeRabbit