From f901a6bf606dff615b076b40179a1a817e7738ba Mon Sep 17 00:00:00 2001 From: Farhan Date: Mon, 14 Sep 2026 16:42:54 +0500 Subject: [PATCH 1/5] Validate reserved state names before class and dynamic registration --- docs/state/overview.md | 13 ++ news/+reserved-state-names.breaking.md | 1 + .../news/+reserved-state-names.deprecation.md | 1 + .../src/reflex_base/environment.py | 3 + reflex/istate/validation.py | 125 +++++++++++ reflex/state.py | 59 ++--- tests/units/istate/test_proxy.py | 19 +- tests/units/istate/test_validation.py | 210 ++++++++++++++++++ 8 files changed, 387 insertions(+), 44 deletions(-) create mode 100644 news/+reserved-state-names.breaking.md create mode 100644 packages/reflex-base/news/+reserved-state-names.deprecation.md create mode 100644 reflex/istate/validation.py create mode 100644 tests/units/istate/test_validation.py diff --git a/docs/state/overview.md b/docs/state/overview.md index e5e040a5aee..ec8ebfb10a4 100644 --- a/docs/state/overview.md +++ b/docs/state/overview.md @@ -52,6 +52,19 @@ A state class is made up of two parts: vars and event handlers. **Event handlers** are functions that modify these vars in response to events. +State declarations cannot reuse framework method or bookkeeping names, such as +`get_state`, `_get_was_touched`, or `dirty_vars`. Reflex checks these names when +creating a state class and when adding vars, event handlers, or route arguments +dynamically. Rename a conflicting declaration and update its references. Ordinary +backend names such as `_count` remain supported. + +For existing apps, setting `REFLEX_STATE_ALLOW_RESERVED_NAMES=1` before starting +Reflex temporarily restores legacy handling of conflicting vars and emits a +deprecation warning. This compatibility option will be removed in Reflex 1.0. +It preserves the old behavior, including any crashes caused by a collision; rename +the conflicting members to resolve those crashes. Existing restrictions on +overriding framework methods still apply. + These are the main concepts to understand how state works in Reflex: ```python eval diff --git a/news/+reserved-state-names.breaking.md b/news/+reserved-state-names.breaking.md new file mode 100644 index 00000000000..50bd1bb2773 --- /dev/null +++ b/news/+reserved-state-names.breaking.md @@ -0,0 +1 @@ +State vars, event handlers, and dynamic route arguments now reject names reserved by framework methods and bookkeeping before registration. Rename conflicting members; `REFLEX_STATE_ALLOW_RESERVED_NAMES=1` temporarily preserves legacy behavior with a deprecation warning until Reflex 1.0. diff --git a/packages/reflex-base/news/+reserved-state-names.deprecation.md b/packages/reflex-base/news/+reserved-state-names.deprecation.md new file mode 100644 index 00000000000..b577e1b8a0b --- /dev/null +++ b/packages/reflex-base/news/+reserved-state-names.deprecation.md @@ -0,0 +1 @@ +Add the temporary `REFLEX_STATE_ALLOW_RESERVED_NAMES` compatibility option for apps migrating away from reserved state names. It defaults to false and will be removed in Reflex 1.0. diff --git a/packages/reflex-base/src/reflex_base/environment.py b/packages/reflex-base/src/reflex_base/environment.py index 3bb80d6970b..b6b0fb72af6 100644 --- a/packages/reflex-base/src/reflex_base/environment.py +++ b/packages/reflex-base/src/reflex_base/environment.py @@ -673,6 +673,9 @@ class EnvironmentVariables: # The maximum size of the reflex state in kilobytes. REFLEX_STATE_SIZE_LIMIT: EnvVar[int] = env_var(1000) + # Temporary compatibility for state declarations that shadow framework members. + REFLEX_STATE_ALLOW_RESERVED_NAMES: EnvVar[bool] = env_var(False) + # Additional paths to include in the hot reload. Separated by a colon. REFLEX_HOT_RELOAD_INCLUDE_PATHS: EnvVar[list[Path]] = env_var([]) diff --git a/reflex/istate/validation.py b/reflex/istate/validation.py new file mode 100644 index 00000000000..e87b66f0cbd --- /dev/null +++ b/reflex/istate/validation.py @@ -0,0 +1,125 @@ +"""Validate the framework namespace before constructing or extending a state.""" + +from functools import cache +from types import FunctionType +from typing import Any + +from reflex_base.environment import environment +from reflex_base.utils import console +from reflex_base.utils.compat import annotations_from_namespace +from reflex_base.utils.exceptions import ( + EventHandlerShadowsBuiltInStateMethodError, + StateValueError, +) +from reflex_base.vars.base import ( + BaseStateMeta, + EvenMoreBasicBaseState, + _linearize_bases, +) + +_FIELD_MAP_NAMES = frozenset({"__fields__", "__own_fields__", "__inherited_fields__"}) + + +@cache +def _reserved_state_members() -> dict[str, Any]: + """Return framework members, excluding state vars and Python protocols. + + Returns: + Reserved names and their original descriptors, without invoking them. + """ + # BaseState must exist before its namespace can be inspected. + from reflex.state import BaseState + + members = {} + for base in reversed(BaseState.__mro__[:-1]): + namespace = vars(base) + members.update( + (name, namespace.get(name)) + for name in namespace.keys() | annotations_from_namespace(namespace).keys() + if not name.startswith("__") or name in _FIELD_MAP_NAMES + ) + for name, field in BaseState.__fields__.items(): + if field.is_var: + members.pop(name, None) + return members + + +def _validate_state_name(name: str, value: Any = None) -> None: + """Reject declarations that replace framework methods or bookkeeping. + + Args: + name: The declared or dynamically registered name. + value: The raw class declaration, when available. + + Raises: + StateValueError: If a declaration uses a reserved name. + EventHandlerShadowsBuiltInStateMethodError: If a method overrides a builtin. + """ + members = _reserved_state_members() + if name not in members: + return + method = value.__func__ if isinstance(value, (classmethod, staticmethod)) else value + if isinstance(method, FunctionType): + if value is members[name] or getattr(method, "__override_base_method__", False): + return + msg = f"The event handler name `{name}` shadows a builtin State method; use a different name instead" + raise EventHandlerShadowsBuiltInStateMethodError(msg) + reason = ( + f"State name `{name}` is reserved by BaseState; use a different name instead." + ) + if environment.REFLEX_STATE_ALLOW_RESERVED_NAMES.get(): + console.deprecate( + feature_name="REFLEX_STATE_ALLOW_RESERVED_NAMES", + reason=reason, + deprecation_version="0.9.12", + removal_version="1.0", + ) + return + msg = f"{reason} Set REFLEX_STATE_ALLOW_RESERVED_NAMES=1 temporarily to retain legacy behavior." + raise StateValueError(msg) + + +class _StateMeta(BaseStateMeta): + """Check state declarations before field collection and subclass initialization.""" + + def __new__( + cls, + name: str, + bases: tuple[type, ...], + namespace: dict[str, Any], + mixin: bool = False, + ) -> type: + """Construct a state after checking its declarations and Python mixins. + + Args: + name: The class name. + bases: The parent classes. + namespace: The unmodified class namespace. + mixin: Whether the class is a state mixin. + + Returns: + The validated state class. + """ + if any(isinstance(base, _StateMeta) for base in bases): + seen = namespace.keys() | annotations_from_namespace(namespace).keys() + for member in seen: + _validate_state_name(member, namespace.get(member)) + for base in _linearize_bases(bases): + if not isinstance(base, _StateMeta) and base not in ( + EvenMoreBasicBaseState, + object, + ): + if isinstance(base, BaseStateMeta): + # Model fields are inherited even when an earlier base + # masks their class attributes in the MRO. + for member in base.__own_fields__: + _validate_state_name(member) + seen.update(base.__own_fields__) + for member, value in vars(base).items(): + if member not in seen and not ( + isinstance(base, BaseStateMeta) + and (member in _FIELD_MAP_NAMES or member == "_mixin") + ): + _validate_state_name(member, value) + seen.update(vars(base)) + return super().__new__(cls, name, bases, namespace, mixin=mixin) diff --git a/reflex/state.py b/reflex/state.py index 1ea080cc863..8f1ec19a1b9 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -44,7 +44,6 @@ ComputedVarShadowsStateVarError, DynamicComponentInvalidSignatureError, DynamicRouteArgShadowsStateVarError, - EventHandlerShadowsBuiltInStateMethodError, ReflexRuntimeError, SetUndefinedStateVarError, StateMismatchError, @@ -77,6 +76,7 @@ from reflex.istate.proxy import ImmutableMutableProxy as ImmutableMutableProxy from reflex.istate.proxy import MutableProxy, is_mutable_type from reflex.istate.storage import ClientStorageBase +from reflex.istate.validation import _StateMeta, _validate_state_name from reflex.utils import console, format, types from reflex.utils.exec import is_testing_env @@ -426,7 +426,7 @@ def _is_user_descriptor(value: Any) -> bool: }) -class BaseState(EvenMoreBasicBaseState): +class BaseState(EvenMoreBasicBaseState, metaclass=_StateMeta): """The state of the app.""" # A map from the var name to the var. @@ -616,9 +616,6 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): # Validate the module name. cls._validate_module_name() - # Event handlers should not shadow builtin state methods. - cls._check_overridden_methods() - # Computed vars should not shadow builtin state props. cls._check_overridden_basevars() @@ -825,6 +822,7 @@ def _add_event_handler( name: The name of the event handler. fn: The function to call when the event is triggered. """ + _validate_state_name(name) handler = cls._create_event_handler(fn) cls.event_handlers[name] = handler setattr(cls, name, handler) @@ -1059,29 +1057,6 @@ def _iter_functions(cls) -> Iterator[tuple[str, FunctionType]]: if isinstance(value, FunctionType): yield name, value - @classmethod - def _check_overridden_methods(cls): - """Check for shadow methods and raise error if any. - - Raises: - EventHandlerShadowsBuiltInStateMethodError: When an event handler shadows an inbuilt state method. - """ - overridden_methods = set() - state_base_functions = cls._get_base_functions() - for name, method in cls._iter_functions(): - # Check if the method is overridden and not a dunder method - if ( - not name.startswith("__") - and method.__name__ in state_base_functions - and state_base_functions[method.__name__] != method - and not getattr(method, "__override_base_method__", False) - ): - overridden_methods.add(method.__name__) - - for method_name in overridden_methods: - msg = f"The event handler name `{method_name}` shadows a builtin State method; use a different name instead" - raise EventHandlerShadowsBuiltInStateMethodError(msg) - @classmethod def _check_overridden_basevars(cls): """Check for shadow base vars and raise error if any. @@ -1290,6 +1265,19 @@ def _init_var(cls, name: str, prop: Var): cls._create_setter(name, prop) cls._set_default_value(name, prop) + @classmethod + @_override_base_method + def add_field(cls, name: str, var: Var, default_value: Any): + """Validate a dynamically added field before updating the field map. + + Args: + name: The name of the field to add. + var: The variable to add a field for. + default_value: The default value of the field. + """ + _validate_state_name(name) + super().add_field(name, var, default_value) + @classmethod def add_var(cls, name: str, type_: Any, default_value: Any = None): """Add dynamically a variable to the State. @@ -1442,19 +1430,6 @@ def _get_var_default(cls, name: str, annotation_value: Any) -> Any: except TypeError: return None - @staticmethod - def _get_base_functions() -> builtins.dict[str, FunctionType]: - """Get all functions of the state class excluding dunder methods. - - Returns: - The functions of rx.State class as a dict. - """ - return { - func[0]: func[1] - for func in inspect.getmembers(BaseState, predicate=inspect.isfunction) - if not func[0].startswith("__") - } - @classmethod def _update_substate_inherited_vars(cls, vars_to_add: builtins.dict[str, Var]): """Update the inherited vars of substates recursively when new vars are added. @@ -1507,6 +1482,8 @@ def setup_dynamic_args(cls, args: builtins.dict[str, str]): if not args: return + for name in args: + _validate_state_name(name) cls._check_overwritten_dynamic_args(list(args.keys())) def argsingle_factory(param: str): diff --git a/tests/units/istate/test_proxy.py b/tests/units/istate/test_proxy.py index 285349688b0..ab0412d75c5 100644 --- a/tests/units/istate/test_proxy.py +++ b/tests/units/istate/test_proxy.py @@ -1003,7 +1003,7 @@ def timed(self) -> int: assert IntervalState._interval_computed_var_names == frozenset({"timed"}) -def test_fast_path_skips_names_a_subclass_defines(): +def test_fast_path_skips_names_a_subclass_defines(monkeypatch: pytest.MonkeyPatch): """A subclass defining a fast-pathed framework name keeps the full lookup for it. The fast path bypasses var resolution, so it must not apply to a name the @@ -1011,9 +1011,14 @@ def test_fast_path_skips_names_a_subclass_defines(): backend var named like a framework method). The class is a detached root (not a substate of ``State``) so the shadowed method never reaches the framework paths that other tests exercise on the shared state tree. + + Args: + monkeypatch: Enable legacy reserved names for this lookup regression. """ from reflex.state import BaseState + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") + def get_value(self, key: str): return f"shadow:{key}" @@ -1039,12 +1044,20 @@ def get_value(self, key: str): assert state._get_was_touched == 7 -def test_fast_path_prunes_names_registered_after_class_creation(): - """Vars and handlers added after class creation also leave the fast path.""" +def test_fast_path_prunes_names_registered_after_class_creation( + monkeypatch: pytest.MonkeyPatch, +): + """Vars and handlers added after class creation also leave the fast path. + + Args: + monkeypatch: Enable legacy reserved names for this lookup regression. + """ from reflex_base.constants import RouteArgType from reflex.state import BaseState + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") + DynamicState = type( "DynamicState", (BaseState,), diff --git a/tests/units/istate/test_validation.py b/tests/units/istate/test_validation.py new file mode 100644 index 00000000000..1b12ccd31bc --- /dev/null +++ b/tests/units/istate/test_validation.py @@ -0,0 +1,210 @@ +"""Tests for reserved state names at class creation and dynamic registration.""" + +from unittest.mock import patch + +import pytest +from reflex_base.constants import RouteArgType +from reflex_base.utils.exceptions import StateValueError +from reflex_base.vars.base import EvenMoreBasicBaseState, LiteralVar, computed_var + +from reflex.state import BaseState, _override_base_method + + +@pytest.mark.parametrize( + "name", + [ + "_get_was_touched", + "_update_was_touched", + "_was_touched", + "dirty_vars", + "get_fields", + "get_full_name", + "backend_vars", + "__fields__", + "setvar", + ], +) +@pytest.mark.parametrize("annotated", [False, True]) +def test_reserved_state_var(name: str, annotated: bool, clean_registration_context): + """Reject framework names before state initialization can call them. + + Args: + name: The reserved member to shadow. + annotated: Whether to explicitly annotate the variable. + clean_registration_context: An isolated state registry. + """ + namespace = {"__module__": __name__, "__qualname__": "ShadowState", name: 7} + if annotated: + namespace["__annotations__"] = {name: int} + with pytest.raises(StateValueError, match=name): + type("ShadowState", (BaseState,), namespace) + + +def test_reserved_annotation_only(clean_registration_context): + """Reject a reserved var even when no default is declared. + + Args: + clean_registration_context: An isolated state registry. + """ + with pytest.raises(StateValueError, match="_get_was_touched"): + + class ShadowState(BaseState): + _get_was_touched: int + + +@pytest.mark.parametrize("state_mixin", [False, True]) +def test_reserved_mixin_var(state_mixin: bool, clean_registration_context): + """Reject collisions from both ordinary Python mixins and state mixins. + + Args: + state_mixin: Whether the mixin subclasses BaseState. + clean_registration_context: An isolated state registry. + """ + with pytest.raises(StateValueError, match="_update_was_touched"): + mixin = type( + "Mixin", + (BaseState,) if state_mixin else (), + {"__module__": __name__, "_update_was_touched": 7}, + **({"mixin": True} if state_mixin else {}), + ) + type("MixedState", (mixin, BaseState), {"__module__": __name__}) + + +@pytest.mark.parametrize("name", ["_get_was_touched", "get_fields"]) +def test_reserved_computed_var(name: str, clean_registration_context): + """Reject computed vars that replace framework methods. + + Args: + name: The reserved method to replace. + clean_registration_context: An isolated state registry. + """ + + def value(self) -> int: + """Return a constant computed value.""" + return 7 + + value.__name__ = name + with pytest.raises(StateValueError, match=name): + type( + "ComputedState", + (BaseState,), + {"__module__": __name__, name: computed_var(value)}, + ) + + +@pytest.mark.parametrize("registration", ["var", "route", "event", "field"]) +def test_dynamic_reserved_name(registration: str, clean_registration_context): + """Reject dynamic collisions before any field or event map is changed. + + Args: + registration: The dynamic registration path to exercise. + clean_registration_context: An isolated state registry. + """ + + class DynamicState(BaseState): + """State receiving a dynamic declaration.""" + + fields = dict(DynamicState.get_fields()) + with pytest.raises(StateValueError, match="get_state"): + if registration == "var": + DynamicState.add_var("get_state", int, 7) + elif registration == "route": + DynamicState.setup_dynamic_args({"get_state": RouteArgType.SINGLE}) + elif registration == "event": + DynamicState._add_event_handler("get_state", lambda self: None) + else: + DynamicState.add_field("get_state", LiteralVar.create(7), 7) + assert DynamicState.get_fields() == fields + assert "get_state" not in DynamicState.vars + assert "get_state" not in DynamicState.event_handlers + assert "get_state" not in DynamicState.__dict__ + + +def test_user_vars_and_marked_override(clean_registration_context): + """Keep normal vars, inherited vars, and explicitly marked method overrides. + + Args: + clean_registration_context: An isolated state registry. + """ + + class Parent(BaseState): + value: int = 1 + _backend: int = 2 + + class Child(Parent): + @_override_base_method + def get_value(self, key: str): + """Return a value through a supported framework override.""" + return f"override:{key}" + + parent = Parent() + child = parent.substates[Child.get_name()] + assert isinstance(child, Child) + assert child.value == 1 + assert child._backend == 2 + assert child.get_value("value") == "override:value" + + +def test_non_state_models_keep_their_namespace(): + """Do not reserve Reflex state names on unrelated base models.""" + + class Model(EvenMoreBasicBaseState): + get_state: int = 7 + + assert Model().get_state == 7 + + +def test_legacy_state_names( + monkeypatch: pytest.MonkeyPatch, clean_registration_context +): + """Preserve the old lookup behavior only with the deprecated legacy opt-in. + + Args: + monkeypatch: Set the temporary compatibility environment variable. + clean_registration_context: An isolated state registry. + """ + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") + with patch("reflex_base.utils.console.deprecate") as deprecate: + + class LegacyState(BaseState): + _get_was_touched: int = 7 + + assert LegacyState()._get_was_touched == 7 + deprecate.assert_called_once() + assert deprecate.call_args.kwargs["removal_version"] == "1.0" + + +@pytest.mark.parametrize("name", ["get_fields", "_get_was_touched"]) +@pytest.mark.parametrize("state_first", [False, True]) +def test_reserved_model_mixin(name: str, state_first: bool, clean_registration_context): + """Reject inherited model fields before the field collector sees them. + + Args: + name: The framework name declared as a model field. + state_first: Whether BaseState precedes the model in the MRO. + clean_registration_context: An isolated state registry. + """ + model = type("Model", (EvenMoreBasicBaseState,), {"__module__": __name__, name: 7}) + bases = (BaseState, model) if state_first else (model, BaseState) + with pytest.raises(StateValueError, match=name): + type("MixedState", bases, {"__module__": __name__}) + + +def test_reserved_descriptor(clean_registration_context): + """Reject a descriptor without executing its class access behavior. + + Args: + clean_registration_context: An isolated state registry. + """ + + class Descriptor: + def __get__(self, instance, owner): + """Fail if validation invokes this descriptor.""" + pytest.fail("Reserved descriptor was evaluated") + + with pytest.raises(StateValueError, match="get_fields"): + type( + "DescriptorState", + (BaseState,), + {"__module__": __name__, "get_fields": Descriptor()}, + ) From 7ed33c7c2531c90e21787e0530e1e833f68e5e34 Mon Sep 17 00:00:00 2001 From: Farhan Date: Thu, 17 Sep 2026 22:37:33 +0500 Subject: [PATCH 2/5] docs: rename the dynamic form handler so it does not shadow State.add_field --- docs/library/forms/form.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/library/forms/form.md b/docs/library/forms/form.md index 8fdbda66583..1bd2480aa3f 100644 --- a/docs/library/forms/form.md +++ b/docs/library/forms/form.md @@ -298,7 +298,7 @@ class DynamicFormState(rx.State): ] @rx.event - def add_field(self, form_data: dict): + def add_form_field(self, form_data: dict): new_field = form_data.get("new_field") if not new_field: return @@ -331,7 +331,7 @@ def dynamic_form(): rx.input(placeholder="New Field", name="new_field"), rx.button("+", type="submit"), ), - on_submit=DynamicFormState.add_field, + on_submit=DynamicFormState.add_form_field, reset_on_submit=True, ), rx.divider(), From c5adb5efe7d844b19ddb50f563dfc90cbbc3710e Mon Sep 17 00:00:00 2001 From: Farhan Date: Thu, 17 Sep 2026 22:48:38 +0500 Subject: [PATCH 3/5] fix(state): report every reserved name under the legacy flag and flatten the validating metaclass Set the deprecation version to the next release, fold the colliding name into the deprecation key so each collision in a class is reported, move the inherited-member check into a helper, and drop the unneeded override marker on BaseState.add_field. The shadowed-touched-method tests from #7132 opt into the legacy flag. --- reflex/istate/validation.py | 38 +++++++++++++++--------- reflex/state.py | 1 - tests/units/istate/manager/test_redis.py | 6 +++- tests/units/istate/manager/test_token.py | 4 ++- tests/units/istate/test_shared.py | 2 ++ 5 files changed, 34 insertions(+), 17 deletions(-) diff --git a/reflex/istate/validation.py b/reflex/istate/validation.py index e87b66f0cbd..6940393b2dc 100644 --- a/reflex/istate/validation.py +++ b/reflex/istate/validation.py @@ -69,9 +69,9 @@ def _validate_state_name(name: str, value: Any = None) -> None: ) if environment.REFLEX_STATE_ALLOW_RESERVED_NAMES.get(): console.deprecate( - feature_name="REFLEX_STATE_ALLOW_RESERVED_NAMES", + feature_name=f"REFLEX_STATE_ALLOW_RESERVED_NAMES for `{name}`", reason=reason, - deprecation_version="0.9.12", + deprecation_version="0.9.11", removal_version="1.0", ) return @@ -79,6 +79,27 @@ def _validate_state_name(name: str, value: Any = None) -> None: raise StateValueError(msg) +def _validate_inherited_members(base: type, seen: set[str]) -> None: + """Check the members a Python mixin or model base adds to a state. + + Args: + base: A base class that is not itself a validated state. + seen: Names an earlier base already provides in the MRO. + """ + is_model = isinstance(base, BaseStateMeta) + if is_model: + # Model fields are inherited even when an earlier base masks their + # class attributes in the MRO. + for member in base.__own_fields__: + _validate_state_name(member) + seen.update(base.__own_fields__) + for member, value in vars(base).items(): + if member not in seen and not ( + is_model and (member in _FIELD_MAP_NAMES or member == "_mixin") + ): + _validate_state_name(member, value) + + class _StateMeta(BaseStateMeta): """Check state declarations before field collection and subclass initialization.""" @@ -109,17 +130,6 @@ def __new__( EvenMoreBasicBaseState, object, ): - if isinstance(base, BaseStateMeta): - # Model fields are inherited even when an earlier base - # masks their class attributes in the MRO. - for member in base.__own_fields__: - _validate_state_name(member) - seen.update(base.__own_fields__) - for member, value in vars(base).items(): - if member not in seen and not ( - isinstance(base, BaseStateMeta) - and (member in _FIELD_MAP_NAMES or member == "_mixin") - ): - _validate_state_name(member, value) + _validate_inherited_members(base, seen) seen.update(vars(base)) return super().__new__(cls, name, bases, namespace, mixin=mixin) diff --git a/reflex/state.py b/reflex/state.py index 79cd8344606..1c2c9510d46 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1308,7 +1308,6 @@ def _init_var(cls, name: str, prop: Var): cls._set_default_value(name, prop) @classmethod - @_override_base_method def add_field(cls, name: str, var: Var, default_value: Any): """Validate a dynamically added field before updating the field map. diff --git a/tests/units/istate/manager/test_redis.py b/tests/units/istate/manager/test_redis.py index 3a336501b84..b0c1075fecf 100644 --- a/tests/units/istate/manager/test_redis.py +++ b/tests/units/istate/manager/test_redis.py @@ -115,12 +115,16 @@ async def test_basic_get_set( ) -async def test_set_state_with_shadowed_touched_method(clean_registration_context): +async def test_set_state_with_shadowed_touched_method( + clean_registration_context, monkeypatch: pytest.MonkeyPatch +): """Persist a backend var that shadows the touched-state method. Args: clean_registration_context: A fresh, empty registration context. + monkeypatch: Enable legacy reserved names for this persistence regression. """ + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") class ShadowState(BaseState): """State with an intentional framework-method collision.""" diff --git a/tests/units/istate/manager/test_token.py b/tests/units/istate/manager/test_token.py index a3c3c515371..38888162b40 100644 --- a/tests/units/istate/manager/test_token.py +++ b/tests/units/istate/manager/test_token.py @@ -138,13 +138,15 @@ class TouchState(BaseState): def test_base_state_token_get_and_reset_touched_shadowed_by_var( - clean_registration_context, + clean_registration_context, monkeypatch: pytest.MonkeyPatch ): """A state var named like the touched-state method does not break persistence. Args: clean_registration_context: A fresh, empty registration context. + monkeypatch: Enable legacy reserved names for this persistence regression. """ + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") from reflex.state import BaseState TouchState = type( diff --git a/tests/units/istate/test_shared.py b/tests/units/istate/test_shared.py index f87c7893eaa..b92bfa4f121 100644 --- a/tests/units/istate/test_shared.py +++ b/tests/units/istate/test_shared.py @@ -131,6 +131,8 @@ async def test_shared_updates_with_shadowed_touched_method( State, "_always_dirty_substates", State._always_dirty_substates.copy() ) + monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") + class ShadowState(SharedState): """State with an intentional framework-method collision.""" From dfe6dedcb67bb82be75ba278b8deee93adcac963 Mon Sep 17 00:00:00 2001 From: Farhan Date: Thu, 17 Sep 2026 22:55:42 +0500 Subject: [PATCH 4/5] fix(state): drop the reserved-name compatibility flag Reserved names are always rejected. The tests that constructed states with a reserved backend var can no longer exist and are removed; the fast-path test keeps its marked-override case. --- docs/state/overview.md | 7 --- news/+reserved-state-names.breaking.md | 2 +- .../news/+reserved-state-names.deprecation.md | 1 - .../src/reflex_base/environment.py | 3 - reflex/istate/validation.py | 15 +---- tests/units/istate/manager/test_redis.py | 27 -------- tests/units/istate/manager/test_token.py | 30 --------- tests/units/istate/test_proxy.py | 63 ++----------------- tests/units/istate/test_shared.py | 48 +------------- tests/units/istate/test_validation.py | 22 ------- 10 files changed, 8 insertions(+), 210 deletions(-) delete mode 100644 packages/reflex-base/news/+reserved-state-names.deprecation.md diff --git a/docs/state/overview.md b/docs/state/overview.md index ec8ebfb10a4..bd6a297a112 100644 --- a/docs/state/overview.md +++ b/docs/state/overview.md @@ -58,13 +58,6 @@ creating a state class and when adding vars, event handlers, or route arguments dynamically. Rename a conflicting declaration and update its references. Ordinary backend names such as `_count` remain supported. -For existing apps, setting `REFLEX_STATE_ALLOW_RESERVED_NAMES=1` before starting -Reflex temporarily restores legacy handling of conflicting vars and emits a -deprecation warning. This compatibility option will be removed in Reflex 1.0. -It preserves the old behavior, including any crashes caused by a collision; rename -the conflicting members to resolve those crashes. Existing restrictions on -overriding framework methods still apply. - These are the main concepts to understand how state works in Reflex: ```python eval diff --git a/news/+reserved-state-names.breaking.md b/news/+reserved-state-names.breaking.md index 50bd1bb2773..81c4fadae44 100644 --- a/news/+reserved-state-names.breaking.md +++ b/news/+reserved-state-names.breaking.md @@ -1 +1 @@ -State vars, event handlers, and dynamic route arguments now reject names reserved by framework methods and bookkeeping before registration. Rename conflicting members; `REFLEX_STATE_ALLOW_RESERVED_NAMES=1` temporarily preserves legacy behavior with a deprecation warning until Reflex 1.0. +State vars, event handlers, and dynamic route arguments now reject names reserved by framework methods and bookkeeping before registration. Rename conflicting members. diff --git a/packages/reflex-base/news/+reserved-state-names.deprecation.md b/packages/reflex-base/news/+reserved-state-names.deprecation.md deleted file mode 100644 index b577e1b8a0b..00000000000 --- a/packages/reflex-base/news/+reserved-state-names.deprecation.md +++ /dev/null @@ -1 +0,0 @@ -Add the temporary `REFLEX_STATE_ALLOW_RESERVED_NAMES` compatibility option for apps migrating away from reserved state names. It defaults to false and will be removed in Reflex 1.0. diff --git a/packages/reflex-base/src/reflex_base/environment.py b/packages/reflex-base/src/reflex_base/environment.py index 429dcbf5a13..c937d0e9b34 100644 --- a/packages/reflex-base/src/reflex_base/environment.py +++ b/packages/reflex-base/src/reflex_base/environment.py @@ -751,9 +751,6 @@ class EnvironmentVariables: # The maximum size of the reflex state in kilobytes. REFLEX_STATE_SIZE_LIMIT: EnvVar[int] = env_var(1000) - # Temporary compatibility for state declarations that shadow framework members. - REFLEX_STATE_ALLOW_RESERVED_NAMES: EnvVar[bool] = env_var(False) - # Additional paths to include in the hot reload. Separated by a colon. REFLEX_HOT_RELOAD_INCLUDE_PATHS: EnvVar[list[Path]] = env_var([]) diff --git a/reflex/istate/validation.py b/reflex/istate/validation.py index 6940393b2dc..85b83f071eb 100644 --- a/reflex/istate/validation.py +++ b/reflex/istate/validation.py @@ -4,8 +4,6 @@ from types import FunctionType from typing import Any -from reflex_base.environment import environment -from reflex_base.utils import console from reflex_base.utils.compat import annotations_from_namespace from reflex_base.utils.exceptions import ( EventHandlerShadowsBuiltInStateMethodError, @@ -64,18 +62,7 @@ def _validate_state_name(name: str, value: Any = None) -> None: return msg = f"The event handler name `{name}` shadows a builtin State method; use a different name instead" raise EventHandlerShadowsBuiltInStateMethodError(msg) - reason = ( - f"State name `{name}` is reserved by BaseState; use a different name instead." - ) - if environment.REFLEX_STATE_ALLOW_RESERVED_NAMES.get(): - console.deprecate( - feature_name=f"REFLEX_STATE_ALLOW_RESERVED_NAMES for `{name}`", - reason=reason, - deprecation_version="0.9.11", - removal_version="1.0", - ) - return - msg = f"{reason} Set REFLEX_STATE_ALLOW_RESERVED_NAMES=1 temporarily to retain legacy behavior." + msg = f"State name `{name}` is reserved by BaseState; use a different name instead." raise StateValueError(msg) diff --git a/tests/units/istate/manager/test_redis.py b/tests/units/istate/manager/test_redis.py index b0c1075fecf..5ec8de77be6 100644 --- a/tests/units/istate/manager/test_redis.py +++ b/tests/units/istate/manager/test_redis.py @@ -115,33 +115,6 @@ async def test_basic_get_set( ) -async def test_set_state_with_shadowed_touched_method( - clean_registration_context, monkeypatch: pytest.MonkeyPatch -): - """Persist a backend var that shadows the touched-state method. - - Args: - clean_registration_context: A fresh, empty registration context. - monkeypatch: Enable legacy reserved names for this persistence regression. - """ - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - - class ShadowState(BaseState): - """State with an intentional framework-method collision.""" - - _get_was_touched: int = 7 - - state = ShadowState() - state._get_was_touched = 8 - manager = StateManagerRedis(redis=mock_redis()) - token = BaseStateToken(ident="shadowed", cls=ShadowState) - - await manager.set_state(token, state) - - restored = BaseState._deserialize(data=await manager.redis.get(str(token))) - assert restored._get_was_touched == 8 - - async def test_modify( state_manager_redis: StateManagerRedis, root_state: type[RedisTestState], diff --git a/tests/units/istate/manager/test_token.py b/tests/units/istate/manager/test_token.py index 38888162b40..d6f9411b608 100644 --- a/tests/units/istate/manager/test_token.py +++ b/tests/units/istate/manager/test_token.py @@ -137,36 +137,6 @@ class TouchState(BaseState): assert BaseStateToken.get_and_reset_touched_state(state) is False -def test_base_state_token_get_and_reset_touched_shadowed_by_var( - clean_registration_context, monkeypatch: pytest.MonkeyPatch -): - """A state var named like the touched-state method does not break persistence. - - Args: - clean_registration_context: A fresh, empty registration context. - monkeypatch: Enable legacy reserved names for this persistence regression. - """ - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - from reflex.state import BaseState - - TouchState = type( - "TouchState", - (BaseState,), - { - "__module__": __name__, - "__qualname__": "TouchState", - "__annotations__": {"_get_was_touched": int, "x": int}, - "_get_was_touched": 7, - "x": 0, - }, - ) - state = TouchState() - state.x = 1 - assert BaseStateToken.get_and_reset_touched_state(state) is True - assert state._was_touched is False - assert state._get_was_touched == 7 - - def test_from_legacy_token(clean_registration_context): """from_legacy_token parses 'ident_state.path' into a BaseStateToken. diff --git a/tests/units/istate/test_proxy.py b/tests/units/istate/test_proxy.py index ab0412d75c5..a1b09f1f358 100644 --- a/tests/units/istate/test_proxy.py +++ b/tests/units/istate/test_proxy.py @@ -1003,22 +1003,17 @@ def timed(self) -> int: assert IntervalState._interval_computed_var_names == frozenset({"timed"}) -def test_fast_path_skips_names_a_subclass_defines(monkeypatch: pytest.MonkeyPatch): +def test_fast_path_skips_names_a_subclass_defines(): """A subclass defining a fast-pathed framework name keeps the full lookup for it. The fast path bypasses var resolution, so it must not apply to a name the - state itself defines (here a marked override of a BaseState method, and a - backend var named like a framework method). The class is a detached root - (not a substate of ``State``) so the shadowed method never reaches the - framework paths that other tests exercise on the shared state tree. - - Args: - monkeypatch: Enable legacy reserved names for this lookup regression. + state itself defines (here a marked override of a BaseState method). The + class is a detached root (not a substate of ``State``) so the shadowed + method never reaches the framework paths that other tests exercise on the + shared state tree. """ from reflex.state import BaseState - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - def get_value(self, key: str): return f"shadow:{key}" @@ -1030,59 +1025,11 @@ def get_value(self, key: str): { "__module__": __name__, "__qualname__": "ShadowState", - "__annotations__": {"_get_was_touched": int}, - "_get_was_touched": 7, "get_value": get_value, }, ) assert "get_value" in BaseState._fast_attr_names assert "get_value" not in ShadowState._fast_attr_names - assert "_get_was_touched" not in ShadowState._fast_attr_names assert "dirty_vars" in ShadowState._fast_attr_names state = ShadowState(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] assert state.get_value("k") == "shadow:k" - assert state._get_was_touched == 7 - - -def test_fast_path_prunes_names_registered_after_class_creation( - monkeypatch: pytest.MonkeyPatch, -): - """Vars and handlers added after class creation also leave the fast path. - - Args: - monkeypatch: Enable legacy reserved names for this lookup regression. - """ - from reflex_base.constants import RouteArgType - - from reflex.state import BaseState - - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - - DynamicState = type( - "DynamicState", - (BaseState,), - {"__module__": __name__, "__qualname__": "DynamicState"}, - ) - DynamicSubState = type( - "DynamicSubState", - (DynamicState,), - {"__module__": __name__, "__qualname__": "DynamicSubState"}, - ) - DynamicGrandChild = type( - "DynamicGrandChild", - (DynamicSubState,), - {"__module__": __name__, "__qualname__": "DynamicGrandChild"}, - ) - tree = (DynamicState, DynamicSubState, DynamicGrandChild) - for cls in tree: - assert {"get_value", "get_delta", "get_state"} <= cls._fast_attr_names - - DynamicState.setup_dynamic_args({"get_value": RouteArgType.SINGLE}) - DynamicState._add_event_handler("get_delta", lambda self: None) - DynamicState.add_var("get_state", int, 0) - # Registered on the root and inherited down the tree, so pruned everywhere. - for cls in tree: - assert "get_value" not in cls._fast_attr_names - assert "get_delta" not in cls._fast_attr_names - assert "get_state" not in cls._fast_attr_names - assert "dirty_vars" in cls._fast_attr_names diff --git a/tests/units/istate/test_shared.py b/tests/units/istate/test_shared.py index b92bfa4f121..c8b16c874dd 100644 --- a/tests/units/istate/test_shared.py +++ b/tests/units/istate/test_shared.py @@ -7,11 +7,7 @@ import pytest -from reflex.istate.shared import ( - SharedState, - SharedStateBaseInternal, - _do_update_other_tokens, -) +from reflex.istate.shared import _do_update_other_tokens from reflex.state import State from reflex.utils.token_manager import ( LocalTokenManager, @@ -113,45 +109,3 @@ async def test_update_other_tokens_redis_cross_instance(redis_manager, mock_redi # Locally owned sockets are authoritative and never require a redis lookup. local_key = redis_manager._get_redis_key("local") assert local_key not in [call.args[0] for call in mock_redis.get.call_args_list] - - -@pytest.mark.parametrize("held_lock", [False, True], ids=["direct", "linked"]) -async def test_shared_updates_with_shadowed_touched_method( - clean_registration_context, monkeypatch: pytest.MonkeyPatch, held_lock: bool -): - """Notify linked clients when a backend var shadows the touched-state method. - - Args: - clean_registration_context: A fresh, empty registration context. - monkeypatch: Restore shared-state defaults after the test. - held_lock: Whether to collect the shared state from held locks. - """ - monkeypatch.setitem(State.backend_vars, "_reflex_internal_links", {}) - monkeypatch.setattr( - State, "_always_dirty_substates", State._always_dirty_substates.copy() - ) - - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - - class ShadowState(SharedState): - """State with an intentional framework-method collision.""" - - _get_was_touched: int = 7 - - root = State() - parent = await root.get_state(SharedStateBaseInternal) - state = await root.get_state(ShadowState) - state._linked_from = {"other-client"} - root._clean() - state._was_touched = False - state._previous_dirty_vars.clear() - - with patch("reflex.istate.shared._do_update_other_tokens") as update: - async with parent._modify_linked_states(): - if held_lock: - parent._held_locks = {"shared": {ShadowState: state}} - state._get_was_touched = 8 - - update.assert_called_once() - assert update.call_args.kwargs["affected_tokens"] == {"other-client"} - assert state._get_was_touched == 8 diff --git a/tests/units/istate/test_validation.py b/tests/units/istate/test_validation.py index 1b12ccd31bc..ec567b60285 100644 --- a/tests/units/istate/test_validation.py +++ b/tests/units/istate/test_validation.py @@ -1,7 +1,5 @@ """Tests for reserved state names at class creation and dynamic registration.""" -from unittest.mock import patch - import pytest from reflex_base.constants import RouteArgType from reflex_base.utils.exceptions import StateValueError @@ -154,26 +152,6 @@ class Model(EvenMoreBasicBaseState): assert Model().get_state == 7 -def test_legacy_state_names( - monkeypatch: pytest.MonkeyPatch, clean_registration_context -): - """Preserve the old lookup behavior only with the deprecated legacy opt-in. - - Args: - monkeypatch: Set the temporary compatibility environment variable. - clean_registration_context: An isolated state registry. - """ - monkeypatch.setenv("REFLEX_STATE_ALLOW_RESERVED_NAMES", "1") - with patch("reflex_base.utils.console.deprecate") as deprecate: - - class LegacyState(BaseState): - _get_was_touched: int = 7 - - assert LegacyState()._get_was_touched == 7 - deprecate.assert_called_once() - assert deprecate.call_args.kwargs["removal_version"] == "1.0" - - @pytest.mark.parametrize("name", ["get_fields", "_get_was_touched"]) @pytest.mark.parametrize("state_first", [False, True]) def test_reserved_model_mixin(name: str, state_first: bool, clean_registration_context): From 3aa4c66ba64682118e843fc072b2a2db91414bf6 Mon Sep 17 00:00:00 2001 From: Farhan Date: Thu, 17 Sep 2026 22:59:11 +0500 Subject: [PATCH 5/5] refactor(state): compute the fast-path names once at class creation Reserved-name validation means only a marked method override can define a fast-pathed framework name, so the recursive prune after dynamic registration is removed. --- reflex/state.py | 40 +++++++--------------------------------- 1 file changed, 7 insertions(+), 33 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 1c2c9510d46..499e2895650 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -382,8 +382,8 @@ def _is_user_descriptor(value: Any) -> bool: # Instance bookkeeping fields and framework methods read on every event. They # bypass the var-resolution logic below, so nothing stored in `_backend_vars` # (e.g. `_reflex_internal_links`) or delegated to the parent (`router_data`) -# may appear here. A subclass that defines one of these names itself (as a var -# or an event handler) drops it from its own `_fast_attr_names`. +# may appear here. A subclass that overrides one of these methods drops the +# name from its own `_fast_attr_names`. _FRAMEWORK_ATTR_NAMES = frozenset({ "dirty_vars", "dirty_substates", @@ -781,38 +781,15 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): cls._var_dependencies = {} cls._init_var_dependency_dicts() - cls._prune_fast_attr_names() - - all_base_state_classes[cls.get_full_name()] = None - - @classmethod - def _prune_fast_attr_names(cls) -> None: - """Recompute which framework attribute names this state tree may fast-path. - - A name the state defines (as a var, a backend var, an event handler or - a marked method override) must keep going through the full lookup in - ``_get_attribute``. The set is rebuilt from the parent's current set - minus this class's own names, then recomputed for every substate, so a - var or handler registered after class creation (dynamic route args, - ``add_var``, ...) drops the name for the whole subtree that inherits it. - """ + # A marked override of a framework method must keep the full lookup. parent_state = cls.get_parent_state() - inherited = ( + cls._fast_attr_names = ( parent_state._fast_attr_names if parent_state is not None else _FRAMEWORK_ATTR_NAMES - ) - cls._fast_attr_names = inherited - ( - _FRAMEWORK_ATTR_NAMES - & ( - set(cls.__dict__) - | set(cls.vars) - | set(cls.backend_vars) - | set(cls.event_handlers) - ) - ) - for substate_class in cls.get_substates(): - substate_class._prune_fast_attr_names() + ) - cls.__dict__.keys() + + all_base_state_classes[cls.get_full_name()] = None @classmethod def _add_event_handler( @@ -830,7 +807,6 @@ def _add_event_handler( handler = cls._create_event_handler(fn) cls.event_handlers[name] = handler setattr(cls, name, handler) - cls._prune_fast_attr_names() @staticmethod def _copy_fn(fn: Callable) -> Callable: @@ -1360,7 +1336,6 @@ def add_var(cls, name: str, type_: Any, default_value: Any = None): # let substates know about the new variable for substate_class in cls.get_substates(): substate_class.vars.setdefault(name, var) - cls._prune_fast_attr_names() # Reinitialize dependency tracking dicts. cls._init_var_dependency_dicts() @@ -1491,7 +1466,6 @@ def _update_substate_inherited_vars(cls, vars_to_add: builtins.dict[str, Var]): substate_class._update_substate_inherited_vars(vars_to_add) # Reinitialize dependency tracking dicts. cls._init_var_dependency_dicts() - cls._prune_fast_attr_names() @classmethod def _dynamic_route_arg_types(cls) -> builtins.dict[str, str]: