PropsBase converts EventHandler-annotated props to EventChain#5765
PropsBase converts EventHandler-annotated props to EventChain#5765adhami3310 merged 4 commits intomainfrom
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR refactors event handling in the Reflex framework by centralizing EventHandler-to-EventChain conversion at the PropsBase level. The changes extract the logic for processing EventHandler-annotated fields from the Component class into a new shared utility function args_specs_from_fields in the event module, and introduce automatic conversion in PropsBase initialization.
Key Changes:
- New utility function:
args_specs_from_fieldsinevent.pyextracts argument specifications from fields annotated with EventHandler types, checking for metadata or defaulting to no-args specs - Component simplification: The
get_event_triggersmethod incomponent.pynow uses the new utility function instead of inline logic - PropsBase enhancement: The
__init__method now automatically converts EventHandler-annotated fields to EventChain objects using the extracted arg specs - Test coverage: New tests verify that State event methods assigned to EventHandler fields are properly converted to EventChain instances
This architectural change moves event handling logic from the component layer to the props layer, creating consistency across all components that inherit from PropsBase. The refactoring enables developers to assign State methods directly to EventHandler-annotated fields (like on_click=FooState.handle_click), while the system automatically converts them to the required EventChain format internally. The utility function is also added as a static method to EventNamespace for broader accessibility.
PR Description Notes:
- Contains a typo: "pyi processing" should likely be "type processing" or reference to
.pyistub files
Confidence score: 4/5
- This PR introduces significant architectural changes to event handling but appears well-implemented with proper testing
- Score reflects the acknowledged typing limitation where EventCallback/EventType cannot be assigned to EventHandler fields without workarounds
- Pay close attention to
reflex/components/props.pyand the event handling conversion logic in PropsBase
4 files reviewed, 1 comment
| # Convert EventHandler to EventChain | ||
| args_specs = args_specs_from_fields(self.get_fields()) | ||
| for handler_name, args_spec in args_specs.items(): | ||
| if (handler := getattr(self, handler_name, None)) is not None: | ||
| setattr( | ||
| self, | ||
| handler_name, | ||
| EventChain.create( | ||
| value=handler, | ||
| args_spec=args_spec, | ||
| key=handler_name, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
style: The EventChain conversion logic executes for every PropsBase instantiation even when no EventHandler fields exist. Consider adding a quick check to skip this block if args_specs is empty for better performance.
CodSpeed Performance ReportMerging #5765 will not alter performanceComparing Summary
|
adhami3310
dismissed
their stale review
the unit tests aren't passing, but otherwise looks fine
Allow both Component and PropsBase to resolve annotations from the module namespace.
2 participants
Typing issue remains in that EventCallback/EventType cannot be assigned to a field annotated as EventHandler without pyi processing... not sure if we want to merge without support for this.