From acb13a864a99e44096dce164457cc43d32e1071e Mon Sep 17 00:00:00 2001 From: Elijah Date: Mon, 21 Aug 2023 14:07:15 +0000 Subject: [PATCH 1/5] Validate component children: This PR allows a component to explicitly state only allowed child components --- reflex/components/component.py | 26 +++++++++++++++++++------- tests/components/test_component.py | 26 +++++++++++++++++++++++--- 2 files changed, 42 insertions(+), 10 deletions(-) diff --git a/reflex/components/component.py b/reflex/components/component.py index 0f4756fd0e..14a6825a39 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -66,6 +66,10 @@ class Component(Base, ABC): # components that cannot be children invalid_children: List[str] = [] + + # components that are only allowed as children + valid_children: List[str] = [] + # custom attribute custom_attrs: Dict[str, str] = {} @@ -435,29 +439,37 @@ def render(self) -> Dict: ), autofocus=self.autofocus, ) - self._validate_component_children( - rendered_dict["name"], rendered_dict["children"] - ) + self._validate_component_children(rendered_dict) return rendered_dict - def _validate_component_children(self, comp_name: str, children: List[Dict]): + def _validate_component_children(self, rendered_dict: Dict[str, Any]): """Validate the children components. Args: - comp_name: name of the component. - children: list of children components. + rendered_dict: The component's rendered dictionary. Raises: ValueError: when an unsupported component is matched. """ - if not self.invalid_children: + if not self.invalid_children or not self.valid_children: return + children = rendered_dict["children"] + comp_name = rendered_dict["name"] + + # check that explicitly stated invalid components are not allowed. for child in children: name = child["name"] if name in self.invalid_children: raise ValueError( f"The component `{comp_name.lower()}` cannot have `{name.lower()}` as a child component" ) + # check that only explicitly stated valid components are allowed. + for child in children: + name = child["name"] + if name not in self.valid_children: + raise ValueError( + f"The component `{comp_name.lower()}` only allows the components: {','.join([f'`{v_child}`' for v_child in self.valid_children])} as children. Got `{name.lower()}` instead." + ) def _get_custom_code(self) -> Optional[str]: """Get custom code for the component. diff --git a/tests/components/test_component.py b/tests/components/test_component.py index 23274b80dc..c6ebcd8c3c 100644 --- a/tests/components/test_component.py +++ b/tests/components/test_component.py @@ -121,10 +121,12 @@ def component5() -> Type[Component]: """ class TestComponent5(Component): - tag = "Tag" + tag = "RandomComponent" invalid_children: List[str] = ["Text"] + valid_children: List[str] = ["Text"] + return TestComponent5 @@ -462,7 +464,8 @@ def test_get_hooks_nested2(component3, component4): def test_unsupported_child_components(component5): - """Test that a value error is raised when an unsupported component is provided as a child. + """Test that a value error is raised when an unsupported component (a child component found in the + component's invalid children list) is provided as a child. Args: component5: the test component @@ -472,5 +475,22 @@ def test_unsupported_child_components(component5): comp.render() assert ( err.value.args[0] - == f"The component `tag` cannot have `text` as a child component" + == f"The component `randomcomponent` cannot have `text` as a child component" + ) + + +def test_component_with_only_valid_children(component5): + """Test that a value error is raised when an unsupported component (a child component not found in the + component's valid children list) is provided as a child. + + Args: + component5: the test component + """ + with pytest.raises(ValueError) as err: + comp = component5.create(rx.box("testing component")) + comp.render() + assert ( + err.value.args[0] + == f"The component `randomcomponent` only allows the components: `Text` as children. " + f"Got `box` instead." ) From b750c4996abfc7b599ffeefe6530053ed0e1ef83 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 23 Aug 2023 13:33:16 +0000 Subject: [PATCH 2/5] refactor --- reflex/components/component.py | 24 +++++++++++++++--------- reflex/components/forms/button.py | 4 ++++ reflex/components/overlay/menu.py | 5 ++++- tests/components/test_component.py | 4 ++-- 4 files changed, 25 insertions(+), 12 deletions(-) diff --git a/reflex/components/component.py b/reflex/components/component.py index 14a6825a39..184e071e18 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -107,9 +107,10 @@ def __init__(self, *args, **kwargs): TypeError: If an invalid prop is passed. """ # Set the id and children initially. + children = kwargs.get("children", []) initial_kwargs = { "id": kwargs.get("id"), - "children": kwargs.get("children", []), + "children": children, **{ prop: Var.create(kwargs[prop]) for prop in self.get_initial_props() @@ -118,6 +119,8 @@ def __init__(self, *args, **kwargs): } super().__init__(**initial_kwargs) + self._validate_component_children(children) + # Get the component fields, triggers, and props. fields = self.get_fields() triggers = self.get_triggers() @@ -385,6 +388,7 @@ def create(cls, *children, **props) -> Component: else Bare.create(contents=Var.create(child, is_string=True)) for child in children ] + return cls(children=children, **props) def _add_style(self, style): @@ -439,33 +443,35 @@ def render(self) -> Dict: ), autofocus=self.autofocus, ) - self._validate_component_children(rendered_dict) return rendered_dict - def _validate_component_children(self, rendered_dict: Dict[str, Any]): + def _validate_component_children(self, children: List[Component]): """Validate the children components. Args: - rendered_dict: The component's rendered dictionary. + children: The children of the component. Raises: ValueError: when an unsupported component is matched. """ - if not self.invalid_children or not self.valid_children: + if not self.invalid_children and not self.valid_children: return - children = rendered_dict["children"] - comp_name = rendered_dict["name"] + comp_name = type(self).__name__ # check that explicitly stated invalid components are not allowed. for child in children: - name = child["name"] + if not self.invalid_children: + break + name = type(child).__name__ if name in self.invalid_children: raise ValueError( f"The component `{comp_name.lower()}` cannot have `{name.lower()}` as a child component" ) # check that only explicitly stated valid components are allowed. for child in children: - name = child["name"] + if not self.valid_children: + break + name = type(child).__name__ if name not in self.valid_children: raise ValueError( f"The component `{comp_name.lower()}` only allows the components: {','.join([f'`{v_child}`' for v_child in self.valid_children])} as children. Got `{name.lower()}` instead." diff --git a/reflex/components/forms/button.py b/reflex/components/forms/button.py index 3ec573bd93..5c336f8e89 100644 --- a/reflex/components/forms/button.py +++ b/reflex/components/forms/button.py @@ -1,4 +1,5 @@ """A button component.""" +from typing import List from reflex.components.libs.chakra import ChakraComponent from reflex.vars import Var @@ -42,6 +43,9 @@ class Button(ChakraComponent): # The type of button. type_: Var[str] + # Components that are not allowed as children. + invalid_children: List[str] = ["Button", "MenuButton"] + class ButtonGroup(ChakraComponent): """A group of buttons.""" diff --git a/reflex/components/overlay/menu.py b/reflex/components/overlay/menu.py index 2c23407b0e..f477b36cc5 100644 --- a/reflex/components/overlay/menu.py +++ b/reflex/components/overlay/menu.py @@ -1,6 +1,6 @@ """Menu components.""" -from typing import Set +from typing import List, Set from reflex.components.component import Component from reflex.components.libs.chakra import ChakraComponent @@ -100,6 +100,9 @@ class MenuButton(ChakraComponent): # The variant of the menu button. variant: Var[str] + # Components that are not allowed as children. + invalid_children: List[str] = ["Button", "MenuButton"] + # The tag to use for the menu button. as_: Var[str] diff --git a/tests/components/test_component.py b/tests/components/test_component.py index c6ebcd8c3c..71328a2f55 100644 --- a/tests/components/test_component.py +++ b/tests/components/test_component.py @@ -475,7 +475,7 @@ def test_unsupported_child_components(component5): comp.render() assert ( err.value.args[0] - == f"The component `randomcomponent` cannot have `text` as a child component" + == f"The component `testcomponent5` cannot have `text` as a child component" ) @@ -491,6 +491,6 @@ def test_component_with_only_valid_children(component5): comp.render() assert ( err.value.args[0] - == f"The component `randomcomponent` only allows the components: `Text` as children. " + == f"The component `testcomponent5` only allows the components: `Text` as children. " f"Got `box` instead." ) From 107de21ee69f1586d2f05a3beb61f0fd412227cb Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 23 Aug 2023 13:40:04 +0000 Subject: [PATCH 3/5] refactor --- reflex/components/component.py | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/reflex/components/component.py b/reflex/components/component.py index 184e071e18..031bb671b0 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -456,27 +456,33 @@ def _validate_component_children(self, children: List[Component]): """ if not self.invalid_children and not self.valid_children: return + comp_name = type(self).__name__ - # check that explicitly stated invalid components are not allowed. - for child in children: - if not self.invalid_children: - break - name = type(child).__name__ - if name in self.invalid_children: + def validate_invalid_child(child_name): + if child_name in self.invalid_children: raise ValueError( - f"The component `{comp_name.lower()}` cannot have `{name.lower()}` as a child component" + f"The component `{comp_name.lower()}` cannot have `{child_name.lower()}` as a child component" + ) + + def validate_valid_child(child_name): + if child_name not in self.valid_children: + valid_child_list = ", ".join( + [f"`{v_child}`" for v_child in self.valid_children] ) - # check that only explicitly stated valid components are allowed. - for child in children: - if not self.valid_children: - break - name = type(child).__name__ - if name not in self.valid_children: raise ValueError( - f"The component `{comp_name.lower()}` only allows the components: {','.join([f'`{v_child}`' for v_child in self.valid_children])} as children. Got `{name.lower()}` instead." + f"The component `{comp_name.lower()}` only allows the components: {valid_child_list} as children. Got `{child_name.lower()}` instead." ) + for child in children: + name = type(child).__name__ + + if self.invalid_children: + validate_invalid_child(name) + + if self.valid_children: + validate_valid_child(name) + def _get_custom_code(self) -> Optional[str]: """Get custom code for the component. From 68a5f89f2ed6033b77a019784c72917bef380bf2 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 23 Aug 2023 13:41:37 +0000 Subject: [PATCH 4/5] refactor --- reflex/components/component.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/reflex/components/component.py b/reflex/components/component.py index 031bb671b0..ed98354a12 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -451,8 +451,6 @@ def _validate_component_children(self, children: List[Component]): Args: children: The children of the component. - Raises: - ValueError: when an unsupported component is matched. """ if not self.invalid_children and not self.valid_children: return From aef3350ccf836cfa4f07c1aeaf59d766d4cfa2f4 Mon Sep 17 00:00:00 2001 From: Elijah Date: Wed, 23 Aug 2023 13:57:16 +0000 Subject: [PATCH 5/5] more tests --- reflex/components/component.py | 4 +-- tests/components/test_component.py | 56 +++++++++++++++++++++++++----- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/reflex/components/component.py b/reflex/components/component.py index ed98354a12..2b8f89bf38 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -460,7 +460,7 @@ def _validate_component_children(self, children: List[Component]): def validate_invalid_child(child_name): if child_name in self.invalid_children: raise ValueError( - f"The component `{comp_name.lower()}` cannot have `{child_name.lower()}` as a child component" + f"The component `{comp_name}` cannot have `{child_name}` as a child component" ) def validate_valid_child(child_name): @@ -469,7 +469,7 @@ def validate_valid_child(child_name): [f"`{v_child}`" for v_child in self.valid_children] ) raise ValueError( - f"The component `{comp_name.lower()}` only allows the components: {valid_child_list} as children. Got `{child_name.lower()}` instead." + f"The component `{comp_name}` only allows the components: {valid_child_list} as children. Got `{child_name}` instead." ) for child in children: diff --git a/tests/components/test_component.py b/tests/components/test_component.py index 71328a2f55..c676e543af 100644 --- a/tests/components/test_component.py +++ b/tests/components/test_component.py @@ -130,6 +130,38 @@ class TestComponent5(Component): return TestComponent5 +@pytest.fixture +def component6() -> Type[Component]: + """A test component. + + Returns: + A test component. + """ + + class TestComponent6(Component): + tag = "RandomComponent" + + invalid_children: List[str] = ["Text"] + + return TestComponent6 + + +@pytest.fixture +def component7() -> Type[Component]: + """A test component. + + Returns: + A test component. + """ + + class TestComponent7(Component): + tag = "RandomComponent" + + valid_children: List[str] = ["Text"] + + return TestComponent7 + + @pytest.fixture def on_click1() -> EventHandler: """A sample on click function. @@ -463,34 +495,40 @@ def test_get_hooks_nested2(component3, component4): ) -def test_unsupported_child_components(component5): +@pytest.mark.parametrize("fixture", ["component5", "component6"]) +def test_unsupported_child_components(fixture, request): """Test that a value error is raised when an unsupported component (a child component found in the component's invalid children list) is provided as a child. Args: - component5: the test component + fixture: the test component as a fixture. + request: Pytest request. """ + component = request.getfixturevalue(fixture) with pytest.raises(ValueError) as err: - comp = component5.create(rx.text("testing component")) + comp = component.create(rx.text("testing component")) comp.render() assert ( err.value.args[0] - == f"The component `testcomponent5` cannot have `text` as a child component" + == f"The component `{component.__name__}` cannot have `Text` as a child component" ) -def test_component_with_only_valid_children(component5): +@pytest.mark.parametrize("fixture", ["component5", "component7"]) +def test_component_with_only_valid_children(fixture, request): """Test that a value error is raised when an unsupported component (a child component not found in the component's valid children list) is provided as a child. Args: - component5: the test component + fixture: the test component as a fixture. + request: Pytest request. """ + component = request.getfixturevalue(fixture) with pytest.raises(ValueError) as err: - comp = component5.create(rx.box("testing component")) + comp = component.create(rx.box("testing component")) comp.render() assert ( err.value.args[0] - == f"The component `testcomponent5` only allows the components: `Text` as children. " - f"Got `box` instead." + == f"The component `{component.__name__}` only allows the components: `Text` as children. " + f"Got `Box` instead." )