fix(bindgen): fix ts bytearray type mapping - #3247
Conversation
WalkthroughOhayo sensei! This update adjusts TypeScript code generation for Cairo byte arrays. It removes the unnecessary Changes
Sequence Diagram(s)sequenceDiagram
participant CairoType
participant JsTypeMapping
participant TypeScriptGenerator
CairoType->>JsTypeMapping: Request mapping for ByteArray
JsTypeMapping-->>TypeScriptGenerator: Returns JS_STRING
TypeScriptGenerator->>TypeScriptGenerator: Omits ByteArray from starknet imports
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🧪 Generate Unit Tests
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
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)
160-167: Ohayo sensei –ByteArrayshort-form is still unmappedThe new match arm correctly maps the full path (
CAIRO_BYTE_ARRAY) toJS_STRING, but the test you add below expects the short literal"ByteArray"to resolve the same way.
Currently"ByteArray"falls through to the default branch and will remain"ByteArray", making the test (and generated TS) fail.A minimal fix is to add an explicit arm (or a suffix check) for the short form in both
JsPrimitiveInputTypeandJsPrimitiveType:CAIRO_CONTRACT_ADDRESS => JsPrimitiveInputType(JS_STRING.to_owned()), CAIRO_BYTE_ARRAY => JsPrimitiveInputType(JS_STRING.to_owned()), + // Allow using the short type without the full module path + "ByteArray" => JsPrimitiveInputType(JS_STRING.to_owned()),Without this,
cargo testwill fail ontest_byte_array_input_type.
🧹 Nitpick comments (1)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)
378-395: Imported constants look good – minor style nitNice addition of
JS_STRINGto the test imports and thePartialEqshim forJsPrimitiveInputType.
You can simplify the comparison closure slightly:- fn eq(&self, other: &JsPrimitiveInputType) -> bool { - self == &other.0.as_str() + fn eq(&self, other: &JsPrimitiveInputType) -> bool { + *self == other.0 }No functional gain, just removes an extra reference & call.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
crates/dojo/bindgen/src/plugins/typescript/generator/function.rs(1 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (3)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)
876-889: Test will fail until short-form mapping is addedAs noted above, the second
assert_eq!relies on"ByteArray"mapping toJS_STRING, which is not implemented yet. Merge will break CI unless the mapping arm is added.
After adding the extra match arm, this test will pass.crates/dojo/bindgen/src/plugins/typescript/generator/function.rs (2)
22-25: Removal ofByteArrayimport LGTMWith
ByteArraynow mapped to plainstringin generated TS, dropping it from the Starknet import keeps the bundle clean and avoids unused-import warnings.
No further action needed.
13-28: Quick sanity-check: detect residualByteArrayreferencesJust to be safe, run a quick grep to ensure no stale
ByteArraysymbols remain in the generator after this change:#!/bin/bash rg -n "ByteArray" crates/dojo/bindgen/src/plugins/typescript | headExpect: only occurrences in tests or constant definitions, none inside production generator code.
0a554ca to
10d2dbe
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/dojo/bindgen/src/lib.rs (1)
267-267: Ohayo sensei! Replace magic number with self-documenting expectationBumping the expected model count from
8→9works today but is brittle: any future model addition/removal will break the test again. Prefer deriving the expectation from the data you already have, or enumerating the expected tags and asserting on that list’s length—this keeps intent obvious and failures more informative.- assert_eq!(data.models.len(), 9); + // Expected models we explicitly care about; update list, not magic number. + let expected = ["ns-Position", "ns-Moves", "ns-Message", "ns-PlayerConfig", /* … */]; + assert_eq!(data.models.len(), expected.len()); + for tag in expected { + assert!(data.models.contains_key(tag), "{tag} missing"); + }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/dojo/bindgen/src/lib.rs(2 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/function.rs(1 hunks)crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- crates/dojo/bindgen/src/plugins/typescript/generator/function.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: fmt
🔇 Additional comments (1)
crates/dojo/bindgen/src/lib.rs (1)
15-17: Ohayo sensei! Import re-order looks finePure re-ordering; no semantic changes, compilation unaffected.
f9ba1a9 to
ad4a2b8
Compare
ad4a2b8 to
ae83f42
Compare
* Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
…3266) * feat: add external contract registering in the world (#3195) * Use the world as external contract reference and manage upgrades with sozo * update spawn-and-move with new external contract features * update test artifacts + world_address in dojo_dev.toml * export entrypoints + fix tests + remove TODO RBA * restore test_metadata_updated_event test * fix tests * rabbit improvements * fix test * feat: add proc macros (#3212) * WIP * add macros + core-tests crates + update them from 1.6.0 * update scripts/cairo_fmt.sh + fix fmt * update CI + Scarb.toml files * fix fmt * change from 2.11.2 to 2.11.4 * fix scarb issue + fmt * one testing library for each test runner (cairo, snfoundry) + update examples * fix fmt * update comment * chore: apply patches to sync exact same commits than scarb * fix(ci): update rust-toolchain version * fix(ci): bump clippy to use nightly compatible with 1.86 * fix: clippy and fmt * update spawn-and-move to use dojo-snf-test * update world address --------- Co-authored-by: glihm <dev@glihm.net> * Scarb crate removing (#3223) * introduce scarb_interop crate * handle missing scarb * introduce metadata instead of workspace to manage dojo related paths and config * add stats back and ensure profile is propagated until build not only metadata * cleanup and add error message if SCARB is not set correctly * add features + packages to build command * update scarb build/test command * refactor packages/features * fix utils.rs + enable auth command * fix rust fmt * re-enable sozo commands * re-enable the last sozo commands + new version command * fix issue with conflicts_with_all * check if manifest_path does not exist * set run() functions as async instead of using tokio * update init command management * refactor metadata loading + dead code cleaning * add build_simple_dev() function to be used to easily build spawn-and-move for tests/benches * update tests + fmt + clippy * remove dojo/lang * remove useless sozo files * tiny change * fix CI * use scarb nightly in CI * fix manifest_path/manifest_dir issue * fix fmt * update snfoundry version + update tests accordingly + add cairo-profiler to .tool-versions --------- Co-authored-by: glihm <dev@glihm.net> * feat: add DojoStore trait to handle storage serialization (#3219) * add DojoStore management * fix rust fmt after abigen * fix ModelReader * fix fmt * set DojoStore functions as inline(always) * update test artifacts * fix fmt * remove dojo-core test from CI as all tests are now in core-tests * feat: add block number for external contract events (#3224) * add block_number to ExternalContractRegistered/ExternalContractUpgraded events * remove useless println * restore external_contracts tests + update with block_number * update test artifacts * fix clippy * rework: handle block_number * fix world.dns for external contract + update tests * update sozo events with missing events * add a self-managed external contract called from spawn-and-move * update manifest_dev.json * use dns_address * fix fmt + abigen * update test artifacts * fix fmt * update starkli path * update policies * rebuild test db * fix: restore resource order to avoid storage conflict (#3238) * Resource enum variant order should not change as it is used in the world storage * fix fmt * fix world address and test db * fix fmt --------- Co-authored-by: glihm <dev@glihm.net> * fix(lang): remove warning with unit type (#3255) fix warning with unit type for enumerations serialization generated code. * dev: merge main in dev 1.6.0 (#3263) * Fix broken getting started link in README (#3235) * chore(versions): add torii 1.5.6 * Update DEVELOPMENT.md (#3236) * Update DEVELOPMENT.md with new instructions * Update scripts/rebuild_test_artifacts.sh with Rabbit suggestion Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Respond to review comments * Respond to review comments II * Respond to review comments III --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * feat: update rpc spec to 0.8 (#3179) * release(prepare): v1.6.0-alpha.0 (#3241) Prepare release: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore(versions): bump katana to 1.6.0-alpha.0 * fix(dojoup): remove new line in the source cmd (#3244) fix: fixed new line in the source cmd * chore: edited the build badge and its link (#3221) Co-authored-by: Ammar Arif <evergreenkary@gmail.com> * chore(devcontainer): update image: v1.6.0-alpha.0 (#3242) Update devcontainer image: v1.6.0-alpha.0 Co-authored-by: glihm <glihm@users.noreply.github.com> * chore: add katana 1.5.4 (#3245) * chore: add the missing backticks in the comments (#3243) Signed-off-by: one230six <723682061@qq.com> * fix(bindgen): use world namespace for imported models in TypeScript S… (#3249) * fix(bindgen): filter out Value models (#3248) * fix(bindgen): fix ts bytearray type mapping (#3247) * chore: update katana versions (#3253) add katana versions * sozo(unrealengine): handle UE5.6 and Dojo 1.5 (#3252) * fix(sozo): assert caller permission with match (#3254) * handle call_contract_syscall() result for better panic trace * avoid using 0 as caller address * remove temporarly the RPC version check. Currently, Katana uses the new RPC types, without bumping the spec version. To ensure we can still use sozo with sepolia/mainnet and Katana, Sozo will not check the RPC version for now * update test dbs --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): create standalone bindgen command (#3246) * feat: create standalone bindgen command * cleanup unused inputs * remove dbg * add meaningful error if project is not built --------- Co-authored-by: glihm <dev@glihm.net> * feat(sozo): add MCP sever (#3256) * feat(sozo): add mvp for mcp server * refacto: restructure MCP server * evaluate changes using official rust sdk * feat(mcp): add stdio support * refacto with rmcp crate * wip * bump rmcp * add testing support * wip * refacto * cleanup * add instructions * add instructions * wip test * refacto tests * add debugging * ignore test with katana at the moment * fix typos and sozo path * refacto uri parsing * remove dbg * disable test that should be run with katana * fix test, windows fails because of new reqwest version * refactor(types): schema json sql value (#3257) * refactor(types): schema json sql value * remove excess comma * fix clippy --------- Co-authored-by: glihm <dev@glihm.net> * chore: bump cairo packages to 1.6.0-alpha.0 * fix(mcp): ensure test is using latest version * split scarb metadata ext in a different crate for dependencies * wip: controller issue * wip * wip: resolve conflicts and update deps * cleanup * lint * wip * wip tests * fix all tests * rebuild test db * fix: run linter * fix slot dep with github commit * fix clippy * fix inspect by adding missing json format * fix mcp formatting * fix stdio_test * use sozo from path --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com> * feat(core): use metaprogramming for tuple and fixed size array Introspect and DojoStore (#3260) * reuse Starkware metaprogramming stuff to manage tuples * fix fmt + clippy + artifacts * use metaprogramming to handle tuple introspect * fix fmt + clippy + artifacts * add fixed size array support * update dojo-world * update abigen + artifacts after rebase * add a sum_sizes function * update artifacts * tooling: add cairo-bench tool + bench tests (#3240) * add cairo-bench tool * add bench tests * first optimisation batch * fix fmt * add license info * update artifacts after rebase * fix cairo-bench tests * update artifacts * update artifacts * update scarb lock --------- Co-authored-by: glihm <dev@glihm.net> * fix(cairo-bench): use write and threshold + refactor fixed size array (#3267) * remove update-ref-test-list argument * add threshold parameter to cairo-bench (set to 3% by default) * max_fee_raw and fee_estimate_multiplier don't exist anymore * update sozo model commands for fixed size arrays * fix fmt + clippy * rename --update-ref to --write * fix some tests * fix fmt * fix(ci): update katana to 1.6.2 for compatible db --------- Signed-off-by: one230six <723682061@qq.com> Co-authored-by: Rémy Baranx <remy.baranx@gmail.com> Co-authored-by: Ritik <ritikverma0050@gmail.com> Co-authored-by: Daniel Kronovet <kronovet@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Ammar Arif <evergreenkary@gmail.com> Co-authored-by: Tarrence van As <tarrencev@users.noreply.github.com> Co-authored-by: glihm <glihm@users.noreply.github.com> Co-authored-by: Benjamin <158306087+bengineer42@users.noreply.github.com> Co-authored-by: braveocheretovych <braveocheretovych@gmail.com> Co-authored-by: one230six <163239332+one230six@users.noreply.github.com> Co-authored-by: Brother MartianGreed <valentin@pupucecorp.com> Co-authored-by: Corentin Cailleaud <corentin.cailleaud@caillef.com> Co-authored-by: Larko <59736843+Larkooo@users.noreply.github.com>
Description
Related issue
#3216
Summary by CodeRabbit