-
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
Improve Var type handling for better rx.Model attribute access #2010
Conversation
Allows a Var to access relationship attributes on a model from the frontend rendering code. Fix: REF-904
Fix #1814 * Remove py3.7 workarounds, use get_args and get_origin directly * Move `can_access_attribute` to types and call recursively for union types * Extend `is_valid_var_type` to allow Union where all args are valid var types
Allows access to relationship attributes on rx.Model subclasses outside of __fields__. Use null coalescing when accessing attributes to avoid frontend errors when fields are optional. (Fix REF-905) Use the new `can_access_attribute` helper to support vars with Union types. If a state var is Optional and no default is provided, use None
* only check for default default when the field is marked required * grab the first item in a Literal if no default is given
Ensure that null coalescing is applied when accessing vars that default to None
All the type: ignore comments are because the annotations in the UnionState are respected...and they're not Var typed. For the Union, it's technically a type error to access fields on a Union type without a discriminator, but we'll ignore that for the purposes of Var chaining.
"""Test that state can be defined with Union and Optional vars.""" | ||
|
||
class UnionState(State): | ||
int_float: Union[int, float] = 0 |
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 don't think other Var operations are prepared to handle bare Union types...
@@ -77,7 +78,22 @@ def dict(self, **kwargs): | |||
Returns: | |||
The object as a dictionary. | |||
""" | |||
return {name: getattr(self, name) for name in self.__fields__} | |||
base_fields = {name: getattr(self, name) for name in self.__fields__} | |||
relationships = {} |
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 we add some comments explaining these lines
reflex/utils/types.py
Outdated
return is_union(cls) and type(None) in get_args(cls) | ||
|
||
|
||
def can_access_attribute(cls: GenericType, name: str) -> GenericType | None: |
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.
Rename function since it doesn't return a bool
reflex/utils/types.py
Outdated
Returns: | ||
The type of the attribute, if accessible, or None | ||
""" | ||
from ..model import Model |
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.
from reflex.model import Model
for arg in get_args(cls): | ||
type_ = can_access_attribute(arg, name) | ||
if type_ is not None: | ||
return type_ |
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.
Add return None
Support accessing relationship attributes on Models
Support accessing all possible attributes when Var is annotated as a Union (fix #1814)
Use null coalescing when the var is Optional or has a default value of None
reflex.utils.types.is_union
supports new styleUnionType
on py3.10