Skip to content

feat: add enum const types - #3091

Merged
glihm merged 1 commit into
dojoengine:mainfrom
MartianGreed:feat/ts-enum-const
Mar 11, 2025
Merged

feat: add enum const types#3091
glihm merged 1 commit into
dojoengine:mainfrom
MartianGreed:feat/ts-enum-const

Conversation

@MartianGreed

@MartianGreed MartianGreed commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Description

Related issue

Tests

  • Yes
  • No, because they aren't needed
  • No, because I need help

Added to documentation?

  • README.md
  • Dojo Book
  • No documentation needed

Checklist

  • I've formatted my code (scripts/prettier.sh, scripts/rust_fmt.sh, scripts/cairo_fmt.sh)
  • I've linted my code (scripts/clippy.sh, scripts/docs.sh)
  • I've commented my code
  • I've requested a review after addressing the comments

Summary by CodeRabbit

  • New Features

    • Introduced two approaches for TypeScript enum generation: one using a simple array format and another offering a custom format with dedicated type definitions, providing more flexible output.
    • Enhanced token classification with new functions to determine characteristics of composite types.
  • Tests

    • Expanded test coverage to validate the new custom enum generation logic and ensure consistent output formatting across various scenarios.

@coderabbitai

coderabbitai Bot commented Mar 10, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Ohayo sensei! This pull request enhances TypeScript enum generation in the Bindgen module. It introduces two new methods—generate_simple_enum and generate_custom_enum—in the TypeScript enum generator. The main generate method now conditionally calls one of these based on a check using the new token_is_custom_enum function. Additionally, two helper functions, token_has_inner_composite and token_is_custom_enum, have been added to better classify token types, and the corresponding test cases and import statements have been updated.

Changes

File Path Changes Summary
crates/dojo/bindgen/.../enum.rs - Added generate_simple_enum and generate_custom_enum methods.
- Updated generate method to call the appropriate enum generator based on token_is_custom_enum.
- Revised import statements and test cases.
crates/dojo/bindgen/.../mod.rs - Introduced token_has_inner_composite to evaluate inner composite types.
- Added token_is_custom_enum to determine whether a token represents a custom enum.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Client
    participant Gen as TsEnumGenerator
    Caller->>Gen: generate(token, buffer)
    Gen->>Gen: token_is_custom_enum(token)
    alt Custom Enum
        Gen->>Gen: generate_custom_enum(token, buffer)
    else Simple Enum
        Gen->>Gen: generate_simple_enum(token, buffer)
    end
    Gen-->>Caller: Return TypeScript enum string
Loading

Possibly related PRs

Suggested reviewers

  • glihm

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between 4a69635 and 5afc1ce.

📒 Files selected for processing (2)
  • crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (6 hunks)
  • crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build-explorer
🔇 Additional comments (11)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (2)

51-60: Ohayo! Nice addition of token inspection utilities!

The new token_has_inner_composite function is well-implemented, efficiently checking for nested composite types by examining different token variants. The pattern matching approach is clean and covers all the necessary cases.


68-72: The custom enum detection looks great, sensei!

This function combines the enum type check with the inner composite check to properly identify custom enums. It's a clean, focused function that will help with specialized type generation.

crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (9)

2-2: Ohayo! Good addition of convert_case for proper casing in generated code

Adding the Case and Casing imports from convert_case will help ensure consistent naming conventions in the generated TypeScript code.


5-5: Nice import of the new token_is_custom_enum function, sensei!

This properly brings in the custom enum detection utility from the mod.rs file.


26-45: Excellent implementation of simple enum generation!

The generate_simple_enum function creates clean TypeScript code for simple enums using modern TypeScript features. The format of generating a const array with as const and then a type based on that array is idiomatic TypeScript.


47-76: Well-structured custom enum generator, sensei!

The generate_custom_enum method creates a more complex TypeScript type definition that includes type information for each variant. The approach is consistent with the simple enum generation while adding the necessary complexity for custom enums.


79-97: Smart conditional enum generation logic!

The updated generate method now dynamically chooses between simple and custom enum generation based on the enum's structure. This follows the single responsibility principle and keeps the code clean.


149-154: Nice test update to match new enum format!

You've properly updated the assertions to match the new TypeScript enum generation format.


162-165: Updated test buffer to match new enum format!

This change ensures the duplicate detection test works correctly with the new enum format.


176-189: Excellent test case for custom enums, sensei!

Adding a dedicated test for custom enum generation ensures that the new functionality works as expected. The test case is well-structured with clear assertions.


220-254: Great test helper function for custom enum creation!

This helper creates a complex custom enum with nested structures, which is perfect for testing the custom enum generation logic. The detailed structure gives confidence that the generation will handle real-world complex enums.


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 (1)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)

51-60: Ohayo sensei! Consider recursive checking for multi-level composites.
The current implementation only checks direct children of the token. If you have deeper nesting (e.g., an array of tuples containing composites), you may need a recursive approach to detect them. Otherwise, this is a neat utility.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5b83295 and 55dc93c.

📒 Files selected for processing (2)
  • crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (6 hunks)
  • crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build-explorer
🔇 Additional comments (7)
crates/dojo/bindgen/src/plugins/typescript/generator/mod.rs (1)

68-73: Ohayo sensei! Custom enum detection looks solid.
This function correctly identifies custom enums by confirming they have inner composites. The logic is straightforward and consistent with the rest of the code.

crates/dojo/bindgen/src/plugins/typescript/generator/enum.rs (6)

2-2: Ohayo sensei! Good job importing necessary utilities.
Adding convert_case::{Case, Casing} and referencing token_is_custom_enum / token_is_enum fits nicely with your usage in this file.

Also applies to: 5-5


26-50: Ohayo sensei! Simple enum generation workflow looks good.
The generate_simple_enum function neatly uses a string array plus a mapped type. It's an elegant representation for basic enums in TypeScript.


51-85: Ohayo sensei! Custom enum generation is well-structured.
The split between simple and custom enum generation is clear. The template format here is easy to follow, ensuring each variant receives the right type mapping.


87-106: Ohayo sensei! Conditional logic for enum generation is concise.
Using token_is_custom_enum vs. generate_simple_enum is a clean pattern. The short-circuit for non-enum or empty inners is a good safeguard.


184-185: Ohayo sensei! New test block is well-labeled.
Defining test_custom_enum clarifies the coverage for custom enum generation. This naming convention aids test suite readability.


228-262: Ohayo sensei! The custom enum token creation is nicely structured.
Constructing a composite with multiple nested variants is thorough. This test fixture demonstrates a realistic scenario for verifying custom enum logic.

fn generate_simple_enum(
&self,
token: &Composite,
buffer: &mut Buffer,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not used, can be removed if not planned to be re-introduced later.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be wrong but I think this is used L94 underneath BindgenModelGenerator.generate fn ser

fn generate_custom_enum(
&self,
token: &Composite,
buffer: &mut Buffer,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, buffer is not used anymore, can be removed if not planned to be used (or not part of a trait that must be implemented).

@codecov

codecov Bot commented Mar 11, 2025

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 98.19820% with 2 lines in your changes missing coverage. Please review.

Project coverage is 57.21%. Comparing base (5b83295) to head (5afc1ce).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...jo/bindgen/src/plugins/typescript/generator/mod.rs 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3091      +/-   ##
==========================================
- Coverage   57.22%   57.21%   -0.01%     
==========================================
  Files         441      446       +5     
  Lines       60121    60547     +426     
==========================================
+ Hits        34403    34643     +240     
- Misses      25718    25904     +186     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@glihm
glihm merged commit 2cc5eab into dojoengine:main Mar 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants