From 23fa20bf70225ba9a8675e32994c7ce0774ea8fb Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:24:41 +0530 Subject: [PATCH 1/2] Warn when a base var shadows a var inherited from a parent state A substate field whose name matches an inherited base var is dropped silently: get_skip_vars() includes inherited_vars, so base_vars filters the field out. _init_var never runs for it, class access returns the raw default instead of a Var, and reads and writes are delegated to the parent. A component built from it renders a static value rather than a reactive binding, with no diagnostic anywhere. Emit a deduped console.warn at class creation naming the var and both states. A redeclaration whose purpose is to win over a descriptor reached through a non-state base is exempt, since re-annotating is how that MRO conflict is resolved (see test_hybrid_property_shadowed_by_closer_base_stays_a_field). Warning rather than raising, per the repository's policy of not breaking downstream users; the shadowing computed-var guards raise, so this can be escalated later if preferred. Also drops a redundant `is_hydrated: bool = False` from DynamicState in tests/units/test_app.py: it shadowed the identical root State default, so it was already a no-op. Fixes #7074 --- news/7074.bugfix.md | 1 + reflex/state.py | 69 +++++++++++++++++++++++++++++++++++++-- tests/units/test_app.py | 1 - tests/units/test_state.py | 50 ++++++++++++++++++++++++++++ 4 files changed, 117 insertions(+), 4 deletions(-) create mode 100644 news/7074.bugfix.md diff --git a/news/7074.bugfix.md b/news/7074.bugfix.md new file mode 100644 index 00000000000..17fd03f35e5 --- /dev/null +++ b/news/7074.bugfix.md @@ -0,0 +1 @@ +Warn when a substate declares a var whose name is already a var on a parent state. Such a declaration is ignored — reads and writes resolve to the parent's var and class-level access returns the raw default instead of a reactive `Var` — and previously produced no diagnostic at all. diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..97d884fb4f1 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -344,7 +344,7 @@ def _has_data_descriptor(cls: type, name: str) -> bool: return False -def _is_user_descriptor(value: Any) -> bool: +def _is_user_descriptor(value: Any, *, include_properties: bool = False) -> bool: """Whether a class attribute is a user-defined descriptor. Excludes framework-recognized callables and var types so user-defined @@ -353,6 +353,7 @@ def _is_user_descriptor(value: Any) -> bool: Args: value: The class attribute value to check. + include_properties: Whether property-like descriptors also count. Returns: True if the value is a custom descriptor. @@ -365,14 +366,16 @@ def _is_user_descriptor(value: Any) -> bool: FunctionType, classmethod, staticmethod, - property, - functools.cached_property, EventHandler, Var, Field, ), ): return False + if not include_properties and isinstance( + value, (property, functools.cached_property) + ): + return False return not is_computed_var(value) @@ -663,6 +666,9 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): if k not in own_descriptor_names } + # Base vars silently lose to an inherited var of the same name; warn about it. + cls._check_overridden_inherited_vars() + # Get computed vars. computed_vars = cls._get_computed_vars() cls._check_overridden_computed_vars() @@ -1110,6 +1116,63 @@ def _check_overridden_computed_vars(cls) -> None: msg = f"The computed var name `{cv._js_expr}` shadows a var in {cls.__module__}.{cls.__name__}; use a different name instead" raise ComputedVarShadowsStateVarError(msg) + @classmethod + def _shadows_non_state_descriptor(cls, name: str) -> bool: + """Whether a base outside the state hierarchy defines name as a descriptor. + + Args: + name: The var name to look up. + + Returns: + True if a non-state base in the MRO defines name as a user descriptor. + """ + return any( + not issubclass(base, BaseState) + and _is_user_descriptor(base.__dict__[name], include_properties=True) + for base in cls.__mro__ + if name in base.__dict__ + ) + + @classmethod + def _check_overridden_inherited_vars(cls) -> None: + """Warn about base vars that shadow a var inherited from a parent state. + + Such a redeclaration is dropped silently: the field never becomes a base var, + so reads and writes resolve to the parent's var and class-level access returns + the raw default instead of a Var. + + A redeclaration that exists to win over a descriptor reached through a + non-state base is left alone, since re-annotating is how that MRO conflict + is resolved. + """ + parent_state = cls.get_parent_state() + if parent_state is None: + return + parent_fields = parent_state.get_fields() + for name, own_field in cls.get_fields().items(): + if ( + name.startswith("_") + or not own_field.is_var + or name not in cls.inherited_vars + ): + continue + # A field redeclared on this class is a distinct object from the parent's; + # a merely inherited one is the same object. + parent_field = parent_fields.get(name) + if ( + parent_field is None + or parent_field is own_field + or cls._shadows_non_state_descriptor(name) + ): + continue + console.warn( + f"The var `{name}` in {cls.__module__}.{cls.__name__} shadows a var " + f"inherited from {parent_state.__module__}.{parent_state.__name__} and " + "is ignored: reads and writes resolve to the parent's var. Use a " + "different name instead.", + dedupe=True, + ) + @classmethod def get_skip_vars(cls) -> set[str]: """Get the vars to skip when serializing. diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 932456cd6ed..9d0b8ca7e4e 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -1779,7 +1779,6 @@ class DynamicState(State): recalculated when the dynamic route var was dirty """ - is_hydrated: bool = False loaded: int = 0 counter: int = 0 diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 37c0ed2fc4c..b5e942b6ab0 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5380,3 +5380,53 @@ def test_setattr_alias_annotated_var(mocker: MockerFixture): state.key = 1 # pyright: ignore[reportAttributeAccessIssue] assert state.key == 1 error_mock.assert_called_once() + + +def test_base_var_shadowing_inherited_var_warns(mocker: MockerFixture) -> None: + """A base var shadowing an inherited var warns instead of being dropped silently. + + Args: + mocker: Pytest mock fixture. + """ + warn_mock = mocker.patch("reflex.state.console.warn") + + class ShadowParent(BaseState): + shadowed_value: int = 1 + + class ShadowChild(ShadowParent): + shadowed_value: str = "ninety-nine" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + assert any("shadowed_value" in call.args[0] for call in warn_mock.call_args_list), ( + "expected a warning naming the shadowed var" + ) + + +def test_base_var_shadowing_non_state_descriptor_does_not_warn( + mocker: MockerFixture, +) -> None: + """Re-annotating to win over a descriptor from a non-state base is not a shadow. + + Args: + mocker: Pytest mock fixture. + """ + from reflex_base.vars.hybrid_property import hybrid_property + + warn_mock = mocker.patch("reflex.state.console.warn") + + class SharedMixin: + @hybrid_property + def descriptor_value(self) -> int: + return 1 + + class PlainBase(SharedMixin): + pass + + class OverridingState(SharedMixin, BaseState): + descriptor_value: int = 5 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + class DescriptorChild(PlainBase, OverridingState): + descriptor_value: int # pyright: ignore[reportGeneralTypeIssues, reportIncompatibleVariableOverride] + + assert not [ + call for call in warn_mock.call_args_list if "descriptor_value" in call.args[0] + ], "re-annotation resolving a descriptor MRO conflict must not warn" From b4c2d768c7d29b4970a1e3e1887f0d3b4d31c8d2 Mon Sep 17 00:00:00 2001 From: blockgroot <170620375+blockgroot@users.noreply.github.com> Date: Thu, 10 Sep 2026 16:37:59 +0530 Subject: [PATCH 2/2] Narrow the descriptor exemption to where the state field outranks it The first version exempted any redeclaration whose name appeared as a descriptor on a non-state base anywhere in the MRO. That is broader than the pattern it exists for: in Child(DescriptorMixin, ParentState) the descriptor precedes the state field, the child's declaration is still discarded, and class access still returns the raw default rather than a Var -- yet the warning was suppressed. Exempt only when a state base declaring the name precedes the same-named non-state descriptor, which is the ordering in which the field already wins and the re-annotation is therefore inert. Both conditions are required: with no descriptor in the MRO at all, an ordinary redeclaration still warns. This tracks whether the declaration actually breaks class access: warnings now fire exactly in the cases where the shadowed name no longer resolves to a Var. Adds test_base_var_shadowing_warns_when_descriptor_outranks_state_field. --- reflex/state.py | 28 ++++++++++++++++++---------- tests/units/test_state.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 97d884fb4f1..cee25d4024a 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1117,21 +1117,29 @@ def _check_overridden_computed_vars(cls) -> None: raise ComputedVarShadowsStateVarError(msg) @classmethod - def _shadows_non_state_descriptor(cls, name: str) -> bool: - """Whether a base outside the state hierarchy defines name as a descriptor. + def _state_field_precedes_descriptor(cls, name: str) -> bool: + """Whether a state base declaring name outranks a same-named descriptor. + + Re-annotating is how a state field that already wins over a descriptor on a + non-state base is kept, so that redeclaration is inert rather than a mistake. + A descriptor that instead outranks the state field does not make the + redeclaration take effect, so it is not exempt. Args: name: The var name to look up. Returns: - True if a non-state base in the MRO defines name as a user descriptor. + True if a state base declaring name precedes a non-state descriptor. """ - return any( - not issubclass(base, BaseState) - and _is_user_descriptor(base.__dict__[name], include_properties=True) - for base in cls.__mro__ - if name in base.__dict__ - ) + state_first = False + for base in cls.__mro__[1:]: + if name not in base.__dict__: + continue + if issubclass(base, BaseState): + state_first = True + elif _is_user_descriptor(base.__dict__[name], include_properties=True): + return state_first + return False @classmethod def _check_overridden_inherited_vars(cls) -> None: @@ -1162,7 +1170,7 @@ def _check_overridden_inherited_vars(cls) -> None: if ( parent_field is None or parent_field is own_field - or cls._shadows_non_state_descriptor(name) + or cls._state_field_precedes_descriptor(name) ): continue console.warn( diff --git a/tests/units/test_state.py b/tests/units/test_state.py index b5e942b6ab0..4801cc340df 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5430,3 +5430,34 @@ class DescriptorChild(PlainBase, OverridingState): assert not [ call for call in warn_mock.call_args_list if "descriptor_value" in call.args[0] ], "re-annotation resolving a descriptor MRO conflict must not warn" + + +def test_base_var_shadowing_warns_when_descriptor_outranks_state_field( + mocker: MockerFixture, +) -> None: + """A descriptor closer than the state field does not exempt a dropped declaration. + + Args: + mocker: Pytest mock fixture. + """ + from reflex_base.vars.hybrid_property import hybrid_property + + warn_mock = mocker.patch("reflex.state.console.warn") + + class CloserMixin: + @hybrid_property + def outranked_value(self) -> int: + return 1 + + class OutrankedParent(BaseState): + outranked_value: int = 1 # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + class OutrankedChild(CloserMixin, OutrankedParent): + outranked_value: str = "x" # pyright: ignore[reportIncompatibleVariableOverride, reportAssignmentType] + + assert "outranked_value" not in OutrankedChild.base_vars, ( + "declaration is still dropped, so it must not be treated as effective" + ) + assert any( + "outranked_value" in call.args[0] for call in warn_mock.call_args_list + ), "expected a warning when the descriptor outranks the state field"