From 608f448faf9a3e7175691e6037bbb8c6aa805314 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 28 Aug 2026 09:29:28 +0000 Subject: [PATCH 01/27] Split router into per-field base vars; gather static router data at connect Reimagines #6906 without touching the delta machinery: instead of eliding unchanged RouterData fields during delta serialization, the router is no longer a single serialized base var. Each kind of router data lives in its own base var on the root state (router_session, router_headers, router_page, router_url, router_route_id), so the existing per-var delta machinery naturally re-sends only what changed: - a navigation dirties only page/url/route_id (measured: 1898 -> 619 bytes, -67%, on a realistic connection; session and headers ship once per connection instead of on every page change) - a reconnect dirties only the session - an event without a route change re-sends no router vars at all State.router remains as a switchboard. On instances it is a property that composes a RouterData view from the per-field vars (and decomposes on assignment), so all existing reads/writes keep working. On classes it returns RouterDataVar, whose attributes resolve directly to the per-field base vars, so State.router.session.client_token and friends compile to the new var names with no frontend changes needed. ComputedVar dependency tracking recurses into the property getter, so vars reading self.router depend on the per-field vars; an explicit legacy deps=["router"] is expanded to all of them with a deprecation warning. The URL is stored as URLData, a dataclass mirroring ReflexURL's parsed components: json.dumps serializes str subclasses natively (bypassing the serializer registry), so a bare ReflexURL field would reach the frontend as a string instead of the component dict. Also move the static per-connection router_data gathering (headers, client IP, session id) from on_event to on_connect: they cannot change without going through on_connect again, so decoding every header on every event was wasted work. on_event now merges a per-sid cached fragment (~4.4x faster router_data prep, benchmarked by test_on_event_router_data), falling back to the connection environ if the connect was not seen; the cache is dropped on disconnect. Old pickled states are discarded by the existing schema check (the root state's base vars changed); __setstate__ drops the legacy router entry so unpickling them does not crash before that check. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../src/reflex_base/constants/__init__.py | 2 + .../src/reflex_base/constants/route.py | 14 +- .../event/processor/base_state_processor.py | 11 +- reflex/app.py | 102 +++++--- reflex/istate/data.py | 222 ++++++++++++++++- reflex/istate/shared.py | 27 +- reflex/state.py | 233 +++++++++++++++++- tests/benchmarks/test_event_processing.py | 82 ++++++ tests/units/istate/test_data.py | 66 +++++ .../processor/test_base_state_processor.py | 87 +++++++ tests/units/test_app.py | 130 +++++++++- tests/units/test_state.py | 139 +++++++++-- tests/units/utils/test_format.py | 22 +- 13 files changed, 1027 insertions(+), 110 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/constants/__init__.py b/packages/reflex-base/src/reflex_base/constants/__init__.py index f83bcfd1e25..fdd7994e9b5 100644 --- a/packages/reflex-base/src/reflex_base/constants/__init__.py +++ b/packages/reflex-base/src/reflex_base/constants/__init__.py @@ -60,6 +60,7 @@ ROUTER, ROUTER_DATA, ROUTER_DATA_INCLUDE, + ROUTER_VARS, DefaultPage, Page404, RouteArgType, @@ -86,6 +87,7 @@ "ROUTER", "ROUTER_DATA", "ROUTER_DATA_INCLUDE", + "ROUTER_VARS", "ROUTE_NOT_FOUND", "SESSION_STORAGE", "SETTER_PREFIX", diff --git a/packages/reflex-base/src/reflex_base/constants/route.py b/packages/reflex-base/src/reflex_base/constants/route.py index 30e7b32170e..abf2fea266f 100644 --- a/packages/reflex-base/src/reflex_base/constants/route.py +++ b/packages/reflex-base/src/reflex_base/constants/route.py @@ -11,10 +11,22 @@ class RouteArgType(SimpleNamespace): LIST = "arg_list" -# the name of the backend var containing path and client information +# the name of the state attribute exposing path and client information ROUTER = "router" ROUTER_DATA = "router_data" +# The names of the per-field base vars holding router data on the root state. +# Session and headers are constant for the lifetime of a websocket connection, +# while page, url, and route_id change on every navigation; keeping them in +# separate vars means a navigation delta only re-sends the navigation fields. +ROUTER_VARS = ( + "router_session", + "router_headers", + "router_page", + "router_url", + "router_route_id", +) + class RouteVar(SimpleNamespace): """Names of variables used in the router_data dict stored in State.""" diff --git a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py index d69baa8f77d..3196d3dfc81 100644 --- a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py +++ b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py @@ -12,7 +12,6 @@ from importlib.util import find_spec from typing import TYPE_CHECKING, Any -from reflex.istate.data import RouterData from reflex.istate.manager.token import BaseStateToken from reflex.istate.proxy import StateProxy from reflex.utils import types @@ -414,12 +413,16 @@ async def _execute_event( ) # re-assign only when the value is set and different - if router_data and state.router_data != router_data: + if ( + router_data + and (previous_router_data := state.router_data) != router_data + ): # assignment will recurse into substates and force recalculation of # dependent ComputedVar (dynamic route variables) state.router_data = router_data - if state.router != (router := RouterData.from_router_data(router_data)): - state.router = router + # only the router vars whose backing keys changed are rebuilt + # and re-sent; session/headers stay put across navigations + state._update_router_vars(router_data, previous_router_data) # Preprocess the event. if ( diff --git a/reflex/app.py b/reflex/app.py index a24116d42b0..3da583cb63e 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -73,7 +73,6 @@ from reflex.app_mixins import AppMixin, LifespanMixin, MiddlewareMixin from reflex.compiler import compiler from reflex.compiler.compiler import readable_name_from_component -from reflex.istate.data import RouterData from reflex.istate.manager import StateManager, StateModificationContext from reflex.istate.manager.token import BaseStateToken from reflex.route import ( @@ -1959,6 +1958,10 @@ def __init__(self, namespace: str, app: App): # Number of client_error reports logged per SID, for rate limiting. self._client_error_counts: dict[str, int] = {} + # Connection-scoped router_data entries per SID, computed once at + # connect time instead of for every event on the connection. + self._static_router_data: dict[str, dict[str, Any]] = {} + # Start time and count of the current process-wide client_error window. self._client_error_window_start = 0.0 self._client_error_window_count = 0 @@ -2008,6 +2011,51 @@ async def on_connect(self, sid: str, environ: dict): f"Frontend version {subprotocol} for session {sid} does not match the backend version {constants.Reflex.VERSION}." ) + # Headers, client IP, and session id cannot change for the lifetime of + # the connection; compute them once instead of on every event. + self._static_router_data[sid] = self._build_static_router_data(sid, environ) + + def _build_static_router_data(self, sid: str, environ: dict) -> dict[str, Any]: + """Build the connection-scoped router_data entries for a socket. + + Args: + sid: The Socket.IO session id. + environ: The request information, including HTTP headers. + + Returns: + The router_data entries that are constant for the connection. + """ + asgi_scope = environ.get("asgi.scope", {}) + + # Get the client headers. + headers = { + k.decode("utf-8"): v.decode("utf-8") + for (k, v) in asgi_scope.get("headers", []) + } + + # Get the client IP + try: + client_ip = asgi_scope["client"][0] + headers["asgi-scope-client"] = client_ip + except (KeyError, IndexError): + client_ip = environ.get("REMOTE_ADDR", "0.0.0.0") + + # Unroll reverse proxy forwarded headers. + client_ip = ( + headers + .get( + "x-forwarded-for", + client_ip, + ) + .partition(",")[0] + .strip() + ) + return { + constants.RouteVar.SESSION_ID: sid, + constants.RouteVar.HEADERS: headers, + constants.RouteVar.CLIENT_IP: client_ip, + } + def on_disconnect(self, sid: str) -> asyncio.Task | None: """Event for when the websocket disconnects. @@ -2018,6 +2066,7 @@ def on_disconnect(self, sid: str) -> asyncio.Task | None: An asyncio Task for cleaning up the token, or None. """ self._client_error_counts.pop(sid, None) + self._static_router_data.pop(sid, None) # Get token before cleaning up disconnect_token = self.sid_to_token.get(sid) if disconnect_token: @@ -2110,45 +2159,25 @@ async def on_event(self, sid: str, data: Any): msg = f"Failed to deserialize event data: {fields}." raise exceptions.EventDeserializationError(msg) from ex - # Get the event environment. - if self.app.sio is None: - msg = "Socket.IO is not initialized." - raise RuntimeError(msg) - environ = self.app.sio.get_environ(sid, self.namespace) - if environ is None: - msg = "Socket.IO environ is not initialized." - raise RuntimeError(msg) - - # Get the client headers. - headers = { - k.decode("utf-8"): v.decode("utf-8") - for (k, v) in environ["asgi.scope"]["headers"] - } - - # Get the client IP - try: - client_ip = environ["asgi.scope"]["client"][0] - headers["asgi-scope-client"] = client_ip - except (KeyError, IndexError): - client_ip = environ.get("REMOTE_ADDR", "0.0.0.0") - - # Unroll reverse proxy forwarded headers. - client_ip = ( - headers - .get( - "x-forwarded-for", - client_ip, + static_router_data = self._static_router_data.get(sid) + if static_router_data is None: + # The connection was not seen by on_connect (e.g. namespace created + # after the socket connected); fall back to the connection environ. + if self.app.sio is None: + msg = "Socket.IO is not initialized." + raise RuntimeError(msg) + environ = self.app.sio.get_environ(sid, self.namespace) + if environ is None: + msg = "Socket.IO environ is not initialized." + raise RuntimeError(msg) + static_router_data = self._static_router_data[sid] = ( + self._build_static_router_data(sid, environ) ) - .partition(",")[0] - .strip() - ) router_data = event.router_data + router_data.update(static_router_data) router_data.update({ constants.RouteVar.QUERY: format.format_query_params(event.router_data), constants.RouteVar.CLIENT_TOKEN: token, - constants.RouteVar.SESSION_ID: sid, - constants.RouteVar.HEADERS: headers, - constants.RouteVar.CLIENT_IP: client_ip, }) router_data[constants.RouteVar.PATH] = "/" + ( self.app.router(path) or "404" @@ -2263,4 +2292,5 @@ async def link_token_to_sid(self, sid: str, token: str): BaseStateToken(ident=new_token or token, cls=self.app._state) ) as state: state.router_data[constants.RouteVar.SESSION_ID] = sid - state.router = RouterData.from_router_data(state.router_data) + if (session := state.router_session).session_id != sid: + state.router_session = dataclasses.replace(session, session_id=sid) diff --git a/reflex/istate/data.py b/reflex/istate/data.py index 74e13c60899..bf7f2b1132c 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -382,6 +382,86 @@ def _serialize_page_data(obj: PageData) -> dict: return {key.name: getattr(obj, key.name) for key in dataclasses.fields(obj)} +def _url_from_router_data(router_data: dict) -> ReflexURL: + """Build the browser URL for the page described by a router_data dict. + + Args: + router_data: the router_data dict. + + Returns: + The parsed browser URL (origin header + prefixed path). + """ + return ReflexURL( + router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") + + get_config().prepend_frontend_path( + router_data.get(constants.RouteVar.ORIGIN, "") + ) + ) + + +@dataclasses.dataclass(frozen=True) +class URLData: + """The parsed components of the current page URL. + + Storage form of ``RouterData.url`` in the state: unlike ``ReflexURL`` (a + ``str`` subclass, which ``json.dumps`` would serialize as a bare string), + a dataclass goes through the registered serializer, so the frontend + receives the parsed component dict. + """ + + scheme: str = "" + netloc: str = "" + origin: str = "" + path: str = "" + query: str = "" + query_parameters: Mapping[str, str] = dataclasses.field( + default_factory=_FrozenDictStrStr + ) + fragment: str = "" + # Annotated str so the frontend var for this field renders the raw href + # string, but always holds a ReflexURL at runtime so the backend keeps + # parsed-component access without re-splitting the URL. + href: str = ReflexURL("") + + @classmethod + def from_url(cls, url: ReflexURL) -> "URLData": + """Create a URLData object from an already-parsed ReflexURL. + + Args: + url: the parsed URL. + + Returns: + A URLData object mirroring the URL's components. + """ + return cls( + scheme=url.scheme, + netloc=url.netloc, + origin=url.origin, + path=url.path, + query=url.query, + query_parameters=url.query_parameters, + fragment=url.fragment, + href=url, + ) + + @classmethod + def from_router_data(cls, router_data: dict) -> "URLData": + """Create a URLData object from the given router_data. + + Args: + router_data: the router_data dict. + + Returns: + A URLData object for the page described by the router_data. + """ + return cls.from_url(_url_from_router_data(router_data)) + + +@serializer(to=dict) +def _serialize_url_data(obj: URLData) -> dict: + return {key.name: getattr(obj, key.name) for key in dataclasses.fields(obj)} + + @dataclasses.dataclass(frozen=True) class SessionData: """An object containing session data.""" @@ -451,12 +531,7 @@ def from_router_data(cls, router_data: dict) -> "RouterData": session=SessionData.from_router_data(router_data), headers=HeaderData.from_router_data(router_data), _page=PageData.from_router_data(router_data), - url=ReflexURL( - router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") - + get_config().prepend_frontend_path( - router_data.get(constants.RouteVar.ORIGIN, "") - ) - ), + url=_url_from_router_data(router_data), route_id=router_data.get(constants.RouteVar.PATH, ""), ) @@ -482,3 +557,138 @@ def serialize_router_data(obj: RouterData) -> dict: "url": _serialize_reflex_url(obj.url), "route_id": obj.route_id, } + + +def _null_var() -> Var: + """Placeholder default for RouterDataVar component fields. + + Returns: + A null Var. + """ + return Var(_js_expr="null", _var_type=None) + + +@dataclasses.dataclass( + eq=False, + frozen=True, + slots=True, +) +class RouterDataVar(CachedVarOperation, ObjectVar[RouterData]): + """Switchboard Var for ``State.router``. + + Router data is stored in separate per-field base vars on the root state + (session, headers, page, url, route_id) so that unchanged + connection-scoped data is not re-sent in the delta on every navigation. + This var stitches them back together: each attribute resolves directly to + the underlying per-field base var, and rendering the var itself produces + an object literal matching the pre-split serialized router shape. + """ + + # _url_var first: VarData.merge picks the first non-empty field_name, so + # `deps=[State.router]` registers against the navigation-scoped var. + _url_var: Var = dataclasses.field(default_factory=_null_var) + _page_var: Var = dataclasses.field(default_factory=_null_var) + _session_var: Var = dataclasses.field(default_factory=_null_var) + _headers_var: Var = dataclasses.field(default_factory=_null_var) + _route_id_var: Var = dataclasses.field(default_factory=_null_var) + _default_var_type: ClassVar[Any] = RouterData + + @cached_property_no_lock + def _cached_var_name(self) -> str: + """Render the router as an object literal over the per-field vars. + + Returns: + The JS expression for the assembled router object. + """ + return ( + "({ " + f'"session": {self._session_var!s}, ' + f'"headers": {self._headers_var!s}, ' + f'"page": {self._page_var!s}, ' + f'"url": {self._url_var!s}, ' + f'"route_id": {self._route_id_var!s}' + " })" + ) + + @property + def session(self) -> ObjectVar[SessionData]: + """The per-connection session data. + + Returns: + ObjectVar for the ``router_session`` base var. + """ + return self._session_var.to(ObjectVar, SessionData) + + @property + def headers(self) -> ObjectVar[HeaderData]: + """The headers of the websocket connection request. + + Returns: + ObjectVar for the ``router_headers`` base var. + """ + return self._headers_var.to(ObjectVar, HeaderData) + + @property + def page(self) -> ObjectVar[PageData]: + """The page data for the current page (deprecated, use ``url``). + + Returns: + ObjectVar for the ``router_page`` base var. + """ + return self._page_var.to(ObjectVar, PageData) + + # RouterData exposes the page data under both `page` and `_page`. + _page = page + + @property + def url(self) -> ReflexURLCastedVar: + """The parsed URL of the current page. + + Returns: + ReflexURLCastedVar over the ``router_url`` base var. + """ + return ReflexURLCastedVar.create(self._url_var) + + @property + def route_id(self) -> StringVar: + """The route pattern that matched the current page. + + Returns: + StringVar for the ``router_route_id`` base var. + """ + return self._route_id_var.to(str) + + @classmethod + def create( + cls, + *, + session: Var, + headers: Var, + page: Var, + url: Var, + route_id: Var, + _var_data: VarData | None = None, + ) -> "RouterDataVar": + """Create a RouterDataVar over the per-field router base vars. + + Args: + session: The ``router_session`` base var. + headers: The ``router_headers`` base var. + page: The ``router_page`` base var. + url: The ``router_url`` base var. + route_id: The ``router_route_id`` base var. + _var_data: Additional VarData to merge in. + + Returns: + The new RouterDataVar. + """ + return cls( + _js_expr="", + _var_type=RouterData, + _var_data=_var_data, + _url_var=url, + _page_var=page, + _session_var=session, + _headers_var=headers, + _route_id_var=route_id, + ) diff --git a/reflex/istate/shared.py b/reflex/istate/shared.py index e38517fef66..8b164ac45d5 100644 --- a/reflex/istate/shared.py +++ b/reflex/istate/shared.py @@ -6,7 +6,7 @@ from collections.abc import AsyncIterator from typing import TypeVar -from reflex_base.constants import ROUTER_DATA +from reflex_base.constants import ROUTER_DATA, ROUTER_VARS from reflex_base.event import Event, get_hydrate_event from reflex_base.registry import RegistrationContext from reflex_base.utils.exceptions import ReflexRuntimeError @@ -106,7 +106,7 @@ async def _patch_state( linked_state._mark_dirty() # Apply the updates into the existing state tree for rehydrate. root_state = original_state._get_root_state() - root_state.dirty_vars.add("router") + root_state.dirty_vars.update(ROUTER_VARS) root_state.dirty_vars.add(ROUTER_DATA) root_state._mark_dirty() await root_state._get_resolved_delta() @@ -237,7 +237,7 @@ async def _link_to(self, token: str) -> Self: return self # already linked to this token if self._linked_to and self._linked_to != token: # Disassociate from previous linked token since unlink will not be called. - self._linked_from.discard(self.router.session.client_token) + self._linked_from.discard(self.router_session.client_token) # TODO: Change StateManager to accept token + class instead of combining them in a string. if "_" in token: msg = f"Invalid token {token} for linking state {self.get_full_name()}, cannot use underscore (_) in the token name." @@ -272,12 +272,12 @@ async def _unlink(self): # Break the linkage for future events. self._reflex_internal_links.pop(state_name) - self._linked_from.discard(self.router.session.client_token) + self._linked_from.discard(self.router_session.client_token) # Patch in the original state, apply updates, then rehydrate. private_root_state = await get_state_manager().get_state( BaseStateToken( - ident=self.router.session.client_token, + ident=self.router_session.client_token, cls=type(self), ) ) @@ -326,14 +326,13 @@ async def _internal_patch_linked_state( # Set client_token on the linked root so that subsequent get_state # calls when directly modifying a linked token will load the # associated instance. - if linked_root_state.router.session.client_token != token: + if ( + session := linked_root_state.router_session + ).client_token != token: import dataclasses as dc - linked_root_state.router = dc.replace( - linked_root_state.router, - session=dc.replace( - linked_root_state.router.session, client_token=token - ), + linked_root_state.router_session = dc.replace( + session, client_token=token ) if linked_root_state is None: linked_root_state = await get_state_manager().get_state( @@ -346,8 +345,8 @@ async def _internal_patch_linked_state( # Avoid unnecessary dirtiness of shared state when there are no changes. if type(self) not in self._held_locks[token]: self._held_locks[token][type(self)] = linked_state - if self.router.session.client_token not in linked_state._linked_from: - linked_state._linked_from.add(self.router.session.client_token) + if self.router_session.client_token not in linked_state._linked_from: + linked_state._linked_from.add(self.router_session.client_token) if linked_state._linked_to != token: linked_state._linked_to = token await self._exit_stack.enter_async_context( @@ -438,7 +437,7 @@ async def _modify_linked_states( affected_tokens.update( token for token in linked_state._linked_from - if token != self.router.session.client_token + if token != self.router_session.client_token ) # When modifying a shared token directly (empty _reflex_internal_links), # the held locks will be empty. Check SharedState substates for linked diff --git a/reflex/state.py b/reflex/state.py index 452c80e7109..9dcbc29c66b 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -25,7 +25,9 @@ Final, ParamSpec, TypeVar, + cast, get_type_hints, + overload, ) from reflex_base import constants @@ -73,7 +75,15 @@ import reflex.istate.dynamic from reflex import event from reflex.istate import HANDLED_PICKLE_ERRORS, debug_failed_pickles -from reflex.istate.data import RouterData +from reflex.istate.data import ( + HeaderData, + PageData, + ReflexURL, + RouterData, + RouterDataVar, + SessionData, + URLData, +) from reflex.istate.proxy import ImmutableMutableProxy as ImmutableMutableProxy from reflex.istate.proxy import MutableProxy, is_mutable_type from reflex.istate.storage import ClientStorageBase @@ -358,6 +368,124 @@ def _is_user_descriptor(value: Any) -> bool: return not is_computed_var(value) +def _router_fget(self: BaseState) -> RouterData: + """Assemble the RouterData view over the per-field router vars. + + Args: + self: The state instance. + + Returns: + The RouterData for the current connection and page. + """ + return RouterData( + session=self.router_session, + headers=self.router_headers, + _page=self.router_page, + # URLData.href always holds a ReflexURL at runtime (see URLData). + url=cast("ReflexURL", self.router_url.href), + route_id=self.router_route_id, + ) + + +def _router_fset(self: BaseState, value: RouterData) -> None: + """Decompose a RouterData assignment into the per-field router vars. + + Args: + self: The state instance. + value: The RouterData to store. + """ + self.router_session = value.session + self.router_headers = value.headers + self.router_page = value._page + self.router_url = URLData.from_url(value.url) + self.router_route_id = value.route_id + + +def _get_router_var(cls: type[BaseState]) -> RouterDataVar: + """Get (or build and cache) the router switchboard var for a state class. + + Args: + cls: The state class the ``router`` attribute was accessed on. + + Returns: + The RouterDataVar over the root state's per-field router vars. + """ + root_cls = cls.get_root_state() + router_var = root_cls.__dict__.get("_reflex_router_var") + if router_var is None: + base_vars = root_cls.base_vars + if "router_session" not in base_vars: + # BaseState itself and mixins never initialize base vars; give + # introspection-style access an unbound switchboard. + return RouterDataVar(_js_expr="", _var_type=RouterData) + router_var = RouterDataVar.create( + session=base_vars["router_session"], + headers=base_vars["router_headers"], + page=base_vars["router_page"], + url=base_vars["router_url"], + route_id=base_vars["router_route_id"], + ) + setattr(root_cls, "_reflex_router_var", router_var) # noqa: B010 + return router_var + + +class _RouterDescriptor(property): + """Property exposing the per-field router vars as a single ``router`` attribute. + + Instance access composes a ``RouterData`` view from the per-field router + vars and assignment decomposes one into them, so existing reads and writes + of ``state.router`` keep working unchanged. Class-level access returns the + ``RouterDataVar`` switchboard, resolving ``State.router.`` to the + underlying per-field base var. Subclassing ``property`` keeps the state + field machinery from treating this as a base var and lets ComputedVar + dependency tracking recurse into the getter, so any computed var reading + ``self.router`` depends on the per-field vars. + """ + + if TYPE_CHECKING: + + @overload + def __get__(self, instance: None, owner: type, /) -> RouterDataVar: ... + + @overload + def __get__(self, instance: BaseState, owner: type, /) -> RouterData: ... + + def __get__(self, instance: Any, owner: type | None = None, /) -> Any: + """Get the switchboard var (class) or RouterData view (instance). + + Args: + instance: The state instance, or None for class access. + owner: The class through which the attribute was accessed. + + Returns: + The RouterDataVar for class access, or the RouterData view. + """ + + def __set__(self, instance: Any, value: RouterData) -> None: + """Set the router data on the instance. + + Args: + instance: The state instance. + value: The RouterData to store. + """ + + else: + + def __get__(self, instance: Any, owner: type | None = None, /): + """Get the switchboard var (class) or RouterData view (instance). + + Args: + instance: The state instance, or None for class access. + owner: The class through which the attribute was accessed. + + Returns: + The RouterDataVar for class access, or the RouterData view. + """ + if instance is None: + return _get_router_var(owner) + return super().__get__(instance, owner) + + all_base_state_classes: dict[str, None] = {} CLASS_VAR_NAMES = frozenset({ @@ -435,8 +563,27 @@ class BaseState(EvenMoreBasicBaseState): default_factory=builtins.dict, is_var=False ) - # The router data for the current page - router: Field[RouterData] = field(default_factory=RouterData) + # The per-connection session data (constant for the socket lifetime). + router_session: Field[SessionData] = field(default_factory=SessionData) + + # The headers of the connection request (constant for the socket lifetime). + router_headers: Field[HeaderData] = field(default_factory=HeaderData) + + # The page data for the current page (deprecated; params feeds dynamic route vars). + router_page: Field[PageData] = field(default_factory=PageData) + + # The parsed URL of the current page. + router_url: Field[URLData] = field(default_factory=URLData) + + # The route pattern that matched the current page. + router_route_id: Field[str] = field(default="") + + # Switchboard for the router vars above: instance reads compose a + # RouterData view, writes decompose into the per-field vars, and class + # access returns the RouterDataVar. Deliberately not a Field: storing each + # kind of router data in its own base var means a navigation delta only + # re-sends the navigation-scoped vars, not session/headers. + router = _RouterDescriptor(_router_fget, _router_fset) # Whether the state has ever been touched since instantiation. _was_touched: bool = field(default=False, is_var=False) @@ -899,6 +1046,19 @@ def _init_var_dependency_dicts(cls): # Do not perform dep calculation when cache=False (these are always dirty). continue for state_name, dvar_set in cvar._deps(objclass=cls).items(): + if constants.ROUTER in dvar_set: + # Legacy explicit dependency on the pre-split `router` var: + # depend on all the per-field router vars instead. + console.deprecate( + feature_name='ComputedVar deps=["router"]', + reason="the router var was split; depend on the specific" + ' router var instead (e.g. deps=["router_url"]).', + deprecation_version="0.9.9", + removal_version="1.0", + ) + dvar_set = (dvar_set - {constants.ROUTER}) | set( + constants.ROUTER_VARS + ) state_cls = cls.get_root_state().get_class_substate(state_name) for dvar in dvar_set: defining_state_cls = state_cls @@ -971,7 +1131,9 @@ def _check_overridden_basevars(cls): """ hints = cls._get_type_hints() for name, computed_var_ in cls._get_computed_vars(): - if name in hints: + # `router` is not a field, but shadowing the descriptor would + # silently break router access for the whole state tree. + if name in hints or name == constants.ROUTER: msg = f"The computed var name `{computed_var_._js_expr}` shadows a base var in {cls.__module__}.{cls.__name__}; use a different name instead" raise ComputedVarShadowsBaseVarsError(msg) @@ -1005,6 +1167,8 @@ def get_skip_vars(cls) -> set[str]: "dirty_vars", "dirty_substates", "router_data", + # Not a var: assignment must reach the _RouterDescriptor. + constants.ROUTER, } | types.RESERVED_BACKEND_VAR_NAMES ) @@ -1375,7 +1539,7 @@ def setup_dynamic_args(cls, args: builtins.dict[str, str]): def argsingle_factory(param: str): def inner_func(self: BaseState) -> str: - return self.router._page.params.get(param, "") + return self.router_page.params.get(param, "") inner_func.__name__ = param @@ -1383,7 +1547,7 @@ def inner_func(self: BaseState) -> str: def arglist_factory(param: str): def inner_func(self: BaseState) -> list[str]: - return self.router._page.params.get(param, []) + return self.router_page.params.get(param, []) inner_func.__name__ = param @@ -1400,7 +1564,7 @@ def inner_func(self: BaseState) -> list[str]: dynamic_vars[param] = DynamicRouteVar( fget=func, auto_deps=False, - deps=["router"], + deps=["router_page"], _var_data=VarData.from_state(cls, param), ) setattr(cls, param, dynamic_vars[param]) @@ -1565,7 +1729,7 @@ def reset(self): # Reset the base vars. fields = self.get_fields() for prop_name in self.base_vars: - if prop_name == constants.ROUTER: + if prop_name in constants.ROUTER_VARS: continue # never reset the router data field = fields[prop_name] if default_factory := field.default_factory: @@ -1583,6 +1747,48 @@ def reset(self): for substate in self.substates.values(): substate.reset() + def _update_router_vars( + self, + router_data: builtins.dict[str, Any], + previous_router_data: builtins.dict[str, Any], + ) -> None: + """Update the per-field router vars from a new router_data dict. + + Only rebuilds and reassigns the vars whose backing router_data keys + actually changed, so connection-scoped data (session, headers) is not + recomputed or re-sent in the delta on every navigation. + + Args: + router_data: The new router_data dict. + previous_router_data: The router_data dict this state last saw. + """ + get = router_data.get + prev_get = previous_router_data.get + if any( + prev_get(key) != get(key) + for key in ( + constants.RouteVar.CLIENT_TOKEN, + constants.RouteVar.SESSION_ID, + constants.RouteVar.CLIENT_IP, + ) + ): + self.router_session = SessionData.from_router_data(router_data) + headers_changed = prev_get(constants.RouteVar.HEADERS) != get( + constants.RouteVar.HEADERS + ) + if headers_changed: + self.router_headers = HeaderData.from_router_data(router_data) + if ( + # The origin header feeds the URL/page host. + headers_changed + or prev_get(constants.RouteVar.PATH) != get(constants.RouteVar.PATH) + or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) + or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) + ): + self.router_page = PageData.from_router_data(router_data) + self.router_url = URLData.from_router_data(router_data) + self.router_route_id = get(constants.RouteVar.PATH, "") + @classmethod @functools.lru_cache def _is_client_storage(cls, prop_name_or_field: str | Field) -> bool: @@ -1695,7 +1901,7 @@ async def _get_state_from_redis(self, state_cls: type[T_STATE]) -> T_STATE: ) raise RuntimeError(msg) state_in_redis = await state_manager.get_state( - token=BaseStateToken(ident=self.router.session.client_token, cls=state_cls), + token=BaseStateToken(ident=self.router_session.client_token, cls=state_cls), top_level=False, for_state_instance=self, ) @@ -2069,7 +2275,8 @@ def __getstate__(self): state = state.copy() if state.get("parent_state") is not None: # Do not serialize router data in substates (only the root state). - state.pop("router", None) + for router_var in constants.ROUTER_VARS: + state.pop(router_var, None) state.pop("router_data", None) # Never serialize parent_state or substates. state.pop("parent_state", None) @@ -2090,6 +2297,10 @@ def __setstate__(self, state: builtins.dict[str, Any]): """ state["parent_state"] = None state["substates"] = {} + # Pre-split pickles stored a RouterData under `router`, which is now a + # descriptor; drop it so unpickling does not route through the setter. + # The schema check in _deserialize discards such states anyway. + state.pop("router", None) for key, value in state.items(): object.__setattr__(self, key, value) @@ -2466,7 +2677,7 @@ def on_load_internal(self) -> list[Event | EventSpec | event.EventCallback] | No The list of events to queue for on load handling. """ load_events = RegistrationContext.get().app.get_load_events( - self.router.url.path + self.router_url.path ) if not load_events: self.is_hydrated = True diff --git a/tests/benchmarks/test_event_processing.py b/tests/benchmarks/test_event_processing.py index 15acf8094d4..e912e86a143 100644 --- a/tests/benchmarks/test_event_processing.py +++ b/tests/benchmarks/test_event_processing.py @@ -120,3 +120,85 @@ def test_process_event( @benchmark def _(): loop.run_until_complete(run_events(num_events=3, num_expected_deltas=3)) + + +@pytest.fixture +def on_event_harness(): + """Set up an EventNamespace with a connected socket for benchmarking on_event. + + The event processor's enqueue is mocked out so the benchmark isolates the + per-event router_data preparation (which reuses the connection-scoped + data gathered once in on_connect). + + Yields: + An async callable that feeds the given number of events through + ``EventNamespace.on_event``, and the event loop to drive it with. + """ + from reflex.app import App, EventNamespace + + app = App() + app._event_processor = mock.Mock(enqueue=mock.AsyncMock()) + namespace = EventNamespace("/event", app) + + sid = "benchmark-sid" + environ = { + "QUERY_STRING": "token=benchmark-token", + "asgi.scope": { + "headers": [ + (b"host", b"localhost:3000"), + (b"origin", b"http://localhost:3000"), + (b"user-agent", b"Mozilla/5.0 (X11; Linux x86_64) benchmark"), + (b"accept-encoding", b"gzip, deflate, br"), + (b"accept-language", b"en-US,en;q=0.9"), + (b"cookie", b"session=abc123; theme=dark"), + (b"upgrade", b"websocket"), + (b"connection", b"Upgrade"), + (b"sec-websocket-version", b"13"), + (b"sec-websocket-key", b"dGhlIHNhbXBsZSBub25jZQ=="), + (b"x-forwarded-for", b"203.0.113.7, 10.0.0.1"), + ], + "client": ("127.0.0.1", 54321), + }, + } + + async def run_events(num_events: int) -> None: + """Feed events through on_event. + + Args: + num_events: Number of events to process. + """ + for _ in range(num_events): + await namespace.on_event( + sid, + { + "name": "state.hydrate", + "router_data": {"pathname": "/", "query": {}, "asPath": "/"}, + "payload": {}, + }, + ) + + loop = asyncio.new_event_loop() + loop.run_until_complete(namespace.on_connect(sid, environ)) + yield run_events, loop + loop.close() + + +def test_on_event_router_data( + on_event_harness, + benchmark: BenchmarkFixture, +): + """Benchmark the per-event router_data preparation in on_event. + + Headers and client IP are gathered once at connect time, so the + per-event path is reduced to merging the cached connection-scoped dict + with the event's navigation data. + + Args: + on_event_harness: The run_events async callable and its event loop. + benchmark: The codspeed benchmark fixture. + """ + run_events, loop = on_event_harness + + @benchmark + def _(): + loop.run_until_complete(run_events(num_events=10)) diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index 6ff0b6e805a..62f4a8c5b32 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -146,3 +146,69 @@ def test_router_url_var_renders_as_href_at_top_level(): """ url_var = rx.State.router.url assert str(url_var) == f'{url_var._original!s}?.["href"]' + + +def test_url_data_serializes_like_reflex_url(): + """URLData (the per-field storage form of the router URL) must serialize + to the same component dict shape as the eager ReflexURL serialization, so + the frontend var access patterns are unchanged by the router var split. + """ + import json + + from reflex_base.utils.format import json_dumps + + from reflex.istate.data import URLData, _serialize_reflex_url + + url = ReflexURL(SAMPLE_URL) + payload = json.loads(json_dumps(URLData.from_url(url))) + assert payload == json.loads(json_dumps(_serialize_reflex_url(url))) + # The runtime value of href keeps parsed-component access on the backend. + assert isinstance(URLData.from_url(url).href, ReflexURL) + + +def test_router_var_resolves_to_per_field_base_vars(): + """State.router is a switchboard: each attribute must resolve directly to + the per-field base var, so a navigation delta that only carries the + navigation-scoped vars still updates every rendered router expression. + """ + prefix = "reflex___state____state" + assert ( + str(rx.State.router.session.client_token) + == f'{prefix}.router_session_rx_state_?.["client_token"]' + ) + assert ( + str(rx.State.router.headers.user_agent) + == f'{prefix}.router_headers_rx_state_?.["user_agent"]' + ) + assert ( + str(rx.State.router.page.raw_path) + == f'{prefix}.router_page_rx_state_?.["raw_path"]' + ) + assert str(rx.State.router.url) == f'{prefix}.router_url_rx_state_?.["href"]' + assert str(rx.State.router.url.path) == f'{prefix}.router_url_rx_state_?.["path"]' + assert str(rx.State.router.route_id) == f"{prefix}.router_route_id_rx_state_" + + +def test_router_var_renders_composed_object(): + """Rendering State.router itself produces an object literal over the + per-field vars, matching the pre-split serialized router shape. + """ + prefix = "reflex___state____state" + assert str(rx.State.router) == ( + "({ " + f'"session": {prefix}.router_session_rx_state_, ' + f'"headers": {prefix}.router_headers_rx_state_, ' + f'"page": {prefix}.router_page_rx_state_, ' + f'"url": {prefix}.router_url_rx_state_, ' + f'"route_id": {prefix}.router_route_id_rx_state_' + " })" + ) + + +def test_router_var_carries_state_var_data(): + """The switchboard var must merge the per-field vars' VarData so hooks + and context wiring for the root state are set up when it renders. + """ + var_data = rx.State.router._get_all_var_data() + assert var_data is not None + assert var_data.state == rx.State.get_full_name() diff --git a/tests/units/reflex_base/event/processor/test_base_state_processor.py b/tests/units/reflex_base/event/processor/test_base_state_processor.py index c8d0476edb3..ae5413e78b8 100644 --- a/tests/units/reflex_base/event/processor/test_base_state_processor.py +++ b/tests/units/reflex_base/event/processor/test_base_state_processor.py @@ -615,3 +615,90 @@ def raise_on_modify(*args, **kwargs): object.__setattr__(root_ctx.state_manager, "modify_state_with_links", original) assert proxy._self_entered_context is False + + +async def test_navigation_delta_elides_connection_scoped_router_vars( + wired_app: App, + real_base_state_processor: BaseStateEventProcessor, + emitted_deltas: list, + token: str, +): + """A navigation only re-sends the navigation-scoped router vars. + + Session and headers cannot change without going through a reconnect, so + re-shipping them in the delta of every client event is pure overhead. + The router is stored in per-field base vars precisely so that a + navigation marks only page/url/route_id dirty; a reconnect (new sid) + marks only the session dirty. + + Args: + wired_app: The App wired to the processor's state manager. + real_base_state_processor: The unmocked BaseStateEventProcessor. + emitted_deltas: List of deltas captured from the processor. + token: The client token. + """ + + class NavState(State): + n: int = 0 + + @event + def bump(self): + self.n += 1 + + headers = {"origin": "http://localhost:3000", "user-agent": "test-agent"} + + def view(path: str, sid: str = "sid1") -> dict[str, Any]: + return { + "pathname": path, + "asPath": path, + "query": {}, + "token": token, + "sid": sid, + "ip": "127.0.0.1", + "headers": headers, + } + + def client_event(router_data: dict[str, Any]) -> Event: + return dataclasses.replace( + Event.from_event_type(NavState.bump())[0], router_data=router_data + ) + + def router_vars_in_deltas() -> set[str]: + return { + key.removesuffix(FIELD_MARKER) + for _token, delta in emitted_deltas + for key in delta.get(State.get_full_name(), {}) + if key.startswith("router") + } + + async def run_event(router_data: dict[str, Any]) -> None: + emitted_deltas.clear() + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(router_data)) + await processor.join(10) + + # First event on the connection populates every router var. + await run_event(view("/a")) + assert router_vars_in_deltas() == { + "router_session", + "router_headers", + "router_page", + "router_url", + "router_route_id", + } + + # A navigation only re-sends the navigation-scoped vars. + await run_event(view("/b")) + assert router_vars_in_deltas() == { + "router_page", + "router_url", + "router_route_id", + } + + # An event without a route change re-sends no router vars at all. + await run_event(view("/b")) + assert router_vars_in_deltas() == set() + + # A reconnect (new sid, same headers) re-sends only the session. + await run_event(view("/b", sid="sid2")) + assert router_vars_in_deltas() == {"router_session"} diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 484796337a5..4c063d0ef27 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -14,7 +14,7 @@ from contextlib import nullcontext as does_not_raise from importlib.util import find_spec from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast from unittest.mock import AsyncMock, Mock import pytest @@ -52,7 +52,7 @@ ) from reflex.compiler.plugins import default_page_plugins from reflex.environment import environment -from reflex.istate.data import RouterData +from reflex.istate.data import RouterData, URLData from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory from reflex.istate.manager.redis import StateManagerRedis @@ -290,9 +290,9 @@ def test_add_page_set_route_dynamic(index_page: ComponentCallable): assert app._pages.keys() == {"test/[dynamic]"} assert "dynamic" in app._state.computed_vars assert app._state.computed_vars["dynamic"]._deps(objclass=EmptyState) == { - EmptyState.get_full_name(): {constants.ROUTER}, + EmptyState.get_full_name(): {"router_page"}, } - assert constants.ROUTER in app._state()._var_dependencies + assert "router_page" in app._state()._var_dependencies def test_add_page_set_route_nested(app: App, index_page: ComponentCallable): @@ -1884,9 +1884,9 @@ async def test_dynamic_route_var_route_change_completed_on_load( assert arg_name in app._state.vars assert arg_name in app._state.computed_vars assert app._state.computed_vars[arg_name]._deps(objclass=DynamicState) == { - DynamicState.get_full_name(): {constants.ROUTER}, + DynamicState.get_full_name(): {"router_page"}, } - assert constants.ROUTER in app._state()._var_dependencies + assert "router_page" in app._state()._var_dependencies substate_token = BaseStateToken(ident=token, cls=DynamicState) exp_vals = ["foo", "foobar", "baz"] @@ -1920,6 +1920,13 @@ def _dynamic_state_event(name, val, **kwargs): val=exp_val, ) exp_router = RouterData.from_router_data(on_load_internal.router_data) + # Only the navigation-scoped router vars change (no session/headers in + # the router_data), so only those land in the delta. + exp_router_delta = { + "router_page" + FIELD_MARKER: exp_router._page, + "router_url" + FIELD_MARKER: URLData.from_url(exp_router.url), + "router_route_id" + FIELD_MARKER: exp_router.route_id, + } async with mock_base_state_event_processor as processor: await processor.enqueue( token, @@ -1933,7 +1940,7 @@ def _dynamic_state_event(name, val, **kwargs): State.get_full_name(): { arg_name + FIELD_MARKER: exp_val, constants.CompileVars.IS_HYDRATED + FIELD_MARKER: False, - "router" + FIELD_MARKER: exp_router, + **exp_router_delta, }, DynamicState.get_full_name(): { f"comp_{arg_name}" + FIELD_MARKER: exp_val, @@ -4283,3 +4290,112 @@ def test_client_error_constants_match_frontend(): f'const ERROR_TYPE_STATE_UPDATE = "{constants.ClientErrorType.STATE_UPDATE}"' in state_js ) + + +@pytest.fixture +def event_namespace_with_processor_mock() -> EventNamespace: + """An EventNamespace whose app has a mocked event processor. + + Returns: + The EventNamespace instance. + """ + app = App() + app._event_processor = Mock(enqueue=AsyncMock()) + return EventNamespace("/event", app) + + +def _connect_environ(token: str) -> dict[str, Any]: + return { + "QUERY_STRING": f"token={token}", + "asgi.scope": { + "headers": [ + (b"origin", b"http://localhost:3000"), + (b"user-agent", b"test-agent"), + ], + "client": ("127.0.0.1", 1234), + }, + } + + +def _client_event_payload() -> dict[str, Any]: + return { + "name": "state.hydrate", + "router_data": {"pathname": "/", "query": {}, "asPath": "/"}, + "payload": {}, + } + + +@pytest.mark.asyncio +async def test_on_event_uses_connect_time_router_data( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """on_event merges the connection-scoped router_data gathered at connect. + + Headers, client IP, and session id are computed once in on_connect; the + per-event path must not re-read the connection environ at all. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace.on_connect("sid1", _connect_environ(token)) + assert "sid1" in event_namespace._static_router_data + + # The per-event path must not re-read the connection environ. + event_namespace.app.sio = Mock( + get_environ=Mock(side_effect=AssertionError("environ must not be consulted")) + ) + await event_namespace.on_event("sid1", _client_event_payload()) + + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + enqueue_mock.assert_called_once() + enqueued_token, event = enqueue_mock.call_args[0] + assert enqueued_token == token + assert event.router_data[constants.RouteVar.CLIENT_TOKEN] == token + assert event.router_data[constants.RouteVar.SESSION_ID] == "sid1" + assert event.router_data[constants.RouteVar.CLIENT_IP] == "127.0.0.1" + assert event.router_data[constants.RouteVar.HEADERS] == { + "origin": "http://localhost:3000", + "user-agent": "test-agent", + "asgi-scope-client": "127.0.0.1", + } + assert event.router_data[constants.RouteVar.PATH] == "/404" + assert event.router_data[constants.RouteVar.QUERY] == {} + + # Disconnect drops the cached connection data. + event_namespace.on_disconnect("sid1") + assert "sid1" not in event_namespace._static_router_data + + +@pytest.mark.asyncio +async def test_on_event_falls_back_to_environ_without_connect( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """on_event computes and caches the static router_data if connect was missed. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace._token_manager.link_token_to_sid(token, "sid1") + event_namespace.app.sio = Mock( + get_environ=Mock(return_value=_connect_environ(token)) + ) + + await event_namespace.on_event("sid1", _client_event_payload()) + await event_namespace.on_event("sid1", _client_event_payload()) + + # The environ is only consulted once; the result is cached for the sid. + event_namespace.app.sio.get_environ.assert_called_once() + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + assert enqueue_mock.call_count == 2 + for call in enqueue_mock.call_args_list: + _, event = call[0] + assert event.router_data[constants.RouteVar.SESSION_ID] == "sid1" + assert ( + event.router_data[constants.RouteVar.HEADERS]["user-agent"] == "test-agent" + ) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index e49e2926809..785c5b55f08 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -42,7 +42,7 @@ import reflex as rx from reflex.app import App from reflex.environment import environment -from reflex.istate.data import HeaderData, RouterData, _FrozenDictStrStr +from reflex.istate.data import HeaderData, RouterData, URLData, _FrozenDictStrStr from reflex.istate.manager import StateManager from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory @@ -71,20 +71,24 @@ LOCK_EXPIRE_SLEEP = 2.5 if CI else 0.4 -formatted_router = { - "route_id": "", - "url": { +formatted_router_vars = { + "router_route_id" + FIELD_MARKER: "", + "router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "://", + "origin": "", "path": "", "query": "", "query_parameters": {}, "fragment": "", "href": "", }, - "session": {"client_token": "", "client_ip": "", "session_id": ""}, - "headers": { + "router_session" + FIELD_MARKER: { + "client_token": "", + "client_ip": "", + "session_id": "", + }, + "router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -100,7 +104,7 @@ "accept_language": "", "raw_headers": {}, }, - "page": { + "router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", @@ -379,7 +383,7 @@ def test_class_vars(test_state): """ cls = type(test_state) assert cls.vars.keys() == { - "router", + *constants.ROUTER_VARS, "num1", "num2", "key", @@ -1216,7 +1220,8 @@ def test_interdependent_state_initial_dict() -> None: s = InterdependentState() state_name = s.get_name() d = s.dict(initial=True)[state_name] - d.pop("router" + FIELD_MARKER) + for router_var in constants.ROUTER_VARS: + d.pop(router_var + FIELD_MARKER) assert d == { "x" + FIELD_MARKER: 0, "v1" + FIELD_MARKER: 0, @@ -1509,19 +1514,19 @@ def dep_v(self) -> int: dict1 = json.loads(json_dumps(ps.dict())) assert dict1[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 1, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict1[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 2} dict2 = json.loads(json_dumps(ps.dict())) assert dict2[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 3, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict2[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 4} dict3 = json.loads(json_dumps(ps.dict())) assert dict3[ps.get_full_name()] == { "no_cache_v" + FIELD_MARKER: 5, - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, } assert dict3[cs.get_full_name()] == {"dep_v" + FIELD_MARKER: 6} assert counter == 6 @@ -2397,7 +2402,13 @@ async def test_state_proxy( ( token, { - TestState.get_full_name(): {"router" + FIELD_MARKER: router_data}, + TestState.get_full_name(): { + "router_session" + FIELD_MARKER: router_data.session, + "router_headers" + FIELD_MARKER: router_data.headers, + "router_page" + FIELD_MARKER: router_data._page, + "router_url" + FIELD_MARKER: URLData.from_url(router_data.url), + "router_route_id" + FIELD_MARKER: router_data.route_id, + }, grandchild_state.get_full_name(): { "value2" + FIELD_MARKER: "42", }, @@ -3094,7 +3105,7 @@ class MutableContainsBase(BaseState): assert json.loads(val) == { MutableContainsBase.get_full_name(): { f"items{FIELD_MARKER}": [{"tags": ["123", "456"]}], - f"router{FIELD_MARKER}": formatted_router, + **formatted_router_vars, } } @@ -3393,7 +3404,10 @@ def index(): assert len(emitted_deltas) == 1 + len(expected) first_token, first_delta = emitted_deltas[0] assert first_token == token - assert first_delta[State.get_full_name()].pop("router" + FIELD_MARKER) is not None + first_state_delta = first_delta[State.get_full_name()] + assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + for router_var in constants.ROUTER_VARS: + first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) # Find the deltas containing the test handler's state change @@ -3453,7 +3467,10 @@ def index(): # First delta: router + is_hydrated=False assert len(emitted_deltas) >= 2 first_delta = emitted_deltas[0][1] - assert first_delta[State.get_full_name()].pop("router" + FIELD_MARKER) is not None + first_state_delta = first_delta[State.get_full_name()] + assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + for router_var in constants.ROUTER_VARS: + first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) # Find deltas containing the test handler's state change (num incremented twice) @@ -3726,12 +3743,15 @@ def foo(self) -> str: foo = RouterVarDepState.computed_vars["foo"] State._init_var_dependency_dicts() + # Reading self.router recurses into the router property getter, so the + # dependency lands on each of the per-field router vars. assert foo._deps(objclass=RouterVarDepState) == { - RouterVarDepState.get_full_name(): {"router"} + RouterVarDepState.get_full_name(): set(constants.ROUTER_VARS) } - assert (RouterVarDepState.get_full_name(), "foo") in State._var_dependencies[ - "router" - ] + for router_var in constants.ROUTER_VARS: + assert (RouterVarDepState.get_full_name(), "foo") in State._var_dependencies[ + router_var + ] # Get state from state manager. rx_state = await state_manager.get_state(BaseStateToken(ident=token, cls=State)) @@ -3744,11 +3764,86 @@ def foo(self) -> str: # Reassign router var state.router = state.router - assert rx_state.dirty_vars == {"router"} + assert rx_state.dirty_vars == set(constants.ROUTER_VARS) assert state.dirty_vars == {"foo"} assert parent_state.dirty_substates == {RouterVarDepState.get_name()} +def test_router_var_dep_legacy_string() -> None: + """An explicit deps=["router"] still fires when any router var changes. + + The `router` base var was split into per-field vars; a legacy string dep + on "router" is expanded to all of them (with a deprecation warning). + """ + + class LegacyRouterDepState(State): + """A state with a legacy string dependency on the router var.""" + + @rx.var(deps=["router"], auto_deps=False) + def foo(self) -> str: + return self.router.url.path + + for router_var in constants.ROUTER_VARS: + assert ( + LegacyRouterDepState.get_full_name(), + "foo", + ) in State._var_dependencies[router_var] + assert "router" not in State._var_dependencies + + +def test_update_router_vars_granular_delta(test_state: TestState) -> None: + """_update_router_vars only dirties the vars whose source keys changed. + + Args: + test_state: A state. + """ + full_router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.CLIENT_TOKEN: "tok", + RouteVar.SESSION_ID: "sid1", + RouteVar.CLIENT_IP: "127.0.0.1", + RouteVar.HEADERS: {"origin": "http://localhost:3000"}, + } + test_state._update_router_vars(full_router_data, {}) + assert set(constants.ROUTER_VARS) <= test_state.dirty_vars + test_state._clean() + + # Navigation: only the navigation-scoped vars are rebuilt. + nav_router_data = {**full_router_data, RouteVar.PATH: "/b", RouteVar.ORIGIN: "/b"} + test_state._update_router_vars(nav_router_data, full_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_page", + "router_url", + "router_route_id", + } + assert test_state.router.url.path == "/b" + assert test_state.router.session.session_id == "sid1" + test_state._clean() + + # Reconnect: only the session var is rebuilt. + reconnect_router_data = {**nav_router_data, RouteVar.SESSION_ID: "sid2"} + test_state._update_router_vars(reconnect_router_data, nav_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_session"} + assert test_state.router.session.session_id == "sid2" + test_state._clean() + + # Header change: headers and the URL (whose host derives from them) update. + new_headers_router_data = { + **reconnect_router_data, + RouteVar.HEADERS: {"origin": "http://example.com"}, + } + test_state._update_router_vars(new_headers_router_data, reconnect_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_headers", + "router_page", + "router_url", + "router_route_id", + } + assert test_state.router.url.origin == "http://example.com" + + @pytest.mark.asyncio async def test_setvar( state_manager: StateManager, diff --git a/tests/units/utils/test_format.py b/tests/units/utils/test_format.py index 83e718411fd..1b062d0f8f2 100644 --- a/tests/units/utils/test_format.py +++ b/tests/units/utils/test_format.py @@ -657,20 +657,24 @@ def test_format_query_params(input, output): assert format.format_query_params(input) == output -formatted_router = { - "route_id": "", - "url": { +formatted_router_vars = { + "router_route_id" + FIELD_MARKER: "", + "router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "://", + "origin": "", "path": "", "query": "", "query_parameters": {}, "fragment": "", "href": "", }, - "session": {"client_token": "", "client_ip": "", "session_id": ""}, - "headers": { + "router_session" + FIELD_MARKER: { + "client_token": "", + "client_ip": "", + "session_id": "", + }, + "router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -686,7 +690,7 @@ def test_format_query_params(input, output): "accept_language": "", "raw_headers": {}, }, - "page": { + "router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", @@ -720,7 +724,7 @@ def test_format_query_params(input, output): "obj" + FIELD_MARKER: {"prop1": 42, "prop2": "hello"}, "sum" + FIELD_MARKER: 3.15, "upper" + FIELD_MARKER: "", - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, "asynctest" + FIELD_MARKER: 0, }, ChildState.get_full_name(): { @@ -742,7 +746,7 @@ def test_format_query_params(input, output): "dt" + FIELD_MARKER: "1989-11-09 18:53:00+01:00", "t" + FIELD_MARKER: "18:53:00+01:00", "td" + FIELD_MARKER: "11 days, 0:11:00", - "router" + FIELD_MARKER: formatted_router, + **formatted_router_vars, }, }, ), From 752f403b228369e5d846b8bc3c7ee20da7023693 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:36:38 +0000 Subject: [PATCH 02/27] Add news fragments; stop router dep tests leaking class-level registrations test_router_var_dep and test_router_var_dep_legacy_string define state classes locally, which register themselves in State's class-level _var_dependencies / _potentially_dirty_states and outlive the test. A later test that dirties a router var on a fresh State tree then resolves the stale entry and raises on the missing substate -- which already made test_chained_event_keeps_originating_router_data fail whenever it ran after test_state.py. Drop the registrations at the end of each test. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- news/7068.deprecation.md | 1 + news/7068.performance.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 news/7068.deprecation.md create mode 100644 news/7068.performance.md diff --git a/news/7068.deprecation.md b/news/7068.deprecation.md new file mode 100644 index 00000000000..feee20812a1 --- /dev/null +++ b/news/7068.deprecation.md @@ -0,0 +1 @@ +Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the specific router var instead, e.g. `deps=["router_url"]`. diff --git a/news/7068.performance.md b/news/7068.performance.md new file mode 100644 index 00000000000..3c56ecabc0e --- /dev/null +++ b/news/7068.performance.md @@ -0,0 +1 @@ +Store router data in separate base vars (session, headers, page, url, route_id) so a navigation delta only re-sends the fields that changed instead of the whole router, and gather the connection-scoped router data (headers, client IP, session id) once at connect time rather than on every event. `State.router` is unchanged for app code. From a2af873ba4c859b3d8528445eb3a67d56212484a Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:38:50 +0000 Subject: [PATCH 03/27] Add reflex-base news fragment for the router var split The changelog check requires a news fragment under every package whose source the PR touches; this change also edits reflex-base's route constants and event processor. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- packages/reflex-base/news/7068.performance.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 packages/reflex-base/news/7068.performance.md diff --git a/packages/reflex-base/news/7068.performance.md b/packages/reflex-base/news/7068.performance.md new file mode 100644 index 00000000000..41445cf7826 --- /dev/null +++ b/packages/reflex-base/news/7068.performance.md @@ -0,0 +1 @@ +The event processor now refreshes only the router vars whose backing `router_data` keys actually changed, so a navigation no longer rebuilds and re-sends the connection-scoped session and header data. `ROUTER_VARS` names the per-field router vars that replaced the single `router` var on the root state. From bfb89fada8da74b2a31ae6714d7a84bb889c5f5f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 22:54:03 +0000 Subject: [PATCH 04/27] Rebuild the session var from router_data when linking a token to a sid dataclasses.replace() on the existing router_session value rejects anything that is not a dataclass instance, which broke the redis token manager tests: they drive on_connect with a mocked state whose router_session is a Mock. Rebuilding from router_data (as the pre-split code did, and as the event processor does) keeps the session var and router_data in step and works with the mocked state. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/app.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/reflex/app.py b/reflex/app.py index 14371fc18ec..31dc87e59ab 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -73,6 +73,7 @@ from reflex.app_mixins import AppMixin, LifespanMixin, MiddlewareMixin from reflex.compiler import compiler from reflex.compiler.compiler import readable_name_from_component +from reflex.istate.data import SessionData from reflex.istate.manager import StateManager, StateModificationContext from reflex.istate.manager.token import BaseStateToken from reflex.route import ( @@ -2313,5 +2314,10 @@ async def link_token_to_sid(self, sid: str, token: str): BaseStateToken(ident=new_token or token, cls=self.app._state) ) as state: state.router_data[constants.RouteVar.SESSION_ID] = sid - if (session := state.router_session).session_id != sid: - state.router_session = dataclasses.replace(session, session_id=sid) + # Rebuild from router_data (rather than replacing the field on + # the existing value) to keep the session var and router_data + # in step, the same way the event processor refreshes it. + if ( + session := SessionData.from_router_data(state.router_data) + ) != state.router_session: + state.router_session = session From 605b68108c9dcfd15f8d7735d1b6e894a124e152 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:02:19 +0000 Subject: [PATCH 05/27] Assign a router var only when its rebuilt value differs _update_router_vars gated on the router_data keys each var derives from, then assigned unconditionally. Different keys can still yield an equal value -- an absent key and an empty one both produce the default -- so a router_data update that changed nothing observable still dirtied the var, which marked the state touched and persisted it to redis. That showed up as an extra token in test_redis_token_manager_enumerate_tokens. The pre-split code compared the rebuilt RouterData before assigning; restore that, keeping the key check as the cheap gate that avoids rebuilding HeaderData on every navigation. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 30 +++++++++++++++++++++--------- tests/units/test_state.py | 14 ++++++++++++-- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index 4c7edfd72e2..521292392f7 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1903,9 +1903,12 @@ def _update_router_vars( ) -> None: """Update the per-field router vars from a new router_data dict. - Only rebuilds and reassigns the vars whose backing router_data keys - actually changed, so connection-scoped data (session, headers) is not - recomputed or re-sent in the delta on every navigation. + Each var is rebuilt only when the router_data keys it derives from + changed, so connection-scoped data (session, headers) is not recomputed + on every navigation, and is then assigned only when the rebuilt value + actually differs -- different keys can still yield an equal value (an + absent key and an empty one both produce the default), and assigning + regardless would dirty the var, mark the state touched, and persist it. Args: router_data: The new router_data dict. @@ -1920,13 +1923,19 @@ def _update_router_vars( constants.RouteVar.SESSION_ID, constants.RouteVar.CLIENT_IP, ) + ) and (session := SessionData.from_router_data(router_data)) != ( + self.router_session ): - self.router_session = SessionData.from_router_data(router_data) + self.router_session = session headers_changed = prev_get(constants.RouteVar.HEADERS) != get( constants.RouteVar.HEADERS ) - if headers_changed: - self.router_headers = HeaderData.from_router_data(router_data) + if ( + headers_changed + and (headers := HeaderData.from_router_data(router_data)) + != self.router_headers + ): + self.router_headers = headers if ( # The origin header feeds the URL/page host. headers_changed @@ -1934,9 +1943,12 @@ def _update_router_vars( or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) ): - self.router_page = PageData.from_router_data(router_data) - self.router_url = URLData.from_router_data(router_data) - self.router_route_id = get(constants.RouteVar.PATH, "") + if (page := PageData.from_router_data(router_data)) != self.router_page: + self.router_page = page + if (url := URLData.from_router_data(router_data)) != self.router_url: + self.router_url = url + if (route_id := get(constants.RouteVar.PATH, "")) != self.router_route_id: + self.router_route_id = route_id @classmethod @functools.lru_cache diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 8054e2f7ce4..f7adfb97af3 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3846,7 +3846,8 @@ def test_update_router_vars_granular_delta(test_state: TestState) -> None: assert test_state.router.session.session_id == "sid2" test_state._clean() - # Header change: headers and the URL (whose host derives from them) update. + # Header change: headers, and the page/URL whose host derives from them. + # route_id derives from the path alone, so it is left clean. new_headers_router_data = { **reconnect_router_data, RouteVar.HEADERS: {"origin": "http://example.com"}, @@ -3856,9 +3857,18 @@ def test_update_router_vars_granular_delta(test_state: TestState) -> None: "router_headers", "router_page", "router_url", - "router_route_id", } assert test_state.router.url.origin == "http://example.com" + test_state._clean() + + # Keys that differ but derive the same values leave every var clean: an + # absent key and an empty one both produce the default, and dirtying on + # that alone would mark the state touched and persist it. + equivalent_router_data = { + k: v for k, v in new_headers_router_data.items() if k != RouteVar.QUERY + } + test_state._update_router_vars(equivalent_router_data, new_headers_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == set() @pytest.mark.asyncio From d5c82f468a7c28363280b8b92c77c17ac09ebfaf Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:08:29 +0000 Subject: [PATCH 06/27] Keep the new on_event tests from leaking tokens into a shared redis EventNamespace's token manager is redis-backed when a redis URL is configured, so linking a token in these tests left it visible to test_redis_token_manager_enumerate_tokens, which asserts an exact token count. Tear the namespace down the way the token manager tests' own factory does. Also stop expecting router_route_id in every on_load delta of test_dynamic_route_var_route_change_completed_on_load: those navigations all match the same route, so the route pattern only changes on the first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- tests/units/test_app.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 071a03d93bd..49f84d649ac 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -1925,8 +1925,11 @@ def _dynamic_state_event(name, val, **kwargs): exp_router_delta = { "router_page" + FIELD_MARKER: exp_router._page, "router_url" + FIELD_MARKER: URLData.from_url(exp_router.url), - "router_route_id" + FIELD_MARKER: exp_router.route_id, } + if exp_index == 0: + # Every navigation here matches the same route, so the route_id + # only changes on the first one. + exp_router_delta["router_route_id" + FIELD_MARKER] = exp_router.route_id async with mock_base_state_event_processor as processor: await processor.enqueue( token, @@ -4319,15 +4322,20 @@ def test_client_error_constants_match_frontend(): @pytest.fixture -def event_namespace_with_processor_mock() -> EventNamespace: +def event_namespace_with_processor_mock() -> Generator[EventNamespace, None, None]: """An EventNamespace whose app has a mocked event processor. - Returns: + Yields: The EventNamespace instance. """ app = App() app._event_processor = Mock(enqueue=AsyncMock()) - return EventNamespace("/event", app) + event_namespace = EventNamespace("/event", app) + yield event_namespace + # The token manager is backed by redis when one is configured; drop the + # tokens these tests link so they do not show up in another test's + # enumeration of the shared instance. + asyncio.run(event_namespace._token_manager.disconnect_all()) def _connect_environ(token: str) -> dict[str, Any]: From c7a08e51e17f67f84543c22aaae0ddeae6b8176b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:19:32 +0000 Subject: [PATCH 07/27] Address review findings on the router var split Whole-router dependency (greptile P1, cubic P2): deps=[State.router] registered only router_url, because VarData.merge surfaces the first non-empty field name, so a cached var declaring the whole router went stale when any other router field changed. Vars now name the state fields a dependency must track via _dependency_field_names(); RouterDataVar names all five. Non-composite vars keep their existing behaviour. Cached headers aliasing (greptile P2, cubic P2): the cached headers dict was shared with every event and, through it, with the mutable state.router_data, so a handler mutating self.router_data["headers"] corrupted the connection cache. Copy it per event -- measured at 0.081us against the 1.347us decode it replaced, so the cache still pays for itself 17x over. My note on the PR claiming the copy was the cost being removed was simply wrong. Omitted static keys (cubic P2): a router_data carrying only the navigation keys says nothing about the session or headers, but _update_router_vars read the omission as a change and reset them to their defaults. A key absent from the new payload is now left alone. Non-origin headers (cubic P2): only the origin header feeds the page and URL, so a cookie change no longer rebuilds the navigation vars. Hardcoded identifiers (greptile P2): the router field names are now named constants, used at the lookup sites and in the dynamic route dependency. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../src/reflex_base/constants/__init__.py | 10 ++ .../src/reflex_base/constants/route.py | 16 +++- .../reflex-base/src/reflex_base/vars/base.py | 42 +++++--- reflex/app.py | 8 ++ reflex/istate/data.py | 26 ++++- reflex/state.py | 56 +++++++---- tests/units/test_app.py | 40 ++++++++ tests/units/test_state.py | 96 +++++++++++++++++++ 8 files changed, 256 insertions(+), 38 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/constants/__init__.py b/packages/reflex-base/src/reflex_base/constants/__init__.py index fdd7994e9b5..3f46a794461 100644 --- a/packages/reflex-base/src/reflex_base/constants/__init__.py +++ b/packages/reflex-base/src/reflex_base/constants/__init__.py @@ -60,6 +60,11 @@ ROUTER, ROUTER_DATA, ROUTER_DATA_INCLUDE, + ROUTER_HEADERS, + ROUTER_PAGE, + ROUTER_ROUTE_ID, + ROUTER_SESSION, + ROUTER_URL, ROUTER_VARS, DefaultPage, Page404, @@ -87,6 +92,11 @@ "ROUTER", "ROUTER_DATA", "ROUTER_DATA_INCLUDE", + "ROUTER_HEADERS", + "ROUTER_PAGE", + "ROUTER_ROUTE_ID", + "ROUTER_SESSION", + "ROUTER_URL", "ROUTER_VARS", "ROUTE_NOT_FOUND", "SESSION_STORAGE", diff --git a/packages/reflex-base/src/reflex_base/constants/route.py b/packages/reflex-base/src/reflex_base/constants/route.py index abf2fea266f..02281381e58 100644 --- a/packages/reflex-base/src/reflex_base/constants/route.py +++ b/packages/reflex-base/src/reflex_base/constants/route.py @@ -19,12 +19,18 @@ class RouteArgType(SimpleNamespace): # Session and headers are constant for the lifetime of a websocket connection, # while page, url, and route_id change on every navigation; keeping them in # separate vars means a navigation delta only re-sends the navigation fields. +ROUTER_SESSION = "router_session" +ROUTER_HEADERS = "router_headers" +ROUTER_PAGE = "router_page" +ROUTER_URL = "router_url" +ROUTER_ROUTE_ID = "router_route_id" + ROUTER_VARS = ( - "router_session", - "router_headers", - "router_page", - "router_url", - "router_route_id", + ROUTER_SESSION, + ROUTER_HEADERS, + ROUTER_PAGE, + ROUTER_URL, + ROUTER_ROUTE_ID, ) diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 3d541ada126..86b7f966acd 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -720,6 +720,21 @@ def _get_all_var_data(self) -> VarData | None: """ return self._var_data + def _dependency_field_names(self) -> tuple[str, ...]: + """The state field names a ComputedVar depending on this Var must track. + + A Var normally stands for a single state field, the one named by its + VarData. A Var composed of several state fields must name all of them, + or a ``deps=[that_var]`` dependency would only track the one field + VarData.merge happened to surface, leaving the computed var stale when + any of the others change. + + Returns: + The field names to register the dependency against. + """ + all_var_data = self._get_all_var_data() + return (all_var_data.field_name if all_var_data is not None else "",) + def __deepcopy__(self, memo: dict[int, Any]) -> Self: """Deepcopy the var. @@ -2411,10 +2426,11 @@ def _add_static_dep( else None ) if all_var_data is not None: - var_name = all_var_data.field_name + # A composite Var names every state field it is built from. + var_names = dep._dependency_field_names() else: - var_name = dep._js_expr - deps.setdefault(state_name, set()).add(var_name) + var_names = (dep._js_expr,) + deps.setdefault(state_name, set()).update(var_names) elif isinstance(dep, str) and dep != "": deps.setdefault(None, set()).add(dep) else: @@ -2692,18 +2708,20 @@ def add_dependency(self, objclass: type[BaseState], dep: Var): if all_var_data := dep._get_all_var_data(): state_name = all_var_data.state if state_name: - var_name = all_var_data.field_name - if var_name: - self._static_deps.setdefault(state_name, set()).add(var_name) + # A composite Var names every state field it is built from. + var_names = tuple(filter(None, dep._dependency_field_names())) + if var_names: + self._static_deps.setdefault(state_name, set()).update(var_names) target_state_class = objclass.get_root_state().get_class_substate( state_name ) - target_state_class._var_dependencies.setdefault( - var_name, set() - ).add(( - objclass.get_full_name(), - self._name, - )) + for var_name in var_names: + target_state_class._var_dependencies.setdefault( + var_name, set() + ).add(( + objclass.get_full_name(), + self._name, + )) target_state_class._potentially_dirty_states.add( objclass.get_full_name() ) diff --git a/reflex/app.py b/reflex/app.py index 31dc87e59ab..bbd91bd3501 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -2194,6 +2194,14 @@ async def on_event(self, sid: str, data: Any): ) router_data = event.router_data router_data.update(static_router_data) + # The cached headers reach the event, and from there `state.router_data`, + # which is a plain mutable dict: sharing the mapping would let a handler + # mutating `self.router_data["headers"]` corrupt the connection cache for + # every later event on this socket. The shallow copy is ~17x cheaper than + # the per-event header decode it replaced, so the cache still pays off. + router_data[constants.RouteVar.HEADERS] = static_router_data[ + constants.RouteVar.HEADERS + ].copy() router_data.update({ constants.RouteVar.QUERY: format.format_query_params(event.router_data), constants.RouteVar.CLIENT_TOKEN: token, diff --git a/reflex/istate/data.py b/reflex/istate/data.py index bf7f2b1132c..b70272a1aad 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -584,8 +584,6 @@ class RouterDataVar(CachedVarOperation, ObjectVar[RouterData]): an object literal matching the pre-split serialized router shape. """ - # _url_var first: VarData.merge picks the first non-empty field_name, so - # `deps=[State.router]` registers against the navigation-scoped var. _url_var: Var = dataclasses.field(default_factory=_null_var) _page_var: Var = dataclasses.field(default_factory=_null_var) _session_var: Var = dataclasses.field(default_factory=_null_var) @@ -610,6 +608,30 @@ def _cached_var_name(self) -> str: " })" ) + def _dependency_field_names(self) -> tuple[str, ...]: + """Name every per-field router var backing this switchboard. + + VarData.merge surfaces only the first non-empty field name, so without + this a ``deps=[State.router]`` dependency would track one router var + and leave the computed var stale when any of the others changed (a + reconnect updates the session without touching the URL, for example). + + Returns: + The field names of all five per-field router vars. + """ + return tuple( + field_name + for var in ( + self._session_var, + self._headers_var, + self._page_var, + self._url_var, + self._route_id_var, + ) + if (all_var_data := var._get_all_var_data()) is not None + and (field_name := all_var_data.field_name) + ) + @property def session(self) -> ObjectVar[SessionData]: """The per-connection session data. diff --git a/reflex/state.py b/reflex/state.py index 521292392f7..63a4221fd17 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -432,16 +432,16 @@ def _get_router_var(cls: type[BaseState]) -> RouterDataVar: router_var = root_cls.__dict__.get("_reflex_router_var") if router_var is None: base_vars = root_cls.base_vars - if "router_session" not in base_vars: + if constants.ROUTER_SESSION not in base_vars: # BaseState itself and mixins never initialize base vars; give # introspection-style access an unbound switchboard. return RouterDataVar(_js_expr="", _var_type=RouterData) router_var = RouterDataVar.create( - session=base_vars["router_session"], - headers=base_vars["router_headers"], - page=base_vars["router_page"], - url=base_vars["router_url"], - route_id=base_vars["router_route_id"], + session=base_vars[constants.ROUTER_SESSION], + headers=base_vars[constants.ROUTER_HEADERS], + page=base_vars[constants.ROUTER_PAGE], + url=base_vars[constants.ROUTER_URL], + route_id=base_vars[constants.ROUTER_ROUTE_ID], ) setattr(root_cls, "_reflex_router_var", router_var) # noqa: B010 return router_var @@ -1700,7 +1700,7 @@ def inner_func(self: BaseState) -> list[str]: dynamic_vars[param] = DynamicRouteVar( fget=func, auto_deps=False, - deps=["router_page"], + deps=[constants.ROUTER_PAGE], _var_data=VarData.from_state(cls, param), ) setattr(cls, param, dynamic_vars[param]) @@ -1910,14 +1910,32 @@ def _update_router_vars( absent key and an empty one both produce the default), and assigning regardless would dirty the var, mark the state touched, and persist it. + A key missing from ``router_data`` carries no information about the + value it feeds, so it is not treated as a change: a payload holding + only the navigation keys must not reset the connection-scoped vars to + their defaults. + Args: router_data: The new router_data dict. previous_router_data: The router_data dict this state last saw. """ - get = router_data.get - prev_get = previous_router_data.get + + def changed(key: str) -> bool: + return ( + key in router_data and previous_router_data.get(key) != router_data[key] + ) + + headers_changed = changed(constants.RouteVar.HEADERS) + # Only the origin header feeds the URL/page host, so the navigation + # vars must not be rebuilt for a change to any other header. Read both + # sides from router_data: the headers var may already be updated below. + origin_changed = headers_changed and ( + previous_router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") + != router_data[constants.RouteVar.HEADERS].get("origin", "") + ) + if any( - prev_get(key) != get(key) + changed(key) for key in ( constants.RouteVar.CLIENT_TOKEN, constants.RouteVar.SESSION_ID, @@ -1927,9 +1945,6 @@ def _update_router_vars( self.router_session ): self.router_session = session - headers_changed = prev_get(constants.RouteVar.HEADERS) != get( - constants.RouteVar.HEADERS - ) if ( headers_changed and (headers := HeaderData.from_router_data(router_data)) @@ -1937,17 +1952,20 @@ def _update_router_vars( ): self.router_headers = headers if ( - # The origin header feeds the URL/page host. - headers_changed - or prev_get(constants.RouteVar.PATH) != get(constants.RouteVar.PATH) - or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) - or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) + origin_changed + or changed(constants.RouteVar.PATH) + or changed(constants.RouteVar.ORIGIN) + or changed(constants.RouteVar.QUERY) ): if (page := PageData.from_router_data(router_data)) != self.router_page: self.router_page = page if (url := URLData.from_router_data(router_data)) != self.router_url: self.router_url = url - if (route_id := get(constants.RouteVar.PATH, "")) != self.router_route_id: + if ( + route_id := router_data.get( + constants.RouteVar.PATH, self.router_route_id + ) + ) != self.router_route_id: self.router_route_id = route_id @classmethod diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 49f84d649ac..2d1d50d2133 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -4403,6 +4403,46 @@ async def test_on_event_uses_connect_time_router_data( assert "sid1" not in event_namespace._static_router_data +@pytest.mark.asyncio +async def test_on_event_does_not_share_the_cached_headers( + token: str, + event_namespace_with_processor_mock: EventNamespace, +): + """Each event gets its own headers mapping, not the cached one. + + The headers reach `state.router_data`, a plain mutable dict, so sharing + the cached mapping would let a handler mutating it corrupt the connection + cache for every later event on the socket. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + """ + event_namespace = event_namespace_with_processor_mock + await event_namespace.on_connect("sid1", _connect_environ(token)) + cached_headers = event_namespace._static_router_data["sid1"][ + constants.RouteVar.HEADERS + ] + + await event_namespace.on_event("sid1", _client_event_payload()) + enqueue_mock = cast(AsyncMock, event_namespace.app.event_processor.enqueue) + _, event = enqueue_mock.call_args[0] + event_headers = event.router_data[constants.RouteVar.HEADERS] + + assert event_headers == cached_headers + assert event_headers is not cached_headers + # Mutating what the handler sees must not reach the connection cache. + event_headers["user-agent"] = "mutated" + assert cached_headers["user-agent"] == "test-agent" + + enqueue_mock.reset_mock() + await event_namespace.on_event("sid1", _client_event_payload()) + _, next_event = enqueue_mock.call_args[0] + assert ( + next_event.router_data[constants.RouteVar.HEADERS]["user-agent"] == "test-agent" + ) + + @pytest.mark.asyncio async def test_on_event_falls_back_to_environ_without_connect( token: str, diff --git a/tests/units/test_state.py b/tests/units/test_state.py index f7adfb97af3..c6ed9d4c822 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3808,6 +3808,102 @@ def foo(self) -> str: State._potentially_dirty_states.discard(LegacyRouterDepState.get_full_name()) +def test_router_var_dep_whole_router() -> None: + """deps=[State.router] must track every per-field router var. + + The switchboard's VarData surfaces only one field name, so without the + composite dependency hook a cached var declaring the whole router would go + stale when any other router field changed -- a reconnect updates the + session without touching the URL, for instance. + """ + + class WholeRouterDepState(State): + """A state depending on the whole router var.""" + + @rx.var(deps=[State.router], auto_deps=False) + def summary(self) -> str: + return "" + + assert WholeRouterDepState.computed_vars["summary"]._static_deps == { + State.get_full_name(): set(constants.ROUTER_VARS) + } + for router_var in constants.ROUTER_VARS: + assert ( + WholeRouterDepState.get_full_name(), + "summary", + ) in State._var_dependencies[router_var] + + # Drop the class-level registrations; see the note in test_router_var_dep. + for dep_set in State._var_dependencies.values(): + dep_set.discard((WholeRouterDepState.get_full_name(), "summary")) + State._potentially_dirty_states.discard(WholeRouterDepState.get_full_name()) + + +def test_update_router_vars_ignores_omitted_static_keys( + test_state: TestState, +) -> None: + """A navigation-only payload must not reset the connection-scoped vars. + + A router_data carrying only the navigation keys says nothing about the + session or headers; treating the omission as a change would wipe them to + their defaults and ship a destructive delta. + + Args: + test_state: A state. + """ + full_router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.CLIENT_TOKEN: "tok", + RouteVar.SESSION_ID: "sid1", + RouteVar.CLIENT_IP: "127.0.0.1", + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "a=b"}, + } + test_state._update_router_vars(full_router_data, {}) + test_state._clean() + + navigation_only = { + RouteVar.PATH: "/b", + RouteVar.ORIGIN: "/b", + RouteVar.QUERY: {}, + } + test_state._update_router_vars(navigation_only, full_router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { + "router_page", + "router_url", + "router_route_id", + } + assert test_state.router.session.client_token == "tok" + assert test_state.router.session.session_id == "sid1" + assert test_state.router.headers.cookie == "a=b" + + +def test_update_router_vars_non_origin_header_leaves_navigation_clean( + test_state: TestState, +) -> None: + """Only the origin header feeds the page/URL, so other headers leave them alone. + + Args: + test_state: A state. + """ + router_data = { + RouteVar.PATH: "/a", + RouteVar.ORIGIN: "/a", + RouteVar.QUERY: {}, + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "a=b"}, + } + test_state._update_router_vars(router_data, {}) + test_state._clean() + + new_cookie = { + **router_data, + RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "c=d"}, + } + test_state._update_router_vars(new_cookie, router_data) + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_headers"} + + def test_update_router_vars_granular_delta(test_state: TestState) -> None: """_update_router_vars only dirties the vars whose source keys changed. From c7e8c1804d6a80faa14a9e8587499e714288ee6c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:28:38 +0000 Subject: [PATCH 08/27] Record the connecting token in router_data when linking a sid Both reviewers flagged this as P1. Rebuilding SessionData from router_data left router_session.client_token empty until the first event filled it in, and duplicate-token handling makes that worse: the state is loaded under a freshly issued token, so anything reading client_token in the meantime -- a background task, a shared-state link -- addresses the wrong state tree. Record the identity the state was actually loaded under. Also make the EventNamespace fixture async so its token-manager teardown awaits on the test's own event loop, rather than driving a redis client bound to that loop from a fresh one via asyncio.run. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/app.py | 6 ++++ tests/units/test_app.py | 61 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/reflex/app.py b/reflex/app.py index bbd91bd3501..cb8421d6b78 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -2322,6 +2322,12 @@ async def link_token_to_sid(self, sid: str, token: str): BaseStateToken(ident=new_token or token, cls=self.app._state) ) as state: state.router_data[constants.RouteVar.SESSION_ID] = sid + # The state is loaded under this identity, so record it rather + # than waiting for the first event to fill it in: duplicate-token + # handling hands back a fresh token here, and until router_data + # carries it, anything reading router_session.client_token (a + # background task, a shared-state link) addresses the wrong tree. + state.router_data[constants.RouteVar.CLIENT_TOKEN] = new_token or token # Rebuild from router_data (rather than replacing the field on # the existing value) to keep the session var and router_data # in step, the same way the event processor refreshes it. diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 2d1d50d2133..0e92dadae19 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -10,7 +10,7 @@ import re import unittest.mock import uuid -from collections.abc import Generator +from collections.abc import AsyncGenerator, Generator from contextlib import nullcontext as does_not_raise from importlib.util import find_spec from pathlib import Path @@ -18,6 +18,7 @@ from unittest.mock import AsyncMock, Mock import pytest +import pytest_asyncio import reflex_base from pytest_mock import MockerFixture from reflex_base.components.component import Component @@ -4321,8 +4322,8 @@ def test_client_error_constants_match_frontend(): ) -@pytest.fixture -def event_namespace_with_processor_mock() -> Generator[EventNamespace, None, None]: +@pytest_asyncio.fixture +async def event_namespace_with_processor_mock() -> AsyncGenerator[EventNamespace, None]: """An EventNamespace whose app has a mocked event processor. Yields: @@ -4334,8 +4335,9 @@ def event_namespace_with_processor_mock() -> Generator[EventNamespace, None, Non yield event_namespace # The token manager is backed by redis when one is configured; drop the # tokens these tests link so they do not show up in another test's - # enumeration of the shared instance. - asyncio.run(event_namespace._token_manager.disconnect_all()) + # enumeration of the shared instance. Awaited rather than run in a fresh + # loop via asyncio.run: the redis client is bound to the test's loop. + await event_namespace._token_manager.disconnect_all() def _connect_environ(token: str) -> dict[str, Any]: @@ -4403,6 +4405,55 @@ async def test_on_event_uses_connect_time_router_data( assert "sid1" not in event_namespace._static_router_data +@pytest.mark.asyncio +async def test_link_token_to_sid_records_the_connecting_identity( + token: str, + event_namespace_with_processor_mock: EventNamespace, + mocker: MockerFixture, +): + """The session var carries the token the state was loaded under. + + Duplicate-token handling hands back a fresh token, and the state is loaded + under it. Leaving `router_session.client_token` empty until the first event + would let anything reading it in between -- a background task, a + shared-state link -- address the wrong state tree. + + Args: + token: A token. + event_namespace_with_processor_mock: The event namespace fixture. + mocker: pytest-mock fixture. + """ + event_namespace = event_namespace_with_processor_mock + state = Mock() + state.router_data = {} + mocker.patch.object( + event_namespace.app.state_manager, + "modify_state", + Mock(return_value=AsyncMock(__aenter__=AsyncMock(return_value=state))), + ) + + # No duplicate: the connecting token is recorded. + await event_namespace.link_token_to_sid("sid1", token) + assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == token + assert state.router_session.client_token == token + assert state.router_session.session_id == "sid1" + + # Duplicate: the *new* token is recorded, not the one the client sent. + # The duplicate branch emits the replacement token to the client, which + # needs a server the bare namespace does not have. + event_namespace.emit = AsyncMock() # pyright: ignore[reportAttributeAccessIssue] + new_token = "a-fresh-token" + mocker.patch.object( + event_namespace._token_manager, + "link_token_to_sid", + AsyncMock(return_value=new_token), + ) + await event_namespace.link_token_to_sid("sid2", token) + assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == new_token + assert state.router_session.client_token == new_token + assert state.router_session.session_id == "sid2" + + @pytest.mark.asyncio async def test_on_event_does_not_share_the_cached_headers( token: str, From 05372dd1c340ebba43da38195424751c36cfaa5f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:39:52 +0000 Subject: [PATCH 09/27] Carry omitted router_data keys forward when rebuilding the router vars The previous commit stopped an absent key from being read as a change, but the constructors still read the incoming dict directly, so a payload holding only some navigation keys rebuilt page and url without the origin header that gives them their host. Merge the new data over what the state last saw and build from that; the merged dict is also what the caller now stores, so a partial payload does not drop keys for the next comparison either. Merging additionally makes an absent key compare equal to what it replaced, so the explicit presence check is no longer needed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../event/processor/base_state_processor.py | 13 ++-- reflex/state.py | 76 ++++++++++--------- tests/units/test_state.py | 20 ++++- 3 files changed, 69 insertions(+), 40 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py index 33b77ff6737..31011505473 100644 --- a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py +++ b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py @@ -417,12 +417,15 @@ async def _execute_event( router_data and (previous_router_data := state.router_data) != router_data ): - # assignment will recurse into substates and force recalculation of - # dependent ComputedVar (dynamic route variables) - state.router_data = router_data # only the router vars whose backing keys changed are rebuilt - # and re-sent; session/headers stay put across navigations - state._update_router_vars(router_data, previous_router_data) + # and re-sent; session/headers stay put across navigations. + # Store what it merged rather than the payload: a partial one + # would otherwise drop the keys it omits for the next event. + # The assignment recurses into substates and forces + # recalculation of dependent ComputedVar (dynamic route vars). + state.router_data = state._update_router_vars( + router_data, previous_router_data + ) # Preprocess the event. if ( diff --git a/reflex/state.py b/reflex/state.py index 63a4221fd17..c0b292ba407 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1900,7 +1900,7 @@ def _update_router_vars( self, router_data: builtins.dict[str, Any], previous_router_data: builtins.dict[str, Any], - ) -> None: + ) -> builtins.dict[str, Any]: """Update the per-field router vars from a new router_data dict. Each var is rebuilt only when the router_data keys it derives from @@ -1911,62 +1911,70 @@ def _update_router_vars( regardless would dirty the var, mark the state touched, and persist it. A key missing from ``router_data`` carries no information about the - value it feeds, so it is not treated as a change: a payload holding - only the navigation keys must not reset the connection-scoped vars to - their defaults. + value it feeds, so the previous one is carried forward rather than + letting the constructors default it away: a payload holding only the + navigation keys must not empty the connection-scoped vars, nor rebuild + the page and URL without the origin header that gives them their host. Args: router_data: The new router_data dict. previous_router_data: The router_data dict this state last saw. - """ - def changed(key: str) -> bool: - return ( - key in router_data and previous_router_data.get(key) != router_data[key] - ) + Returns: + The router_data to store on the state: the new values over the + previous ones, so a partial payload does not drop keys for the + next comparison either. + """ + # Merging also makes an absent key compare equal to what it replaced, + # so it is not read as a change without a special case for it. + merged = ( + {**previous_router_data, **router_data} + if previous_router_data + else router_data + ) + get = merged.get + prev_get = previous_router_data.get - headers_changed = changed(constants.RouteVar.HEADERS) + headers_changed = prev_get(constants.RouteVar.HEADERS) != get( + constants.RouteVar.HEADERS + ) # Only the origin header feeds the URL/page host, so the navigation - # vars must not be rebuilt for a change to any other header. Read both - # sides from router_data: the headers var may already be updated below. + # vars must not be rebuilt for a change to any other header. origin_changed = headers_changed and ( - previous_router_data.get(constants.RouteVar.HEADERS, {}).get("origin", "") - != router_data[constants.RouteVar.HEADERS].get("origin", "") + prev_get(constants.RouteVar.HEADERS, {}).get("origin", "") + != get(constants.RouteVar.HEADERS, {}).get("origin", "") ) - if any( - changed(key) - for key in ( - constants.RouteVar.CLIENT_TOKEN, - constants.RouteVar.SESSION_ID, - constants.RouteVar.CLIENT_IP, + if ( + any( + prev_get(key) != get(key) + for key in ( + constants.RouteVar.CLIENT_TOKEN, + constants.RouteVar.SESSION_ID, + constants.RouteVar.CLIENT_IP, + ) ) - ) and (session := SessionData.from_router_data(router_data)) != ( - self.router_session + and (session := SessionData.from_router_data(merged)) != self.router_session ): self.router_session = session if ( headers_changed - and (headers := HeaderData.from_router_data(router_data)) - != self.router_headers + and (headers := HeaderData.from_router_data(merged)) != self.router_headers ): self.router_headers = headers if ( origin_changed - or changed(constants.RouteVar.PATH) - or changed(constants.RouteVar.ORIGIN) - or changed(constants.RouteVar.QUERY) + or prev_get(constants.RouteVar.PATH) != get(constants.RouteVar.PATH) + or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) + or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) ): - if (page := PageData.from_router_data(router_data)) != self.router_page: + if (page := PageData.from_router_data(merged)) != self.router_page: self.router_page = page - if (url := URLData.from_router_data(router_data)) != self.router_url: + if (url := URLData.from_router_data(merged)) != self.router_url: self.router_url = url - if ( - route_id := router_data.get( - constants.RouteVar.PATH, self.router_route_id - ) - ) != self.router_route_id: + if (route_id := get(constants.RouteVar.PATH, "")) != self.router_route_id: self.router_route_id = route_id + return merged @classmethod @functools.lru_cache diff --git a/tests/units/test_state.py b/tests/units/test_state.py index c6ed9d4c822..4ba068664af 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3868,7 +3868,7 @@ def test_update_router_vars_ignores_omitted_static_keys( RouteVar.ORIGIN: "/b", RouteVar.QUERY: {}, } - test_state._update_router_vars(navigation_only, full_router_data) + merged = test_state._update_router_vars(navigation_only, full_router_data) assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { "router_page", "router_url", @@ -3877,6 +3877,24 @@ def test_update_router_vars_ignores_omitted_static_keys( assert test_state.router.session.client_token == "tok" assert test_state.router.session.session_id == "sid1" assert test_state.router.headers.cookie == "a=b" + # The rebuilt navigation vars keep the host from the headers the payload + # omitted, rather than being reconstructed from the partial dict alone. + assert test_state.router.url.origin == "http://localhost:3000" + assert test_state.router.url.path == "/b" + assert test_state.router.page.host == "http://localhost:3000" + # The merged data is what the caller stores, so the omitted keys are still + # there to compare against next time. + assert merged[RouteVar.CLIENT_TOKEN] == "tok" + assert merged[RouteVar.HEADERS] == full_router_data[RouteVar.HEADERS] + + # A second consecutive partial payload still has the full picture. + test_state._clean() + merged2 = test_state._update_router_vars( + {RouteVar.PATH: "/c", RouteVar.ORIGIN: "/c", RouteVar.QUERY: {}}, merged + ) + assert test_state.router.url.origin == "http://localhost:3000" + assert test_state.router.session.client_token == "tok" + assert merged2[RouteVar.HEADERS] == full_router_data[RouteVar.HEADERS] def test_update_router_vars_non_origin_header_leaves_navigation_clean( From 14d7662db9acb34ca14c90d22a00436c9236ad48 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:47:12 +0000 Subject: [PATCH 10/27] Give the serialized router keys a single source of truth serialize_router_data and RouterDataVar independently spelled out the five keys of the serialized router shape, and the two have to agree: the literal a whole-router render produces is the object a component reads, and it must match what the delta carries. Name them once and use them in both places, with a test pinning the rendered keys to the serializer's and to what actually reaches the client. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/istate/data.py | 44 ++++++++++++++++++++++++--------- tests/units/istate/test_data.py | 20 +++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/reflex/istate/data.py b/reflex/istate/data.py index b70272a1aad..3eb9737a483 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -3,7 +3,7 @@ import dataclasses from collections.abc import Mapping from types import MappingProxyType -from typing import TYPE_CHECKING, Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar, Final from urllib.parse import _NetlocResultMixinStr, parse_qsl, urlsplit from reflex_base import constants @@ -536,6 +536,16 @@ def from_router_data(cls, router_data: dict) -> "RouterData": ) +# Keys of the serialized RouterData: the object shape the frontend receives. +# `serialize_router_data` emits it, and `RouterDataVar` composes the same shape +# when the whole router is rendered, so the two must not drift apart. +SESSION_KEY: Final = "session" +HEADERS_KEY: Final = "headers" +PAGE_KEY: Final = "page" +URL_KEY: Final = "url" +ROUTE_ID_KEY: Final = "route_id" + + @serializer(to=dict) def serialize_router_data(obj: RouterData) -> dict: """Serialize a RouterData object to a dict. @@ -547,15 +557,15 @@ def serialize_router_data(obj: RouterData) -> dict: A dict representation of the RouterData object. """ return { - "session": obj.session, - "headers": obj.headers, - "page": obj._page, + SESSION_KEY: obj.session, + HEADERS_KEY: obj.headers, + PAGE_KEY: obj._page, # ReflexURL is a str subclass, so json.dumps handles it natively and # never invokes the `default=serialize` hook. Call the URL serializer # eagerly here so the frontend receives the parsed component dict # instead of just the raw URL string. - "url": _serialize_reflex_url(obj.url), - "route_id": obj.route_id, + URL_KEY: _serialize_reflex_url(obj.url), + ROUTE_ID_KEY: obj.route_id, } @@ -600,14 +610,24 @@ def _cached_var_name(self) -> str: """ return ( "({ " - f'"session": {self._session_var!s}, ' - f'"headers": {self._headers_var!s}, ' - f'"page": {self._page_var!s}, ' - f'"url": {self._url_var!s}, ' - f'"route_id": {self._route_id_var!s}' - " })" + + ", ".join(f'"{key}": {var!s}' for key, var in self._wire_fields().items()) + + " })" ) + def _wire_fields(self) -> dict[str, Var]: + """Map each serialized RouterData key to the var backing it. + + Returns: + The keys of the serialized router shape, in order, to their vars. + """ + return { + SESSION_KEY: self._session_var, + HEADERS_KEY: self._headers_var, + PAGE_KEY: self._page_var, + URL_KEY: self._url_var, + ROUTE_ID_KEY: self._route_id_var, + } + def _dependency_field_names(self) -> tuple[str, ...]: """Name every per-field router var backing this switchboard. diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index 62f4a8c5b32..81c58f111cb 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -205,6 +205,26 @@ def test_router_var_renders_composed_object(): ) +def test_router_var_shape_matches_the_serializer(): + """The composed router literal and the serializer must emit the same keys. + + Rendering `State.router` as a whole has to produce the object shape the + backend serializes a `RouterData` into, or a component reading the whole + router would see different keys from the ones the delta carries. The two + are built in different places, so pin them to each other. + """ + import json + + from reflex_base.utils.format import json_dumps + + from reflex.istate.data import RouterData, serialize_router_data + + rendered_keys = list(rx.State.router._wire_fields()) + assert rendered_keys == list(serialize_router_data(RouterData())) + # And that is what actually reaches the client for a whole-router value. + assert rendered_keys == list(json.loads(json_dumps(RouterData()))) + + def test_router_var_carries_state_var_data(): """The switchboard var must merge the per-field vars' VarData so hooks and context wiring for the root state are set up when it renders. From 9c270ca6c08d3a730623034dfa98df160ef7c835 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 23:51:00 +0000 Subject: [PATCH 11/27] Do not store merged router_data when it changes nothing A partial payload is never equal to the full dict the state holds, so it always reaches the merge -- and when it merges to what is already there, assigning it still dirtied router_data, marked the state touched, and persisted it for an event that moved nothing. Same class of bug as the spurious var dirtying fixed earlier, reached through the router_data assignment instead. Assign only when the merge actually differs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../event/processor/base_state_processor.py | 12 ++- .../processor/test_base_state_processor.py | 73 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py index 31011505473..2c8b2f962e7 100644 --- a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py +++ b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py @@ -419,13 +419,19 @@ async def _execute_event( ): # only the router vars whose backing keys changed are rebuilt # and re-sent; session/headers stay put across navigations. + merged_router_data = state._update_router_vars( + router_data, previous_router_data + ) # Store what it merged rather than the payload: a partial one # would otherwise drop the keys it omits for the next event. + # Only when that actually differs, though -- a payload that + # merges to what is already there changed nothing, and the + # assignment would still dirty router_data and mark the state + # touched, persisting it for an event that moved nothing. # The assignment recurses into substates and forces # recalculation of dependent ComputedVar (dynamic route vars). - state.router_data = state._update_router_vars( - router_data, previous_router_data - ) + if merged_router_data != previous_router_data: + state.router_data = merged_router_data # Preprocess the event. if ( diff --git a/tests/units/reflex_base/event/processor/test_base_state_processor.py b/tests/units/reflex_base/event/processor/test_base_state_processor.py index 3ddb031e955..29e215d1948 100644 --- a/tests/units/reflex_base/event/processor/test_base_state_processor.py +++ b/tests/units/reflex_base/event/processor/test_base_state_processor.py @@ -747,6 +747,79 @@ def raise_on_modify(*args, **kwargs): assert proxy._self_entered_context is False +async def test_no_op_partial_router_data_leaves_the_state_untouched( + wired_app: App, + real_base_state_processor: BaseStateEventProcessor, + emitted_deltas: list, + token: str, +): + """A payload that merges to what is already there must not touch the state. + + A partial router_data (only the navigation keys, as `fix_events` produces) + is never equal to the full dict the state holds, so it reaches the merge. + If it merges to the same thing, nothing moved: assigning it anyway would + dirty router_data, mark the state touched, and persist it for an event + that changed nothing. + + Args: + wired_app: The App wired to the processor's state manager. + real_base_state_processor: The unmocked BaseStateEventProcessor. + emitted_deltas: List of deltas captured from the processor. + token: The client token. + """ + + class NoOpRouterState(State): + n: int = 0 + + @event + def bump(self): + self.n += 1 + + full_view = { + "pathname": "/a", + "asPath": "/a", + "query": {}, + "token": token, + "sid": "sid1", + "ip": "127.0.0.1", + "headers": {"origin": "http://localhost:3000"}, + } + # Same navigation, but carrying only the keys a chained event keeps. + navigation_only = {"pathname": "/a", "asPath": "/a", "query": {}} + + def client_event(router_data: dict[str, Any]) -> Event: + return dataclasses.replace( + Event.from_event_type(NoOpRouterState.bump())[0], router_data=router_data + ) + + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(full_view)) + await processor.join(10) + + root_ctx = real_base_state_processor._root_context + assert root_ctx is not None + state = await root_ctx.state_manager.get_state( + BaseStateToken(ident=token, cls=State) + ) + state._was_touched = False + emitted_deltas.clear() + + async with real_base_state_processor as processor: + await processor.enqueue(token, client_event(navigation_only)) + await processor.join(10) + + # The connection-scoped data survived the partial payload... + assert state.router_data["headers"] == full_view["headers"] + assert state.router_session.client_token == token + # ...and nothing about the router was re-sent or marked dirty. + assert not any( + key.startswith("router") + for _token, delta in emitted_deltas + for key in delta.get(State.get_full_name(), {}) + ) + assert not state._get_was_touched() + + async def test_navigation_delta_elides_connection_scoped_router_vars( wired_app: App, real_base_state_processor: BaseStateEventProcessor, From 4dd07c9f7c5f5aee6fe724e56112e49cdb7b827c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 22:21:43 +0000 Subject: [PATCH 12/27] List `router` as a var so substates inherit the switchboard `_RouterDescriptor` subclasses `property`, which `_is_user_descriptor` excludes, so after the split `router` disappeared from `State.vars` (and from every substate's `inherited_vars`) even though it is still usable as a Var everywhere one is accepted. Add it explicitly. Nothing that serializes vars is affected: `get_delta` and `dict` iterate `base_vars`/`computed_vars`, so a switchboard with no backing field can never reach the wire. `test_dict` asserted the wire keys by deriving them from `vars`; it now derives them from the two dicts that actually back fields, which says the same thing without assuming every var has one. Since `inherited_vars` aliases the parent's `vars`, substates now resolve `self.router` through one parent delegation in `__getattribute__` instead of five (one per field), measured at 22.5us -> 16.2us at depth 3. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 5 +++++ tests/units/test_state.py | 37 +++++++++++++++++++++++++++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index c0b292ba407..35a43a70c0c 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -877,6 +877,11 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): **cls.inherited_vars, **cls.base_vars, **cls.computed_vars, + # `router` is a switchboard over the per-field router vars rather + # than a field of its own, but it is usable as a Var everywhere one + # is accepted, so it is listed here (and thus inherited by + # substates). It has no backing field, so it never reaches a delta. + constants.ROUTER: _get_router_var(cls), } cls.event_handlers = {} diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 4ba068664af..ff06d4023da 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -43,7 +43,13 @@ import reflex as rx from reflex.app import App from reflex.environment import environment -from reflex.istate.data import HeaderData, RouterData, URLData, _FrozenDictStrStr +from reflex.istate.data import ( + HeaderData, + RouterData, + RouterDataVar, + URLData, + _FrozenDictStrStr, +) from reflex.istate.manager import StateManager from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory @@ -384,6 +390,7 @@ def test_class_vars(test_state): """ cls = type(test_state) assert cls.vars.keys() == { + constants.ROUTER, *constants.ROUTER_VARS, "num1", "num2", @@ -465,8 +472,10 @@ def test_dict(test_state: TestState): } test_state_dict = test_state.dict() assert set(test_state_dict) == substates + # Only vars with a backing field are serialized; `router` is a switchboard + # over the per-field router vars and has no field of its own. assert set(test_state_dict[test_state.get_name()]) == { - var + FIELD_MARKER for var in test_state.vars + var + FIELD_MARKER for var in (*test_state.base_vars, *test_state.computed_vars) } assert set(test_state.dict(include_computed=False)[test_state.get_name()]) == { var + FIELD_MARKER for var in test_state.base_vars @@ -3839,6 +3848,30 @@ def summary(self) -> str: State._potentially_dirty_states.discard(WholeRouterDepState.get_full_name()) +def test_router_is_listed_as_a_var_and_inherited_by_substates() -> None: + """`router` is usable as a Var, so it is listed in vars and inherited. + + It has no backing field of its own, so it must stay out of anything that + serializes vars: the switchboard resolves to the root state's per-field + base vars instead. + """ + + class RouterVarListingState(State): + """A substate that only inherits the router.""" + + assert constants.ROUTER in State.vars + assert constants.ROUTER in RouterVarListingState.inherited_vars + assert constants.ROUTER not in State.base_vars + assert constants.ROUTER not in State.computed_vars + + # The substate's entry is the root's switchboard, resolving to the root's + # per-field base vars rather than to anything on the substate. + router_var = RouterVarListingState.vars[constants.ROUTER] + assert isinstance(router_var, RouterDataVar) + assert router_var.equals(State.router) + assert str(router_var.route_id) == str(State.router_route_id) + + def test_update_router_vars_ignores_omitted_static_keys( test_state: TestState, ) -> None: From 755126026a43d396329ca83f9d4ef1b7a1bb155e Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 22:34:10 +0000 Subject: [PATCH 13/27] Correct the stale get_skip_vars comment for `router` `router` is listed in `vars` now, so "Not a var" no longer describes why it is skipped. Say what the entry actually guards: substates are already covered by `set(cls.inherited_vars)`, and this catches a root state class whose inherited_vars has no `router` to skip. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/reflex/state.py b/reflex/state.py index 35a43a70c0c..fa5256b319c 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1292,7 +1292,10 @@ def get_skip_vars(cls) -> set[str]: "dirty_vars", "dirty_substates", "router_data", - # Not a var: assignment must reach the _RouterDescriptor. + # Listed in `vars` but backed by no field of its own, so a + # `router` annotation must never become a base var that would + # half-shadow the descriptor. Substates are already covered by + # `inherited_vars` above; this catches a root state class. constants.ROUTER, } | types.RESERVED_BACKEND_VAR_NAMES From 5de53287b9f88edde478eba6ab6f535b39de4864 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 02:13:19 +0000 Subject: [PATCH 14/27] Track every field a composite var reads in VarData.field_names `VarData.merge` kept only the first non-empty `field_name`, so a var built from several state fields could only ever report one of them. Any `deps=[composite_var]` therefore tracked a single field and went stale when the others changed. `RouterDataVar` worked around this with a bespoke `_dependency_field_names` override that reached into its five sub-vars. Fix it where the information is lost instead: `VarData` now stores `field_names` and merge collects them in order, deduped. `field_name` becomes a property returning the first, so every existing reader is unaffected. Names carrying a different `state` are dropped, because callers pair these names with the single merged `state` and registering a field against a state that does not have it fails dependency validation. The `RouterDataVar` override is gone; the generic path now produces the same five names for `deps=[State.router]`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- packages/reflex-base/news/7068.feature.md | 1 + .../reflex-base/src/reflex_base/vars/base.py | 77 +++++++++++++----- reflex/istate/data.py | 44 +++------- tests/units/reflex_base/vars/test_base.py | 47 ++++++++++- tests/units/test_state.py | 81 ++++++++++++------- 5 files changed, 167 insertions(+), 83 deletions(-) create mode 100644 packages/reflex-base/news/7068.feature.md diff --git a/packages/reflex-base/news/7068.feature.md b/packages/reflex-base/news/7068.feature.md new file mode 100644 index 00000000000..23460935a50 --- /dev/null +++ b/packages/reflex-base/news/7068.feature.md @@ -0,0 +1 @@ +`VarData` now tracks every state field a var is built from in `field_names`, collected and deduped as vars merge. `field_name` still reports the first of them, so existing readers are unaffected. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes, not just the one the merge happened to surface first. diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 86b7f966acd..c00e78a9332 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -255,8 +255,11 @@ class VarData: # The name of the enclosing state. state: str = dataclasses.field(default="") - # The name of the field in the state. - field_name: str = dataclasses.field(default="") + # The names of the state fields this var is built from. A var normally + # stands for a single field, but one composed of several (see + # `Var._dependency_field_names`) names all of them so a dependency on it + # tracks every field it reads. + field_names: tuple[str, ...] = dataclasses.field(default_factory=tuple) # Imports needed to render this var imports: ParsedImportTuple = dataclasses.field(default_factory=tuple) @@ -283,6 +286,7 @@ def __init__( self, state: str = "", field_name: str = "", + field_names: Sequence[str] | None = None, imports: ImmutableImportDict | ImmutableParsedImportDict | None = None, hooks: Mapping[str, VarData | None] | Sequence[str] | str | None = None, deps: list[Var] | None = None, @@ -294,7 +298,9 @@ def __init__( Args: state: The name of the enclosing state. - field_name: The name of the field in the state. + field_name: The name of the field in the state. Shorthand for a + single-entry ``field_names``; ignored when that is given. + field_names: The names of every state field this var is built from. imports: Imports needed to render this var. hooks: Hooks that need to be present in the component to render this var. deps: Dependencies of the var for useCallback. @@ -310,7 +316,13 @@ def __init__( (k, tuple(v)) for k, v in parse_imports(imports or {}).items() ) object.__setattr__(self, "state", state) - object.__setattr__(self, "field_name", field_name) + object.__setattr__( + self, + "field_names", + tuple(dict.fromkeys(field_names)) + if field_names is not None + else ((field_name,) if field_name else ()), + ) object.__setattr__(self, "imports", immutable_imports) object.__setattr__(self, "hooks", tuple(hooks or {})) object.__setattr__(self, "deps", tuple(deps or [])) @@ -323,7 +335,7 @@ def __init__( merged_var_data = VarData.merge(*hooks.values(), self) if merged_var_data is not None: object.__setattr__(self, "state", merged_var_data.state) - object.__setattr__(self, "field_name", merged_var_data.field_name) + object.__setattr__(self, "field_names", merged_var_data.field_names) object.__setattr__(self, "imports", merged_var_data.imports) object.__setattr__(self, "hooks", merged_var_data.hooks) object.__setattr__(self, "deps", merged_var_data.deps) @@ -331,6 +343,18 @@ def __init__( object.__setattr__(self, "components", merged_var_data.components) object.__setattr__(self, "app_wraps", merged_var_data.app_wraps) + @property + def field_name(self) -> str: + """The name of the field in the state. + + A var built from several fields reports the first; use ``field_names`` + to see all of them. + + Returns: + The first field name, or an empty string if there is none. + """ + return self.field_names[0] if self.field_names else "" + def old_school_imports(self) -> ImportDict: """Return the imports as a mutable dict. @@ -361,17 +385,26 @@ def merge(*all: VarData | None) -> VarData | None: if len(all_var_datas) == 1: return all_var_datas[0] - # Get the first non-empty field name or default to empty string. - field_name = next( - (var_data.field_name for var_data in all_var_datas if var_data.field_name), - "", - ) - # Get the first non-empty state or default to empty string. state = next( (var_data.state for var_data in all_var_datas if var_data.state), "" ) + # Collect every field name belonging to that state, in order, deduped, + # so a var composed of several fields (and therefore a dependency on + # it) knows about all of them. Field names carrying a different state + # are dropped: callers pair these names with the single `state` above, + # so keeping them would register a dependency on a field that state + # does not have. + field_names = tuple( + dict.fromkeys( + field_name + for var_data in all_var_datas + if not var_data.state or var_data.state == state + for field_name in var_data.field_names + ) + ) + hooks: dict[str, VarData | None] = { hook: None for var_data in all_var_datas for hook in var_data.hooks } @@ -407,7 +440,7 @@ def merge(*all: VarData | None) -> VarData | None: return VarData( state=state, - field_name=field_name, + field_names=field_names, imports=imports_, hooks=hooks, deps=deps, @@ -428,7 +461,7 @@ def __bool__(self) -> bool: self.state or self.imports or self.hooks - or self.field_name + or self.field_names or self.deps or self.position or self.components @@ -451,7 +484,7 @@ def _identity_key(self) -> tuple: """ return ( self.state, - self.field_name, + self.field_names, self.imports, self.hooks, self.deps, @@ -723,17 +756,21 @@ def _get_all_var_data(self) -> VarData | None: def _dependency_field_names(self) -> tuple[str, ...]: """The state field names a ComputedVar depending on this Var must track. - A Var normally stands for a single state field, the one named by its - VarData. A Var composed of several state fields must name all of them, - or a ``deps=[that_var]`` dependency would only track the one field - VarData.merge happened to surface, leaving the computed var stale when - any of the others change. + A Var normally stands for a single state field, but one composed of + several must name all of them, or a ``deps=[that_var]`` dependency + would track only some of the fields it reads and leave the computed var + stale when any of the others change. ``VarData.merge`` collects the + names as vars combine, so the merged VarData already knows all of them. + + Only fields of the VarData's own state are named; callers pair these + names with that single state, so a var spanning two states still + reports just the first one's fields. Returns: The field names to register the dependency against. """ all_var_data = self._get_all_var_data() - return (all_var_data.field_name if all_var_data is not None else "",) + return all_var_data.field_names if all_var_data is not None else () def __deepcopy__(self, memo: dict[int, Any]) -> Self: """Deepcopy the var. diff --git a/reflex/istate/data.py b/reflex/istate/data.py index 3eb9737a483..75a43fab77f 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -628,36 +628,12 @@ def _wire_fields(self) -> dict[str, Var]: ROUTE_ID_KEY: self._route_id_var, } - def _dependency_field_names(self) -> tuple[str, ...]: - """Name every per-field router var backing this switchboard. - - VarData.merge surfaces only the first non-empty field name, so without - this a ``deps=[State.router]`` dependency would track one router var - and leave the computed var stale when any of the others changed (a - reconnect updates the session without touching the URL, for example). - - Returns: - The field names of all five per-field router vars. - """ - return tuple( - field_name - for var in ( - self._session_var, - self._headers_var, - self._page_var, - self._url_var, - self._route_id_var, - ) - if (all_var_data := var._get_all_var_data()) is not None - and (field_name := all_var_data.field_name) - ) - @property def session(self) -> ObjectVar[SessionData]: """The per-connection session data. Returns: - ObjectVar for the ``router_session`` base var. + ObjectVar for the ``rx_router_session`` base var. """ return self._session_var.to(ObjectVar, SessionData) @@ -666,7 +642,7 @@ def headers(self) -> ObjectVar[HeaderData]: """The headers of the websocket connection request. Returns: - ObjectVar for the ``router_headers`` base var. + ObjectVar for the ``rx_router_headers`` base var. """ return self._headers_var.to(ObjectVar, HeaderData) @@ -675,7 +651,7 @@ def page(self) -> ObjectVar[PageData]: """The page data for the current page (deprecated, use ``url``). Returns: - ObjectVar for the ``router_page`` base var. + ObjectVar for the ``rx_router_page`` base var. """ return self._page_var.to(ObjectVar, PageData) @@ -687,7 +663,7 @@ def url(self) -> ReflexURLCastedVar: """The parsed URL of the current page. Returns: - ReflexURLCastedVar over the ``router_url`` base var. + ReflexURLCastedVar over the ``rx_router_url`` base var. """ return ReflexURLCastedVar.create(self._url_var) @@ -696,7 +672,7 @@ def route_id(self) -> StringVar: """The route pattern that matched the current page. Returns: - StringVar for the ``router_route_id`` base var. + StringVar for the ``rx_router_route_id`` base var. """ return self._route_id_var.to(str) @@ -714,11 +690,11 @@ def create( """Create a RouterDataVar over the per-field router base vars. Args: - session: The ``router_session`` base var. - headers: The ``router_headers`` base var. - page: The ``router_page`` base var. - url: The ``router_url`` base var. - route_id: The ``router_route_id`` base var. + session: The ``rx_router_session`` base var. + headers: The ``rx_router_headers`` base var. + page: The ``rx_router_page`` base var. + url: The ``rx_router_url`` base var. + route_id: The ``rx_router_route_id`` base var. _var_data: Additional VarData to merge in. Returns: diff --git a/tests/units/reflex_base/vars/test_base.py b/tests/units/reflex_base/vars/test_base.py index df1c6046bcd..af13a6241c6 100644 --- a/tests/units/reflex_base/vars/test_base.py +++ b/tests/units/reflex_base/vars/test_base.py @@ -6,7 +6,13 @@ import pytest from reflex_base.utils.types import get_field_type -from reflex_base.vars.base import EvenMoreBasicBaseState, Var, _linearize_bases, field +from reflex_base.vars.base import ( + EvenMoreBasicBaseState, + Var, + VarData, + _linearize_bases, + field, +) from reflex_base.vars.object import ObjectVar from reflex_base.vars.sequence import ArrayVar, StringVar from typing_extensions import TypeAliasType, TypeVarTuple, Unpack @@ -246,3 +252,42 @@ def __hash__(cls) -> int: _linearize_bases((b, c)), created.__mro__[1:], strict=True ) ) + + +def test_var_data_merge_collects_field_names(): + """Merging vars of one state keeps every field name, deduped and in order.""" + merged = VarData.merge( + VarData(state="s", field_name="a"), + VarData(state="s", field_name="b"), + VarData(state="s", field_name="a"), + ) + + assert merged is not None + assert merged.field_names == ("a", "b") + # `field_name` stays the first, so existing single-field readers are intact. + assert merged.field_name == "a" + + +def test_var_data_merge_drops_field_names_of_other_states(): + """Field names are paired with a single state, so foreign ones are dropped. + + Callers register these names against `VarData.state`; keeping a name from + a different state would declare a dependency on a field that state has no + knowledge of. + """ + merged = VarData.merge( + VarData(state="s", field_name="a"), + VarData(state="other", field_name="b"), + ) + + assert merged is not None + assert merged.state == "s" + assert merged.field_names == ("a",) + + +def test_var_data_field_name_shorthand_round_trips(): + """`field_name` is shorthand for a single-entry `field_names`.""" + assert VarData(field_name="a").field_names == ("a",) + assert VarData(field_names=["a", "b"]).field_name == "a" + assert VarData().field_names == () + assert VarData().field_name == "" diff --git a/tests/units/test_state.py b/tests/units/test_state.py index ff06d4023da..82c165371bf 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -79,8 +79,8 @@ formatted_router_vars = { - "router_route_id" + FIELD_MARKER: "", - "router_url" + FIELD_MARKER: { + "rx_router_route_id" + FIELD_MARKER: "", + "rx_router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", "origin": "", @@ -90,12 +90,12 @@ "fragment": "", "href": "", }, - "router_session" + FIELD_MARKER: { + "rx_router_session" + FIELD_MARKER: { "client_token": "", "client_ip": "", "session_id": "", }, - "router_headers" + FIELD_MARKER: { + "rx_router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -111,7 +111,7 @@ "accept_language": "", "raw_headers": {}, }, - "router_page" + FIELD_MARKER: { + "rx_router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", @@ -2413,11 +2413,11 @@ async def test_state_proxy( token, { TestState.get_full_name(): { - "router_session" + FIELD_MARKER: router_data.session, - "router_headers" + FIELD_MARKER: router_data.headers, - "router_page" + FIELD_MARKER: router_data._page, - "router_url" + FIELD_MARKER: URLData.from_url(router_data.url), - "router_route_id" + FIELD_MARKER: router_data.route_id, + "rx_router_session" + FIELD_MARKER: router_data.session, + "rx_router_headers" + FIELD_MARKER: router_data.headers, + "rx_router_page" + FIELD_MARKER: router_data._page, + "rx_router_url" + FIELD_MARKER: URLData.from_url(router_data.url), + "rx_router_route_id" + FIELD_MARKER: router_data.route_id, }, grandchild_state.get_full_name(): { "value2" + FIELD_MARKER: "42", @@ -3415,7 +3415,7 @@ def index(): first_token, first_delta = emitted_deltas[0] assert first_token == token first_state_delta = first_delta[State.get_full_name()] - assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + assert first_state_delta.pop("rx_router_url" + FIELD_MARKER) is not None for router_var in constants.ROUTER_VARS: first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) @@ -3478,7 +3478,7 @@ def index(): assert len(emitted_deltas) >= 2 first_delta = emitted_deltas[0][1] first_state_delta = first_delta[State.get_full_name()] - assert first_state_delta.pop("router_url" + FIELD_MARKER) is not None + assert first_state_delta.pop("rx_router_url" + FIELD_MARKER) is not None for router_var in constants.ROUTER_VARS: first_state_delta.pop(router_var + FIELD_MARKER, None) assert first_delta == exp_is_hydrated(State, False) @@ -3817,13 +3817,38 @@ def foo(self) -> str: State._potentially_dirty_states.discard(LegacyRouterDepState.get_full_name()) +def test_router_var_dep_legacy_string_still_compiles() -> None: + """An app declaring deps=["router"] must still pass dependency validation. + + `_validate_var_dependencies` checks the raw `_deps()` names against + `state_cls.vars` rather than the expanded registrations, so the deprecated + string only keeps working while `router` is itself listed as a var. + """ + + class LegacyRouterCompileState(State): + """A state with a legacy string dependency on the router var.""" + + @rx.var(deps=["router"], auto_deps=False) + def foo(self) -> str: + return self.router.url.path + + assert constants.ROUTER in State.vars + # Raises VarDependencyError if the dependency does not resolve to a var. + App()._validate_var_dependencies() + + for dep_set in State._var_dependencies.values(): + dep_set.discard((LegacyRouterCompileState.get_full_name(), "foo")) + State._potentially_dirty_states.discard(LegacyRouterCompileState.get_full_name()) + + def test_router_var_dep_whole_router() -> None: """deps=[State.router] must track every per-field router var. - The switchboard's VarData surfaces only one field name, so without the - composite dependency hook a cached var declaring the whole router would go - stale when any other router field changed -- a reconnect updates the - session without touching the URL, for instance. + The switchboard is composed of the five per-field vars, so its VarData + must carry all five field names; if it reported only one, a cached var + declaring the whole router would go stale when any other router field + changed -- a reconnect updates the session without touching the URL, for + instance. """ class WholeRouterDepState(State): @@ -3869,7 +3894,7 @@ class RouterVarListingState(State): router_var = RouterVarListingState.vars[constants.ROUTER] assert isinstance(router_var, RouterDataVar) assert router_var.equals(State.router) - assert str(router_var.route_id) == str(State.router_route_id) + assert str(router_var.route_id) == str(State.rx_router_route_id) def test_update_router_vars_ignores_omitted_static_keys( @@ -3903,9 +3928,9 @@ def test_update_router_vars_ignores_omitted_static_keys( } merged = test_state._update_router_vars(navigation_only, full_router_data) assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { - "router_page", - "router_url", - "router_route_id", + "rx_router_page", + "rx_router_url", + "rx_router_route_id", } assert test_state.router.session.client_token == "tok" assert test_state.router.session.session_id == "sid1" @@ -3952,7 +3977,7 @@ def test_update_router_vars_non_origin_header_leaves_navigation_clean( RouteVar.HEADERS: {"origin": "http://localhost:3000", "cookie": "c=d"}, } test_state._update_router_vars(new_cookie, router_data) - assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_headers"} + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"rx_router_headers"} def test_update_router_vars_granular_delta(test_state: TestState) -> None: @@ -3978,9 +4003,9 @@ def test_update_router_vars_granular_delta(test_state: TestState) -> None: nav_router_data = {**full_router_data, RouteVar.PATH: "/b", RouteVar.ORIGIN: "/b"} test_state._update_router_vars(nav_router_data, full_router_data) assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { - "router_page", - "router_url", - "router_route_id", + "rx_router_page", + "rx_router_url", + "rx_router_route_id", } assert test_state.router.url.path == "/b" assert test_state.router.session.session_id == "sid1" @@ -3989,7 +4014,7 @@ def test_update_router_vars_granular_delta(test_state: TestState) -> None: # Reconnect: only the session var is rebuilt. reconnect_router_data = {**nav_router_data, RouteVar.SESSION_ID: "sid2"} test_state._update_router_vars(reconnect_router_data, nav_router_data) - assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"router_session"} + assert test_state.dirty_vars & set(constants.ROUTER_VARS) == {"rx_router_session"} assert test_state.router.session.session_id == "sid2" test_state._clean() @@ -4001,9 +4026,9 @@ def test_update_router_vars_granular_delta(test_state: TestState) -> None: } test_state._update_router_vars(new_headers_router_data, reconnect_router_data) assert test_state.dirty_vars & set(constants.ROUTER_VARS) == { - "router_headers", - "router_page", - "router_url", + "rx_router_headers", + "rx_router_page", + "rx_router_url", } assert test_state.router.url.origin == "http://example.com" test_state._clean() From c9ea01cb8155c0782a450657b98d057d840edcd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 02:13:25 +0000 Subject: [PATCH 15/27] Prefix the per-field router vars with rx_ `router_session` and friends are plausible names for an app's own field, and a collision is silent: the framework var wins and the app's field is dropped without taking effect (see #7074). Prefixing them makes that collision unlikely rather than merely unlucky. `router` itself keeps its name -- it is the public switchboard. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- news/7068.breaking.md | 1 + news/7068.deprecation.md | 2 +- .../src/reflex_base/constants/route.py | 12 ++-- reflex/app.py | 6 +- reflex/istate/shared.py | 16 ++--- reflex/state.py | 66 ++++++++++--------- tests/units/istate/test_data.py | 24 +++---- .../processor/test_base_state_processor.py | 26 ++++---- tests/units/test_app.py | 24 +++---- tests/units/utils/test_format.py | 10 +-- 10 files changed, 99 insertions(+), 88 deletions(-) create mode 100644 news/7068.breaking.md diff --git a/news/7068.breaking.md b/news/7068.breaking.md new file mode 100644 index 00000000000..41b1dce8c62 --- /dev/null +++ b/news/7068.breaking.md @@ -0,0 +1 @@ +The root state gained five reserved base vars holding the router data: `rx_router_session`, `rx_router_headers`, `rx_router_page`, `rx_router_url` and `rx_router_route_id`. A `State` subclass that already defines a field under one of those names must rename it — the framework's var wins, and the app's own field is dropped without taking effect. `State.router` itself is unchanged. diff --git a/news/7068.deprecation.md b/news/7068.deprecation.md index feee20812a1..87a6ba3b4bd 100644 --- a/news/7068.deprecation.md +++ b/news/7068.deprecation.md @@ -1 +1 @@ -Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the specific router var instead, e.g. `deps=["router_url"]`. +Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the specific router var instead, e.g. `deps=["rx_router_url"]`. diff --git a/packages/reflex-base/src/reflex_base/constants/route.py b/packages/reflex-base/src/reflex_base/constants/route.py index 02281381e58..4c8606561eb 100644 --- a/packages/reflex-base/src/reflex_base/constants/route.py +++ b/packages/reflex-base/src/reflex_base/constants/route.py @@ -19,11 +19,13 @@ class RouteArgType(SimpleNamespace): # Session and headers are constant for the lifetime of a websocket connection, # while page, url, and route_id change on every navigation; keeping them in # separate vars means a navigation delta only re-sends the navigation fields. -ROUTER_SESSION = "router_session" -ROUTER_HEADERS = "router_headers" -ROUTER_PAGE = "router_page" -ROUTER_URL = "router_url" -ROUTER_ROUTE_ID = "router_route_id" +# The `rx_` prefix keeps them from colliding with a field an app already +# defines; `router` itself stays unprefixed as the public switchboard. +ROUTER_SESSION = "rx_router_session" +ROUTER_HEADERS = "rx_router_headers" +ROUTER_PAGE = "rx_router_page" +ROUTER_URL = "rx_router_url" +ROUTER_ROUTE_ID = "rx_router_route_id" ROUTER_VARS = ( ROUTER_SESSION, diff --git a/reflex/app.py b/reflex/app.py index 9bc97d8372f..51ca0405d57 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -2390,7 +2390,7 @@ async def link_token_to_sid(self, sid: str, token: str): # The state is loaded under this identity, so record it rather # than waiting for the first event to fill it in: duplicate-token # handling hands back a fresh token here, and until router_data - # carries it, anything reading router_session.client_token (a + # carries it, anything reading rx_router_session.client_token (a # background task, a shared-state link) addresses the wrong tree. state.router_data[constants.RouteVar.CLIENT_TOKEN] = new_token or token # Rebuild from router_data (rather than replacing the field on @@ -2398,5 +2398,5 @@ async def link_token_to_sid(self, sid: str, token: str): # in step, the same way the event processor refreshes it. if ( session := SessionData.from_router_data(state.router_data) - ) != state.router_session: - state.router_session = session + ) != state.rx_router_session: + state.rx_router_session = session diff --git a/reflex/istate/shared.py b/reflex/istate/shared.py index bf5642c2a10..fb4d86d497d 100644 --- a/reflex/istate/shared.py +++ b/reflex/istate/shared.py @@ -240,7 +240,7 @@ async def _link_to(self, token: str) -> Self: return self # already linked to this token if self._linked_to and self._linked_to != token: # Disassociate from previous linked token since unlink will not be called. - self._linked_from.discard(self.router_session.client_token) + self._linked_from.discard(self.rx_router_session.client_token) # TODO: Change StateManager to accept token + class instead of combining them in a string. if "_" in token: msg = f"Invalid token {token} for linking state {self.get_full_name()}, cannot use underscore (_) in the token name." @@ -275,12 +275,12 @@ async def _unlink(self): # Break the linkage for future events. self._reflex_internal_links.pop(state_name) - self._linked_from.discard(self.router_session.client_token) + self._linked_from.discard(self.rx_router_session.client_token) # Patch in the original state, apply updates, then rehydrate. private_root_state = await get_state_manager().get_state( BaseStateToken( - ident=self.router_session.client_token, + ident=self.rx_router_session.client_token, cls=type(self), ) ) @@ -330,11 +330,11 @@ async def _internal_patch_linked_state( # calls when directly modifying a linked token will load the # associated instance. if ( - session := linked_root_state.router_session + session := linked_root_state.rx_router_session ).client_token != token: import dataclasses as dc - linked_root_state.router_session = dc.replace( + linked_root_state.rx_router_session = dc.replace( session, client_token=token ) if linked_root_state is None: @@ -348,8 +348,8 @@ async def _internal_patch_linked_state( # Avoid unnecessary dirtiness of shared state when there are no changes. if type(self) not in self._held_locks[token]: self._held_locks[token][type(self)] = linked_state - if self.router_session.client_token not in linked_state._linked_from: - linked_state._linked_from.add(self.router_session.client_token) + if self.rx_router_session.client_token not in linked_state._linked_from: + linked_state._linked_from.add(self.rx_router_session.client_token) if linked_state._linked_to != token: linked_state._linked_to = token await self._exit_stack.enter_async_context( @@ -440,7 +440,7 @@ async def _modify_linked_states( affected_tokens.update( token for token in linked_state._linked_from - if token != self.router_session.client_token + if token != self.rx_router_session.client_token ) # When modifying a shared token directly (empty _reflex_internal_links), # the held locks will be empty. Check SharedState substates for linked diff --git a/reflex/state.py b/reflex/state.py index fa5256b319c..dd211a0afe9 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -396,12 +396,12 @@ def _router_fget(self: BaseState) -> RouterData: The RouterData for the current connection and page. """ return RouterData( - session=self.router_session, - headers=self.router_headers, - _page=self.router_page, + session=self.rx_router_session, + headers=self.rx_router_headers, + _page=self.rx_router_page, # URLData.href always holds a ReflexURL at runtime (see URLData). - url=cast("ReflexURL", self.router_url.href), - route_id=self.router_route_id, + url=cast("ReflexURL", self.rx_router_url.href), + route_id=self.rx_router_route_id, ) @@ -412,11 +412,11 @@ def _router_fset(self: BaseState, value: RouterData) -> None: self: The state instance. value: The RouterData to store. """ - self.router_session = value.session - self.router_headers = value.headers - self.router_page = value._page - self.router_url = URLData.from_url(value.url) - self.router_route_id = value.route_id + self.rx_router_session = value.session + self.rx_router_headers = value.headers + self.rx_router_page = value._page + self.rx_router_url = URLData.from_url(value.url) + self.rx_router_route_id = value.route_id def _get_router_var(cls: type[BaseState]) -> RouterDataVar: @@ -623,19 +623,19 @@ class BaseState(EvenMoreBasicBaseState): ) # The per-connection session data (constant for the socket lifetime). - router_session: Field[SessionData] = field(default_factory=SessionData) + rx_router_session: Field[SessionData] = field(default_factory=SessionData) # The headers of the connection request (constant for the socket lifetime). - router_headers: Field[HeaderData] = field(default_factory=HeaderData) + rx_router_headers: Field[HeaderData] = field(default_factory=HeaderData) # The page data for the current page (deprecated; params feeds dynamic route vars). - router_page: Field[PageData] = field(default_factory=PageData) + rx_router_page: Field[PageData] = field(default_factory=PageData) # The parsed URL of the current page. - router_url: Field[URLData] = field(default_factory=URLData) + rx_router_url: Field[URLData] = field(default_factory=URLData) # The route pattern that matched the current page. - router_route_id: Field[str] = field(default="") + rx_router_route_id: Field[str] = field(default="") # Switchboard for the router vars above: instance reads compose a # RouterData view, writes decompose into the per-field vars, and class @@ -1153,7 +1153,7 @@ def _init_var_dependency_dicts(cls): console.deprecate( feature_name='ComputedVar deps=["router"]', reason="the router var was split; depend on the specific" - ' router var instead (e.g. deps=["router_url"]).', + ' router var instead (e.g. deps=["rx_router_url"]).', deprecation_version="0.9.9", removal_version="1.0", ) @@ -1683,7 +1683,7 @@ def setup_dynamic_args(cls, args: builtins.dict[str, str]): def argsingle_factory(param: str): def inner_func(self: BaseState) -> str: - return self.router_page.params.get(param, "") + return self.rx_router_page.params.get(param, "") inner_func.__name__ = param @@ -1691,7 +1691,7 @@ def inner_func(self: BaseState) -> str: def arglist_factory(param: str): def inner_func(self: BaseState) -> list[str]: - return self.router_page.params.get(param, []) + return self.rx_router_page.params.get(param, []) inner_func.__name__ = param @@ -1962,26 +1962,30 @@ def _update_router_vars( constants.RouteVar.CLIENT_IP, ) ) - and (session := SessionData.from_router_data(merged)) != self.router_session + and (session := SessionData.from_router_data(merged)) + != self.rx_router_session ): - self.router_session = session + self.rx_router_session = session if ( headers_changed - and (headers := HeaderData.from_router_data(merged)) != self.router_headers + and (headers := HeaderData.from_router_data(merged)) + != self.rx_router_headers ): - self.router_headers = headers + self.rx_router_headers = headers if ( origin_changed or prev_get(constants.RouteVar.PATH) != get(constants.RouteVar.PATH) or prev_get(constants.RouteVar.ORIGIN) != get(constants.RouteVar.ORIGIN) or prev_get(constants.RouteVar.QUERY) != get(constants.RouteVar.QUERY) ): - if (page := PageData.from_router_data(merged)) != self.router_page: - self.router_page = page - if (url := URLData.from_router_data(merged)) != self.router_url: - self.router_url = url - if (route_id := get(constants.RouteVar.PATH, "")) != self.router_route_id: - self.router_route_id = route_id + if (page := PageData.from_router_data(merged)) != self.rx_router_page: + self.rx_router_page = page + if (url := URLData.from_router_data(merged)) != self.rx_router_url: + self.rx_router_url = url + if ( + route_id := get(constants.RouteVar.PATH, "") + ) != self.rx_router_route_id: + self.rx_router_route_id = route_id return merged @classmethod @@ -2096,7 +2100,9 @@ async def _get_state_from_redis(self, state_cls: type[T_STATE]) -> T_STATE: ) raise RuntimeError(msg) state_in_redis = await state_manager.get_state( - token=BaseStateToken(ident=self.router_session.client_token, cls=state_cls), + token=BaseStateToken( + ident=self.rx_router_session.client_token, cls=state_cls + ), top_level=False, for_state_instance=self, ) @@ -2876,7 +2882,7 @@ def on_load_internal(self) -> list[Event | EventSpec | event.EventCallback] | No The list of events to queue for on load handling. """ load_events = RegistrationContext.get().app.get_load_events( - self.router_url.path + self.rx_router_url.path ) if not load_events: self.is_hydrated = True diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index 81c58f111cb..375f4aabb89 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -174,19 +174,21 @@ def test_router_var_resolves_to_per_field_base_vars(): prefix = "reflex___state____state" assert ( str(rx.State.router.session.client_token) - == f'{prefix}.router_session_rx_state_?.["client_token"]' + == f'{prefix}.rx_router_session_rx_state_?.["client_token"]' ) assert ( str(rx.State.router.headers.user_agent) - == f'{prefix}.router_headers_rx_state_?.["user_agent"]' + == f'{prefix}.rx_router_headers_rx_state_?.["user_agent"]' ) assert ( str(rx.State.router.page.raw_path) - == f'{prefix}.router_page_rx_state_?.["raw_path"]' + == f'{prefix}.rx_router_page_rx_state_?.["raw_path"]' ) - assert str(rx.State.router.url) == f'{prefix}.router_url_rx_state_?.["href"]' - assert str(rx.State.router.url.path) == f'{prefix}.router_url_rx_state_?.["path"]' - assert str(rx.State.router.route_id) == f"{prefix}.router_route_id_rx_state_" + assert str(rx.State.router.url) == f'{prefix}.rx_router_url_rx_state_?.["href"]' + assert ( + str(rx.State.router.url.path) == f'{prefix}.rx_router_url_rx_state_?.["path"]' + ) + assert str(rx.State.router.route_id) == f"{prefix}.rx_router_route_id_rx_state_" def test_router_var_renders_composed_object(): @@ -196,11 +198,11 @@ def test_router_var_renders_composed_object(): prefix = "reflex___state____state" assert str(rx.State.router) == ( "({ " - f'"session": {prefix}.router_session_rx_state_, ' - f'"headers": {prefix}.router_headers_rx_state_, ' - f'"page": {prefix}.router_page_rx_state_, ' - f'"url": {prefix}.router_url_rx_state_, ' - f'"route_id": {prefix}.router_route_id_rx_state_' + f'"session": {prefix}.rx_router_session_rx_state_, ' + f'"headers": {prefix}.rx_router_headers_rx_state_, ' + f'"page": {prefix}.rx_router_page_rx_state_, ' + f'"url": {prefix}.rx_router_url_rx_state_, ' + f'"route_id": {prefix}.rx_router_route_id_rx_state_' " })" ) diff --git a/tests/units/reflex_base/event/processor/test_base_state_processor.py b/tests/units/reflex_base/event/processor/test_base_state_processor.py index fc8b2448445..8d6ecefdb23 100644 --- a/tests/units/reflex_base/event/processor/test_base_state_processor.py +++ b/tests/units/reflex_base/event/processor/test_base_state_processor.py @@ -12,7 +12,7 @@ import pytest import pytest_asyncio from opentelemetry.trace import SpanKind, StatusCode -from reflex_base import otel +from reflex_base import constants, otel from reflex_base.constants import CompileVars, RouteVar from reflex_base.constants.state import FIELD_MARKER from reflex_base.environment import environment @@ -1403,10 +1403,10 @@ def client_event(router_data: dict[str, Any]) -> Event: # The connection-scoped data survived the partial payload... assert state.router_data["headers"] == full_view["headers"] - assert state.router_session.client_token == token + assert state.rx_router_session.client_token == token # ...and nothing about the router was re-sent or marked dirty. assert not any( - key.startswith("router") + key.removesuffix(FIELD_MARKER) in constants.ROUTER_VARS for _token, delta in emitted_deltas for key in delta.get(State.get_full_name(), {}) ) @@ -1464,7 +1464,7 @@ def router_vars_in_deltas() -> set[str]: key.removesuffix(FIELD_MARKER) for _token, delta in emitted_deltas for key in delta.get(State.get_full_name(), {}) - if key.startswith("router") + if key.removesuffix(FIELD_MARKER) in constants.ROUTER_VARS } async def run_event(router_data: dict[str, Any]) -> None: @@ -1476,19 +1476,19 @@ async def run_event(router_data: dict[str, Any]) -> None: # First event on the connection populates every router var. await run_event(view("/a")) assert router_vars_in_deltas() == { - "router_session", - "router_headers", - "router_page", - "router_url", - "router_route_id", + "rx_router_session", + "rx_router_headers", + "rx_router_page", + "rx_router_url", + "rx_router_route_id", } # A navigation only re-sends the navigation-scoped vars. await run_event(view("/b")) assert router_vars_in_deltas() == { - "router_page", - "router_url", - "router_route_id", + "rx_router_page", + "rx_router_url", + "rx_router_route_id", } # An event without a route change re-sends no router vars at all. @@ -1497,4 +1497,4 @@ async def run_event(router_data: dict[str, Any]) -> None: # A reconnect (new sid, same headers) re-sends only the session. await run_event(view("/b", sid="sid2")) - assert router_vars_in_deltas() == {"router_session"} + assert router_vars_in_deltas() == {"rx_router_session"} diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 1af2fb6f06f..cb6b245892d 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -317,9 +317,9 @@ def test_add_page_set_route_dynamic(index_page: ComponentCallable): assert app._pages.keys() == {"test/[dynamic]"} assert "dynamic" in app._state.computed_vars assert app._state.computed_vars["dynamic"]._deps(objclass=EmptyState) == { - EmptyState.get_full_name(): {"router_page"}, + EmptyState.get_full_name(): {"rx_router_page"}, } - assert "router_page" in app._state()._var_dependencies + assert "rx_router_page" in app._state()._var_dependencies def test_add_page_set_route_nested(app: App, index_page: ComponentCallable): @@ -1948,9 +1948,9 @@ async def test_dynamic_route_var_route_change_completed_on_load( assert arg_name in app._state.vars assert arg_name in app._state.computed_vars assert app._state.computed_vars[arg_name]._deps(objclass=DynamicState) == { - DynamicState.get_full_name(): {"router_page"}, + DynamicState.get_full_name(): {"rx_router_page"}, } - assert "router_page" in app._state()._var_dependencies + assert "rx_router_page" in app._state()._var_dependencies substate_token = BaseStateToken(ident=token, cls=DynamicState) exp_vals = ["foo", "foobar", "baz"] @@ -1987,13 +1987,13 @@ def _dynamic_state_event(name, val, **kwargs): # Only the navigation-scoped router vars change (no session/headers in # the router_data), so only those land in the delta. exp_router_delta = { - "router_page" + FIELD_MARKER: exp_router._page, - "router_url" + FIELD_MARKER: URLData.from_url(exp_router.url), + "rx_router_page" + FIELD_MARKER: exp_router._page, + "rx_router_url" + FIELD_MARKER: URLData.from_url(exp_router.url), } if exp_index == 0: # Every navigation here matches the same route, so the route_id # only changes on the first one. - exp_router_delta["router_route_id" + FIELD_MARKER] = exp_router.route_id + exp_router_delta["rx_router_route_id" + FIELD_MARKER] = exp_router.route_id async with mock_base_state_event_processor as processor: await processor.enqueue( token, @@ -4564,7 +4564,7 @@ async def test_link_token_to_sid_records_the_connecting_identity( """The session var carries the token the state was loaded under. Duplicate-token handling hands back a fresh token, and the state is loaded - under it. Leaving `router_session.client_token` empty until the first event + under it. Leaving `rx_router_session.client_token` empty until the first event would let anything reading it in between -- a background task, a shared-state link -- address the wrong state tree. @@ -4585,8 +4585,8 @@ async def test_link_token_to_sid_records_the_connecting_identity( # No duplicate: the connecting token is recorded. await event_namespace.link_token_to_sid("sid1", token) assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == token - assert state.router_session.client_token == token - assert state.router_session.session_id == "sid1" + assert state.rx_router_session.client_token == token + assert state.rx_router_session.session_id == "sid1" # Duplicate: the *new* token is recorded, not the one the client sent. # The duplicate branch emits the replacement token to the client, which @@ -4600,8 +4600,8 @@ async def test_link_token_to_sid_records_the_connecting_identity( ) await event_namespace.link_token_to_sid("sid2", token) assert state.router_data[constants.RouteVar.CLIENT_TOKEN] == new_token - assert state.router_session.client_token == new_token - assert state.router_session.session_id == "sid2" + assert state.rx_router_session.client_token == new_token + assert state.rx_router_session.session_id == "sid2" @pytest.mark.asyncio diff --git a/tests/units/utils/test_format.py b/tests/units/utils/test_format.py index 1b062d0f8f2..06519d46b55 100644 --- a/tests/units/utils/test_format.py +++ b/tests/units/utils/test_format.py @@ -658,8 +658,8 @@ def test_format_query_params(input, output): formatted_router_vars = { - "router_route_id" + FIELD_MARKER: "", - "router_url" + FIELD_MARKER: { + "rx_router_route_id" + FIELD_MARKER: "", + "rx_router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", "origin": "", @@ -669,12 +669,12 @@ def test_format_query_params(input, output): "fragment": "", "href": "", }, - "router_session" + FIELD_MARKER: { + "rx_router_session" + FIELD_MARKER: { "client_token": "", "client_ip": "", "session_id": "", }, - "router_headers" + FIELD_MARKER: { + "rx_router_headers" + FIELD_MARKER: { "host": "", "origin": "", "upgrade": "", @@ -690,7 +690,7 @@ def test_format_query_params(input, output): "accept_language": "", "raw_headers": {}, }, - "router_page" + FIELD_MARKER: { + "rx_router_page" + FIELD_MARKER: { "host": "", "path": "", "raw_path": "", From 6b2afe76e218e1652a1b751fdb0896dd02b6a807 Mon Sep 17 00:00:00 2001 From: Masen Furer Date: Wed, 16 Sep 2026 16:04:02 -0700 Subject: [PATCH 16/27] Fix router name collisions and composed-var dependencies --- news/7068.breaking.md | 2 +- .../src/reflex_base/vars/dep_tracking.py | 4 ++-- reflex/state.py | 9 ++++++-- tests/units/test_state.py | 12 +++++++++++ tests/units/vars/test_dep_tracking.py | 21 +++++++++++++++++++ 5 files changed, 43 insertions(+), 5 deletions(-) diff --git a/news/7068.breaking.md b/news/7068.breaking.md index 41b1dce8c62..b276bf0b52d 100644 --- a/news/7068.breaking.md +++ b/news/7068.breaking.md @@ -1 +1 @@ -The root state gained five reserved base vars holding the router data: `rx_router_session`, `rx_router_headers`, `rx_router_page`, `rx_router_url` and `rx_router_route_id`. A `State` subclass that already defines a field under one of those names must rename it — the framework's var wins, and the app's own field is dropped without taking effect. `State.router` itself is unchanged. +The root state gained five reserved base vars holding the router data: `rx_router_session`, `rx_router_headers`, `rx_router_page`, `rx_router_url` and `rx_router_route_id`. A state subclass that declares one of these names now raises a clear error and must rename its field. `State.router` itself is unchanged. diff --git a/packages/reflex-base/src/reflex_base/vars/dep_tracking.py b/packages/reflex-base/src/reflex_base/vars/dep_tracking.py index 4d4ad5e8333..0c3db139c4a 100644 --- a/packages/reflex-base/src/reflex_base/vars/dep_tracking.py +++ b/packages/reflex-base/src/reflex_base/vars/dep_tracking.py @@ -388,8 +388,8 @@ def handle_getting_var(self, instruction: dis.Instruction) -> None: if the_var_data is None: msg = f"Cannot determine the source code for the var in {self.func!r}." raise VarValueError(msg) - self.dependencies.setdefault(the_var_data.state, set()).add( - the_var_data.field_name + self.dependencies.setdefault(the_var_data.state, set()).update( + the_var._dependency_field_names() ) self.scan_status = ScanStatus.SCANNING diff --git a/reflex/state.py b/reflex/state.py index 8217cf9afe7..34b7492817f 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -748,10 +748,15 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): **kwargs: The kwargs to pass to the init_subclass method. Raises: - StateValueError: If a substate class shadows another. + StateValueError: If a substate shadows another or declares a reserved router field. """ from reflex_base.utils.exceptions import StateValueError + for name in constants.ROUTER_VARS: + if name in cls.__own_fields__ or name in cls.__dict__: + msg = f"The state name `{name}` is reserved for router data; use a different name instead" + raise StateValueError(msg) + super().__init_subclass__(**kwargs) if cls._mixin: @@ -1158,7 +1163,7 @@ def _init_var_dependency_dicts(cls): feature_name='ComputedVar deps=["router"]', reason="the router var was split; depend on the specific" ' router var instead (e.g. deps=["rx_router_url"]).', - deprecation_version="0.9.9", + deprecation_version="0.9.12", removal_version="1.0", ) dvar_set = (dvar_set - {constants.ROUTER}) | set( diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 5b5203e03e8..f405146aa27 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -3799,6 +3799,18 @@ def foo(self) -> str: State._potentially_dirty_states.discard(RouterVarDepState.get_full_name()) +@pytest.mark.parametrize("name", constants.ROUTER_VARS) +@pytest.mark.parametrize("base", [BaseState, State]) +def test_router_field_names_are_reserved(name, base): + """Application fields cannot replace framework-owned router storage.""" + with pytest.raises(ValueError, match="reserved"): + type( + "InvalidRouterState", + (base,), + {"__module__": __name__, "__annotations__": {name: int}, name: 1}, + ) + + def test_router_var_dep_legacy_string() -> None: """An explicit deps=["router"] still fires when any router var changes. diff --git a/tests/units/vars/test_dep_tracking.py b/tests/units/vars/test_dep_tracking.py index 6c3c1782316..7f5fd27c482 100644 --- a/tests/units/vars/test_dep_tracking.py +++ b/tests/units/vars/test_dep_tracking.py @@ -300,6 +300,27 @@ async def func_with_get_var_value(self: DependencyTestState): assert tracker.dependencies == expected_deps +@pytest.mark.skipif( + sys.version_info < (3, 11), reason="Requires Python 3.11+ for positions" +) +def test_get_var_value_tracks_all_composed_fields(): + """Composed get_var_value arguments register every state field they read.""" + composed_var = DependencyTestState.count + DependencyTestState.items.length() + + async def composed(self: DependencyTestState): + """Read a composite expression. + + Returns: + The combined field value. + """ + return await self.get_var_value(composed_var) + + tracker = DependencyTracker(composed, DependencyTestState) + assert tracker.dependencies == { + DependencyTestState.get_full_name(): {"count", "items"} + } + + @pytest.mark.skipif( sys.version_info < (3, 11), reason="Requires Python 3.11+ for positions" ) From db3caadc100f66caeced1340d21723e7663f2294 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 16 Sep 2026 23:32:42 +0000 Subject: [PATCH 17/27] Point the router deps deprecation at the Var form The message steered users to `deps=["rx_router_url"]`, a string naming an internal-prefixed backing var -- which makes the `rx_` names user-facing API exactly where the PR is trying to keep them internal. Recommend the Var form this PR already supports instead: `deps=[State.router.url]` for one field (verified: tracks only `rx_router_url`), or `deps=[State.router]` for all five, which is what the legacy string meant. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- news/7068.deprecation.md | 2 +- reflex/state.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/news/7068.deprecation.md b/news/7068.deprecation.md index 87a6ba3b4bd..cac4544b070 100644 --- a/news/7068.deprecation.md +++ b/news/7068.deprecation.md @@ -1 +1 @@ -Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the specific router var instead, e.g. `deps=["rx_router_url"]`. +Declaring a computed var dependency on the `router` var (`deps=["router"]`) is deprecated; depend on the router Var instead, e.g. `deps=[State.router.url]` for a single field or `deps=[State.router]` to keep tracking all of them. diff --git a/reflex/state.py b/reflex/state.py index 34b7492817f..bd5a49969eb 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -1161,8 +1161,9 @@ def _init_var_dependency_dicts(cls): # depend on all the per-field router vars instead. console.deprecate( feature_name='ComputedVar deps=["router"]', - reason="the router var was split; depend on the specific" - ' router var instead (e.g. deps=["rx_router_url"]).', + reason="the router var was split; depend on the router" + " Var instead (e.g. deps=[State.router.url] for one" + " field, or deps=[State.router] for all of them).", deprecation_version="0.9.12", removal_version="1.0", ) From 153b1ef291db747b716f437edca1167146570c8f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 16 Sep 2026 23:38:37 +0000 Subject: [PATCH 18/27] Hoist the reserved router name check out of __init_subclass__ `__init_subclass__` was already well past the complexity guideline on main (19); the reserved-name loop pushed it to 21. Move the loop into a `_check_reserved_router_names` classmethod alongside the other `_check_*` / `_validate_*` helpers, restoring the function to 19 and leaving the behaviour (including the check running for mixins, before `super().__init_subclass__`) unchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index bd5a49969eb..b83f500e002 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -739,6 +739,20 @@ def _validate_module_name(cls) -> None: ) raise NameError(msg) + @classmethod + def _check_reserved_router_names(cls) -> None: + """Check that the class does not declare one of the router base vars. + + Raises: + StateValueError: If the class declares a reserved router field. + """ + from reflex_base.utils.exceptions import StateValueError + + for name in constants.ROUTER_VARS: + if name in cls.__own_fields__ or name in cls.__dict__: + msg = f"The state name `{name}` is reserved for router data; use a different name instead" + raise StateValueError(msg) + @classmethod def __init_subclass__(cls, mixin: bool = False, **kwargs): """Do some magic for the subclass initialization. @@ -748,14 +762,11 @@ def __init_subclass__(cls, mixin: bool = False, **kwargs): **kwargs: The kwargs to pass to the init_subclass method. Raises: - StateValueError: If a substate shadows another or declares a reserved router field. + StateValueError: If a substate shadows another. """ from reflex_base.utils.exceptions import StateValueError - for name in constants.ROUTER_VARS: - if name in cls.__own_fields__ or name in cls.__dict__: - msg = f"The state name `{name}` is reserved for router data; use a different name instead" - raise StateValueError(msg) + cls._check_reserved_router_names() super().__init_subclass__(**kwargs) From 2d7603256984e5f8556bfd72e8f57768429e2f2d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 00:49:47 +0000 Subject: [PATCH 19/27] Share the empty router defaults across states The split gave the root state four extra base vars, and three of their `default_factory` defaults were rebuilt for every state instance even though `SessionData`, `HeaderData` and `URLData` are frozen dataclasses whose members are themselves immutable (`URLData.query_parameters` is a `_FrozenDictStrStr`). One instance can back every state's field. `field()` cannot express this: it shares a `default` only when its type is in `IMMUTABLE_TYPES`, and deep-copies anything else per instance, so these three use `Field(default=...)` directly. `PageData` keeps `default_factory` -- its `params` is a plain mutable dict. Root state instantiation, measured on 3.14 (min of 7 x 2000 runs): main 12.78us before 15.02us after 10.92us so this removes the instantiation regression the split introduced and lands 15% below main. Payload size is unchanged; the remaining CodSpeed delta is serialization of the extra keys. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index b83f500e002..d9519160fb3 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -91,6 +91,13 @@ from reflex.utils import console, format, types from reflex.utils.exec import is_testing_env +# Shared empty router defaults. Each is a frozen dataclass whose members are +# themselves immutable, so one instance can back every state's field instead +# of being rebuilt per state. +_DEFAULT_SESSION_DATA = SessionData() +_DEFAULT_HEADER_DATA = HeaderData() +_DEFAULT_URL_DATA = URLData() + logger = logging.getLogger(__name__) if TYPE_CHECKING: @@ -624,16 +631,21 @@ class BaseState(EvenMoreBasicBaseState): ) # The per-connection session data (constant for the socket lifetime). - rx_router_session: Field[SessionData] = field(default_factory=SessionData) + # These three defaults are frozen dataclasses holding only immutable + # members, so every state can share one instance instead of building a + # fresh one per field per state. `field()` cannot be used for that: it + # only shares a `default` whose type is in `IMMUTABLE_TYPES`, and + # otherwise deep-copies it per instance. + rx_router_session: Field[SessionData] = Field(default=_DEFAULT_SESSION_DATA) # The headers of the connection request (constant for the socket lifetime). - rx_router_headers: Field[HeaderData] = field(default_factory=HeaderData) + rx_router_headers: Field[HeaderData] = Field(default=_DEFAULT_HEADER_DATA) # The page data for the current page (deprecated; params feeds dynamic route vars). rx_router_page: Field[PageData] = field(default_factory=PageData) # The parsed URL of the current page. - rx_router_url: Field[URLData] = field(default_factory=URLData) + rx_router_url: Field[URLData] = Field(default=_DEFAULT_URL_DATA) # The route pattern that matched the current page. rx_router_route_id: Field[str] = field(default="") From 8cc79e5ba7d3ab9d66dff0a9f6f253943cba2fbf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 00:55:11 +0000 Subject: [PATCH 20/27] Make ReflexURL reject attribute assignment `URLData.href` defaults to a class-level `ReflexURL("")`, so the empty URL object is shared by every state that has not navigated yet -- that sharing comes from the dataclass default, not from how the field is declared, and it behaves the same whether `rx_router_url` uses `default_factory` or a shared default. `ReflexURL` had no assignment guard, so `state.router.url.path = "/x"` rewrote that shared object and every other such state saw `/x`. Reject `__setattr__`/`__delattr__`; `__new__` keeps filling the parsed components through `object.__setattr__`. `__slots__` is not available here because `str` is variable-length. Pickle, `copy` and `deepcopy` still round-trip every component, since they restore instance state through `__dict__` rather than `setattr`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/istate/data.py | 33 +++++++++++++++++++++++- tests/units/istate/test_data.py | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/reflex/istate/data.py b/reflex/istate/data.py index 75a43fab77f..b43557d513d 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -3,7 +3,7 @@ import dataclasses from collections.abc import Mapping from types import MappingProxyType -from typing import TYPE_CHECKING, Any, ClassVar, Final +from typing import TYPE_CHECKING, Any, ClassVar, Final, NoReturn from urllib.parse import _NetlocResultMixinStr, parse_qsl, urlsplit from reflex_base import constants @@ -159,6 +159,37 @@ def __new__(cls, url: str): object.__setattr__(obj, "fragment", fragment) return obj + def __setattr__(self, name: str, value: Any) -> NoReturn: + """Reject attribute assignment. + + A `ReflexURL` is a parsed view of an immutable `str`, and the empty + one is the class-level default of `URLData.href`, so it is shared by + every state that has not navigated yet. Letting a component assign to + a parsed component would rewrite that shared object for every state. + `__new__` fills the components with `object.__setattr__`. + + Args: + name: The attribute being assigned. + value: The value it would take. + + Raises: + AttributeError: Always. + """ + msg = f"cannot assign to {name!r}: ReflexURL is immutable" + raise AttributeError(msg) + + def __delattr__(self, name: str) -> NoReturn: + """Reject attribute deletion. + + Args: + name: The attribute being deleted. + + Raises: + AttributeError: Always. + """ + msg = f"cannot delete {name!r}: ReflexURL is immutable" + raise AttributeError(msg) + @serializer(to=dict) def _serialize_reflex_url(obj: ReflexURL) -> dict: diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index 375f4aabb89..c27be366b7e 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -1,8 +1,10 @@ """Tests for ReflexURL parsing, serialization, and Var attribute access.""" from collections.abc import Mapping +from typing import cast from urllib.parse import parse_qsl +import pytest from reflex_base.vars.object import ObjectVar from reflex_base.vars.sequence import StringVar @@ -234,3 +236,46 @@ def test_router_var_carries_state_var_data(): var_data = rx.State.router._get_all_var_data() assert var_data is not None assert var_data.state == rx.State.get_full_name() + + +@pytest.mark.parametrize("attr", ["path", "scheme", "netloc", "query", "fragment"]) +def test_reflex_url_rejects_attribute_assignment(attr: str): + """A parsed component must not be assignable. + + `URLData.href` defaults to a class-level `ReflexURL("")`, so the empty URL + object is shared by every state that has not navigated yet. If a component + could be assigned, writing through one state's `router.url` would rewrite + that shared object for all of them. + """ + url = ReflexURL(SAMPLE_URL) + before = getattr(url, attr) + + with pytest.raises(AttributeError, match="immutable"): + setattr(url, attr, "/mutated") + with pytest.raises(AttributeError, match="immutable"): + delattr(url, attr) + + assert getattr(url, attr) == before + + +def test_shared_empty_url_default_cannot_be_mutated_through_a_state(): + """Writing through one state's router.url must not leak into another.""" + from reflex.istate.data import URLData + from reflex.state import BaseState + + # A root state, so the router fields live on the instance under test + # rather than being delegated to a parent that is not in a tree here. + class _URLIsolationState(BaseState): + pass + + one = _URLIsolationState(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] + two = _URLIsolationState(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] + + with pytest.raises(AttributeError, match="immutable"): + one.router.url.path = "/mutated" + + one.rx_router_url = URLData.from_url(ReflexURL("https://example.com/real")) + + assert one.router.url.path == "/real" + assert two.router.url.path == "" + assert cast("ReflexURL", URLData().href).path == "" From c67a9bc69158dfbdf17223a210429bde38e607cc Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 17:28:58 +0000 Subject: [PATCH 21/27] Group VarData field dependencies by owning state `field_names` was a flat tuple paired with a single `state`, so merging a var built from one state's field with a var built from another's dropped the second: `VarData.merge` kept only the fields whose state matched the first non-empty one. A computed var depending on such a composite var never tracked the other state's field and went stale when it changed. Make the canonical form a `Mapping[str, tuple[str, ...]]` of state to its field names, unioned across merges, and register dependencies against every state in it -- in `_add_static_dep`, in `add_dependency`, and in `DependencyTracker`, all three of which previously collapsed a composite var to one state. `state`, `field_names` and `field_name` stay as fallback accessors reporting the first state and its first field, which is what the remaining readers in `state.py`, `exceptions.py` and the memoize plugin want. The mapping is a plain dict rather than a `MappingProxyType`: VarData is pickled along with the states holding it, and mappingproxy cannot be pickled. It is built fresh per VarData and never mutated afterwards. Verified against a var spanning two states: the second state's field goes from untracked to tracked. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- packages/reflex-base/news/7068.feature.md | 2 +- .../reflex-base/src/reflex_base/vars/base.py | 238 +++++++++++------- .../src/reflex_base/vars/dep_tracking.py | 5 +- tests/units/reflex_base/vars/test_base.py | 36 ++- tests/units/test_state.py | 44 +++- 5 files changed, 225 insertions(+), 100 deletions(-) diff --git a/packages/reflex-base/news/7068.feature.md b/packages/reflex-base/news/7068.feature.md index 23460935a50..6997058d111 100644 --- a/packages/reflex-base/news/7068.feature.md +++ b/packages/reflex-base/news/7068.feature.md @@ -1 +1 @@ -`VarData` now tracks every state field a var is built from in `field_names`, collected and deduped as vars merge. `field_name` still reports the first of them, so existing readers are unaffected. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes, not just the one the merge happened to surface first. +`VarData` now tracks every state field a var is built from in `field_dependencies`, a mapping of state name to that state's field names, unioned and deduped as vars merge. `state`, `field_name` and `field_names` remain as fallback accessors reporting the first state and its fields, so existing readers are unaffected. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes — including fields belonging to a different state, which previously went untracked. diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index f53143f7d93..44e124a7b8f 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -276,6 +276,39 @@ def insert_app_wraps( target[key] = wrapper +def _normalize_field_dependencies( + field_dependencies: Mapping[str, Sequence[str]] | None, + state: str, + field_name: str, + field_names: Sequence[str] | None, +) -> Mapping[str, tuple[str, ...]]: + """Build the canonical state -> fields mapping from the accepted shorthands. + + Args: + field_dependencies: The canonical mapping, if the caller gave one. + state: The single enclosing state, for the shorthand forms. + field_name: A single field of `state`. + field_names: Several fields of `state`; takes precedence over `field_name`. + + Returns: + An immutable mapping of state name to its deduped field names. + """ + if field_dependencies is not None: + return { + state_name: tuple(dict.fromkeys(names)) + for state_name, names in field_dependencies.items() + } + if field_names is not None: + names = tuple(dict.fromkeys(field_names)) + else: + names = (field_name,) if field_name else () + # A state with no named field still has to be recorded: plenty of vars + # carry only the state (for imports and hooks) and nothing reads a field. + if not state and not names: + return {} + return {state: names} + + @dataclasses.dataclass( eq=True, frozen=True, @@ -283,14 +316,17 @@ def insert_app_wraps( class VarData: """Metadata associated with a x.""" - # The name of the enclosing state. - state: str = dataclasses.field(default="") - - # The names of the state fields this var is built from. A var normally - # stands for a single field, but one composed of several (see - # `Var._dependency_field_names`) names all of them so a dependency on it - # tracks every field it reads. - field_names: tuple[str, ...] = dataclasses.field(default_factory=tuple) + # Every state field this var is built from, grouped by the state that owns + # it. A var normally stands for a single field of a single state, but one + # composed of several -- possibly spanning several states -- names all of + # them, so a dependency on it tracks each field it actually reads. + # Built fresh for every VarData and never mutated afterwards, so it is + # effectively frozen like the tuples beside it. A plain dict rather than a + # MappingProxyType because VarData is pickled along with the states holding + # it, and mappingproxy cannot be pickled. + field_dependencies: Mapping[str, tuple[str, ...]] = dataclasses.field( + default_factory=dict + ) # Imports needed to render this var imports: ParsedImportTuple = dataclasses.field(default_factory=tuple) @@ -318,6 +354,7 @@ def __init__( state: str = "", field_name: str = "", field_names: Sequence[str] | None = None, + field_dependencies: Mapping[str, Sequence[str]] | None = None, imports: ImmutableImportDict | ImmutableParsedImportDict | None = None, hooks: Mapping[str, VarData | None] | Sequence[str] | str | None = None, deps: list[Var] | None = None, @@ -328,10 +365,15 @@ def __init__( """Initialize the var data. Args: - state: The name of the enclosing state. - field_name: The name of the field in the state. Shorthand for a + state: The name of the enclosing state. Shorthand for a + single-state ``field_dependencies``; ignored when that is given. + field_name: The name of the field in ``state``. Shorthand for a single-entry ``field_names``; ignored when that is given. - field_names: The names of every state field this var is built from. + field_names: The names of the ``state`` fields this var is built + from. Ignored when ``field_dependencies`` is given. + field_dependencies: Every state field this var is built from, + grouped by owning state. The canonical form; the three + arguments above are shorthands for a single state. imports: Imports needed to render this var. hooks: Hooks that need to be present in the component to render this var. deps: Dependencies of the var for useCallback. @@ -346,13 +388,12 @@ def __init__( immutable_imports: ParsedImportTuple = tuple( (k, tuple(v)) for k, v in parse_imports(imports or {}).items() ) - object.__setattr__(self, "state", state) object.__setattr__( self, - "field_names", - tuple(dict.fromkeys(field_names)) - if field_names is not None - else ((field_name,) if field_name else ()), + "field_dependencies", + _normalize_field_dependencies( + field_dependencies, state, field_name, field_names + ), ) object.__setattr__(self, "imports", immutable_imports) object.__setattr__(self, "hooks", tuple(hooks or {})) @@ -365,8 +406,11 @@ def __init__( # Merge our dependencies first, so they can be referenced. merged_var_data = VarData.merge(*hooks.values(), self) if merged_var_data is not None: - object.__setattr__(self, "state", merged_var_data.state) - object.__setattr__(self, "field_names", merged_var_data.field_names) + object.__setattr__( + self, + "field_dependencies", + merged_var_data.field_dependencies, + ) object.__setattr__(self, "imports", merged_var_data.imports) object.__setattr__(self, "hooks", merged_var_data.hooks) object.__setattr__(self, "deps", merged_var_data.deps) @@ -374,17 +418,44 @@ def __init__( object.__setattr__(self, "components", merged_var_data.components) object.__setattr__(self, "app_wraps", merged_var_data.app_wraps) + @property + def state(self) -> str: + """The name of the enclosing state. + + Deprecated fallback accessor: a var may be built from fields of more + than one state, and this reports only the first. Read + ``field_dependencies`` to see every state. + + Returns: + The first state name, or an empty string if there is none. + """ + return next(iter(self.field_dependencies), "") + + @property + def field_names(self) -> tuple[str, ...]: + """The names of the fields this var is built from, in ``state``. + + Deprecated fallback accessor: fields owned by any other state are not + reported. Read ``field_dependencies`` to see every state's fields. + + Returns: + The first state's field names, empty if there are none. + """ + return self.field_dependencies.get(self.state, ()) + @property def field_name(self) -> str: """The name of the field in the state. - A var built from several fields reports the first; use ``field_names`` - to see all of them. + Deprecated fallback accessor: a var built from several fields reports + only the first, of the first state. Read ``field_dependencies`` to see + all of them. Returns: The first field name, or an empty string if there is none. """ - return self.field_names[0] if self.field_names else "" + field_names = self.field_names + return field_names[0] if field_names else "" def old_school_imports(self) -> ImportDict: """Return the imports as a mutable dict. @@ -416,25 +487,15 @@ def merge(*all: VarData | None) -> VarData | None: if len(all_var_datas) == 1: return all_var_datas[0] - # Get the first non-empty state or default to empty string. - state = next( - (var_data.state for var_data in all_var_datas if var_data.state), "" - ) - - # Collect every field name belonging to that state, in order, deduped, - # so a var composed of several fields (and therefore a dependency on - # it) knows about all of them. Field names carrying a different state - # are dropped: callers pair these names with the single `state` above, - # so keeping them would register a dependency on a field that state - # does not have. - field_names = tuple( - dict.fromkeys( - field_name - for var_data in all_var_datas - if not var_data.state or var_data.state == state - for field_name in var_data.field_names - ) - ) + # Union every state's fields, in order and deduped, so a var composed + # of several fields -- across as many states as it reaches -- carries + # all of them and a dependency on it tracks each one. + field_dependencies: dict[str, tuple[str, ...]] = {} + for var_data in all_var_datas: + for state_name, names in var_data.field_dependencies.items(): + field_dependencies[state_name] = tuple( + dict.fromkeys((*field_dependencies.get(state_name, ()), *names)) + ) hooks: dict[str, VarData | None] = { hook: None for var_data in all_var_datas for hook in var_data.hooks @@ -470,8 +531,7 @@ def merge(*all: VarData | None) -> VarData | None: insert_app_wraps(app_wraps, var_data.app_wraps) return VarData( - state=state, - field_names=field_names, + field_dependencies=field_dependencies, imports=imports_, hooks=hooks, deps=deps, @@ -489,10 +549,9 @@ def __bool__(self) -> bool: True if any field is set to a non-default value. """ return bool( - self.state + self.field_dependencies or self.imports or self.hooks - or self.field_names or self.deps or self.position or self.components @@ -514,8 +573,7 @@ def _identity_key(self) -> tuple: A hashable tuple uniquely identifying this VarData. """ return ( - self.state, - self.field_names, + tuple(self.field_dependencies.items()), self.imports, self.hooks, self.deps, @@ -784,24 +842,22 @@ def _get_all_var_data(self) -> VarData | None: """ return self._var_data - def _dependency_field_names(self) -> tuple[str, ...]: - """The state field names a ComputedVar depending on this Var must track. - - A Var normally stands for a single state field, but one composed of - several must name all of them, or a ``deps=[that_var]`` dependency - would track only some of the fields it reads and leave the computed var - stale when any of the others change. ``VarData.merge`` collects the - names as vars combine, so the merged VarData already knows all of them. + def _dependency_fields(self) -> Mapping[str, tuple[str, ...]]: + """The state fields a ComputedVar depending on this Var must track. - Only fields of the VarData's own state are named; callers pair these - names with that single state, so a var spanning two states still - reports just the first one's fields. + A Var normally stands for a single field of a single state, but one + composed of several must name all of them, or a ``deps=[that_var]`` + dependency would track only some of the fields it reads and leave the + computed var stale when any of the others change. A composite var may + also span several states, so the fields stay grouped by their owner. + ``VarData.merge`` unions them as vars combine, so the merged VarData + already knows every one. Returns: - The field names to register the dependency against. + The fields to register the dependency against, by state name. """ all_var_data = self._get_all_var_data() - return all_var_data.field_names if all_var_data is not None else () + return all_var_data.field_dependencies if all_var_data is not None else {} def __deepcopy__(self, memo: dict[int, Any]) -> Self: """Deepcopy the var. @@ -2497,17 +2553,17 @@ def _add_static_dep( if deps is None: deps = self._static_deps if isinstance(dep, Var): - state_name = ( - all_var_data.state - if (all_var_data := dep._get_all_var_data()) and all_var_data.state - else None - ) - if all_var_data is not None: - # A composite Var names every state field it is built from. - var_names = dep._dependency_field_names() + if (all_var_data := dep._get_all_var_data()) is not None: + # A composite Var names every state field it is built from, in + # each state that owns them. + field_dependencies = all_var_data.field_dependencies + if field_dependencies: + for state_name, field_names in field_dependencies.items(): + deps.setdefault(state_name or None, set()).update(field_names) + else: + deps.setdefault(None, set()) else: - var_names = (dep._js_expr,) - deps.setdefault(state_name, set()).update(var_names) + deps.setdefault(None, set()).add(dep._js_expr) elif isinstance(dep, str) and dep != "": deps.setdefault(None, set()).add(dep) else: @@ -2783,26 +2839,30 @@ def add_dependency(self, objclass: type[BaseState], dep: Var): state and field name """ if all_var_data := dep._get_all_var_data(): - state_name = all_var_data.state - if state_name: - # A composite Var names every state field it is built from. - var_names = tuple(filter(None, dep._dependency_field_names())) - if var_names: - self._static_deps.setdefault(state_name, set()).update(var_names) - target_state_class = objclass.get_root_state().get_class_substate( - state_name - ) - for var_name in var_names: - target_state_class._var_dependencies.setdefault( - var_name, set() - ).add(( - objclass.get_full_name(), - self._name, - )) - target_state_class._potentially_dirty_states.add( - objclass.get_full_name() - ) - return + # A composite Var names every state field it is built from, and may + # span several states; register against each of them. + registered = False + for state_name, field_names in all_var_data.field_dependencies.items(): + var_names = tuple(filter(None, field_names)) + if not state_name or not var_names: + continue + self._static_deps.setdefault(state_name, set()).update(var_names) + target_state_class = objclass.get_root_state().get_class_substate( + state_name + ) + for var_name in var_names: + target_state_class._var_dependencies.setdefault( + var_name, set() + ).add(( + objclass.get_full_name(), + self._name, + )) + target_state_class._potentially_dirty_states.add( + objclass.get_full_name() + ) + registered = True + if registered: + return msg = ( "ComputedVar dependencies must be Var instances with a state and " f"field name, got {dep!r}." diff --git a/packages/reflex-base/src/reflex_base/vars/dep_tracking.py b/packages/reflex-base/src/reflex_base/vars/dep_tracking.py index 0c3db139c4a..837badc9d8e 100644 --- a/packages/reflex-base/src/reflex_base/vars/dep_tracking.py +++ b/packages/reflex-base/src/reflex_base/vars/dep_tracking.py @@ -388,9 +388,8 @@ def handle_getting_var(self, instruction: dis.Instruction) -> None: if the_var_data is None: msg = f"Cannot determine the source code for the var in {self.func!r}." raise VarValueError(msg) - self.dependencies.setdefault(the_var_data.state, set()).update( - the_var._dependency_field_names() - ) + for state_name, field_names in the_var._dependency_fields().items(): + self.dependencies.setdefault(state_name, set()).update(field_names) self.scan_status = ScanStatus.SCANNING def _populate_dependencies(self) -> None: diff --git a/tests/units/reflex_base/vars/test_base.py b/tests/units/reflex_base/vars/test_base.py index 08010bdc45e..01c87f3bc7b 100644 --- a/tests/units/reflex_base/vars/test_base.py +++ b/tests/units/reflex_base/vars/test_base.py @@ -280,21 +280,45 @@ def test_var_data_merge_collects_field_names(): assert merged.field_name == "a" -def test_var_data_merge_drops_field_names_of_other_states(): - """Field names are paired with a single state, so foreign ones are dropped. +def test_var_data_merge_keeps_field_names_of_every_state(): + """A var spanning several states keeps each state's own fields. - Callers register these names against `VarData.state`; keeping a name from - a different state would declare a dependency on a field that state has no - knowledge of. + Fields stay grouped by the state that owns them, so a dependency on a + composite var tracks every field it reads rather than only those of + whichever state happened to merge first. """ merged = VarData.merge( VarData(state="s", field_name="a"), VarData(state="other", field_name="b"), + VarData(state="s", field_name="c"), ) assert merged is not None + assert dict(merged.field_dependencies) == {"s": ("a", "c"), "other": ("b",)} + # The fallback accessors report the first state and its first field only. assert merged.state == "s" - assert merged.field_names == ("a",) + assert merged.field_names == ("a", "c") + assert merged.field_name == "a" + + +def test_var_data_field_dependencies_round_trip(): + """`state`/`field_name`/`field_names` are shorthands for the mapping.""" + assert dict(VarData(state="s", field_name="a").field_dependencies) == {"s": ("a",)} + assert dict(VarData(state="s", field_names=["a", "b"]).field_dependencies) == { + "s": ("a", "b") + } + # A state with no named field is still recorded: many vars carry only the + # state, for its imports and hooks, and read no field. + assert dict(VarData(state="s").field_dependencies) == {"s": ()} + assert dict(VarData().field_dependencies) == {} + # The canonical form wins over the shorthands. + assert dict( + VarData( + state="ignored", + field_name="ignored", + field_dependencies={"s": ("a",), "other": ("b",)}, + ).field_dependencies + ) == {"s": ("a",), "other": ("b",)} def test_var_data_field_name_shorthand_round_trips(): diff --git a/tests/units/test_state.py b/tests/units/test_state.py index f405146aa27..96818fd9bb0 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -13,7 +13,7 @@ import threading from collections.abc import AsyncGenerator, Callable, Mapping from textwrap import dedent -from typing import Any, ClassVar, Literal, TypeVar +from typing import Any, ClassVar, Literal, TypeVar, cast from unittest.mock import AsyncMock, Mock import pytest @@ -5783,3 +5783,45 @@ class ReannotatingChild(ReannotatedParent): reannotated_value: int # pyright: ignore[reportGeneralTypeIssues] assert isinstance(ReannotatingChild.reannotated_value, Var) + + +def test_composite_var_dep_tracks_fields_in_every_state(): + """A dependency on a var spanning two states must track both states' fields. + + `VarData` groups field names by the state that owns them, so merging a var + built from `StateA.a_field` with one built from `StateB.b_field` keeps + both. Before that grouping the merge kept only the first state's fields and + a computed var depending on the composite went stale whenever the other + state changed. + """ + from reflex_base.vars.base import Var, VarData + + class _CompositeDepStateA(rx.State): + a_field: str = "a" + + class _CompositeDepStateB(rx.State): + b_field: str = "b" + + composite = Var( + "combo", + _var_data=VarData.merge( + cast("Var", _CompositeDepStateA.a_field)._get_all_var_data(), + cast("Var", _CompositeDepStateB.b_field)._get_all_var_data(), + ), + ) + + a_name = _CompositeDepStateA.get_full_name() + b_name = _CompositeDepStateB.get_full_name() + assert dict(composite._dependency_fields()) == { + a_name: ("a_field",), + b_name: ("b_field",), + } + + class _CompositeDepConsumer(rx.State): + @rx.var(deps=[composite], cache=True) + def combined(self) -> str: + return "x" + + static_deps = _CompositeDepConsumer.__dict__["combined"]._static_deps + assert "a_field" in static_deps.get(a_name, set()) + assert "b_field" in static_deps.get(b_name, set()) From 8b4a59d1945e9c0f51d4418789bac867c996ad03 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 17:37:37 +0000 Subject: [PATCH 22/27] Address review of the VarData field dependency grouping Three points from the automated review of c67a9bc69: - `field_dependencies` was inserted before `imports` in `VarData.__init__`, so a caller passing four or more arguments positionally would have bound `imports` to the new parameter. No caller in the repo does, but VarData is public surface in reflex-base. Move it after the existing parameters. - The new composite-dependency test left its consumer registered in both source states' `_var_dependencies` and `_potentially_dirty_states`, which outlive the test; a later test dirtying either field would resolve the stale entry and raise on the missing substate. Tear them down, as the router dependency tests nearby already do. - The news fragment claimed existing readers are unaffected. That holds for a multi-state composite -- the old merge dropped other states' fields, so `field_names` reports the same thing it did before -- but not for a VarData carrying field names and no state, which used to be folded into the first state's list and now sits under its own "" key. Point readers at `field_dependencies` instead of claiming blanket compatibility. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- packages/reflex-base/news/7068.feature.md | 2 +- packages/reflex-base/src/reflex_base/vars/base.py | 10 ++++++---- tests/units/test_state.py | 10 ++++++++++ 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/packages/reflex-base/news/7068.feature.md b/packages/reflex-base/news/7068.feature.md index 6997058d111..718af1ca6ee 100644 --- a/packages/reflex-base/news/7068.feature.md +++ b/packages/reflex-base/news/7068.feature.md @@ -1 +1 @@ -`VarData` now tracks every state field a var is built from in `field_dependencies`, a mapping of state name to that state's field names, unioned and deduped as vars merge. `state`, `field_name` and `field_names` remain as fallback accessors reporting the first state and its fields, so existing readers are unaffected. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes — including fields belonging to a different state, which previously went untracked. +`VarData` now tracks every state field a var is built from in `field_dependencies`, a mapping of state name to that state's field names, unioned and deduped as vars merge. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes — including fields belonging to a different state, which previously went untracked. `state`, `field_name` and `field_names` remain as fallback accessors for the first state and its fields; read `field_dependencies` when you need every state a var reaches. diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 44e124a7b8f..0de4af6326a 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -354,13 +354,13 @@ def __init__( state: str = "", field_name: str = "", field_names: Sequence[str] | None = None, - field_dependencies: Mapping[str, Sequence[str]] | None = None, imports: ImmutableImportDict | ImmutableParsedImportDict | None = None, hooks: Mapping[str, VarData | None] | Sequence[str] | str | None = None, deps: list[Var] | None = None, position: Hooks.HookPosition | None = None, components: Iterable[BaseComponent] | None = None, app_wraps: Iterable[tuple[int, BaseComponent]] | None = None, + field_dependencies: Mapping[str, Sequence[str]] | None = None, ): """Initialize the var data. @@ -371,15 +371,17 @@ def __init__( single-entry ``field_names``; ignored when that is given. field_names: The names of the ``state`` fields this var is built from. Ignored when ``field_dependencies`` is given. - field_dependencies: Every state field this var is built from, - grouped by owning state. The canonical form; the three - arguments above are shorthands for a single state. imports: Imports needed to render this var. hooks: Hooks that need to be present in the component to render this var. deps: Dependencies of the var for useCallback. position: Position of the hook in the component. components: Components that are part of this var. app_wraps: App-level wrapper components this var requires when used. + field_dependencies: Every state field this var is built from, + grouped by owning state. The canonical form; ``state``, + ``field_name`` and ``field_names`` are shorthands for a single + state. Keyword-only in practice: it trails the older + parameters so positional callers of those are unaffected. """ if isinstance(hooks, str): hooks = [hooks] diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 96818fd9bb0..8bc38ebdb86 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5825,3 +5825,13 @@ def combined(self) -> str: static_deps = _CompositeDepConsumer.__dict__["combined"]._static_deps assert "a_field" in static_deps.get(a_name, set()) assert "b_field" in static_deps.get(b_name, set()) + + # The consumer registered itself in both source states' class-level + # dependency maps, which outlive this test. Left behind, a later test that + # dirties a_field or b_field resolves the stale entry and raises on the + # missing substate. Drop them. + consumer_name = _CompositeDepConsumer.get_full_name() + for state_cls in (_CompositeDepStateA, _CompositeDepStateB): + for dep_set in state_cls._var_dependencies.values(): + dep_set.difference_update({(consumer_name, "combined")}) + state_cls._potentially_dirty_states.discard(consumer_name) From 1d93bfecc34d9cdb8bde5824209fe3b180309f60 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 17:55:48 +0000 Subject: [PATCH 23/27] Drop the field_names accessor from VarData `field_names` was not something `main` exposes -- it was introduced by an earlier iteration of this PR, so keeping it as a "fallback accessor" added a net-new public name to `VarData` purely to stay compatible with an intermediate state of the same branch. Nothing downstream has ever seen it. Remove the property, the `field_names=` constructor argument and the branch in `_normalize_field_dependencies` that consumed it. `field_name` now reads the mapping directly. `state` and `field_name` stay, since those are the two names `main` actually has. Against `main`, `field_dependencies` is now the only public name this adds to `VarData`; every remaining `field_names` in the diff is a local loop variable. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- packages/reflex-base/news/7068.feature.md | 2 +- .../reflex-base/src/reflex_base/vars/base.py | 40 +++++-------------- tests/units/reflex_base/vars/test_base.py | 19 ++++----- 3 files changed, 17 insertions(+), 44 deletions(-) diff --git a/packages/reflex-base/news/7068.feature.md b/packages/reflex-base/news/7068.feature.md index 718af1ca6ee..1240ddc60e4 100644 --- a/packages/reflex-base/news/7068.feature.md +++ b/packages/reflex-base/news/7068.feature.md @@ -1 +1 @@ -`VarData` now tracks every state field a var is built from in `field_dependencies`, a mapping of state name to that state's field names, unioned and deduped as vars merge. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes — including fields belonging to a different state, which previously went untracked. `state`, `field_name` and `field_names` remain as fallback accessors for the first state and its fields; read `field_dependencies` when you need every state a var reaches. +`VarData` now tracks every state field a var is built from in `field_dependencies`, a mapping of state name to that state's field names, unioned and deduped as vars merge. A computed var depending on a composite var (`deps=[SomeState.composite]`) is now invalidated when any of its underlying fields changes — including fields belonging to a different state, which previously went untracked. `state` and `field_name` still report the first state and its first field, so existing readers are unaffected; read `field_dependencies` when you need every state a var reaches. diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 0de4af6326a..9835da40557 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -280,28 +280,23 @@ def _normalize_field_dependencies( field_dependencies: Mapping[str, Sequence[str]] | None, state: str, field_name: str, - field_names: Sequence[str] | None, ) -> Mapping[str, tuple[str, ...]]: """Build the canonical state -> fields mapping from the accepted shorthands. Args: field_dependencies: The canonical mapping, if the caller gave one. - state: The single enclosing state, for the shorthand forms. + state: The single enclosing state, for the shorthand form. field_name: A single field of `state`. - field_names: Several fields of `state`; takes precedence over `field_name`. Returns: - An immutable mapping of state name to its deduped field names. + A mapping of state name to its deduped field names. """ if field_dependencies is not None: return { state_name: tuple(dict.fromkeys(names)) for state_name, names in field_dependencies.items() } - if field_names is not None: - names = tuple(dict.fromkeys(field_names)) - else: - names = (field_name,) if field_name else () + names = (field_name,) if field_name else () # A state with no named field still has to be recorded: plenty of vars # carry only the state (for imports and hooks) and nothing reads a field. if not state and not names: @@ -353,7 +348,6 @@ def __init__( self, state: str = "", field_name: str = "", - field_names: Sequence[str] | None = None, imports: ImmutableImportDict | ImmutableParsedImportDict | None = None, hooks: Mapping[str, VarData | None] | Sequence[str] | str | None = None, deps: list[Var] | None = None, @@ -367,10 +361,8 @@ def __init__( Args: state: The name of the enclosing state. Shorthand for a single-state ``field_dependencies``; ignored when that is given. - field_name: The name of the field in ``state``. Shorthand for a - single-entry ``field_names``; ignored when that is given. - field_names: The names of the ``state`` fields this var is built - from. Ignored when ``field_dependencies`` is given. + field_name: The name of the field in ``state``. Ignored when + ``field_dependencies`` is given. imports: Imports needed to render this var. hooks: Hooks that need to be present in the component to render this var. deps: Dependencies of the var for useCallback. @@ -379,8 +371,8 @@ def __init__( app_wraps: App-level wrapper components this var requires when used. field_dependencies: Every state field this var is built from, grouped by owning state. The canonical form; ``state``, - ``field_name`` and ``field_names`` are shorthands for a single - state. Keyword-only in practice: it trails the older + and ``field_name`` are the shorthand for a single state with a + single field. Keyword-only in practice: it trails the older parameters so positional callers of those are unaffected. """ if isinstance(hooks, str): @@ -393,9 +385,7 @@ def __init__( object.__setattr__( self, "field_dependencies", - _normalize_field_dependencies( - field_dependencies, state, field_name, field_names - ), + _normalize_field_dependencies(field_dependencies, state, field_name), ) object.__setattr__(self, "imports", immutable_imports) object.__setattr__(self, "hooks", tuple(hooks or {})) @@ -433,18 +423,6 @@ def state(self) -> str: """ return next(iter(self.field_dependencies), "") - @property - def field_names(self) -> tuple[str, ...]: - """The names of the fields this var is built from, in ``state``. - - Deprecated fallback accessor: fields owned by any other state are not - reported. Read ``field_dependencies`` to see every state's fields. - - Returns: - The first state's field names, empty if there are none. - """ - return self.field_dependencies.get(self.state, ()) - @property def field_name(self) -> str: """The name of the field in the state. @@ -456,7 +434,7 @@ def field_name(self) -> str: Returns: The first field name, or an empty string if there is none. """ - field_names = self.field_names + field_names = self.field_dependencies.get(self.state, ()) return field_names[0] if field_names else "" def old_school_imports(self) -> ImportDict: diff --git a/tests/units/reflex_base/vars/test_base.py b/tests/units/reflex_base/vars/test_base.py index 01c87f3bc7b..c6e59c59f34 100644 --- a/tests/units/reflex_base/vars/test_base.py +++ b/tests/units/reflex_base/vars/test_base.py @@ -275,7 +275,7 @@ def test_var_data_merge_collects_field_names(): ) assert merged is not None - assert merged.field_names == ("a", "b") + assert dict(merged.field_dependencies) == {"s": ("a", "b")} # `field_name` stays the first, so existing single-field readers are intact. assert merged.field_name == "a" @@ -297,21 +297,17 @@ def test_var_data_merge_keeps_field_names_of_every_state(): assert dict(merged.field_dependencies) == {"s": ("a", "c"), "other": ("b",)} # The fallback accessors report the first state and its first field only. assert merged.state == "s" - assert merged.field_names == ("a", "c") assert merged.field_name == "a" def test_var_data_field_dependencies_round_trip(): - """`state`/`field_name`/`field_names` are shorthands for the mapping.""" + """`state`/`field_name` are the shorthand for a single-field mapping.""" assert dict(VarData(state="s", field_name="a").field_dependencies) == {"s": ("a",)} - assert dict(VarData(state="s", field_names=["a", "b"]).field_dependencies) == { - "s": ("a", "b") - } # A state with no named field is still recorded: many vars carry only the # state, for its imports and hooks, and read no field. assert dict(VarData(state="s").field_dependencies) == {"s": ()} assert dict(VarData().field_dependencies) == {} - # The canonical form wins over the shorthands. + # The canonical form wins over the shorthand. assert dict( VarData( state="ignored", @@ -321,11 +317,10 @@ def test_var_data_field_dependencies_round_trip(): ) == {"s": ("a",), "other": ("b",)} -def test_var_data_field_name_shorthand_round_trips(): - """`field_name` is shorthand for a single-entry `field_names`.""" - assert VarData(field_name="a").field_names == ("a",) - assert VarData(field_names=["a", "b"]).field_name == "a" - assert VarData().field_names == () +def test_var_data_field_name_reports_the_first_field(): + """`field_name` reports the first field of the first state.""" + assert VarData(field_name="a").field_name == "a" + assert VarData(field_dependencies={"s": ("a", "b")}).field_name == "a" assert VarData().field_name == "" From 22ef10ce6720603450fc6d9ed2d53c7f8c79c3d3 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 17 Sep 2026 18:03:23 +0000 Subject: [PATCH 24/27] Name the legacy router pickle key and cover the drop `__setstate__` popped a bare `"router"` literal. Extract it, but not to `constants.ROUTER`: that constant names the public switchboard as it is today, while this is a historical name frozen into pickle payloads already on disk. Renaming the switchboard must not change what those are keyed by, so the two are independent despite sharing a value now. The drop also had no test, though the PR description claims it works. Add one, verified to fail without the pop -- restoring the legacy entry routes `router` through the descriptor's setter and raises SetUndefinedStateVarError, which is the crash the line prevents. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- reflex/state.py | 10 ++++++++-- tests/units/test_state.py | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/reflex/state.py b/reflex/state.py index d9519160fb3..1af291f8555 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -91,6 +91,12 @@ from reflex.utils import console, format, types from reflex.utils.exec import is_testing_env +# The key a pre-split pickle stored the whole RouterData under. Deliberately +# not `constants.ROUTER`: that names the public switchboard as it is today, +# while this is a historical name frozen into payloads already on disk, and +# renaming the switchboard must not change what those are keyed by. +_LEGACY_ROUTER_PICKLE_KEY = "router" + # Shared empty router defaults. Each is a frozen dataclass whose members are # themselves immutable, so one instance can back every state's field instead # of being rebuilt per state. @@ -2573,10 +2579,10 @@ def __setstate__(self, state: builtins.dict[str, Any]): """ state["parent_state"] = None state["substates"] = {} - # Pre-split pickles stored a RouterData under `router`, which is now a + # Pre-split pickles stored a RouterData under this key, which is now a # descriptor; drop it so unpickling does not route through the setter. # The schema check in _deserialize discards such states anyway. - state.pop("router", None) + state.pop(_LEGACY_ROUTER_PICKLE_KEY, None) for key, value in state.items(): object.__setattr__(self, key, value) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 8bc38ebdb86..29394407e36 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -5835,3 +5835,30 @@ def combined(self) -> str: for dep_set in state_cls._var_dependencies.values(): dep_set.difference_update({(consumer_name, "combined")}) state_cls._potentially_dirty_states.discard(consumer_name) + + +def test_setstate_drops_the_legacy_router_entry(): + """Unpickling a pre-split state must not route `router` through the setter. + + Older pickles stored the whole `RouterData` under `router`, which is now a + descriptor. Restoring it with `object.__setattr__` would shadow that + descriptor on the instance; assigning it would decompose into the per-field + vars and resurrect stale connection data. The schema check in + `_deserialize` discards such states anyway, so the entry is simply dropped. + """ + state = BaseState(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] + legacy = { + "parent_state": None, + "substates": {}, + "router": RouterData.from_router_data({ + constants.RouteVar.CLIENT_TOKEN: "stale-token", + }), + "dirty_vars": set(), + } + + state.__setstate__(legacy) + + # The entry is gone rather than shadowing the descriptor... + assert "router" not in state.__dict__ + # ...and `router` still resolves through the switchboard to live fields. + assert state.router.session.client_token == "" From 3be9b1ed8a797b3f3b657530ddf1ab3bfba7303a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 02:37:56 +0000 Subject: [PATCH 25/27] Persist the router URL as the URL, not as its parsed pieces Splitting the router into per-field vars moved the page URL into `URLData`, a dataclass mirroring every parsed component of `ReflexURL`. Both of them store the derived components eagerly, so a state write pickled the URL text eight times over -- and `URLData` is the storage form of a router var, so that happened on every write. `ReflexURL.__reduce__` and `URLData.__reduce__` now persist only the URL itself and re-split it on the way back in. Measured on the state-manager benchmark tree (root + three substates), against main: main before after root state pickle 805 B 883 B 741 B whole tree pickle 2117 B 2195 B 2053 B tree pickle time 35.62us 38.95us 34.27us So the write path is now cheaper than main rather than 9% dearer, and loading is cheaper too (URLData unpickle 8.38us -> 5.46us), which is the `test_get_state_uncached` direction. Profiling `set_state` against main leaves one difference: 9 extra `dict.pop` calls per write, 0.1% of the operation, inherent to skipping five router vars instead of one. Fixing this surfaced a real divergence. `URLData`'s defaults were written out by hand as all-empty, but `ReflexURL("").origin` is "://", so `URLData()` and `URLData.from_url(ReflexURL(""))` disagreed on `origin`. A fresh state therefore reported "" where main reports "://", contradicting this PR's claim that the router API is unchanged -- and with `__reduce__` in place the value would also have changed across a save/load cycle. The defaults are now read off the empty URL, so the two construction paths cannot drift again and the emitted payload matches main byte for byte. The two test fixtures that had encoded the wrong origin are corrected. `"://"` as the origin of an empty URL is odd, but it is main's behavior and changing it belongs in its own change, not in a performance one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- news/7068.performance.md | 2 + reflex/istate/data.py | 65 ++++++++++++++++++++++++----- tests/units/istate/test_data.py | 70 ++++++++++++++++++++++++++++++++ tests/units/test_state.py | 2 +- tests/units/utils/test_format.py | 2 +- 5 files changed, 128 insertions(+), 13 deletions(-) diff --git a/news/7068.performance.md b/news/7068.performance.md index 3c56ecabc0e..9908f7676a3 100644 --- a/news/7068.performance.md +++ b/news/7068.performance.md @@ -1 +1,3 @@ Store router data in separate base vars (session, headers, page, url, route_id) so a navigation delta only re-sends the fields that changed instead of the whole router, and gather the connection-scoped router data (headers, client IP, session id) once at connect time rather than on every event. `State.router` is unchanged for app code. + +The page URL is also persisted as the URL itself rather than as its parsed pieces: `ReflexURL` and `URLData` re-split on the way out of the state store instead of writing scheme, netloc, origin, path, query, query parameters and fragment alongside the href on every state write. diff --git a/reflex/istate/data.py b/reflex/istate/data.py index b43557d513d..88bf69a1b8f 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -1,7 +1,7 @@ """This module contains the dataclasses representing the router object.""" import dataclasses -from collections.abc import Mapping +from collections.abc import Callable, Mapping from types import MappingProxyType from typing import TYPE_CHECKING, Any, ClassVar, Final, NoReturn from urllib.parse import _NetlocResultMixinStr, parse_qsl, urlsplit @@ -190,6 +190,19 @@ def __delattr__(self, name: str) -> NoReturn: msg = f"cannot delete {name!r}: ReflexURL is immutable" raise AttributeError(msg) + def __reduce__(self) -> tuple[type["ReflexURL"], tuple[str]]: + """Persist only the URL itself, re-splitting it on the way back in. + + Every parsed component is derived from the string by ``__new__``, so + pickling them as well writes the URL into the state store several times + over. Reconstructing costs one ``urlsplit`` and is cheaper than reading + the components back. + + Returns: + The callable and argument that rebuild this URL. + """ + return (type(self), (str.__str__(self),)) + @serializer(to=dict) def _serialize_reflex_url(obj: ReflexURL) -> dict: @@ -430,6 +443,11 @@ def _url_from_router_data(router_data: dict) -> ReflexURL: ) +# The parsed empty URL, shared as every URLData default: it is immutable, so +# one instance can back every state that has not navigated yet. +_EMPTY_URL: Final = ReflexURL("") + + @dataclasses.dataclass(frozen=True) class URLData: """The parsed components of the current page URL. @@ -440,19 +458,20 @@ class URLData: receives the parsed component dict. """ - scheme: str = "" - netloc: str = "" - origin: str = "" - path: str = "" - query: str = "" - query_parameters: Mapping[str, str] = dataclasses.field( - default_factory=_FrozenDictStrStr - ) - fragment: str = "" + # Every default is read off the empty URL rather than written out here, so + # `URLData()` is exactly `URLData.from_url(ReflexURL(""))`. Spelled out by + # hand they drifted: `ReflexURL("").origin` is "://", not "". + scheme: str = _EMPTY_URL.scheme + netloc: str = _EMPTY_URL.netloc + origin: str = _EMPTY_URL.origin + path: str = _EMPTY_URL.path + query: str = _EMPTY_URL.query + query_parameters: Mapping[str, str] = _EMPTY_URL.query_parameters + fragment: str = _EMPTY_URL.fragment # Annotated str so the frontend var for this field renders the raw href # string, but always holds a ReflexURL at runtime so the backend keeps # parsed-component access without re-splitting the URL. - href: str = ReflexURL("") + href: str = _EMPTY_URL @classmethod def from_url(cls, url: ReflexURL) -> "URLData": @@ -487,6 +506,30 @@ def from_router_data(cls, router_data: dict) -> "URLData": """ return cls.from_url(_url_from_router_data(router_data)) + def __reduce__(self) -> tuple[Callable[[str], "URLData"], tuple[str]]: + """Persist only the href, deriving the components again on the way back. + + Every other field is a parsed piece of ``href``, so storing them too + writes the URL into the state store eight times over. This is the + storage form of a router var, so it is pickled on every state write. + + Returns: + The callable and argument that rebuild this URLData. + """ + return (_url_data_from_href, (str.__str__(self.href),)) + + +def _url_data_from_href(href: str) -> URLData: + """Rebuild a URLData from the raw href alone. + + Args: + href: the full URL string. + + Returns: + A URLData with every component re-derived from the URL. + """ + return URLData.from_url(ReflexURL(href)) + @serializer(to=dict) def _serialize_url_data(obj: URLData) -> dict: diff --git a/tests/units/istate/test_data.py b/tests/units/istate/test_data.py index c27be366b7e..d7af58128dc 100644 --- a/tests/units/istate/test_data.py +++ b/tests/units/istate/test_data.py @@ -279,3 +279,73 @@ class _URLIsolationState(BaseState): assert one.router.url.path == "/real" assert two.router.url.path == "" assert cast("ReflexURL", URLData().href).path == "" + + +def test_url_data_default_matches_the_parsed_empty_url(): + """`URLData()` must equal `URLData.from_url(ReflexURL(""))`. + + The two are different construction paths to the same "not navigated yet" + value: the dataclass defaults back every fresh state, while `from_url` is + what `__reduce__` rebuilds a persisted one through. Written out by hand the + defaults drifted -- `ReflexURL("").origin` is "://", not "" -- so a state + reported one origin before a save and another after. + """ + from reflex.istate.data import URLData + + assert URLData() == URLData.from_url(ReflexURL("")) + + +@pytest.mark.parametrize( + "raw", + [ + "", + SAMPLE_URL, + "http://x/", + "https://a.b/c?d=1&d=2#f", + ], +) +def test_reflex_url_and_url_data_survive_pickling(raw: str): + """Both persist through a pickle round-trip with every component intact. + + `ReflexURL` and `URLData` persist only the URL itself and re-split it on + the way back, so this pins that the derived components come back equal + rather than being silently dropped or recomputed differently. + """ + import pickle + + from reflex.istate.data import URLData + + components = ("scheme", "netloc", "origin", "path", "query", "fragment") + + url = ReflexURL(raw) + restored_url = pickle.loads(pickle.dumps(url)) + assert type(restored_url) is ReflexURL + assert str(restored_url) == raw + for component in components: + assert getattr(restored_url, component) == getattr(url, component) + assert dict(restored_url.query_parameters) == dict(url.query_parameters) + + data = URLData.from_url(url) + restored_data = pickle.loads(pickle.dumps(data)) + assert restored_data == data + # The runtime href must still be a ReflexURL, or backend component access + # through `self.router.url` breaks after a state is loaded from the store. + assert isinstance(restored_data.href, ReflexURL) + + +def test_pickling_a_url_does_not_store_its_derived_components(): + """The persisted form must carry the URL once, not every parsed piece. + + `URLData` is the storage form of a router var, so it is pickled on every + state write. Storing the seven derived components alongside `href` wrote + the URL into the state store eight times over. + """ + import pickle + + from reflex.istate.data import URLData + + blob = pickle.dumps(URLData.from_url(ReflexURL(SAMPLE_URL))) + # Every component is derivable from the href, so the href is the only + # occurrence of the URL text in the payload. + assert blob.count(b"example.com") == 1 + assert b"query_parameters" not in blob diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 870208aa097..9acbc9deb1d 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -93,7 +93,7 @@ "rx_router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "", + "origin": "://", "path": "", "query": "", "query_parameters": {}, diff --git a/tests/units/utils/test_format.py b/tests/units/utils/test_format.py index 06519d46b55..3b89da9c455 100644 --- a/tests/units/utils/test_format.py +++ b/tests/units/utils/test_format.py @@ -662,7 +662,7 @@ def test_format_query_params(input, output): "rx_router_url" + FIELD_MARKER: { "scheme": "", "netloc": "", - "origin": "", + "origin": "://", "path": "", "query": "", "query_parameters": {}, From ef47368a01629477a261f7f84e8d39eeac92092c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:25:19 +0000 Subject: [PATCH 26/27] Resolve get_var_value(State.router) and name the deprecated var Review found four things on the current head. `get_var_value(State.router)` raised UnretrievableVarValueError. The switchboard renders as an object literal over the five per-field vars and had no var data of its own, so there was no field to read. It now names the `router` attribute it stands for, and the existing generic path resolves it through the property to the composed RouterData. Probing every router form against main shows the split traded one case for three rather than simply losing one: main before after router ok FAIL ok router.session FAIL ok ok router.page FAIL ok ok router.route_id FAIL ok ok router.url FAIL FAIL FAIL `router.url` fails identically on main -- an item operation through a cast carries no state on its own var data -- so it is not this PR's and is left alone. Naming the field puts `router` in the declared dep set alongside the five, which made the legacy-string expansion warn for `deps=[State.router]` too. The warning is now gated on the string form, which is the only one that arrives without the per-field names, and it identifies the computed var: the dep scan is lazy, so the caller frame `console.deprecate` reports is unrelated to the declaration and was pointing at `:106`. Registration still resolves to exactly the five fields; nothing is registered against `router`, which has no backing field and would never be dirtied. `deprecation_version` stays 0.9.12. The review read the latest tag as v0.9.11a2 with 0.9.11 unreleased, but `git tag --sort=-v:refname` sorts the prerelease above the release: v0.9.11 was tagged 2026-09-11, after a2, and v0.9.11.post1 on 2026-09-15 is the newest release tag. Also drop "Deprecated fallback accessor" from the `VarData.state` and `field_name` docstrings -- nothing deprecates them and the framework still reads both -- and shorten the narrative comments in app.py, base_state_processor.py, state.py and data.py to the present behavior. tests/units: 9604 passed, 6 skipped, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../event/processor/base_state_processor.py | 11 +-- .../reflex-base/src/reflex_base/vars/base.py | 10 +-- reflex/app.py | 11 +-- reflex/istate/data.py | 5 +- reflex/state.py | 39 +++++---- tests/units/test_state.py | 79 ++++++++++++++++++- 6 files changed, 113 insertions(+), 42 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py index f8990ac6283..0e60983b572 100644 --- a/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py +++ b/packages/reflex-base/src/reflex_base/event/processor/base_state_processor.py @@ -439,14 +439,9 @@ async def _execute_event( merged_router_data = state._update_router_vars( router_data, previous_router_data ) - # Store what it merged rather than the payload: a partial one - # would otherwise drop the keys it omits for the next event. - # Only when that actually differs, though -- a payload that - # merges to what is already there changed nothing, and the - # assignment would still dirty router_data and mark the state - # touched, persisting it for an event that moved nothing. - # The assignment recurses into substates and forces - # recalculation of dependent ComputedVar (dynamic route vars). + # Store what it merged, not the payload, so a partial payload + # does not drop the keys it omits. Only on a real change: the + # assignment dirties router_data and marks the state touched. if merged_router_data != previous_router_data: state.router_data = merged_router_data diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index bc39df81e0a..a849fee851f 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -416,9 +416,8 @@ def __init__( def state(self) -> str: """The name of the enclosing state. - Deprecated fallback accessor: a var may be built from fields of more - than one state, and this reports only the first. Read - ``field_dependencies`` to see every state. + A var may be built from fields of more than one state; this reports + only the first. Read ``field_dependencies`` to see every state. Returns: The first state name, or an empty string if there is none. @@ -429,9 +428,8 @@ def state(self) -> str: def field_name(self) -> str: """The name of the field in the state. - Deprecated fallback accessor: a var built from several fields reports - only the first, of the first state. Read ``field_dependencies`` to see - all of them. + A var built from several fields reports only the first, of the first + state. Read ``field_dependencies`` to see all of them. Returns: The first field name, or an empty string if there is none. diff --git a/reflex/app.py b/reflex/app.py index 7b23b81f9d8..5bed9c5ac11 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -2417,15 +2417,10 @@ async def link_token_to_sid(self, sid: str, token: str): BaseStateToken(ident=new_token or token, cls=self.app._state) ) as state: state.router_data[constants.RouteVar.SESSION_ID] = sid - # The state is loaded under this identity, so record it rather - # than waiting for the first event to fill it in: duplicate-token - # handling hands back a fresh token here, and until router_data - # carries it, anything reading rx_router_session.client_token (a - # background task, a shared-state link) addresses the wrong tree. + # Record the identity the state was loaded under; duplicate-token + # handling can hand back a fresh one here. state.router_data[constants.RouteVar.CLIENT_TOKEN] = new_token or token - # Rebuild from router_data (rather than replacing the field on - # the existing value) to keep the session var and router_data - # in step, the same way the event processor refreshes it. + # Rebuild from router_data to keep the session var in step with it. if ( session := SessionData.from_router_data(state.router_data) ) != state.rx_router_session: diff --git a/reflex/istate/data.py b/reflex/istate/data.py index 88bf69a1b8f..d70962e8b2c 100644 --- a/reflex/istate/data.py +++ b/reflex/istate/data.py @@ -458,9 +458,8 @@ class URLData: receives the parsed component dict. """ - # Every default is read off the empty URL rather than written out here, so - # `URLData()` is exactly `URLData.from_url(ReflexURL(""))`. Spelled out by - # hand they drifted: `ReflexURL("").origin` is "://", not "". + # Read off the empty URL so `URLData()` is exactly + # `URLData.from_url(ReflexURL(""))` -- note `ReflexURL("").origin` is "://". scheme: str = _EMPTY_URL.scheme netloc: str = _EMPTY_URL.netloc origin: str = _EMPTY_URL.origin diff --git a/reflex/state.py b/reflex/state.py index c9308400de0..d332aef5a14 100644 --- a/reflex/state.py +++ b/reflex/state.py @@ -92,10 +92,8 @@ from reflex.utils import console, format, types from reflex.utils.exec import is_testing_env -# The key a pre-split pickle stored the whole RouterData under. Deliberately -# not `constants.ROUTER`: that names the public switchboard as it is today, -# while this is a historical name frozen into payloads already on disk, and -# renaming the switchboard must not change what those are keyed by. +# The key a pre-split pickle stored the whole RouterData under. Not +# `constants.ROUTER`: this name is frozen into payloads already on disk. _LEGACY_ROUTER_PICKLE_KEY = "router" # Shared empty router defaults. Each is a frozen dataclass whose members are @@ -509,6 +507,13 @@ def _get_router_var(cls: type[BaseState]) -> RouterDataVar: page=base_vars[constants.ROUTER_PAGE], url=base_vars[constants.ROUTER_URL], route_id=base_vars[constants.ROUTER_ROUTE_ID], + # Name the `router` attribute the switchboard stands for, so + # `get_var_value(State.router)` resolves it through the property + # and hands back the composed RouterData, as it does on a state + # with a single `router` base var. + _var_data=VarData( + state=root_cls.get_full_name(), field_name=constants.ROUTER + ), ) setattr(root_cls, "_reflex_router_var", router_var) # noqa: B010 return router_var @@ -1197,16 +1202,19 @@ def _init_var_dependency_dicts(cls): continue for state_name, dvar_set in cvar._deps(objclass=cls).items(): if constants.ROUTER in dvar_set: - # Legacy explicit dependency on the pre-split `router` var: - # depend on all the per-field router vars instead. - console.deprecate( - feature_name='ComputedVar deps=["router"]', - reason="the router var was split; depend on the router" - " Var instead (e.g. deps=[State.router.url] for one" - " field, or deps=[State.router] for all of them).", - deprecation_version="0.9.12", - removal_version="1.0", - ) + # `router` names the switchboard, which has no field of its + # own: depend on the per-field router vars instead. The Var + # form already carries them, so only the legacy string form + # arrives here without them, and only it is deprecated. + if dvar_set.isdisjoint(constants.ROUTER_VARS): + console.deprecate( + feature_name=f'ComputedVar deps=["router"] on {cls.__name__}.{cvar_name}', + reason="the router var was split; depend on the router" + " Var instead (e.g. deps=[State.router.url] for one" + " field, or deps=[State.router] for all of them).", + deprecation_version="0.9.12", + removal_version="1.0", + ) dvar_set = (dvar_set - {constants.ROUTER}) | set( constants.ROUTER_VARS ) @@ -2584,9 +2592,8 @@ def __setstate__(self, state: builtins.dict[str, Any]): """ state["parent_state"] = None state["substates"] = {} - # Pre-split pickles stored a RouterData under this key, which is now a + # Pre-split pickles stored a RouterData under this key, now a # descriptor; drop it so unpickling does not route through the setter. - # The schema check in _deserialize discards such states anyway. state.pop(_LEGACY_ROUTER_PICKLE_KEY, None) for key, value in state.items(): object.__setattr__(self, key, value) diff --git a/tests/units/test_state.py b/tests/units/test_state.py index 9acbc9deb1d..f4f6c134fa8 100644 --- a/tests/units/test_state.py +++ b/tests/units/test_state.py @@ -4161,6 +4161,75 @@ def foo(self) -> str: State._potentially_dirty_states.discard(LegacyRouterCompileState.get_full_name()) +@pytest.mark.asyncio +async def test_get_var_value_of_the_whole_router() -> None: + """`get_var_value(State.router)` must hand back the composed RouterData. + + The switchboard renders as an object literal over the five per-field vars, + so it has no field of its own to read. Without naming the `router` + attribute it stands for, this raised UnretrievableVarValueError, while a + state with a single `router` base var resolved it. + """ + state = State(_reflex_internal_init=True) # pyright: ignore [reportCallIssue] + + router = await state.get_var_value(State.router) + + assert isinstance(router, RouterData) + # The per-field vars resolve too, which the pre-split single var could not do. + assert await state.get_var_value(State.router.route_id) == router.route_id + assert ( + await state.get_var_value(State.router.session) + ).client_token == router.session.client_token + + +def test_router_var_dep_does_not_warn_for_the_var_form( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Only the legacy string form is deprecated, and it must name the var. + + `State.router` carries the per-field names as well as `router` itself, so + the expansion has nothing to warn about; `deps=["router"]` arrives with + only `router` and does. The warning has to identify the computed var, + because the lazy dep scan means the reported caller frame is unrelated to + the declaration. + """ + # `console.deprecate` logs and dedupes rather than printing, so record the + # calls instead of scraping output. + from reflex import state as state_module + + deprecations: list[str] = [] + monkeypatch.setattr( + state_module.console, + "deprecate", + lambda *, feature_name, **kwargs: deprecations.append(feature_name), + ) + + class VarFormRouterDepState(State): + """A state depending on the router through the Var.""" + + @rx.var(deps=[State.router], auto_deps=False) + def from_var(self) -> str: + return "" + + assert deprecations == [] + + class StringFormRouterDepState(State): + """A state depending on the router through the legacy string.""" + + @rx.var(deps=["router"], auto_deps=False) + def from_string(self) -> str: + return "" + + assert len(deprecations) == 1 + assert "StringFormRouterDepState.from_string" in deprecations[0] + + for dep_set in State._var_dependencies.values(): + dep_set.discard((VarFormRouterDepState.get_full_name(), "from_var")) + dep_set.discard((StringFormRouterDepState.get_full_name(), "from_string")) + State._potentially_dirty_states.discard(VarFormRouterDepState.get_full_name()) + State._potentially_dirty_states.discard(StringFormRouterDepState.get_full_name()) + + def test_router_var_dep_whole_router() -> None: """deps=[State.router] must track every per-field router var. @@ -4178,14 +4247,22 @@ class WholeRouterDepState(State): def summary(self) -> str: return "" + # The declared set also names `router` itself, the switchboard the five + # fields were read through; it is expanded away before registration. assert WholeRouterDepState.computed_vars["summary"]._static_deps == { - State.get_full_name(): set(constants.ROUTER_VARS) + State.get_full_name(): {constants.ROUTER, *constants.ROUTER_VARS} } for router_var in constants.ROUTER_VARS: assert ( WholeRouterDepState.get_full_name(), "summary", ) in State._var_dependencies[router_var] + # `router` has no backing field, so nothing may be registered against it -- + # it would never be dirtied and the dependent var would go stale. + assert ( + WholeRouterDepState.get_full_name(), + "summary", + ) not in State._var_dependencies.get(constants.ROUTER, set()) # Drop the class-level registrations; see the note in test_router_var_dep. for dep_set in State._var_dependencies.values(): From f07fc21947a39503a0a29b41d8df22d2bcd297fa Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 18 Sep 2026 19:52:14 +0000 Subject: [PATCH 27/27] Build the merged field dependencies in one pass CodSpeed flagged test_cond_operations at -3.7% on the current head. It is this PR's: `rx.cond` merges the VarData of its three operands, and carrying a per-state field mapping costs more than main's two short-circuiting scans over a stored `state`/`field_name` pair. Two passes were wasted. `merge` rebuilt a state's tuple once per contributing var, via `dict.fromkeys` over the previous tuple spread with the new names; then `VarData.__init__` normalized the result and rebuilt every tuple again. It now accumulates ordered sets and hands those to the constructor, which materializes each tuple exactly once. `__init__` already accepted any iterable of names, so only the annotation widens. Measured on the benchmark's own shapes, min of 3 runs of 9: main base before after cond x44 2.359 ms 2.55 ms 2.50 ms VarData.merge x1000 13.34 ms 15.3 ms 14.3 ms So merge goes from ~13% over the base to ~7%, and the benchmark from ~7% to ~5.5%. The remainder is structural: a `Mapping[str, tuple[str, ...]]` cannot be merged as cheaply as picking the first non-empty of two strings, and that mapping is what makes a composite var track fields across every state it reaches. Left there rather than traded away. Note the CodSpeed run compared against 8c06e79 rather than the real base a1ba536, having found no successful run on the latter, so its percentage is against a slightly different tree than this branch merges. tests/units: 9663 passed, 6 skipped, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Mwk1pagH3KZ884Xw55BNMh --- .../reflex-base/src/reflex_base/vars/base.py | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index a849fee851f..49f2e50fab2 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -279,7 +279,7 @@ def insert_app_wraps( def _normalize_field_dependencies( - field_dependencies: Mapping[str, Sequence[str]] | None, + field_dependencies: Mapping[str, Iterable[str]] | None, state: str, field_name: str, ) -> Mapping[str, tuple[str, ...]]: @@ -356,7 +356,7 @@ def __init__( position: Hooks.HookPosition | None = None, components: Iterable[BaseComponent] | None = None, app_wraps: Iterable[tuple[int, BaseComponent]] | None = None, - field_dependencies: Mapping[str, Sequence[str]] | None = None, + field_dependencies: Mapping[str, Iterable[str]] | None = None, ): """Initialize the var data. @@ -469,13 +469,18 @@ def merge(*all: VarData | None) -> VarData | None: # Union every state's fields, in order and deduped, so a var composed # of several fields -- across as many states as it reaches -- carries - # all of them and a dependency on it tracks each one. - field_dependencies: dict[str, tuple[str, ...]] = {} + # all of them and a dependency on it tracks each one. Accumulated as + # ordered sets and materialized once: this runs for every var + # operation, so rebuilding a tuple per contributing var costs. + seen_fields: dict[str, dict[str, None]] = {} for var_data in all_var_datas: for state_name, names in var_data.field_dependencies.items(): - field_dependencies[state_name] = tuple( - dict.fromkeys((*field_dependencies.get(state_name, ()), *names)) - ) + seen = seen_fields.get(state_name) + if seen is None: + seen_fields[state_name] = dict.fromkeys(names) + else: + for name in names: + seen[name] = None hooks: dict[str, VarData | None] = { hook: None for var_data in all_var_datas for hook in var_data.hooks @@ -517,7 +522,7 @@ def merge(*all: VarData | None) -> VarData | None: insert_app_wraps(app_wraps, var_data.app_wraps) return VarData( - field_dependencies=field_dependencies, + field_dependencies=seen_fields, imports=imports_, hooks=hooks, deps=deps,