-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Validate component children #1647
Conversation
This PR allows a component to explicitly state only allowed child components
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.
Looks good to me.
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.
can you also add
Text
andCenter
as invalid children ofText
Tr
as the only valid child ofThead
,Tbody
, andTfoot
TableCaption
,Thead
,Tbody
, andTfoot
as the only valid children ofTable
This should help us reduce incidence of hydrations errors from silly mistakes
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.
Looks good! Some comments but I don't think they're critical, we can come back to them in a follow up
@@ -435,30 +443,44 @@ def render(self) -> Dict: | |||
), | |||
autofocus=self.autofocus, | |||
) | |||
self._validate_component_children( |
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.
should we keep the check here? Only reason is because if they somehow added children dynamically after creating their component, then we wouldn't catch it. Though I don't think I've seen people really do that...
|
||
comp_name = type(self).__name__ | ||
|
||
def validate_invalid_child(child_name): |
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.
nit: we could inline these functions ore define them outside of the function. I've noticed before in Python that defining functions within tight loops is often a performance issue, and these checks will be called many times per compile.
@@ -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] = [] |
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.
I'm also thinking in a follow up if these should maybe be List[Type[Component]]
rather than str
This PR allows a component to explicitly state only allowed child components
usage:
closes #1648 #1588