fix(sozo): adjust typescript bindgen to v1 #2202
Conversation
WalkthroughOhayo, sensei! The recent updates to the Changes
Recent review detailsConfiguration used: .coderabbit.yaml Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
crates/dojo-bindgen/src/plugins/typescript/mod.rs (1)
326-339: Ohayo, sensei!The recent changes to the
format_systemfunction intypescript/mod.rsinclude an additionalnamespaceparameter, which is a great improvement. However, the function calls inunity/mod.rsandtypescript_v2/mod.rsstill use the old signature and need to be updated.Please update the following locations to include the
namespaceparameter:
crates/dojo-bindgen/src/plugins/unity/mod.rs
- Line:
fn format_system(system: &Function, handled_tokens: &HashMap<String, Composite>) -> String- Line:
.map(|system| UnityPlugin::format_system(system.to_function().unwrap(), handled_tokens))
crates/dojo-bindgen/src/plugins/typescript_v2/mod.rs
- Line:
fn format_system(system: &Function, handled_tokens: &[Composite]) -> String- Line:
.map(|system| TypeScriptV2Plugin::format_system(system.to_function().unwrap(), handled_tokens))This will ensure all function calls are consistent with the new signature.
Analysis chain
Ohayo, sensei! The addition of the
namespaceparameter and filtering mechanism is a great improvement.This change enhances the control flow by skipping specific systems during processing.
However, ensure that all function calls to
format_systemmatch the new signature.
Ohayo, sensei!
The recent changes to the
format_systemfunction intypescript/mod.rsinclude an additionalnamespaceparameter, which is a great improvement. However, it appears that other files (unity/mod.rsandtypescript_v2/mod.rs) still use the old signature.Let's ensure all calls to
format_systemmatch the new signature. Here's a follow-up script to locate all calls toformat_systemand verify their signatures:Once we have this information, we can pinpoint the exact locations that need updating.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `format_system` match the new signature. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type rust -A 5 $'format_system'Length of output: 3178
Script:
#!/bin/bash # Description: Verify all function calls to `format_system` match the new signature. # Search for all calls to `format_system` and display context lines to check their signatures. rg --type rust 'format_system' -A 3 -B 3Length of output: 4166
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2202 +/- ##
==========================================
- Coverage 68.17% 68.06% -0.11%
==========================================
Files 336 336
Lines 44053 44122 +69
==========================================
+ Hits 30032 30033 +1
- Misses 14021 14089 +68 ☔ View full report in Codecov by Sentry. |
| // filter out common types | ||
| // TODO: Make cleaner | ||
| if token.type_path == "core::option::Option::<core::integer::u32>" | ||
| || token.type_path == "core::option::Option::<core::integer::u8>" | ||
| || token.type_path == "core::option::Option::<core::integer::u16>" | ||
| || token.type_path == "core::option::Option::<core::integer::u64>" | ||
| || token.type_path == "core::option::Option::<core::integer::u128>" | ||
| || token.type_path == "core::option::Option::<core::integer::u256>" | ||
| { | ||
| return String::new(); // Return an empty string for these enums | ||
| } |
There was a problem hiding this comment.
Ohayo sensei! Optimize the filtering mechanism for enums.
Consider using a set for cleaner and more efficient filtering of Option types.
let excluded_types: HashSet<&str> = [
"core::option::Option::<core::integer::u32>",
"core::option::Option::<core::integer::u8>",
"core::option::Option::<core::integer::u16>",
"core::option::Option::<core::integer::u64>",
"core::option::Option::<core::integer::u128>",
"core::option::Option::<core::integer::u256>",
].iter().cloned().collect();
if excluded_types.contains(token.type_path.as_str()) {
return String::new(); // Return an empty string for these enums
}| fn format_system(system: &Function, handled_tokens: &[Composite], namespace: String) -> String { | ||
| if [ | ||
| "contract_name", | ||
| "namespace", | ||
| "tag", | ||
| "name_hash", | ||
| "selector", | ||
| "dojo_init", | ||
| "namespace_hash", | ||
| ] | ||
| .contains(&system.name.as_str()) | ||
| { | ||
| return String::new(); | ||
| } |
There was a problem hiding this comment.
Ohayo sensei! Optimize the filtering mechanism for systems.
Consider using a set for cleaner and more efficient filtering of system names.
let excluded_systems: HashSet<&str> = [
"contract_name",
"namespace",
"tag",
"name_hash",
"selector",
"dojo_init",
"namespace_hash",
].iter().cloned().collect();
if excluded_systems.contains(system.name.as_str()) {
return String::new();
}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
crates/dojo-bindgen/src/plugins/typescript/mod.rs (3)
142-152: Ohayo sensei! Optimize the filtering mechanism for enums.Consider using a set for cleaner and more efficient filtering of
Optiontypes.let excluded_types: HashSet<&str> = [ "core::option::Option::<core::integer::u32>", "core::option::Option::<core::integer::u8>", "core::option::Option::<core::integer::u16>", "core::option::Option::<core::integer::u64>", "core::option::Option::<core::integer::u128>", "core::option::Option::<core::integer::u256>", ].iter().cloned().collect(); if excluded_types.contains(token.type_path.as_str()) { return String::new(); // Return an empty string for these enums }
206-231: Ohayo sensei! Improve readability of field mapping.Consider breaking down the field mapping logic into smaller helper functions for better readability and maintainability.
346-359: Ohayo sensei! Optimize the filtering mechanism for systems.Consider using a set for cleaner and more efficient filtering of system names.
let excluded_systems: HashSet<&str> = [ "contract_name", "namespace", "tag", "name_hash", "selector", "dojo_init", "namespace_hash", ].iter().cloned().collect(); if excluded_systems.contains(system.name.as_str()) { return String::new(); }
Summary by CodeRabbit
New Features
tokiodependency.Improvements
cainomedependency to a newer version for bug fixes and enhancements.Tests