refactor(lang): reduce events generated code #2662
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,52 @@ | ||
| use dojo::meta::Layout; | ||
| use dojo::meta::{Introspect, introspect::{Struct, Ty}}; | ||
| use dojo::model::model::ModelParser; | ||
|
|
||
| #[derive(Drop, Serde, Debug, PartialEq)] | ||
| pub struct EventDef { | ||
| pub name: ByteArray, | ||
| pub layout: Layout, | ||
| pub schema: Struct | ||
|
Comment on lines
+6
to
9
There was a problem hiding this comment. 💡 Codebase verification Ohayo, sensei! The renaming needs attention in several files. The search results show that
The renaming appears to be incomplete and needs to be consistently applied across all these files. 🔗 Analysis chainOhayo, sensei! Ensure consistent renaming from Please verify that all references to Run the following script to check for any remaining references: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Search for any remaining references to `EventDefinition`.
rg 'EventDefinition' --type cairo
Length of output: 63 Script: #!/bin/bash
# Let's search for EventDefinition without file type restriction since cairo might not be recognized
rg "EventDefinition"
# Also search for EventDef to understand the context better
rg "EventDef"
# Let's also check all cairo files to see their presence
fd "\.cairo$"
Length of output: 9181 |
||
| } | ||
|
Comment on lines
+2
to
10
There was a problem hiding this comment. 💡 Codebase verification Ohayo, sensei! The EventDef renaming is inconsistent and needs attention. The codebase shows mixed usage of both
This dual naming creates confusion and violates consistent naming conventions. Either revert to 🔗 Analysis chainOhayo, sensei! Verify the EventDef renaming across the codebase. The renaming from 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check for any remaining references to EventDefinition
# and verify EventDef usage
echo "Checking for remaining EventDefinition references:"
rg "EventDefinition" --type cairo
echo -e "\nVerifying EventDef usage:"
rg "EventDef" --type cairo
Length of output: 298 Script: #!/bin/bash
# Let's try without file type filtering since cairo isn't recognized
echo "Checking for remaining EventDefinition references:"
rg "EventDefinition"
echo -e "\nVerifying EventDef usage:"
rg "EventDef"
Length of output: 4421 |
||
|
|
||
| pub trait EventDefinition<E> { | ||
| fn name() -> ByteArray; | ||
| } | ||
|
|
||
| pub trait Event<T> { | ||
| fn name() -> ByteArray; | ||
| fn definition() -> EventDef; | ||
| fn layout() -> Layout; | ||
| fn schema() -> Struct; | ||
| fn serialized_keys(self: @T) -> Span<felt252>; | ||
| fn serialized_values(self: @T) -> Span<felt252>; | ||
| /// Returns the selector of the event computed for the given namespace hash. | ||
| fn selector(namespace_hash: felt252) -> felt252; | ||
| } | ||
|
|
||
| pub impl EventImpl<E, +ModelParser<E>, +EventDefinition<E>, +Serde<E>, +Introspect<E>> of Event<E> { | ||
| fn name() -> ByteArray { | ||
| EventDefinition::<E>::name() | ||
| } | ||
| fn definition() -> EventDef { | ||
| EventDef { name: Self::name(), layout: Self::layout(), schema: Self::schema() } | ||
| } | ||
| fn layout() -> Layout { | ||
| Introspect::<E>::layout() | ||
| } | ||
| fn schema() -> Struct { | ||
| match Introspect::<E>::ty() { | ||
| Ty::Struct(s) => s, | ||
| _ => panic!("Event: invalid schema.") | ||
| } | ||
|
Comment on lines
+38
to
+41
There was a problem hiding this comment. 🛠️ Refactor suggestion Ohayo, sensei! Consider graceful error handling in the Using Apply this diff to modify the fn schema() -> Struct {
match Introspect::<E>::ty() {
Ty::Struct(s) => s,
- _ => panic!("Event: invalid schema.")
+ _ => {
+ // Return an error for invalid schema type
+ let err_msg = "Event: invalid schema.";
+ Err(err_msg)
+ }
}
}This change allows calling functions to handle the error appropriately.
|
||
| } | ||
| fn serialized_keys(self: @E) -> Span<felt252> { | ||
| ModelParser::<E>::serialize_keys(self) | ||
| } | ||
| fn serialized_values(self: @E) -> Span<felt252> { | ||
| ModelParser::<E>::serialize_values(self) | ||
| } | ||
| fn selector(namespace_hash: felt252) -> felt252 { | ||
| dojo::utils::selector_from_namespace_and_name(namespace_hash, @Self::name()) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| #[starknet::interface] | ||
| pub trait IEvent<T> { | ||
| fn definition(self: @T) -> super::EventDef; | ||
|
There was a problem hiding this comment. 💡 Codebase verification Ohayo sensei! The EventDef transition is incomplete! The codebase still contains multiple references to
This inconsistency could lead to confusion and potential issues. The change should be applied uniformly across all files. 🔗 Analysis chainOhayo sensei! The return type change looks good! The change from Let's verify the consistency of this change across the codebase: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that EventDefinition has been completely replaced with EventDef
# Test 1: Check for any remaining references to EventDefinition
echo "Checking for remaining EventDefinition references..."
rg "EventDefinition"
# Test 2: Verify the new EventDef usage
echo "Verifying EventDef usage..."
rg "EventDef"
Length of output: 4423 |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohayo sensei! Please revert the hardcoded local paths.
The changes introduce hardcoded local paths that will break builds for other developers. These paths are specific to your local development environment and should not be committed.
Please revert these changes and restore the git dependencies:
If you need to use local paths for development:
.gitignore[patch.crates-io]in a separateCargo.toml.localfile📝 Committable suggestion