feat: add events in typescript bindgen + sort functions by name #2853
Conversation
Ohayo, sensei! 🍵 Let's dive into this TypeScript bindgen adventure!WalkthroughThe pull request modifies the TypeScript code generation process in the Dojo bindgen system. The changes focus on enhancing how events and contracts are processed and transformed into TypeScript code. The Changes
Suggested Reviewers
Possibly Related PRs
Sensei, the code is ready for your wisdom! 🥷🏼🍱 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Nitpick comments (6)
crates/dojo/bindgen/src/plugins/typescript/writer.rs (6)
25-45: Ohayo sensei, consider extracting event composites into a helper
We see code duplication with the model composites. Extracting common logic for collecting enums, structs, and functions into a standalone helper could reduce redundancy and future maintenance overhead.
Line range hint
47-70: Ohayo sensei, unify repeated pattern for model composites
Similarly, the logic for enumerating model tokens mirrors the event composites. Refactoring to a shared function might keep these expansions in sync and more maintainable.
71-72: Ohayo sensei, nice to see deterministic sorting
Sorting bytype_pathensures consistent output. If ties matter or original ordering needs preservation, consider a stable sort.
78-78: Ohayo sensei, watch out for repeated clones
Concatenating cloned vectors is handy but might add overhead for large data sets. Merging them into one vector before iteration could be more efficient.
116-150: Ohayo sensei, handle potential panics gracefully
to_function().unwrap()could panic if conversion fails. While you handle such cases below with apanic!, consider returning an error to propagate gracefully.
157-167: Ohayo sensei, augment error handling
Currently, this loop logs an error if code generation fails and then continues. If partial generation is undesirable, consider failing fast or adding a fallback strategy.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
crates/dojo/bindgen/src/plugins/typescript/writer.rs(3 hunks)
🔇 Additional comments (2)
crates/dojo/bindgen/src/plugins/typescript/writer.rs (2)
24-25: Ohayo sensei, new events vector creation looks straightforward
Your approach here to gather event data before processing appears clean and efficient. No immediate issues spotted.
151-151: Ohayo sensei, function sorting looks good
Sorting the contract functions by name is a straightforward way to achieve consistent ordering.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2853 +/- ##
==========================================
- Coverage 56.16% 56.15% -0.02%
==========================================
Files 440 440
Lines 56635 56670 +35
==========================================
+ Hits 31810 31823 +13
- Misses 24825 24847 +22 ☔ View full report in Codecov by Sentry. |
Solves #2781 and #2782
Summary by CodeRabbit