reflex icon indicating copy to clipboard operation
reflex copied to clipboard

get_event_triggers logic is inconsistent with the rest of the component public API.

Open Lendemor opened this issue 7 months ago β€’ 3 comments

Repro snippet

from reflex.components.component import Component

# Custom component hierarchy for testing
class Child(Component):
    """A child component inheriting from Component."""

    tag = "div"

    def get_event_triggers(self):
        """Return custom event triggers for Child."""
        return {
            "on_child_click": rx.event.no_args_event_spec,
            "on_child_hover": rx.event.no_args_event_spec,
        }


class GrandChild(Child):
    """A grandchild component inheriting from Child."""

    tag = "span"

    on_grandchild_special: rx.EventHandler[rx.event.no_args_event_spec]


# Test the component hierarchy
child_instance = Child.create()
grandchild_instance = GrandChild.create()

print("Child event triggers:", list(child_instance.get_event_triggers().keys()))
print(
    "GrandChild event triggers:", list(grandchild_instance.get_event_triggers().keys())
)

Current result

Child event triggers: ['on_child_click', 'on_child_hover'] GrandChild event triggers: ['on_child_click', 'on_child_hover']

Expected result

Child event triggers: ['on_child_click', 'on_child_hover'] GrandChild event triggers: ['on_child_click', 'on_child_hover', ''on_grandchild_special']

Reasoning

In this case, child component intentionally omit the super().get_event_triggers() because default events are not wanted for that component.

This is the correct way to remove inherited event triggers, but it also remove the event triggers defined in the grandchild component (because 'on_grandchild_special' is usually pushed via super().get_event_triggers() which doesn't quite make sense.

Lendemor avatar May 26 '25 20:05 Lendemor

Hey - I was looking at this issue out of curiosity.

If the desired behavior is for Child component to not include any default events, but GrandChild need to have all Child events without the default ones, isn't the following a bettery way to define GrandChild:

class GrandChild(Child):
        """A grandchild component inheriting from Child."""

        tag = "span"

        def get_event_triggers(self):
            """Return custom event triggers for Child."""
            return {
                **super().get_event_triggers(),
                "on_grandchild_special": rx.event.no_args_event_spec
            }

Without this super() call, Component's field inspection logic (that looks for EventHandler fields) never runs, so on_grandchild_special would be ignored.

priyankc avatar May 30 '25 08:05 priyankc

What you wrote would work yes, but that's not the main issue.

The problem is that by overriding get_event_triggers users may want to remove inherited event_triggers (from base Component or another intermediary component), but this will also remove a part of the internal logic applied in the base get_event_triggers.

If we compare to how we define all other component-related elements (see add_hooks, add_styles, etc..) , this one stand out as being inconsistent.

I'll label this as enhancement instead, but we should really separate the logic from the default event_triggers values.

Lendemor avatar May 31 '25 06:05 Lendemor