From 6ea1eb00958e9f45736240ae3031154a4c867137 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Mon, 4 May 2026 15:27:38 +0500 Subject: [PATCH 01/10] feat: add app_wraps to VarData for Var-driven provider injection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lets Vars declare app-level wrapper components in their VarData so the compiler can mount providers (state context, event loop, upload, etc.) based on what's actually used, instead of relying on hardcoded chains or special-case logic. State/event-loop providers now ride along on VarData.from_state and the events-hook helper, and UploadFilesProvider is mounted when selected_files/upload_file is referenced — even without an Upload component on the page. Layout renders the assembled chain so AppWrap reduces to hooks + children. --- .../src/reflex_base/compiler/templates.py | 13 +- .../src/reflex_base/components/component.py | 16 +- .../reflex_base/components/state_context.py | 51 ++++ .../src/reflex_base/event/__init__.py | 16 +- .../reflex-base/src/reflex_base/vars/base.py | 90 +++++- .../reflex_components_core/base/app_wrap.py | 13 +- .../src/reflex_components_core/core/upload.py | 45 +-- pyi_hashes.json | 4 +- reflex/compiler/compiler.py | 16 ++ reflex/compiler/plugins/builtin.py | 135 +++++++++ reflex/compiler/plugins/memoize.py | 24 +- tests/units/compiler/test_memoize_plugin.py | 47 +++ tests/units/compiler/test_plugins.py | 96 +++++++ tests/units/components/core/test_upload.py | 23 ++ tests/units/test_app.py | 270 ++++++++++++++++-- tests/units/test_event.py | 20 +- tests/units/test_var.py | 69 +++++ 17 files changed, 883 insertions(+), 65 deletions(-) create mode 100644 packages/reflex-base/src/reflex_base/components/state_context.py diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 3c525e99f4e..36ff83d08c4 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -200,7 +200,7 @@ def app_root_template( return f""" {imports_str} {dynamic_imports_str} -import {{ EventLoopProvider, StateProvider, defaultColorMode }} from "$/utils/context"; +import {{ defaultColorMode }} from "$/utils/context"; import {{ ThemeProvider }} from '$/utils/react-theme'; import {{ Layout as AppLayout }} from './_document'; import {{ Outlet }} from 'react-router'; @@ -208,9 +208,12 @@ def app_root_template( {custom_code_str} +// AppWrap is the innermost element of the python app-wrap chain (rendered +// in Layout), so providers in the chain are React-tree ancestors of the +// hooks hoisted here. function AppWrap({{children}}) {{ {_render_hooks(hooks)} -return ({_RenderUtils.render(render)}) +return (children); }} @@ -225,11 +228,7 @@ def app_root_template( return jsx(AppLayout, {{}}, jsx(ThemeProvider, {{defaultTheme: defaultColorMode, attribute: "class"}}, - jsx(StateProvider, {{}}, - jsx(EventLoopProvider, {{}}, - jsx(AppWrap, {{}}, children) - ) - ) + {_RenderUtils.render(render)} ) ); }} diff --git a/packages/reflex-base/src/reflex_base/components/component.py b/packages/reflex-base/src/reflex_base/components/component.py index 79b562ddc78..65143b88c0f 100644 --- a/packages/reflex-base/src/reflex_base/components/component.py +++ b/packages/reflex-base/src/reflex_base/components/component.py @@ -1879,14 +1879,20 @@ def _get_vars_hooks(self) -> dict[str, VarData | None]: def _get_events_hooks(self) -> dict[str, VarData | None]: """Get the hooks required by events referenced in this component. + The ``Hooks.EVENTS`` hook reads ``EventLoopContext``; declaring the + state/event-loop providers in its VarData pulls them into the app + root for every event-triggering component, independent of whether + the app uses ``rx.State`` directly. + Returns: The hooks for the events. """ - return ( - {Hooks.EVENTS: VarData(position=Hooks.HookPosition.INTERNAL)} - if self.event_triggers - else {} - ) + if not self.event_triggers: + return {} + # Lazy import: ``state_context`` imports ``Component`` from this module. + from reflex_base.components.state_context import get_events_hooks_var_data + + return {Hooks.EVENTS: get_events_hooks_var_data()} def _get_hooks_internal(self) -> dict[str, VarData | None]: """Get the React hooks for this component managed by the framework. diff --git a/packages/reflex-base/src/reflex_base/components/state_context.py b/packages/reflex-base/src/reflex_base/components/state_context.py new file mode 100644 index 00000000000..b2e83fb6c0c --- /dev/null +++ b/packages/reflex-base/src/reflex_base/components/state_context.py @@ -0,0 +1,51 @@ +"""App-wrap components mounting the state and event-loop React providers. + +These wrap children in the ``StateProvider`` / ``EventLoopProvider`` JS +functions emitted into ``utils/context.js`` by ``compile_contexts``. They are +attached to the VarData returned by :meth:`reflex_base.vars.base.VarData.from_state` +so the compiler picks them up through the generic Var-driven app-wrap pipeline, +rather than the JS Layout template hard-coding them around every app. +""" + +from __future__ import annotations + +from reflex_base.components.component import Component +from reflex_base.constants import Dirs +from reflex_base.constants.compiler import Hooks +from reflex_base.vars.base import VarData + + +class StateContextProvider(Component): + """App wrap that mounts the React state-context provider around children.""" + + library = f"$/{Dirs.CONTEXTS_PATH}" + tag = "StateProvider" + + +class EventLoopContextProvider(Component): + """App wrap that mounts the websocket event-loop provider around children.""" + + library = f"$/{Dirs.CONTEXTS_PATH}" + tag = "EventLoopProvider" + + +def get_events_hooks_var_data() -> VarData: + """Build the VarData attached to ``Hooks.EVENTS`` for event triggers. + + Higher priority wraps further out, so ``StateProvider`` (100) encloses + ``EventLoopProvider`` (90) — the latter reads ``DispatchContext`` from + the former. The returned providers are fresh per call: the compiler's + ``app_wrap_components`` registry already dedupes by ``(priority, tag)``, + and caching the instances burned us via ``copy.deepcopy`` carrying + ``_cached_render_result`` from a prior compile run forward into the next. + + Returns: + A new VarData carrying both providers as app_wraps. + """ + return VarData( + position=Hooks.HookPosition.INTERNAL, + app_wraps=( + (100, StateContextProvider.create()), + (90, EventLoopContextProvider.create()), + ), + ) diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index 8762e694d18..2b44e16094c 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -1055,14 +1055,14 @@ def _as_event_spec( """ from reflex_components_core.core.upload import ( DEFAULT_UPLOAD_ID, - upload_files_context_var_data, + get_upload_files_context_var_data, ) upload_id = self.upload_id if self.upload_id is not None else DEFAULT_UPLOAD_ID upload_files_var = Var( _js_expr="filesById", _var_type=dict[str, Any], - _var_data=VarData.merge(upload_files_context_var_data), + _var_data=VarData.merge(get_upload_files_context_var_data()), ).to(ObjectVar)[LiteralVar.create(upload_id)] spec_args = [ ( @@ -2335,11 +2335,14 @@ def create( arg_def_expr = Var(_js_expr="args") if value.invocation is None: + # Lazy import: state_context → component → event (this module). + from reflex_base.components.state_context import get_events_hooks_var_data + invocation = FunctionStringVar.create( CompileVars.ADD_EVENTS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + hooks={Hooks.EVENTS: get_events_hooks_var_data()}, ), ) else: @@ -2380,11 +2383,16 @@ def create( _js_expr=f"{{{''.join(f'{statement};' for statement in statements)}}}", ) if value.event_actions: + # Lazy import: state_context → component → event (this module). + from reflex_base.components.state_context import ( + get_events_hooks_var_data, + ) + apply_event_actions = FunctionStringVar.create( CompileVars.APPLY_EVENT_ACTIONS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + hooks={Hooks.EVENTS: get_events_hooks_var_data()}, ), ) return_expr = apply_event_actions.call( diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 4034b69aa3c..c6dce1b6f26 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -141,6 +141,12 @@ class VarData: # Components that are part of this var components: tuple[BaseComponent, ...] = dataclasses.field(default_factory=tuple) + # App-level wrapper components this var requires when used (priority, component). + # Higher priority wraps further out, matching Component._get_app_wrap_components semantics. + app_wraps: tuple[tuple[int, BaseComponent], ...] = dataclasses.field( + default_factory=tuple + ) + def __init__( self, state: str = "", @@ -150,6 +156,7 @@ def __init__( deps: list[Var] | None = None, position: Hooks.HookPosition | None = None, components: Iterable[BaseComponent] | None = None, + app_wraps: Iterable[tuple[int, BaseComponent]] | None = None, ): """Initialize the var data. @@ -161,6 +168,7 @@ def __init__( 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. """ if isinstance(hooks, str): hooks = [hooks] @@ -176,6 +184,7 @@ def __init__( object.__setattr__(self, "deps", tuple(deps or [])) object.__setattr__(self, "position", position or None) object.__setattr__(self, "components", tuple(components or [])) + object.__setattr__(self, "app_wraps", tuple(app_wraps or [])) if hooks and any(hooks.values()): # Merge our dependencies first, so they can be referenced. @@ -188,6 +197,7 @@ def __init__( object.__setattr__(self, "deps", merged_var_data.deps) object.__setattr__(self, "position", merged_var_data.position) object.__setattr__(self, "components", merged_var_data.components) + object.__setattr__(self, "app_wraps", merged_var_data.app_wraps) def old_school_imports(self) -> ImportDict: """Return the imports as a mutable dict. @@ -259,6 +269,16 @@ def merge(*all: VarData | None) -> VarData | None: component for var_data in all_var_datas for component in var_data.components ) + app_wraps_seen: set[tuple[int, str]] = set() + app_wraps_list: list[tuple[int, BaseComponent]] = [] + for var_data in all_var_datas: + for priority, wrapper in var_data.app_wraps: + key = (priority, wrapper.tag or type(wrapper).__name__) + if key in app_wraps_seen: + continue + app_wraps_seen.add(key) + app_wraps_list.append((priority, wrapper)) + return VarData( state=state, field_name=field_name, @@ -267,6 +287,7 @@ def merge(*all: VarData | None) -> VarData | None: deps=deps, position=position, components=components, + app_wraps=tuple(app_wraps_list), ) def __bool__(self) -> bool: @@ -283,8 +304,59 @@ def __bool__(self) -> bool: or self.deps or self.position or self.components + or self.app_wraps ) + def _identity_key(self) -> tuple: + """Return a hashable key for ``__eq__`` and ``__hash__``. + + ``components`` and ``app_wraps`` hold ``BaseComponent`` instances whose + ``__eq__`` override drops the default hash. Use component identity for + embedded components because they can contribute hooks/imports, and use + the compiler's app-wrap registry key for wrappers so fresh provider + instances with the same role still compare equal. + + Returns: + A hashable tuple uniquely identifying this VarData. + """ + return ( + self.state, + self.field_name, + self.imports, + self.hooks, + self.deps, + self.position, + tuple(id(component) for component in self.components), + tuple( + ( + priority, + component.tag or type(component).__name__, + ) + for priority, component in self.app_wraps + ), + ) + + def __eq__(self, other: object) -> bool: + """Compare two VarData by render-time identity. + + Args: + other: The value to compare against. + + Returns: + True if ``other`` is a VarData with matching render-time fields. + """ + if not isinstance(other, VarData): + return NotImplemented + return self._identity_key() == other._identity_key() + + def __hash__(self) -> int: + """Hash consistent with ``__eq__``. + + Returns: + A hash over render-time fields and hashable component metadata. + """ + return hash(self._identity_key()) + @classmethod def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarData: """Set the state of the var. @@ -296,6 +368,11 @@ def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarDa Returns: The var with the set state. """ + # Lazy imports: state_context imports VarData from this module. + from reflex_base.components.state_context import ( + EventLoopContextProvider, + StateContextProvider, + ) from reflex_base.utils import format state_name = state if isinstance(state, str) else state.get_full_name() @@ -311,6 +388,17 @@ def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarDa f"$/{constants.Dirs.CONTEXTS_PATH}": [ImportVar(tag="StateContexts")], "react": [ImportVar(tag="useContext")], }, + app_wraps=( + # Higher priority wraps further out. ``StateProvider`` must + # enclose ``EventLoopProvider`` because the latter reads + # ``DispatchContext`` (provided by StateProvider) at its top. + # Both must enclose the chain's other wraps so the hooks + # AppWrap hosts (e.g. ``useContext(EventLoopContext)``) see + # them as React-tree ancestors. The compiler dedupes by + # ``(priority, tag)`` so fresh per-call instances are fine. + (100, StateContextProvider.create()), + (90, EventLoopContextProvider.create()), + ), ) @@ -362,7 +450,7 @@ def can_use_in_object_var(cls: GenericType) -> bool: Whether the class can be used in an ObjectVar. """ if types.is_union(cls): - return all(can_use_in_object_var(t) for t in types.get_args(cls)) + return all(can_use_in_object_var(t) for t in get_args(cls)) return ( isinstance(cls, type) and not safe_issubclass(cls, Var) diff --git a/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py b/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py index e5a0a491f3a..8a26ffc510b 100644 --- a/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py +++ b/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py @@ -3,11 +3,18 @@ from reflex_base.components.component import Component from reflex_base.vars.base import Var -from reflex_components_core.base.fragment import Fragment +class AppWrap(Component): + """Innermost element of the app-wrap chain. -class AppWrap(Fragment): - """Top-level component that wraps the entire app.""" + Renders as ``{children}`` — the locally-defined JS + function in ``app_root_template`` that hosts all hooks aggregated from + the python chain and returns its children. Library is ``None`` because + the JS function is defined in the same file the component renders into. + """ + + library = None + tag = "AppWrap" @classmethod def create(cls) -> Component: diff --git a/packages/reflex-components-core/src/reflex_components_core/core/upload.py b/packages/reflex-components-core/src/reflex_components_core/core/upload.py index 9bbb28de64d..5aecd609472 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/upload.py @@ -13,6 +13,7 @@ field, ) from reflex_base.components.memoize_helpers import get_memoized_event_triggers +from reflex_base.components.state_context import get_events_hooks_var_data from reflex_base.constants import Dirs from reflex_base.constants.compiler import Hooks, Imports from reflex_base.environment import environment @@ -46,15 +47,29 @@ DEFAULT_UPLOAD_ID: str = "default" -upload_files_context_var_data: VarData = VarData( - imports={ - "react": "useContext", - f"$/{Dirs.CONTEXTS_PATH}": "UploadFilesContext", - }, - hooks={ - "const [filesById, setFilesById] = useContext(UploadFilesContext);": None, - }, -) + +def get_upload_files_context_var_data() -> VarData: + """Build the VarData for vars reading the upload-files React context. + + Defined as a function (not a module-level value) because it references + ``UploadFilesProvider``, which is declared further down in this module. + Returns a fresh instance per call so render-cache state can't leak + across compile runs via ``copy.deepcopy``. + + Returns: + A new VarData carrying the upload-context import, hook, and the + ``UploadFilesProvider`` app_wrap declaration. + """ + return VarData( + imports={ + "react": "useContext", + f"$/{Dirs.CONTEXTS_PATH}": "UploadFilesContext", + }, + hooks={ + "const [filesById, setFilesById] = useContext(UploadFilesContext);": None, + }, + app_wraps=((5, UploadFilesProvider.create()),), + ) def upload_file(id_: str | Var[str] = DEFAULT_UPLOAD_ID) -> Var: @@ -81,7 +96,7 @@ def upload_file(id_: str | Var[str] = DEFAULT_UPLOAD_ID) -> Var: _js_expr=var_name, _var_type=EventChain, _var_data=VarData.merge( - upload_files_context_var_data, id_var._get_all_var_data() + get_upload_files_context_var_data(), id_var._get_all_var_data() ), ) @@ -100,7 +115,7 @@ def selected_files(id_: str | Var[str] = DEFAULT_UPLOAD_ID) -> Var: _js_expr=f"(filesById[{id_var!s}] ? filesById[{id_var!s}].map((f) => f.name) : [])", _var_type=list[str], _var_data=VarData.merge( - upload_files_context_var_data, id_var._get_all_var_data() + get_upload_files_context_var_data(), id_var._get_all_var_data() ), ).guess_type() @@ -384,7 +399,7 @@ def create(cls, *children, **props) -> Component: var_data = VarData.merge( VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + hooks={Hooks.EVENTS: get_events_hooks_var_data()}, ), use_dropzone_arguments._get_all_var_data(), VarData( @@ -442,12 +457,6 @@ def _update_arg_tuple_for_on_drop(cls, arg_value: tuple[Var, Var]): return (arg_value[0], placeholder) return arg_value - @staticmethod - def _get_app_wrap_components() -> dict[tuple[int, str], Component]: - return { - (5, "UploadFilesProvider"): UploadFilesProvider.create(), - } - class StyledUpload(Upload): """The styled Upload Component.""" diff --git a/pyi_hashes.json b/pyi_hashes.json index 801f1660679..d60240cb251 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -3,7 +3,7 @@ "packages/reflex-components-code/src/reflex_components_code/shiki_code_block.pyi": "d3e0c33fdc34f5c154ac387d550c0d29", "packages/reflex-components-core/src/reflex_components_core/__init__.pyi": "82b29d23f2490161d42fd21021bd39c3", "packages/reflex-components-core/src/reflex_components_core/base/__init__.pyi": "7009187aaaf191814d031e5462c48318", - "packages/reflex-components-core/src/reflex_components_core/base/app_wrap.pyi": "ecccfd8a9b0e8b2f4128ff13ff27a9da", + "packages/reflex-components-core/src/reflex_components_core/base/app_wrap.pyi": "26d7284c583b0a5addd96c5ba13d62e0", "packages/reflex-components-core/src/reflex_components_core/base/body.pyi": "2535814d409e5feaf57da63dcf0abeaf", "packages/reflex-components-core/src/reflex_components_core/base/document.pyi": "a2e67a9814dc61853ca2299d9d9c698d", "packages/reflex-components-core/src/reflex_components_core/base/error_boundary.pyi": "59170074a1a228ce58685f3f207954f2", @@ -20,7 +20,7 @@ "packages/reflex-components-core/src/reflex_components_core/core/helmet.pyi": "7fd81a99bde5b0ff94bb52523597fd5c", "packages/reflex-components-core/src/reflex_components_core/core/html.pyi": "753d6ae315369530dad450ed643f5be6", "packages/reflex-components-core/src/reflex_components_core/core/sticky.pyi": "ba60a7d9cba75b27a1133bd63a9fbd59", - "packages/reflex-components-core/src/reflex_components_core/core/upload.pyi": "2dd6ba6e3a4d61fc1d79eb582a7cc548", + "packages/reflex-components-core/src/reflex_components_core/core/upload.pyi": "13f1df7a87202cf74d8cde1717eace73", "packages/reflex-components-core/src/reflex_components_core/core/window_events.pyi": "5e1dcb1130bc8af282783fae329ae6a6", "packages/reflex-components-core/src/reflex_components_core/datadisplay/__init__.pyi": "c96fed4da42a13576d64f84e3c7cb25c", "packages/reflex-components-core/src/reflex_components_core/el/__init__.pyi": "f09129ddefb57ab4c7769c86dc9a3153", diff --git a/reflex/compiler/compiler.py b/reflex/compiler/compiler.py index 39ff4931a9e..e075736bc45 100644 --- a/reflex/compiler/compiler.py +++ b/reflex/compiler/compiler.py @@ -35,6 +35,7 @@ from reflex.compiler import templates, utils from reflex.compiler.plugins import default_page_plugins +from reflex.compiler.plugins.builtin import collect_var_app_wraps_in_subtree from reflex.experimental.memo import ( EXPERIMENTAL_MEMOS, ExperimentalMemoComponentDefinition, @@ -958,6 +959,21 @@ def memoized_toast_provider(): if component is not None: app_wrappers[key] = component + # The page collector only walks pages, but app-wrap components have their + # own subtrees (e.g. ``ErrorBoundary``'s fallback render). Surface their + # Var-declared ``app_wraps`` here, fixpoint-iterating because newly added + # wraps may themselves contain further declarations. + pending: list[Component] = list(app_wrappers.values()) + while pending: + next_pending: list[Component] = [] + for wrapper in pending: + before = set(app_wrappers) + collect_var_app_wraps_in_subtree(app_wrappers, wrapper) + next_pending.extend( + app_wrappers[key] for key in app_wrappers.keys() - before + ) + pending = next_pending + return app_wrappers diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index a4b326be4ab..caa6b522d33 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -7,7 +7,9 @@ from typing import Any from reflex_base.components.component import BaseComponent, Component, ComponentStyle +from reflex_base.components.state_context import get_events_hooks_var_data from reflex_base.config import get_config +from reflex_base.constants.compiler import Hooks from reflex_base.plugins import CompileContext, PageContext, PageDefinition, Plugin from reflex_base.plugins.base import HookOrder from reflex_base.utils.format import make_default_page_title @@ -18,6 +20,109 @@ from reflex.compiler import utils +def collect_var_app_wraps_in_subtree( + page_app_wrap_components: dict[tuple[int, str], Component], + root: Component, +) -> None: + """Walk ``root`` and its descendants, surfacing Var-declared app_wraps. + + Each visited component contributes via :func:`collect_var_app_wraps_for_component`. + Used wherever the page walker doesn't reach — e.g. snapshot-boundary + descendants sealed by ``MemoizeStatefulPlugin``, or the app-wrap chain + components assembled by ``App._app_root`` (their own subtrees, e.g. + ``ErrorBoundary``'s fallback render, are not pages). + """ + visited: set[int] = set() + stack: list[Component] = [root] + while stack: + node = stack.pop() + node_id = id(node) + if node_id in visited: + continue + visited.add(node_id) + page_app_wrap_components.update( + collect_var_app_wraps_for_component(page_app_wrap_components, node) + ) + stack.extend(child for child in node.children if isinstance(child, Component)) + stack.extend( + component + for component in node._get_components_in_props() + if isinstance(component, Component) + ) + + +def collect_var_app_wraps_for_component( + page_app_wrap_components: dict[tuple[int, str], Component], + component: Component, +) -> dict[tuple[int, str], Component]: + """Return Var-declared app_wraps newly contributed by ``component``. + + Scans the component's Vars (props/style/event-trigger args) and the + VarData attached to its framework-managed internal hooks (e.g. + ``Hooks.EVENTS``), so providers required by the hooks themselves — not + just by referenced Vars — surface to the page-level registry. + + Entries already in ``page_app_wrap_components`` are skipped, leaving the + caller to decide how to merge the result and whether to recurse into + each wrapper's own subtree. + + Returns: + Mapping of ``(priority, name)`` -> wrapper for new entries only. + """ + wraps_by_key: dict[tuple[int, str], Component] = {} + for var in component._get_vars(): + var_data = var._get_all_var_data() + if var_data is None: + continue + _ingest_var_data_app_wraps(wraps_by_key, page_app_wrap_components, var_data) + for hook_var_data in component._get_hooks_internal().values(): + if hook_var_data is None: + continue + _ingest_var_data_app_wraps( + wraps_by_key, page_app_wrap_components, hook_var_data + ) + for hook, hook_var_data in component._get_added_hooks().items(): + if hook_var_data is None and hook == Hooks.EVENTS: + hook_var_data = get_events_hooks_var_data() + if hook_var_data is None: + continue + _ingest_var_data_app_wraps( + wraps_by_key, page_app_wrap_components, hook_var_data + ) + return wraps_by_key + + +def _ingest_var_data_app_wraps( + wraps_by_key: dict[tuple[int, str], Component], + existing: dict[tuple[int, str], Component], + var_data: Any, +) -> None: + """Insert app_wraps carried or implied by ``var_data``.""" + if var_data.app_wraps: + _ingest_app_wraps(wraps_by_key, existing, var_data.app_wraps) + if Hooks.EVENTS in var_data.hooks: + _ingest_app_wraps( + wraps_by_key, + existing, + get_events_hooks_var_data().app_wraps, + ) + + +def _ingest_app_wraps( + wraps_by_key: dict[tuple[int, str], Component], + existing: dict[tuple[int, str], Component], + app_wraps: tuple[tuple[int, BaseComponent], ...], +) -> None: + """Insert app_wraps not already present in ``existing`` or ``wraps_by_key``.""" + for priority, wrapper in app_wraps: + if not isinstance(wrapper, Component): + continue + key = (priority, wrapper.tag or type(wrapper).__name__) + if key in existing or key in wraps_by_key: + continue + wraps_by_key[key] = wrapper + + @dataclasses.dataclass(frozen=True, slots=True) class DefaultPagePlugin(Plugin): """Evaluate an unevaluated page into a mutable page context.""" @@ -204,6 +309,8 @@ def leave_component( comp, ) + self._collect_var_app_wraps(page_context.app_wrap_components, comp) + if (dynamic_import := comp._get_dynamic_imports()) is not None: page_context.dynamic_imports.add(dynamic_import) @@ -253,6 +360,7 @@ def _compiler_bind_leave_component( collect_component_hooks = self._collect_component_hooks collect_component_custom_code = self._collect_component_custom_code collect_app_wrap_components = self._collect_app_wrap_components + collect_var_app_wraps = self._collect_var_app_wraps base_get_app_wrap_components = Component._get_app_wrap_components seen_app_wrap_methods: set[object] = set() @@ -281,6 +389,8 @@ def leave_component( seen_app_wrap_methods.add(app_wrap_method) collect_app_wrap_components(app_wrap_components, comp) + collect_var_app_wraps(app_wrap_components, comp) + dynamic_import = comp._get_dynamic_imports() if dynamic_import is not None: dynamic_imports.add(dynamic_import) @@ -352,6 +462,31 @@ def _collect_app_wrap_components( page_app_wrap_components, ) + def _collect_var_app_wraps( + self, + page_app_wrap_components: dict[tuple[int, str], Component], + component: Component, + ) -> None: + """Collect app-wrap components declared by VarData on ``component``.""" + wraps_by_key = collect_var_app_wraps_for_component( + page_app_wrap_components, component + ) + if not wraps_by_key: + return + + ignore_ids = {id(wrapper) for wrapper in page_app_wrap_components.values()} + page_app_wrap_components.update(wraps_by_key) + for wrapper in wraps_by_key.values(): + wrapper_id = id(wrapper) + if wrapper_id in ignore_ids: + continue + ignore_ids.add(wrapper_id) + self._collect_wrapper_subtree_into( + wrapper, + ignore_ids, + page_app_wrap_components, + ) + @staticmethod def _collect_wrapper_subtree_into( component: Component, diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index b596b147a79..7c06c0c6e7f 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -39,6 +39,10 @@ from reflex_base.plugins.base import Plugin from reflex_base.utils import format +from reflex.compiler.plugins.builtin import ( + collect_var_app_wraps_for_component, + collect_var_app_wraps_in_subtree, +) from reflex.experimental.memo import create_passthrough_component_memo @@ -297,7 +301,15 @@ def enter_component( page_context, compile_context, ) - return None if wrapper is None else (wrapper, ()) + if wrapper is not None: + # Snapshot-boundary descendants are sealed from the page walker, so + # ``DefaultCollectorPlugin._collect_var_app_wraps`` never sees Vars + # buried inside the boundary. Surface their app_wraps now (before + # sealing) so providers declared via ``VarData.app_wraps`` still + # reach the page-level app_wrap registry. + collect_var_app_wraps_in_subtree(page_context.app_wrap_components, comp) + return (wrapper, ()) + return None def leave_component( self, @@ -350,6 +362,16 @@ def leave_component( if not _should_memoize(comp): return None + # The collector plugin runs at HookOrder.POST — *after* this plugin + # rewrites ``comp`` into its wrapper — so the wrapper (which holds no + # Vars of its own) is what the collector sees for the current node. + # Surface ``comp``'s own Var-declared app_wraps before returning the + # wrapper. Descendants have already been visited by the collector, so + # this only needs to look at the current node — not its subtree. + page_context.app_wrap_components.update( + collect_var_app_wraps_for_component(page_context.app_wrap_components, comp) + ) + return self._build_wrapper(comp, page_context, compile_context) @staticmethod diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index 18fc3b3e5dc..e8ca4db81ef 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -71,6 +71,19 @@ class LeafComponent(Component): _memoization_mode = MemoizationMode(recursive=False) +class SnapshotWithSlot(Component): + tag = "SnapshotWithSlot" + library = "snapshot-with-slot-lib" + _memoization_mode = MemoizationMode(recursive=False) + + slot: Component | None = field(default=None) + + +class MemoAppWrapProvider(Component): + tag = "MemoAppWrapProvider" + library = "memo-app-wrap-provider-lib" + + class SpecialFormMemoState(BaseState): items: list[str] = ["a"] flag: bool = True @@ -192,6 +205,40 @@ def test_memoize_wrapper_deduped_across_repeated_subtrees() -> None: ) == 1 +def test_passthrough_memo_collects_var_app_wraps_from_replaced_component() -> None: + """Var app_wraps on passthrough-memoized components survive replacement.""" + provider = MemoAppWrapProvider.create() + stateful_var_with_wrap = LiteralVar.create("needs-wrap")._replace( + merge_var_data=VarData( + hooks={"useNeedsWrap": None}, + app_wraps=((70, provider),), + ) + ) + + _ctx, page_ctx = _compile_single_page( + lambda: WithProp.create(label=stateful_var_with_wrap) + ) + + assert (70, "MemoAppWrapProvider") in page_ctx.app_wrap_components + + +def test_snapshot_memo_collects_var_app_wraps_from_prop_components() -> None: + """Snapshot memo boundaries collect app_wraps buried in prop components.""" + provider = MemoAppWrapProvider.create() + var_with_wrap = LiteralVar.create("needs-wrap")._replace( + merge_var_data=VarData(app_wraps=((70, provider),)) + ) + + _ctx, page_ctx = _compile_single_page( + lambda: SnapshotWithSlot.create( + STATE_VAR, + slot=WithProp.create(label=var_with_wrap), + ) + ) + + assert (70, "MemoAppWrapProvider") in page_ctx.app_wrap_components + + @pytest.mark.parametrize( ("special_form", "body_marker"), [ diff --git a/tests/units/compiler/test_plugins.py b/tests/units/compiler/test_plugins.py index 26eb1f39c99..03ff87b0a02 100644 --- a/tests/units/compiler/test_plugins.py +++ b/tests/units/compiler/test_plugins.py @@ -11,6 +11,7 @@ ComponentStyle, field, ) +from reflex_base.constants.compiler import Hooks from reflex_base.plugins import ( BaseContext, CompileContext, @@ -769,6 +770,101 @@ def test_default_collector_matches_legacy_collectors() -> None: ) +class StubVarProvider(Component): + tag = "StubVarProvider" + library = "stub-provider-lib" + + +class DirectEventsHookComponent(Component): + tag = "DirectEventsHookComponent" + library = "direct-events-hook-lib" + + def add_hooks(self) -> list[str]: + """Add the shared event-loop hook directly. + + Returns: + A list with just the events hook. + """ + return [Hooks.EVENTS] + + +def test_default_collector_collects_var_app_wraps() -> None: + """A Var with app_wraps in its VarData injects the wrapper into the page registry.""" + provider = StubVarProvider.create() + var_with_wrap = LiteralVar.create("hello")._replace( + merge_var_data=VarData(app_wraps=((50, provider),)) + ) + + component = RootComponent.create( + ChildComponent.create(id=var_with_wrap), + ) + + page_ctx = collect_page_context( + component, + plugins=(DefaultCollectorPlugin(),), + ) + + assert (50, "StubVarProvider") in page_ctx.app_wrap_components + assert page_ctx.app_wrap_components[50, "StubVarProvider"] is provider + # Existing component-declared wraps are still collected. + assert (10, "Wrap") in page_ctx.app_wrap_components + + +def test_default_collector_dedupes_var_app_wraps_against_component_wraps() -> None: + """A Var-declared wrap with the same (priority, name) as a Component-declared one defers.""" + component_wrap = WrapperComponent.create() + var_wrap = WrapperComponent.create() + var_with_wrap = LiteralVar.create("dup")._replace( + merge_var_data=VarData(app_wraps=((10, var_wrap),)) + ) + + class RootWithSameWrap(Component): + tag = "RootWithSameWrap" + library = "root-with-same-wrap-lib" + + @staticmethod + def _get_app_wrap_components() -> dict[tuple[int, str], Component]: + return {(10, "WrapperComponent"): component_wrap} + + component = RootWithSameWrap.create( + ChildComponent.create(id=var_with_wrap), + ) + + page_ctx = collect_page_context( + component, + plugins=(DefaultCollectorPlugin(),), + ) + + # Component-declared wrap wins because it's collected first; var wrap is skipped. + assert page_ctx.app_wrap_components[10, "WrapperComponent"] is component_wrap + + +def test_default_collector_collects_direct_events_hook_app_wraps() -> None: + """Direct ``Hooks.EVENTS`` users collect the state/event providers.""" + page_ctx = collect_page_context( + DirectEventsHookComponent.create(), + plugins=(DefaultCollectorPlugin(),), + ) + + assert (100, "StateProvider") in page_ctx.app_wrap_components + assert (90, "EventLoopProvider") in page_ctx.app_wrap_components + + +def test_default_collector_collects_var_events_hook_app_wraps() -> None: + """Vars with raw ``Hooks.EVENTS`` metadata collect fresh event providers.""" + var_with_events_hook = LiteralVar.create("hello")._replace( + merge_var_data=VarData(hooks={Hooks.EVENTS: None}) + ) + + page_ctx = collect_page_context( + RootComponent.create(ChildComponent.create(id=var_with_events_hook)), + plugins=(DefaultCollectorPlugin(),), + ) + + assert (100, "StateProvider") in page_ctx.app_wrap_components + assert (90, "EventLoopProvider") in page_ctx.app_wrap_components + + def test_default_collector_collects_nested_prop_tree_custom_code_without_recursion() -> ( None ): diff --git a/tests/units/components/core/test_upload.py b/tests/units/components/core/test_upload.py index d3a1e4c19aa..2bb990bac56 100644 --- a/tests/units/components/core/test_upload.py +++ b/tests/units/components/core/test_upload.py @@ -2,6 +2,7 @@ import pytest from reflex_base.event import EventChain, EventHandler, EventSpec, parse_args_spec +from reflex_base.vars import VarData from reflex_base.vars.base import LiteralVar, Var from reflex_components_core.core.upload import ( GhostUpload, @@ -199,6 +200,28 @@ def test_upload_button_handlers_allow_custom_param_names(): assert chunk_arg_names[:3] == ["files", "stream", "upload_param_name"] +def test_upload_files_event_spec_carries_upload_provider_app_wrap(): + """Upload button event specs carry UploadFilesProvider through VarData.""" + button = rx.button( + "Upload", + on_click=UploadStateTest.drop_handler( + cast(Any, rx.upload_files(upload_id="foo_id")) + ), + ) + chain = cast(EventChain, button.event_triggers["on_click"]) + upload_event = cast(EventSpec, chain.events[0]) + + var_data = VarData.merge( + *(arg_value._get_all_var_data() for _, arg_value in upload_event.args) + ) + + assert var_data is not None + assert any( + priority == 5 and wrapper.tag == "UploadFilesProvider" + for priority, wrapper in var_data.app_wraps + ) + + def test_styled_upload_create(): styled_up_comp_1 = StyledUpload.create() assert isinstance(styled_up_comp_1, StyledUpload) diff --git a/tests/units/test_app.py b/tests/units/test_app.py index a772b3a3eae..987c71babd8 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -22,12 +22,14 @@ from reflex_base.event import Event from reflex_base.event.context import EventContext from reflex_base.event.processor import BaseStateEventProcessor +from reflex_base.plugins import CompileContext, CompilerHooks, PageContext from reflex_base.registry import RegistrationContext from reflex_base.style import Style from reflex_base.utils import console, exceptions, format from reflex_base.vars.base import computed_var from reflex_components_core.base.bare import Bare from reflex_components_core.base.fragment import Fragment +from reflex_components_core.core.upload import selected_files from reflex_components_radix.themes.typography.text import Text from starlette.applications import Starlette from starlette.datastructures import FormData, Headers, UploadFile @@ -38,6 +40,8 @@ import reflex as rx from reflex import AdminDash, constants from reflex.app import App, ComponentCallable, upload +from reflex.compiler.compiler import _compile_app, _resolve_app_wrap_components +from reflex.compiler.plugins import default_page_plugins from reflex.environment import environment from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory @@ -2021,11 +2025,15 @@ async def test_process_events( @pytest.fixture -def compilable_app(tmp_path: Path) -> Generator[tuple[App, Path], None, None]: +def compilable_app( + tmp_path: Path, + forked_registration_context: RegistrationContext, +) -> Generator[tuple[App, Path], None, None]: """Fixture for an app that can be compiled. Args: tmp_path: Temporary path. + forked_registration_context: Isolated state/event registration context. Yields: Tuple containing (app instance, Path to ".web" directory) @@ -2045,15 +2053,64 @@ def compilable_app(tmp_path: Path) -> Generator[tuple[App, Path], None, None]: "postcss-import": {}, autoprefixer: {}, }, -}; -""", + }; + """, ) + reload_state_module(__name__) app = App(theme=None) app._get_frontend_packages = unittest.mock.Mock() with chdir(app_path): yield app, web_dir +EVENT_LOOP_CONTEXT_HOOK = ( + "const [addEvents, connectErrors] = useContext(EventLoopContext);" +) + + +def compile_page_context_for_app_wraps(component: Component): + """Compile one component through the page plugin pipeline. + + Args: + component: The page root component to compile. + + Returns: + The compiled page context. + """ + page_ctx = PageContext(name="page", route="/page", root_component=component) + page_hooks = CompilerHooks(plugins=default_page_plugins(style=None)) + compile_ctx = CompileContext(pages=[], hooks=page_hooks) + + with compile_ctx, page_ctx: + page_ctx.root_component = page_hooks.compile_component( + page_ctx.root_component, + page_context=page_ctx, + compile_context=compile_ctx, + ) + page_hooks.compile_page(page_ctx, compile_context=compile_ctx) + + return page_ctx + + +def compile_app_root_from_page_wraps( + app: App, + page_app_wrap_components: dict[tuple[int, str], Component], +) -> str: + """Render app-root code from an app and pre-collected page app wraps. + + Args: + app: The app whose root wrapper chain should be compiled. + page_app_wrap_components: The app wraps collected from page compilation. + + Returns: + The generated app root source. + """ + app_root = app._app_root( + _resolve_app_wrap_components(app, page_app_wrap_components) + ) + return _compile_app(app_root) + + @pytest.mark.parametrize( "react_strict_mode", [True, False], @@ -2079,17 +2136,27 @@ def test_app_wrap_compile_theme( app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() + # AppWrap is now just hooks + ``return (children)`` — the python app-wrap + # chain is rendered by Layout and wraps AppWrap externally. function_app_definition = app_js_contents[ app_js_contents.index("function AppWrap") : app_js_contents.index( "export function Layout" ) ].strip() - - expected = ( + assert function_app_definition == ( "function AppWrap({children}) {\n" "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" - "return (" - + ("jsx(StrictMode,{}," if react_strict_mode else "") + "return (children);\n}" + ) + + layout_definition = app_js_contents[ + app_js_contents.index("export function Layout") : + ].strip() + + expected_chain = ( + ("jsx(StrictMode,{}," if react_strict_mode else "") + + "jsx(StateProvider,{}," + + "jsx(EventLoopProvider,{}," + "jsx(ErrorBoundary,{" """fallbackRender:((event_args) => (jsx("div", ({css:({ ["height"] : "100%", ["width"] : "100%", ["position"] : "absolute", ["backgroundColor"] : "#fff", ["color"] : "#000", ["display"] : "flex", ["alignItems"] : "center", ["justifyContent"] : "center" })}), (jsx("div", ({css:({ ["display"] : "flex", ["flexDirection"] : "column", ["gap"] : "0.5rem", ["maxWidth"] : "min(80ch, 90vw)", ["borderRadius"] : "0.25rem", ["padding"] : "1rem" })}), (jsx("div", ({css:({ ["opacity"] : "0.5", ["display"] : "flex", ["gap"] : "4vmin", ["alignItems"] : "center" })}), (jsx("svg", ({className:"lucide lucide-frown-icon lucide-frown",fill:"none",stroke:"currentColor","stroke-linecap":"round","stroke-linejoin":"round","stroke-width":"2",viewBox:"0 0 24 24",width:"25vmin",xmlns:"http://www.w3.org/2000/svg"}), (jsx("circle", ({cx:"12",cy:"12",r:"10"}))), (jsx("path", ({d:"M16 16s-1.5-2-4-2-4 2-4 2"}))), (jsx("line", ({x1:"9",x2:"9.01",y1:"9",y2:"9"}))), (jsx("line", ({x1:"15",x2:"15.01",y1:"9",y2:"9"}))))), (jsx("h2", ({css:({ ["fontSize"] : "5vmin", ["fontWeight"] : "bold" })}), "An error occurred while rendering this page.")))), (jsx("p", ({css:({ ["opacity"] : "0.75", ["marginBlock"] : "1rem" })}), "This is an error with the application itself. Refreshing the page might help.")), (jsx("div", ({css:({ ["width"] : "100%", ["background"] : "color-mix(in srgb, currentColor 5%, transparent)", ["maxHeight"] : "15rem", ["overflow"] : "auto", ["borderRadius"] : "0.4rem" })}), (jsx("div", ({css:({ ["padding"] : "0.5rem" })}), (jsx("pre", ({css:({ ["wordBreak"] : "break-word", ["whiteSpace"] : "pre-wrap" })}), event_args.error.name + \': \' + event_args.error.message + \'\\n\' + event_args.error.stack)))))), (jsx("button", ({css:({ ["padding"] : "0.35rem 1.35rem", ["marginBlock"] : "0.5rem", ["marginInlineStart"] : "auto", ["background"] : "color-mix(in srgb, currentColor 15%, transparent)", ["borderRadius"] : "0.4rem", ["width"] : "fit-content", ["&:hover"] : ({ ["background"] : "color-mix(in srgb, currentColor 25%, transparent)" }), ["&:active"] : ({ ["background"] : "color-mix(in srgb, currentColor 35%, transparent)" }) }),onClick:((_e) => (addEvents([(ReflexEvent("_call_function", ({ ["function"] : (() => (navigator?.["clipboard"]?.["writeText"](event_args.error.name + \': \' + event_args.error.message + \'\\n\' + event_args.error.stack))), ["callback"] : null }), ({ })))], [_e], ({ }))))}), "Copy")), (jsx("hr", ({css:({ ["borderColor"] : "currentColor", ["opacity"] : "0.25" })}))), (jsx(ReactRouterLink, ({to:"https://reflex.dev"}), (jsx("div", ({css:({ ["display"] : "flex", ["alignItems"] : "baseline", ["justifyContent"] : "center", ["fontFamily"] : "monospace", ["--default-font-family"] : "monospace", ["gap"] : "0.5rem" })}), "Built with ", (jsx("svg", ({"aria-label":"Reflex",css:({ ["fill"] : "currentColor" }),height:"12",role:"img",width:"56",xmlns:"http://www.w3.org/2000/svg"}), (jsx("path", ({d:"M0 11.5999V0.399902H8.96V4.8799H6.72V2.6399H2.24V4.8799H6.72V7.1199H2.24V11.5999H0ZM6.72 11.5999V7.1199H8.96V11.5999H6.72Z"}))), (jsx("path", ({d:"M11.2 11.5999V0.399902H17.92V2.6399H13.44V4.8799H17.92V7.1199H13.44V9.3599H17.92V11.5999H11.2Z"}))), (jsx("path", ({d:"M20.16 11.5999V0.399902H26.88V2.6399H22.4V4.8799H26.88V7.1199H22.4V11.5999H20.16Z"}))), (jsx("path", ({d:"M29.12 11.5999V0.399902H31.36V9.3599H35.84V11.5999H29.12Z"}))), (jsx("path", ({d:"M38.08 11.5999V0.399902H44.8V2.6399H40.32V4.8799H44.8V7.1199H40.32V9.3599H44.8V11.5999H38.08Z"}))), (jsx("path", ({d:"M47.04 4.8799V0.399902H49.28V4.8799H47.04ZM53.76 4.8799V0.399902H56V4.8799H53.76ZM49.28 7.1199V4.8799H53.76V7.1199H49.28ZM47.04 11.5999V7.1199H49.28V11.5999H47.04ZM53.76 11.5999V7.1199H56V11.5999H53.76Z"}))), (jsx("title", ({}), "Reflex"))))))))))))),""" """onError:((_error, _info) => (addEvents([(ReflexEvent("reflex___state____state.reflex___state____frontend_event_exception_state.handle_frontend_exception", ({ ["info"] : ((((_error?.["name"]+": ")+_error?.["message"])+"\\n")+_error?.["stack"]), ["component_stack"] : _info?.["componentStack"] }), ({ })))], [_error, _info], ({ }))))""" @@ -2100,12 +2167,12 @@ def test_app_wrap_compile_theme( + "jsx(RadixThemesTheme,{accentColor:\"plum\",css:{...theme.styles.global[':root'], ...theme.styles.global.body}}," + "jsx(Fragment,{}," + "jsx(DefaultOverlayComponents,{},)," - + "jsx(Fragment,{}," + + "jsx(AppWrap,{}," + "children" - "))))))" + (")" if react_strict_mode else "") + ")" - "\n}" + "))))))))" + (")" if react_strict_mode else "") ) - assert expected.split(",") == function_app_definition.split(",") + # Layout's body now contains the chain wrapping AppWrap. + assert expected_chain in layout_definition def test_compile_without_radix_components_skips_radix_plugin( @@ -2286,6 +2353,160 @@ def test_compile_writes_upload_files_provider_app_wrap( assert "UploadFilesProvider" in root_contents +def test_app_wrap_event_hook_requires_state_providers(mocker: MockerFixture) -> None: + """App-root hooks from default app-wrap prop components need providers. + + The default error boundary's fallback render contains a copy button, which + contributes the event-loop hook through a component-valued prop. If that + hook is emitted at AppWrap scope, the provider chain must be present too. + """ + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + app = App(theme=None, enable_state=False) + + root_contents = compile_app_root_from_page_wraps(app, {}) + + assert EVENT_LOOP_CONTEXT_HOOK in root_contents + assert "jsx(StateProvider" in root_contents + assert "jsx(EventLoopProvider" in root_contents + + +def test_event_provider_app_wrap_order(mocker: MockerFixture) -> None: + """StateProvider wraps EventLoopProvider, and both wrap AppWrap.""" + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + page_ctx = compile_page_context_for_app_wraps( + rx.button("ping", on_click=rx.console_log("ping")) + ) + app = App(theme=None, enable_state=False) + + root_contents = compile_app_root_from_page_wraps(app, page_ctx.app_wrap_components) + + state_index = root_contents.index("jsx(StateProvider") + event_loop_index = root_contents.index("jsx(EventLoopProvider") + app_wrap_index = root_contents.index("jsx(AppWrap") + assert state_index < event_loop_index < app_wrap_index + + +def test_minimal_static_app_wrap_omits_state_providers( + mocker: MockerFixture, +) -> None: + """An app-root chain with no event/state hooks avoids state providers.""" + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + app = App(theme=None, enable_state=False) + app.app_wraps = {} + app.extra_app_wraps = {} + app.toaster = None + + root_contents = compile_app_root_from_page_wraps(app, {}) + + assert EVENT_LOOP_CONTEXT_HOOK not in root_contents + assert "jsx(StateProvider" not in root_contents + assert "jsx(EventLoopProvider" not in root_contents + + +def test_event_triggers_collect_state_providers_via_var_app_wrap() -> None: + """A component with event triggers collects ``StateProvider`` and + ``EventLoopProvider`` into the page-level app_wrap registry through the + Var-driven path attached to ``Hooks.EVENTS``. + + Uses the page-walk pipeline directly (not ``app._compile()``) so the + assertion is robust against ``rx.State`` registration leaks between + tests in this file. + """ + component = rx.button("ping", on_click=rx.console_log("ping")) + + page_ctx = PageContext(name="page", route="/page", root_component=component) + page_hooks = CompilerHooks(plugins=default_page_plugins(style=None)) + compile_ctx = CompileContext(pages=[], hooks=page_hooks) + + with compile_ctx, page_ctx: + page_ctx.root_component = page_hooks.compile_component( + page_ctx.root_component, + page_context=page_ctx, + compile_context=compile_ctx, + ) + page_hooks.compile_page(page_ctx, compile_context=compile_ctx) + + keys = page_ctx.app_wrap_components.keys() + # ``from_state`` priorities: StateProvider outer (100), EventLoopProvider + # inner (90). Both must be present whenever a component carries event + # triggers, since the ``useContext(EventLoopContext)`` hook hoisted at + # ``AppWrap`` level requires both providers as React-tree ancestors. + assert (100, "StateProvider") in keys + assert (90, "EventLoopProvider") in keys + + +def test_no_event_triggers_omits_state_providers() -> None: + """A static page with no event triggers does not pull state providers in. + + Validates that the migration is *conditional* — apps that don't use + state machinery don't pay for unused providers in the app root. + """ + component = rx.text("hello") + + page_ctx = PageContext(name="page", route="/page", root_component=component) + page_hooks = CompilerHooks(plugins=default_page_plugins(style=None)) + compile_ctx = CompileContext(pages=[], hooks=page_hooks) + + with compile_ctx, page_ctx: + page_ctx.root_component = page_hooks.compile_component( + page_ctx.root_component, + page_context=page_ctx, + compile_context=compile_ctx, + ) + page_hooks.compile_page(page_ctx, compile_context=compile_ctx) + + keys = page_ctx.app_wrap_components.keys() + assert (100, "StateProvider") not in keys + assert (90, "EventLoopProvider") not in keys + + +def test_compile_writes_upload_files_provider_via_var_app_wrap( + compilable_app: tuple[App, Path], + mocker: MockerFixture, +) -> None: + """A page that uses ``selected_files`` *without* an Upload component still + pulls the UploadFilesProvider in via the Var-declared app_wrap path. + """ + conf = rx.Config(app_name="testing") + mocker.patch("reflex_base.config._get_config", return_value=conf) + app, web_dir = compilable_app + + app.add_page( + lambda: rx.text(selected_files("custom-id").to_string()), + route="/", + ) + app._compile() + + root_js = web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT + root_contents = root_js.read_text() + assert "UploadFilesProvider" in root_contents + + +def test_selected_files_collects_upload_provider_without_upload_component() -> None: + """``selected_files`` alone pulls in UploadFilesProvider.""" + page_ctx = compile_page_context_for_app_wraps( + rx.text(selected_files("custom-id").to_string()) + ) + + assert (5, "UploadFilesProvider") in page_ctx.app_wrap_components + assert (100, "StateProvider") not in page_ctx.app_wrap_components + assert (90, "EventLoopProvider") not in page_ctx.app_wrap_components + + +def test_upload_root_collects_upload_and_event_providers() -> None: + """Upload root requires both upload context and event-loop providers.""" + page_ctx = compile_page_context_for_app_wraps( + rx.upload.root(rx.button("Select file")) + ) + + assert (5, "UploadFilesProvider") in page_ctx.app_wrap_components + assert (100, "StateProvider") in page_ctx.app_wrap_components + assert (90, "EventLoopProvider") in page_ctx.app_wrap_components + + @pytest.mark.parametrize( "react_strict_mode", [True, False], @@ -2333,19 +2554,29 @@ def page(): app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() + # AppWrap is now just hooks + ``return (children)``. The chain — including + # priority-ordered wrappers — is rendered by Layout, wrapping AppWrap. function_app_definition = app_js_contents[ app_js_contents.index("function AppWrap") : app_js_contents.index( "export function Layout" ) ].strip() - - expected = ( + assert function_app_definition == ( "function AppWrap({children}) {\n" "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" - "return (" - + ("jsx(StrictMode,{}," if react_strict_mode else "") + "return (children);\n}" + ) + + layout_definition = app_js_contents[ + app_js_contents.index("export function Layout") : + ].strip() + + expected_chain = ( + ("jsx(StrictMode,{}," if react_strict_mode else "") + + "jsx(StateProvider,{}," + "jsx(RadixThemesBox,{}," - "jsx(ErrorBoundary,{" + + "jsx(EventLoopProvider,{}," + + "jsx(ErrorBoundary,{" """fallbackRender:((event_args) => (jsx("div", ({css:({ ["height"] : "100%", ["width"] : "100%", ["position"] : "absolute", ["backgroundColor"] : "#fff", ["color"] : "#000", ["display"] : "flex", ["alignItems"] : "center", ["justifyContent"] : "center" })}), (jsx("div", ({css:({ ["display"] : "flex", ["flexDirection"] : "column", ["gap"] : "0.5rem", ["maxWidth"] : "min(80ch, 90vw)", ["borderRadius"] : "0.25rem", ["padding"] : "1rem" })}), (jsx("div", ({css:({ ["opacity"] : "0.5", ["display"] : "flex", ["gap"] : "4vmin", ["alignItems"] : "center" })}), (jsx("svg", ({className:"lucide lucide-frown-icon lucide-frown",fill:"none",stroke:"currentColor","stroke-linecap":"round","stroke-linejoin":"round","stroke-width":"2",viewBox:"0 0 24 24",width:"25vmin",xmlns:"http://www.w3.org/2000/svg"}), (jsx("circle", ({cx:"12",cy:"12",r:"10"}))), (jsx("path", ({d:"M16 16s-1.5-2-4-2-4 2-4 2"}))), (jsx("line", ({x1:"9",x2:"9.01",y1:"9",y2:"9"}))), (jsx("line", ({x1:"15",x2:"15.01",y1:"9",y2:"9"}))))), (jsx("h2", ({css:({ ["fontSize"] : "5vmin", ["fontWeight"] : "bold" })}), "An error occurred while rendering this page.")))), (jsx("p", ({css:({ ["opacity"] : "0.75", ["marginBlock"] : "1rem" })}), "This is an error with the application itself. Refreshing the page might help.")), (jsx("div", ({css:({ ["width"] : "100%", ["background"] : "color-mix(in srgb, currentColor 5%, transparent)", ["maxHeight"] : "15rem", ["overflow"] : "auto", ["borderRadius"] : "0.4rem" })}), (jsx("div", ({css:({ ["padding"] : "0.5rem" })}), (jsx("pre", ({css:({ ["wordBreak"] : "break-word", ["whiteSpace"] : "pre-wrap" })}), event_args.error.name + \': \' + event_args.error.message + \'\\n\' + event_args.error.stack)))))), (jsx("button", ({css:({ ["padding"] : "0.35rem 1.35rem", ["marginBlock"] : "0.5rem", ["marginInlineStart"] : "auto", ["background"] : "color-mix(in srgb, currentColor 15%, transparent)", ["borderRadius"] : "0.4rem", ["width"] : "fit-content", ["&:hover"] : ({ ["background"] : "color-mix(in srgb, currentColor 25%, transparent)" }), ["&:active"] : ({ ["background"] : "color-mix(in srgb, currentColor 35%, transparent)" }) }),onClick:((_e) => (addEvents([(ReflexEvent("_call_function", ({ ["function"] : (() => (navigator?.["clipboard"]?.["writeText"](event_args.error.name + \': \' + event_args.error.message + \'\\n\' + event_args.error.stack))), ["callback"] : null }), ({ })))], [_e], ({ }))))}), "Copy")), (jsx("hr", ({css:({ ["borderColor"] : "currentColor", ["opacity"] : "0.25" })}))), (jsx(ReactRouterLink, ({to:"https://reflex.dev"}), (jsx("div", ({css:({ ["display"] : "flex", ["alignItems"] : "baseline", ["justifyContent"] : "center", ["fontFamily"] : "monospace", ["--default-font-family"] : "monospace", ["gap"] : "0.5rem" })}), "Built with ", (jsx("svg", ({"aria-label":"Reflex",css:({ ["fill"] : "currentColor" }),height:"12",role:"img",width:"56",xmlns:"http://www.w3.org/2000/svg"}), (jsx("path", ({d:"M0 11.5999V0.399902H8.96V4.8799H6.72V2.6399H2.24V4.8799H6.72V7.1199H2.24V11.5999H0ZM6.72 11.5999V7.1199H8.96V11.5999H6.72Z"}))), (jsx("path", ({d:"M11.2 11.5999V0.399902H17.92V2.6399H13.44V4.8799H17.92V7.1199H13.44V9.3599H17.92V11.5999H11.2Z"}))), (jsx("path", ({d:"M20.16 11.5999V0.399902H26.88V2.6399H22.4V4.8799H26.88V7.1199H22.4V11.5999H20.16Z"}))), (jsx("path", ({d:"M29.12 11.5999V0.399902H31.36V9.3599H35.84V11.5999H29.12Z"}))), (jsx("path", ({d:"M38.08 11.5999V0.399902H44.8V2.6399H40.32V4.8799H44.8V7.1199H40.32V9.3599H44.8V11.5999H38.08Z"}))), (jsx("path", ({d:"M47.04 4.8799V0.399902H49.28V4.8799H47.04ZM53.76 4.8799V0.399902H56V4.8799H53.76ZM49.28 7.1199V4.8799H53.76V7.1199H49.28ZM47.04 11.5999V7.1199H49.28V11.5999H47.04ZM53.76 11.5999V7.1199H56V11.5999H53.76Z"}))), (jsx("title", ({}), "Reflex"))))))))))))),""" """onError:((_error, _info) => (addEvents([(ReflexEvent("reflex___state____state.reflex___state____frontend_event_exception_state.handle_frontend_exception", ({ ["info"] : ((((_error?.["name"]+": ")+_error?.["message"])+"\\n")+_error?.["stack"]), ["component_stack"] : _info?.["componentStack"] }), ({ })))], [_error, _info], ({ }))))""" + "}," @@ -2356,11 +2587,12 @@ def page(): + "jsx(Fragment2,{}," + "jsx(Fragment,{}," + "jsx(DefaultOverlayComponents,{},)," - + "jsx(Fragment,{}," + + "jsx(AppWrap,{}," + "children" - ")))))))" + (")" if react_strict_mode else "") + "))\n}" + + "))))))))))" + + (")" if react_strict_mode else "") ) - assert expected.split(",") == function_app_definition.split(",") + assert expected_chain in layout_definition def test_app_state_determination(): diff --git a/tests/units/test_event.py b/tests/units/test_event.py index c332a57b892..80175b92044 100644 --- a/tests/units/test_event.py +++ b/tests/units/test_event.py @@ -17,7 +17,7 @@ fix_events, ) from reflex_base.utils import format -from reflex_base.vars.base import Field, LiteralVar, Var, VarData, field +from reflex_base.vars.base import Field, LiteralVar, Var, field import reflex as rx from reflex.state import BaseState @@ -500,10 +500,20 @@ def _args_spec(value: Var[int]) -> tuple[Var[int]]: )._get_all_var_data() assert chain_var_data is not None - assert chain_var_data == VarData( - imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, - ) + # Imports include EVENTS, the events hook is registered, and the state + # context providers ride along as app_wraps so the compiler can mount + # them in the app root. Compare structurally — the providers themselves + # are fresh instances per call, so identity-based VarData equality won't + # match exact-instance comparison here. + assert dict(chain_var_data.imports) == { + k: tuple(v) for k, v in Imports.EVENTS.items() + } + assert chain_var_data.hooks == (Hooks.EVENTS,) + assert sorted(p for p, _ in chain_var_data.app_wraps) == [90, 100] + assert {wrapper.tag for _, wrapper in chain_var_data.app_wraps} == { + "StateProvider", + "EventLoopProvider", + } def test_event_chain_statement_block_preserves_nested_var_data(): diff --git a/tests/units/test_var.py b/tests/units/test_var.py index a96e4b2fb2e..3a1f43b0145 100644 --- a/tests/units/test_var.py +++ b/tests/units/test_var.py @@ -21,6 +21,7 @@ ComputedVar, LiteralVar, Var, + _decode_var_immutable, computed_var, var_operation, var_operation_return, @@ -1909,6 +1910,74 @@ def test_var_data_with_hooks_value(): assert var_data == VarData(hooks=["whott", "whot", "what"]) +def test_var_data_app_wraps_merge(): + """Var-declared app_wraps merge and dedupe by the compiler registry key.""" + wrapper_a = rx.fragment() + wrapper_b = rx.fragment() + + vd_a = VarData(app_wraps=((10, wrapper_a),)) + vd_b = VarData(app_wraps=((20, wrapper_b),)) + vd_dup = VarData(app_wraps=((10, wrapper_a),)) + + merged = VarData.merge(vd_a, vd_b, vd_dup) + assert merged is not None + assert (10, wrapper_a) in merged.app_wraps + assert (20, wrapper_b) in merged.app_wraps + assert len(merged.app_wraps) == 2 + + # Same priority/tag dedupes even when provider instances are fresh. + vd_same_key = VarData(app_wraps=((10, wrapper_b),)) + merged_same_key = VarData.merge(vd_a, vd_same_key) + assert merged_same_key is not None + assert merged_same_key.app_wraps == ((10, wrapper_a),) + + # Same component at a different priority is a distinct entry. + vd_alt = VarData(app_wraps=((30, wrapper_a),)) + merged_alt = VarData.merge(vd_a, vd_alt) + assert merged_alt is not None + assert len(merged_alt.app_wraps) == 2 + + # An empty VarData is falsy; one with app_wraps is truthy. + assert not VarData() + assert VarData(app_wraps=((10, wrapper_a),)) + + +def test_var_data_identity_hashes_component_metadata(): + """VarData hashes component metadata without hashing components directly.""" + wrapper_a = rx.fragment() + wrapper_b = rx.fragment() + + base = VarData(hooks="useFoo") + wrap_with_a = VarData(hooks="useFoo", app_wraps=((10, wrapper_a),)) + wrap_with_b = VarData(hooks="useFoo", app_wraps=((10, wrapper_b),)) + component_with_a = VarData(hooks="useFoo", components=(wrapper_a,)) + component_with_b = VarData(hooks="useFoo", components=(wrapper_b,)) + + assert base != wrap_with_a + assert wrap_with_a == wrap_with_b + assert hash(wrap_with_a) == hash(wrap_with_b) + assert component_with_a != component_with_b + + +def test_var_hash_keeps_app_wrap_metadata_distinct(): + """Formatted Var decode keeps app_wrap metadata when JS identity matches.""" + wrapper = rx.fragment() + var_with_wrap = Var( + _js_expr="same", + _var_type=str, + _var_data=VarData(app_wraps=((10, wrapper),)), + ) + plain_var = Var(_js_expr="same", _var_type=str, _var_data=VarData()) + + decoded_var_data, decoded_js_expr = _decode_var_immutable( + f"{var_with_wrap}{plain_var}" + ) + + assert decoded_js_expr == "samesame" + assert decoded_var_data is not None + assert decoded_var_data.app_wraps == ((10, wrapper),) + + def test_str_var_in_components(mocker: MockerFixture): class StateWithVar(rx.State): field: int = 1 From 22e039ec36ba103052d92a1805e29a4453d1ba87 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Mon, 4 May 2026 18:11:33 +0500 Subject: [PATCH 02/10] refactor: reach addEvents via module-level import instead of hook hoist EventLoopProvider now populates a module-level addEvents in $/utils/context, so JSX literals constructed outside the React-tree hoist path (e.g. ErrorBoundary.onError) can dispatch events without useContext(EventLoopContext) being lexically in scope. State and event-loop providers ride along on event-invocation VarData.app_wraps (and via _get_event_app_wraps) so they still mount in the app root. connectErrors stays on useContext since it drives re-renders; AppWrap is now a Fragment rendering the chain in its body. --- .../src/reflex_base/compiler/templates.py | 34 ++++++-- .../src/reflex_base/components/component.py | 41 +++++++--- .../reflex_base/components/state_context.py | 32 +++++--- .../src/reflex_base/constants/compiler.py | 18 ++++- .../src/reflex_base/event/__init__.py | 10 +-- .../reflex_components_core/base/app_wrap.py | 17 ++-- .../src/reflex_components_core/core/banner.py | 6 +- .../src/reflex_components_core/core/upload.py | 6 +- pyi_hashes.json | 4 +- reflex/compiler/plugins/builtin.py | 32 ++++++-- reflex/compiler/plugins/memoize.py | 6 ++ .../test_dynamic_components_codegen.py | 14 ++-- tests/units/test_app.py | 79 ++++++++----------- tests/units/test_event.py | 19 +++-- 14 files changed, 196 insertions(+), 122 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 36ff83d08c4..597683aa071 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -208,12 +208,9 @@ def app_root_template( {custom_code_str} -// AppWrap is the innermost element of the python app-wrap chain (rendered -// in Layout), so providers in the chain are React-tree ancestors of the -// hooks hoisted here. function AppWrap({{children}}) {{ {_render_hooks(hooks)} -return (children); +return ({_RenderUtils.render(render)}) }} @@ -228,7 +225,7 @@ def app_root_template( return jsx(AppLayout, {{}}, jsx(ThemeProvider, {{defaultTheme: defaultColorMode, attribute: "class"}}, - {_RenderUtils.render(render)} + jsx(AppWrap, {{}}, children) ) ); }} @@ -363,6 +360,24 @@ def context_template( export const isDevMode = {json.dumps(is_dev_mode)}; +// Module-level event dispatchers populated by ``EventLoopProvider`` on each +// render. Components reach addEvents/connectErrors via this import instead of +// hoisting ``useContext(EventLoopContext)`` so JSX literals (e.g. +// ``ErrorBoundary.onError``) constructed in any JS scope can dispatch events +// without depending on lexical hook hoisting. +let _addEventsImpl = (events, args, event_actions) => {{ + console.warn("addEvents called before EventLoopProvider mounted", events); +}}; +let _connectErrorsImpl = []; + +export function addEvents(events, args, event_actions) {{ + return _addEventsImpl(events, args, event_actions); +}} + +export function getConnectErrors() {{ + return _connectErrorsImpl; +}} + export function UploadFilesProvider({{ children }}) {{ const [filesById, setFilesById] = useState({{}}) refs["__clear_selected_files"] = (id) => setFilesById(filesById => {{ @@ -393,14 +408,19 @@ def context_template( export function EventLoopProvider({{ children }}) {{ const dispatch = useContext(DispatchContext) - const [addEvents, connectErrors] = useEventLoop( + const [addEventsLocal, connectErrors] = useEventLoop( dispatch, initialEvents, clientStorage, ) + // Populate the module-level dispatchers so JSX literals constructed + // outside the React-tree path (e.g. ``ErrorBoundary.onError``) can call + // ``addEvents`` without needing the events hook hoisted in their scope. + _addEventsImpl = addEventsLocal; + _connectErrorsImpl = connectErrors; return createElement( EventLoopContext.Provider, - {{ value: [addEvents, connectErrors] }}, + {{ value: [addEventsLocal, connectErrors] }}, children ); }} diff --git a/packages/reflex-base/src/reflex_base/components/component.py b/packages/reflex-base/src/reflex_base/components/component.py index 65143b88c0f..196955f7044 100644 --- a/packages/reflex-base/src/reflex_base/components/component.py +++ b/packages/reflex-base/src/reflex_base/components/component.py @@ -1879,20 +1879,15 @@ def _get_vars_hooks(self) -> dict[str, VarData | None]: def _get_events_hooks(self) -> dict[str, VarData | None]: """Get the hooks required by events referenced in this component. - The ``Hooks.EVENTS`` hook reads ``EventLoopContext``; declaring the - state/event-loop providers in its VarData pulls them into the app - root for every event-triggering component, independent of whether - the app uses ``rx.State`` directly. + ``addEvents`` is reached via the module-level import in + ``Imports.EVENTS``, so no in-scope hook is needed for events. + State/event-loop providers ride along on the event invocation's + ``VarData.app_wraps`` and via :meth:`_get_event_app_wraps`. Returns: - The hooks for the events. + An empty dict. """ - if not self.event_triggers: - return {} - # Lazy import: ``state_context`` imports ``Component`` from this module. - from reflex_base.components.state_context import get_events_hooks_var_data - - return {Hooks.EVENTS: get_events_hooks_var_data()} + return {} def _get_hooks_internal(self) -> dict[str, VarData | None]: """Get the React hooks for this component managed by the framework. @@ -2047,6 +2042,30 @@ def _get_app_wrap_components() -> dict[tuple[int, str], Component]: """ return {} + def _get_event_app_wraps(self) -> dict[tuple[int, str], Component]: + """Return state/event-loop providers required by event triggers. + + Kept separate from :meth:`_get_app_wrap_components` so subclass + overrides of that method don't inadvertently strip these out — + the providers must be in the React tree for ``StateContexts``, + ``EventLoopContext``, and the websocket plumbing to stay alive + even though ``addEvents`` itself is reached via module-level + import rather than a hook closure. + + Returns: + The state/event-loop provider entries (empty if no event + triggers are bound). + """ + if not self.event_triggers: + return {} + # Lazy import: state_context imports from this module. + from reflex_base.components.state_context import get_event_app_wraps + + return { + (priority, provider.tag or type(provider).__name__): provider + for priority, provider in get_event_app_wraps() + } + def _get_all_app_wrap_components( self, *, ignore_ids: set[int] | None = None ) -> dict[tuple[int, str], Component]: diff --git a/packages/reflex-base/src/reflex_base/components/state_context.py b/packages/reflex-base/src/reflex_base/components/state_context.py index b2e83fb6c0c..4bccc1c88cd 100644 --- a/packages/reflex-base/src/reflex_base/components/state_context.py +++ b/packages/reflex-base/src/reflex_base/components/state_context.py @@ -29,23 +29,31 @@ class EventLoopContextProvider(Component): tag = "EventLoopProvider" -def get_events_hooks_var_data() -> VarData: - """Build the VarData attached to ``Hooks.EVENTS`` for event triggers. +def get_event_app_wraps() -> tuple[tuple[int, Component], ...]: + """Return state/event-loop providers required when events are dispatched. + + ``StateProvider`` (100) wraps further out than ``EventLoopProvider`` + (90) because the latter reads ``DispatchContext`` from the former. + Providers are constructed fresh per call — module-level caching + breaks because ``copy.deepcopy`` (used when assembling the app-root + chain) carries ``_cached_render_result`` across compile runs. + + Returns: + ``(priority, provider)`` entries deduped by the compiler. + """ + return ( + (100, StateContextProvider.create()), + (90, EventLoopContextProvider.create()), + ) - Higher priority wraps further out, so ``StateProvider`` (100) encloses - ``EventLoopProvider`` (90) — the latter reads ``DispatchContext`` from - the former. The returned providers are fresh per call: the compiler's - ``app_wrap_components`` registry already dedupes by ``(priority, tag)``, - and caching the instances burned us via ``copy.deepcopy`` carrying - ``_cached_render_result`` from a prior compile run forward into the next. + +def get_events_hooks_var_data() -> VarData: + """Build the VarData advertising the state/event-loop app wraps. Returns: A new VarData carrying both providers as app_wraps. """ return VarData( position=Hooks.HookPosition.INTERNAL, - app_wraps=( - (100, StateContextProvider.create()), - (90, EventLoopContextProvider.create()), - ), + app_wraps=get_event_app_wraps(), ) diff --git a/packages/reflex-base/src/reflex_base/constants/compiler.py b/packages/reflex-base/src/reflex_base/constants/compiler.py index 8113f522484..ec68fe546dc 100644 --- a/packages/reflex-base/src/reflex_base/constants/compiler.py +++ b/packages/reflex-base/src/reflex_base/constants/compiler.py @@ -127,20 +127,34 @@ class CompileContext(str, Enum): class Imports(SimpleNamespace): """Common sets of import vars.""" + # ``addEvents`` is a module-level callable populated by + # ``EventLoopProvider``; importing it sidesteps the lexical-scope + # constraint a ``useContext(EventLoopContext)`` hoist would impose. EVENTS = { - "react": [ImportVar(tag="useContext")], - f"$/{Dirs.CONTEXTS_PATH}": [ImportVar(tag="EventLoopContext")], + f"$/{Dirs.CONTEXTS_PATH}": [ImportVar(tag=CompileVars.ADD_EVENTS)], f"$/{Dirs.STATE_PATH}": [ ImportVar(tag=CompileVars.TO_EVENT), ImportVar(tag=CompileVars.APPLY_EVENT_ACTIONS), ], } + # ``connectErrors`` is reactive — it drives connection-banner + # re-renders — so its consumers still go through ``useContext``. + CONNECT_ERRORS = { + "react": [ImportVar(tag="useContext")], + f"$/{Dirs.CONTEXTS_PATH}": [ImportVar(tag="EventLoopContext")], + } + class Hooks(SimpleNamespace): """Common sets of hook declarations.""" + # Kept for legacy callers that still key off this string; the + # compiler no longer auto-hoists it. EVENTS = f"const [{CompileVars.ADD_EVENTS}, {CompileVars.CONNECT_ERROR}] = useContext(EventLoopContext);" + CONNECT_ERRORS = ( + f"const {CompileVars.CONNECT_ERROR} = useContext(EventLoopContext)[1];" + ) class HookPosition(enum.Enum): """The position of the hook in the component.""" diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index 2b44e16094c..730a3672f66 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -2336,13 +2336,13 @@ def create( if value.invocation is None: # Lazy import: state_context → component → event (this module). - from reflex_base.components.state_context import get_events_hooks_var_data + from reflex_base.components.state_context import get_event_app_wraps invocation = FunctionStringVar.create( CompileVars.ADD_EVENTS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: get_events_hooks_var_data()}, + app_wraps=get_event_app_wraps(), ), ) else: @@ -2384,15 +2384,13 @@ def create( ) if value.event_actions: # Lazy import: state_context → component → event (this module). - from reflex_base.components.state_context import ( - get_events_hooks_var_data, - ) + from reflex_base.components.state_context import get_event_app_wraps apply_event_actions = FunctionStringVar.create( CompileVars.APPLY_EVENT_ACTIONS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: get_events_hooks_var_data()}, + app_wraps=get_event_app_wraps(), ), ) return_expr = apply_event_actions.call( diff --git a/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py b/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py index 8a26ffc510b..e9998ec8b7a 100644 --- a/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py +++ b/packages/reflex-components-core/src/reflex_components_core/base/app_wrap.py @@ -3,18 +3,17 @@ from reflex_base.components.component import Component from reflex_base.vars.base import Var +from reflex_components_core.base.fragment import Fragment -class AppWrap(Component): - """Innermost element of the app-wrap chain. - Renders as ``{children}`` — the locally-defined JS - function in ``app_root_template`` that hosts all hooks aggregated from - the python chain and returns its children. Library is ``None`` because - the JS function is defined in the same file the component renders into. - """ +class AppWrap(Fragment): + """Innermost (priority 0) element of the python app-wrap chain. - library = None - tag = "AppWrap" + Renders as ``jsx(Fragment, {}, children)`` — the chain ends here, with + the route ``children`` JS variable flowing through. Same-priority + siblings (e.g. ``StickyBadge``) get appended via the chain reducer and + sit alongside ``children`` inside this Fragment. + """ @classmethod def create(cls) -> Component: diff --git a/packages/reflex-components-core/src/reflex_components_core/core/banner.py b/packages/reflex-components-core/src/reflex_components_core/core/banner.py index 8d9af0bef76..7174ff1b3d5 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/banner.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/banner.py @@ -22,8 +22,8 @@ from reflex_components_core.el.elements.typography import Div connect_error_var_data: VarData = VarData( - imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + imports=Imports.CONNECT_ERRORS, + hooks={Hooks.CONNECT_ERRORS: None}, ) connect_errors = Var( @@ -173,7 +173,7 @@ def add_hooks(self) -> list[str | Var]: ] return [ - Hooks.EVENTS, + Hooks.CONNECT_ERRORS, *individual_hooks, ] diff --git a/packages/reflex-components-core/src/reflex_components_core/core/upload.py b/packages/reflex-components-core/src/reflex_components_core/core/upload.py index 5aecd609472..7951ea60abd 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/upload.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/upload.py @@ -13,9 +13,9 @@ field, ) from reflex_base.components.memoize_helpers import get_memoized_event_triggers -from reflex_base.components.state_context import get_events_hooks_var_data +from reflex_base.components.state_context import get_event_app_wraps from reflex_base.constants import Dirs -from reflex_base.constants.compiler import Hooks, Imports +from reflex_base.constants.compiler import Imports from reflex_base.environment import environment from reflex_base.event import ( CallableEventSpec, @@ -399,7 +399,7 @@ def create(cls, *children, **props) -> Component: var_data = VarData.merge( VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: get_events_hooks_var_data()}, + app_wraps=get_event_app_wraps(), ), use_dropzone_arguments._get_all_var_data(), VarData( diff --git a/pyi_hashes.json b/pyi_hashes.json index d60240cb251..6971b09327e 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -3,7 +3,7 @@ "packages/reflex-components-code/src/reflex_components_code/shiki_code_block.pyi": "d3e0c33fdc34f5c154ac387d550c0d29", "packages/reflex-components-core/src/reflex_components_core/__init__.pyi": "82b29d23f2490161d42fd21021bd39c3", "packages/reflex-components-core/src/reflex_components_core/base/__init__.pyi": "7009187aaaf191814d031e5462c48318", - "packages/reflex-components-core/src/reflex_components_core/base/app_wrap.pyi": "26d7284c583b0a5addd96c5ba13d62e0", + "packages/reflex-components-core/src/reflex_components_core/base/app_wrap.pyi": "ecccfd8a9b0e8b2f4128ff13ff27a9da", "packages/reflex-components-core/src/reflex_components_core/base/body.pyi": "2535814d409e5feaf57da63dcf0abeaf", "packages/reflex-components-core/src/reflex_components_core/base/document.pyi": "a2e67a9814dc61853ca2299d9d9c698d", "packages/reflex-components-core/src/reflex_components_core/base/error_boundary.pyi": "59170074a1a228ce58685f3f207954f2", @@ -20,7 +20,7 @@ "packages/reflex-components-core/src/reflex_components_core/core/helmet.pyi": "7fd81a99bde5b0ff94bb52523597fd5c", "packages/reflex-components-core/src/reflex_components_core/core/html.pyi": "753d6ae315369530dad450ed643f5be6", "packages/reflex-components-core/src/reflex_components_core/core/sticky.pyi": "ba60a7d9cba75b27a1133bd63a9fbd59", - "packages/reflex-components-core/src/reflex_components_core/core/upload.pyi": "13f1df7a87202cf74d8cde1717eace73", + "packages/reflex-components-core/src/reflex_components_core/core/upload.pyi": "0810ae4f1aa3c8fcaa228e7555c59f9a", "packages/reflex-components-core/src/reflex_components_core/core/window_events.pyi": "5e1dcb1130bc8af282783fae329ae6a6", "packages/reflex-components-core/src/reflex_components_core/datadisplay/__init__.pyi": "c96fed4da42a13576d64f84e3c7cb25c", "packages/reflex-components-core/src/reflex_components_core/el/__init__.pyi": "f09129ddefb57ab4c7769c86dc9a3153", diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index caa6b522d33..3f2a2940b4f 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -57,10 +57,12 @@ def collect_var_app_wraps_for_component( ) -> dict[tuple[int, str], Component]: """Return Var-declared app_wraps newly contributed by ``component``. - Scans the component's Vars (props/style/event-trigger args) and the + Scans the component's Vars (props/style/event-trigger args), the VarData attached to its framework-managed internal hooks (e.g. - ``Hooks.EVENTS``), so providers required by the hooks themselves — not - just by referenced Vars — surface to the page-level registry. + ``Hooks.EVENTS``), and any state/event-loop providers it requires + via :meth:`Component._get_event_app_wraps` — so the providers needed + for ``addEvents`` dispatch surface even when the page walker only + sees a memoization wrapper for the original event-triggering node. Entries already in ``page_app_wrap_components`` are skipped, leaving the caller to decide how to merge the result and whether to recurse into @@ -89,13 +91,17 @@ def collect_var_app_wraps_for_component( _ingest_var_data_app_wraps( wraps_by_key, page_app_wrap_components, hook_var_data ) + for key, wrapper in component._get_event_app_wraps().items(): + if key in page_app_wrap_components or key in wraps_by_key: + continue + wraps_by_key[key] = wrapper return wraps_by_key def _ingest_var_data_app_wraps( wraps_by_key: dict[tuple[int, str], Component], existing: dict[tuple[int, str], Component], - var_data: Any, + var_data: VarData, ) -> None: """Insert app_wraps carried or implied by ``var_data``.""" if var_data.app_wraps: @@ -303,7 +309,7 @@ def leave_component( if ( type(comp)._get_app_wrap_components is not Component._get_app_wrap_components - ): + ) or comp.event_triggers: self._collect_app_wrap_components( page_context.app_wrap_components, comp, @@ -382,11 +388,15 @@ def leave_component( collect_component_hooks(hooks, comp) app_wrap_method = type(comp)._get_app_wrap_components - if ( + has_subclass_override = ( app_wrap_method is not base_get_app_wrap_components + ) + if ( + has_subclass_override and app_wrap_method not in seen_app_wrap_methods - ): - seen_app_wrap_methods.add(app_wrap_method) + ) or comp.event_triggers: + if has_subclass_override: + seen_app_wrap_methods.add(app_wrap_method) collect_app_wrap_components(app_wrap_components, comp) collect_var_app_wraps(app_wrap_components, comp) @@ -446,6 +456,12 @@ def _collect_app_wrap_components( ) -> None: """Collect app-wrap components for a structural-tree component.""" direct_wrappers = component._get_app_wrap_components() + # Event-trigger providers ride alongside subclass-declared wraps so + # subclass overrides of ``_get_app_wrap_components`` (e.g. radix + # color-mode) don't strip them. ``setdefault`` preserves a wrap a + # subclass already declared at the same key. + for key, wrapper in component._get_event_app_wraps().items(): + direct_wrappers.setdefault(key, wrapper) if not direct_wrappers: return diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index 7c06c0c6e7f..8147f9904c1 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -140,6 +140,12 @@ def _component_subtree_is_reactive( return True if component._get_hooks() is not None or component._get_added_hooks(): return True + # ``addEvents`` no longer hoists a hook here (it's reached via the + # module-level import in ``Imports.EVENTS``), so a no-arg + # ``on_click=State.ping`` only shows up as ``event_triggers`` — without + # this check the boundary skips memoization and the callback leaks. + if component.event_triggers: + return True for var in component._get_vars(include_children=False): var_data = var._get_all_var_data() if var_data is None: diff --git a/tests/units/compiler/test_dynamic_components_codegen.py b/tests/units/compiler/test_dynamic_components_codegen.py index fe859984346..7f9344a149f 100644 --- a/tests/units/compiler/test_dynamic_components_codegen.py +++ b/tests/units/compiler/test_dynamic_components_codegen.py @@ -25,13 +25,15 @@ def test_dynamic_component_codegen_wires_event_handlers() -> None: assert isinstance(code, str) assert code.startswith("//__reflex_evaluate") - assert "const {Fragment,useContext,useEffect}" in code - assert "const {EventLoopContext} = window['__reflex'][\"$/utils/context\"]" in code + assert "const {Fragment,useEffect}" in code + # ``addEvents`` is now a module-level callable in ``$/utils/context``; + # no more ``useContext(EventLoopContext)`` hoist needed for dispatch. + assert "const {addEvents} = window['__reflex'][\"$/utils/context\"]" in code assert ( "const {ReflexEvent,applyEventActions} = window['__reflex'][\"$/utils/state\"]" in code ) - assert "const [addEvents, connectErrors] = useContext(EventLoopContext);" in code + assert "useContext(EventLoopContext)" not in code assert code.count("onClick:") == 2 assert code.count("addEvents(") == 2 assert code.count("ReflexEvent(") == 2 @@ -91,13 +93,13 @@ def counter_ui(self) -> rx.Component: assert "RadixThemesText" in code assert 'justify:"center"' in code assert 'gap:"5"' in code - assert "const {Fragment,useContext,useEffect}" in code - assert "const {EventLoopContext} = window['__reflex'][\"$/utils/context\"]" in code + assert "const {Fragment,useEffect}" in code + assert "const {addEvents} = window['__reflex'][\"$/utils/context\"]" in code assert ( "const {ReflexEvent,applyEventActions} = window['__reflex'][\"$/utils/state\"]" in code ) - assert "const [addEvents, connectErrors] = useContext(EventLoopContext);" in code + assert "useContext(EventLoopContext)" not in code assert code.count("onClick:") == 2 assert code.count("addEvents(") == 2 assert code.count("ReflexEvent(") == 2 diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 987c71babd8..1bda7259da8 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -43,18 +43,13 @@ from reflex.compiler.compiler import _compile_app, _resolve_app_wrap_components from reflex.compiler.plugins import default_page_plugins from reflex.environment import environment +from reflex.istate.data import RouterData from reflex.istate.manager.disk import StateManagerDisk from reflex.istate.manager.memory import StateManagerMemory from reflex.istate.manager.redis import StateManagerRedis from reflex.istate.manager.token import BaseStateToken from reflex.model import Model -from reflex.state import ( - BaseState, - OnLoadInternalState, - RouterData, - State, - reload_state_module, -) +from reflex.state import BaseState, OnLoadInternalState, State, reload_state_module from .conftest import chdir from .states import GenState @@ -2136,25 +2131,20 @@ def test_app_wrap_compile_theme( app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() - # AppWrap is now just hooks + ``return (children)`` — the python app-wrap - # chain is rendered by Layout and wraps AppWrap externally. + # AppWrap renders the entire chain in its body. ``addEvents`` is now a + # module-level callable (see ``$/utils/context``) so no + # ``useContext(EventLoopContext)`` hoist is needed; the events hook + # block is empty. State/event-loop providers ride along as the highest + # collected app wraps, ahead of ErrorBoundary etc. function_app_definition = app_js_contents[ app_js_contents.index("function AppWrap") : app_js_contents.index( "export function Layout" ) ].strip() - assert function_app_definition == ( - "function AppWrap({children}) {\n" - "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" - "return (children);\n}" - ) - - layout_definition = app_js_contents[ - app_js_contents.index("export function Layout") : - ].strip() - - expected_chain = ( - ("jsx(StrictMode,{}," if react_strict_mode else "") + expected = ( + "function AppWrap({children}) {\n\n\n\n\n" + "return (" + + ("jsx(StrictMode,{}," if react_strict_mode else "") + "jsx(StateProvider,{}," + "jsx(EventLoopProvider,{}," + "jsx(ErrorBoundary,{" @@ -2167,12 +2157,13 @@ def test_app_wrap_compile_theme( + "jsx(RadixThemesTheme,{accentColor:\"plum\",css:{...theme.styles.global[':root'], ...theme.styles.global.body}}," + "jsx(Fragment,{}," + "jsx(DefaultOverlayComponents,{},)," - + "jsx(AppWrap,{}," + + "jsx(Fragment,{}," + "children" - "))))))))" + (")" if react_strict_mode else "") + + "))))))))" + + (")" if react_strict_mode else "") + + ")\n}" ) - # Layout's body now contains the chain wrapping AppWrap. - assert expected_chain in layout_definition + assert expected.split(",") == function_app_definition.split(",") def test_compile_without_radix_components_skips_radix_plugin( @@ -2354,11 +2345,13 @@ def test_compile_writes_upload_files_provider_app_wrap( def test_app_wrap_event_hook_requires_state_providers(mocker: MockerFixture) -> None: - """App-root hooks from default app-wrap prop components need providers. + """App-root chain components with event triggers pull state/event providers. - The default error boundary's fallback render contains a copy button, which - contributes the event-loop hook through a component-valued prop. If that - hook is emitted at AppWrap scope, the provider chain must be present too. + The default error boundary's fallback render contains a Copy button. + The button has an ``on_click`` handler, so the page collector pulls + ``StateProvider`` and ``EventLoopProvider`` into the app root via the + event-trigger app-wrap path. ``addEvents`` itself reaches its call + sites through the module-level import — no ``useContext`` hoist. """ conf = rx.Config(app_name="testing") mocker.patch("reflex_base.config._get_config", return_value=conf) @@ -2366,7 +2359,7 @@ def test_app_wrap_event_hook_requires_state_providers(mocker: MockerFixture) -> root_contents = compile_app_root_from_page_wraps(app, {}) - assert EVENT_LOOP_CONTEXT_HOOK in root_contents + assert EVENT_LOOP_CONTEXT_HOOK not in root_contents assert "jsx(StateProvider" in root_contents assert "jsx(EventLoopProvider" in root_contents @@ -2554,25 +2547,18 @@ def page(): app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() - # AppWrap is now just hooks + ``return (children)``. The chain — including - # priority-ordered wrappers — is rendered by Layout, wrapping AppWrap. + # AppWrap renders the priority-ordered chain inside its body. + # ``addEvents`` is reached via module-level import so the events hook + # block is empty. function_app_definition = app_js_contents[ app_js_contents.index("function AppWrap") : app_js_contents.index( "export function Layout" ) ].strip() - assert function_app_definition == ( - "function AppWrap({children}) {\n" - "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" - "return (children);\n}" - ) - - layout_definition = app_js_contents[ - app_js_contents.index("export function Layout") : - ].strip() - - expected_chain = ( - ("jsx(StrictMode,{}," if react_strict_mode else "") + expected = ( + "function AppWrap({children}) {\n\n\n\n\n" + "return (" + + ("jsx(StrictMode,{}," if react_strict_mode else "") + "jsx(StateProvider,{}," + "jsx(RadixThemesBox,{}," + "jsx(EventLoopProvider,{}," @@ -2587,12 +2573,13 @@ def page(): + "jsx(Fragment2,{}," + "jsx(Fragment,{}," + "jsx(DefaultOverlayComponents,{},)," - + "jsx(AppWrap,{}," + + "jsx(Fragment,{}," + "children" + "))))))))))" + (")" if react_strict_mode else "") + + ")\n}" ) - assert expected_chain in layout_definition + assert expected.split(",") == function_app_definition.split(",") def test_app_state_determination(): diff --git a/tests/units/test_event.py b/tests/units/test_event.py index 80175b92044..9614f69e010 100644 --- a/tests/units/test_event.py +++ b/tests/units/test_event.py @@ -500,15 +500,16 @@ def _args_spec(value: Var[int]) -> tuple[Var[int]]: )._get_all_var_data() assert chain_var_data is not None - # Imports include EVENTS, the events hook is registered, and the state - # context providers ride along as app_wraps so the compiler can mount - # them in the app root. Compare structurally — the providers themselves - # are fresh instances per call, so identity-based VarData equality won't - # match exact-instance comparison here. + # Imports include EVENTS (which now imports module-level ``addEvents``) + # and the state/event-loop providers ride along as app_wraps so the + # compiler can mount them in the app root. ``addEvents`` reaches its + # call sites through the import, not a hoisted hook, so ``hooks`` is + # empty here. Compare structurally — providers are fresh instances per + # call, so identity-based VarData equality wouldn't match. assert dict(chain_var_data.imports) == { k: tuple(v) for k, v in Imports.EVENTS.items() } - assert chain_var_data.hooks == (Hooks.EVENTS,) + assert chain_var_data.hooks == () assert sorted(p for p, _ in chain_var_data.app_wraps) == [90, 100] assert {wrapper.tag for _, wrapper in chain_var_data.app_wraps} == { "StateProvider", @@ -539,7 +540,11 @@ def s(self, value: int): assert chain_var_data.state == x_var_data.state assert chain_var_data.field_name == x_var_data.field_name assert x_var_data.hooks[0] in chain_var_data.hooks - assert Hooks.EVENTS in chain_var_data.hooks + # ``addEvents`` is reached via module-level import, so the events hook + # is no longer hoisted on event-chain VarData. State/event-loop providers + # ride on ``app_wraps`` to surface in the app root when needed. + assert Hooks.EVENTS not in chain_var_data.hooks + assert sorted(p for p, _ in chain_var_data.app_wraps) == [90, 100] def test_event_bound_method() -> None: From d3a56a0a20f9f4225d80a15fdbb939c9afde271c Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 21:18:45 +0500 Subject: [PATCH 03/10] fix: don't memoize subtrees reactive only via no-arg event triggers Per review: a no-arg handler (on_click=State.ping) surfaces only through event_triggers and reaches addEvents via a module-level import, not a hoisted hook. The inline callback carries no reactive data and never drives a re-render, so wrapping it in a memo gains nothing. Drop the event_triggers fast-path from the reactive-data check and let such callbacks render in the page module. Handlers that reference state still surface through the per-Var scan and memoize as before. --- reflex/compiler/plugins/memoize.py | 19 ++++++++----------- tests/units/compiler/test_memoize_plugin.py | 18 +++++++++--------- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index 925030a4675..3eb4bc51873 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -46,9 +46,12 @@ def _subtree_has_reactive_data( ) -> bool: """Whether ``component``'s subtree carries reactive signals worth memoizing. - No-arg event handlers (``on_click=State.ping``) contribute hooks only via - ``event_triggers`` / ``_get_events_hooks``, not as a Var, so the per-Var - scan must be paired with an explicit ``event_triggers`` check. + No-arg event handlers (``on_click=State.ping``) are deliberately not + treated as reactive on their ``event_triggers`` alone: ``addEvents`` is + reached through a module-level import (``Imports.EVENTS``) rather than a + hoisted hook, so the inline callback never drives a re-render and the + subtree gains nothing from being wrapped. Handlers that reference state + (e.g. ``State.set_x(State.y)``) still surface through the per-Var scan. ``useRef`` from a static ``id`` prop is intentionally ignored — it lives in ``_get_hooks_internal``, not in any Var, so static-id-only elements @@ -63,8 +66,8 @@ def _subtree_has_reactive_data( break cycles. Returns: - True if the subtree carries event triggers, explicit hooks, or any - Var whose merged var_data has ``state`` or ``hooks``. + True if the subtree carries explicit or internal hooks, or any Var + whose merged var_data has ``state`` or ``hooks``. """ if _cache is None: _cache = {} @@ -106,12 +109,6 @@ def _component_subtree_is_reactive( return True if component._get_hooks() is not None or component._get_added_hooks(): return True - # ``addEvents`` no longer hoists a hook here (it's reached via the - # module-level import in ``Imports.EVENTS``), so a no-arg - # ``on_click=State.ping`` only shows up as ``event_triggers`` — without - # this check the boundary skips memoization and the callback leaks. - if component.event_triggers: - return True for var in component._get_vars(include_children=False): var_data = var._get_all_var_data() if var_data is None: diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index a872936269b..19c25c42fb7 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -1458,19 +1458,19 @@ def test_snapshot_boundary_with_event_trigger_descendant_is_wrapped() -> None: ) -def test_snapshot_boundary_with_no_arg_event_handler_descendant_is_wrapped() -> None: - """A boundary whose descendant has on_click without arg vars still wraps. - - No-arg handlers (``on_click=State.ping``) contribute to the page only - via the descendant's ``event_triggers`` and ``_get_events_hooks`` — the - per-Var subtree scan misses them. The reactive-data check must also - inspect ``event_triggers`` directly so the boundary wraps and the - callback's ``useCallback`` lands inside the snapshot body. +def test_snapshot_boundary_with_no_arg_event_handler_descendant_not_wrapped() -> None: + """A boundary whose descendant has only a no-arg on_click is not wrapped. + + No-arg handlers (``on_click=State.ping``) surface only through the + descendant's ``event_triggers`` and reach ``addEvents`` via a + module-level import rather than a hoisted hook. The inline callback + carries no reactive data and never drives a re-render, so the boundary + gains nothing from memoization and is left to render in the page module. """ inner = Plain.create() inner.event_triggers["on_click"] = Var(_js_expr="evt") boundary = LeafComponent.create(inner) - assert _should_memoize(boundary) + assert not _should_memoize(boundary) def test_title_with_stateful_var_child_does_not_wrap_bare_independently() -> None: From cf202d0eff7dcd1725da26c4efbe80da6ff4c4d0 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 21:48:33 +0500 Subject: [PATCH 04/10] fix: clear render cache on Component deepcopy to prevent dropped children App._app_root deep-copies app-wrap components and rebinds their children to assemble the provider chain. __copy__ already drops the render-path caches for this reason, but copy.deepcopy preserved them, so a component rendered during page compilation (e.g. a State/EventLoop/UploadFiles provider injected via VarData.app_wraps) returned its stale, childless _cached_render_result after children were appended. The page content (children/Outlet) was silently dropped and the page rendered blank. Add a __deepcopy__ that mirrors __copy__: the clone is for compile-time mutation, so render caches are not carried over. Extract the shared cache attr list to _COMPILE_CACHE_ATTRS. Regression test fails before the fix. --- .../src/reflex_base/components/component.py | 44 ++++++++++++++++--- tests/units/components/test_component.py | 31 +++++++++++++ 2 files changed, 68 insertions(+), 7 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/components/component.py b/packages/reflex-base/src/reflex_base/components/component.py index 1181147d9a6..e04cce1108f 100644 --- a/packages/reflex-base/src/reflex_base/components/component.py +++ b/packages/reflex-base/src/reflex_base/components/component.py @@ -3,6 +3,7 @@ from __future__ import annotations import contextlib +import copy import dataclasses import enum import functools @@ -279,6 +280,15 @@ def _finalize_fields( } +_COMPILE_CACHE_ATTRS = ( + "_cached_render_result", + "_vars_cache", + "_imports_cache", + "_hooks_internal_cache", + "_get_component_prop_property", +) + + class BaseComponent(metaclass=BaseComponentMeta): """The base class for all Reflex components. @@ -349,16 +359,36 @@ def __copy__(self) -> BaseComponent: new._clear_compile_caches() return new + def __deepcopy__(self, memo: dict[int, Any]) -> BaseComponent: + """Return a deep copy suitable for compile-time mutation. + + Like :meth:`__copy__`, the clone exists for the compiler to mutate + (e.g. rebinding ``children`` while assembling the app-wrap chain in + ``App._app_root``), so the render-path caches populated on the + original are not carried over — otherwise ``render()`` on the mutated + clone would return the pre-mutation result. Unlike ``__copy__``, + nested mutable containers are deep-copied so the clone is fully + independent of the original. + + Args: + memo: The deepcopy memo mapping object ids to their copies. + + Returns: + A deep-copied instance with compile-time caches dropped. + """ + new = self.__class__.__new__(self.__class__) + memo[id(self)] = new + new_dict = vars(new) + for key, value in vars(self).items(): + if key in _COMPILE_CACHE_ATTRS: + continue + new_dict[key] = copy.deepcopy(value, memo) + return new + def _clear_compile_caches(self) -> None: """Clear cached render/compiler artifacts after compile-time mutation.""" attrs = cast("dict[str, Any]", vars(self)) - for attr in ( - "_cached_render_result", - "_vars_cache", - "_imports_cache", - "_hooks_internal_cache", - "_get_component_prop_property", - ): + for attr in _COMPILE_CACHE_ATTRS: attrs.pop(attr, None) def __eq__(self, value: Any) -> bool: diff --git a/tests/units/components/test_component.py b/tests/units/components/test_component.py index e22aa1434bc..d0e2aa09c06 100644 --- a/tests/units/components/test_component.py +++ b/tests/units/components/test_component.py @@ -1,3 +1,4 @@ +import copy from contextlib import nullcontext from dataclasses import dataclass from typing import Any, ClassVar @@ -2263,3 +2264,33 @@ class VarProbe(Component): dict_a = VarProbe.create(meta={"k": VarState.text}) dict_b = VarProbe.create(meta={"k": VarState.text}) assert dict_a == dict_b + + +def test_deepcopy_drops_stale_render_cache() -> None: + """A deep-copied component must re-render after its children are mutated. + + ``render()`` memoizes ``_cached_render_result``. The compiler deep-copies + app-wrap components and rebinds their ``children`` (e.g. in + ``App._app_root``); if the clone kept the original's cache, the appended + child would be silently dropped — the page content ``children`` would + never reach the rendered tree and the page would render blank. + """ + original = Fragment.create() + original.render() # populate _cached_render_result with no children + + clone = copy.deepcopy(original) + clone.children.append(Bare.create(contents="page-content")) + + assert len(clone.render()["children"]) == 1 + # The original must be untouched (independent deep copy). + assert original.render()["children"] == [] + + +def test_deepcopy_produces_independent_children() -> None: + """Deep copy must not share the ``children`` list with the original.""" + original = Fragment.create(Bare.create(contents="a")) + clone = copy.deepcopy(original) + clone.children.append(Bare.create(contents="b")) + + assert len(original.children) == 1 + assert len(clone.children) == 2 From ef1964864689a1a257543f13c211f359ab3e0c5b Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 21:59:18 +0500 Subject: [PATCH 05/10] perf: reuse fetched hooks across page-hook and app-wrap collection The page collector already pulls _get_hooks_internal/_get_added_hooks for page-hook aggregation; pass those dicts into the Var app-wrap scan instead of re-fetching, so each component avoids a second (uncached) _get_added_hooks and _get_event_app_wraps per compile. Event-trigger providers now surface solely through the Var scan, dropping the event_triggers special-case in the subclass-override app-wrap path. --- reflex/compiler/plugins/builtin.py | 164 +++++++++++++++++++---------- 1 file changed, 108 insertions(+), 56 deletions(-) diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index 3f2a2940b4f..513da26cf83 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -51,50 +51,73 @@ def collect_var_app_wraps_in_subtree( ) -def collect_var_app_wraps_for_component( - page_app_wrap_components: dict[tuple[int, str], Component], +def _ingest_component_var_app_wraps( + wraps_by_key: dict[tuple[int, str], Component], + existing: dict[tuple[int, str], Component], component: Component, -) -> dict[tuple[int, str], Component]: - """Return Var-declared app_wraps newly contributed by ``component``. - - Scans the component's Vars (props/style/event-trigger args), the - VarData attached to its framework-managed internal hooks (e.g. - ``Hooks.EVENTS``), and any state/event-loop providers it requires - via :meth:`Component._get_event_app_wraps` — so the providers needed - for ``addEvents`` dispatch surface even when the page walker only - sees a memoization wrapper for the original event-triggering node. - - Entries already in ``page_app_wrap_components`` are skipped, leaving the - caller to decide how to merge the result and whether to recurse into - each wrapper's own subtree. - - Returns: - Mapping of ``(priority, name)`` -> wrapper for new entries only. + hooks_internal: dict[str, VarData | None], + added_hooks: dict[str, VarData | None], +) -> None: + """Ingest app_wraps from a component's Vars, pre-fetched hooks, and events. + + Scans the component's Vars (props/style/event-trigger args), the VarData on + its framework-managed internal hooks and ``add_hooks`` output (e.g. + ``Hooks.EVENTS``), and the state/event-loop providers it requires via + :meth:`Component._get_event_app_wraps`. + + ``hooks_internal`` and ``added_hooks`` are supplied by the caller rather than + re-fetched here so the page collector — which already pulls them to populate + ``page_hooks`` — doesn't pay for a second ``_get_hooks_internal`` / + ``_get_added_hooks`` (and the latter is uncached). New entries are written + into ``wraps_by_key``; entries already in ``existing`` are skipped. """ - wraps_by_key: dict[tuple[int, str], Component] = {} for var in component._get_vars(): var_data = var._get_all_var_data() if var_data is None: continue - _ingest_var_data_app_wraps(wraps_by_key, page_app_wrap_components, var_data) - for hook_var_data in component._get_hooks_internal().values(): + _ingest_var_data_app_wraps(wraps_by_key, existing, var_data) + for hook_var_data in hooks_internal.values(): if hook_var_data is None: continue - _ingest_var_data_app_wraps( - wraps_by_key, page_app_wrap_components, hook_var_data - ) - for hook, hook_var_data in component._get_added_hooks().items(): + _ingest_var_data_app_wraps(wraps_by_key, existing, hook_var_data) + for hook, hook_var_data in added_hooks.items(): if hook_var_data is None and hook == Hooks.EVENTS: hook_var_data = get_events_hooks_var_data() if hook_var_data is None: continue - _ingest_var_data_app_wraps( - wraps_by_key, page_app_wrap_components, hook_var_data - ) + _ingest_var_data_app_wraps(wraps_by_key, existing, hook_var_data) for key, wrapper in component._get_event_app_wraps().items(): - if key in page_app_wrap_components or key in wraps_by_key: + if key in existing or key in wraps_by_key: continue wraps_by_key[key] = wrapper + + +def collect_var_app_wraps_for_component( + page_app_wrap_components: dict[tuple[int, str], Component], + component: Component, +) -> dict[tuple[int, str], Component]: + """Return Var-declared app_wraps newly contributed by ``component``. + + Convenience wrapper over :func:`_ingest_component_var_app_wraps` for callers + (snapshot-boundary and app-root walks) that don't already have the + component's hooks in hand. The page collector fetches the hooks once and + calls the underlying helper directly instead. + + Entries already in ``page_app_wrap_components`` are skipped, leaving the + caller to decide how to merge the result and whether to recurse into + each wrapper's own subtree. + + Returns: + Mapping of ``(priority, name)`` -> wrapper for new entries only. + """ + wraps_by_key: dict[tuple[int, str], Component] = {} + _ingest_component_var_app_wraps( + wraps_by_key, + page_app_wrap_components, + component, + component._get_hooks_internal(), + component._get_added_hooks(), + ) return wraps_by_key @@ -303,19 +326,28 @@ def leave_component( self._collect_component_custom_code(page_context.module_code, comp) + # Fetch once and reuse for both page-hook aggregation and the app-wrap + # scan; ``_get_added_hooks`` is uncached, so a second call recomputes. + hooks_internal = comp._get_hooks_internal() + added_hooks = comp._get_added_hooks() + if not in_prop_tree: - self._collect_component_hooks(page_context.hooks, comp) + self._apply_component_hooks( + page_context.hooks, comp, hooks_internal, added_hooks + ) if ( type(comp)._get_app_wrap_components is not Component._get_app_wrap_components - ) or comp.event_triggers: + ): self._collect_app_wrap_components( page_context.app_wrap_components, comp, ) - self._collect_var_app_wraps(page_context.app_wrap_components, comp) + self._collect_var_app_wraps( + page_context.app_wrap_components, comp, hooks_internal, added_hooks + ) if (dynamic_import := comp._get_dynamic_imports()) is not None: page_context.dynamic_imports.add(dynamic_import) @@ -363,7 +395,7 @@ def _compiler_bind_leave_component( refs = page_context.refs app_wrap_components = page_context.app_wrap_components extend_imports = self._extend_imports - collect_component_hooks = self._collect_component_hooks + apply_component_hooks = self._apply_component_hooks collect_component_custom_code = self._collect_component_custom_code collect_app_wrap_components = self._collect_app_wrap_components collect_var_app_wraps = self._collect_var_app_wraps @@ -384,22 +416,25 @@ def leave_component( collect_component_custom_code(module_code, comp) + # Fetch once and reuse for both page-hook aggregation and the + # app-wrap scan; ``_get_added_hooks`` is uncached. + hooks_internal = comp._get_hooks_internal() + added_hooks = comp._get_added_hooks() + if not in_prop_tree: - collect_component_hooks(hooks, comp) + apply_component_hooks(hooks, comp, hooks_internal, added_hooks) app_wrap_method = type(comp)._get_app_wrap_components - has_subclass_override = ( - app_wrap_method is not base_get_app_wrap_components - ) if ( - has_subclass_override + app_wrap_method is not base_get_app_wrap_components and app_wrap_method not in seen_app_wrap_methods - ) or comp.event_triggers: - if has_subclass_override: - seen_app_wrap_methods.add(app_wrap_method) + ): + seen_app_wrap_methods.add(app_wrap_method) collect_app_wrap_components(app_wrap_components, comp) - collect_var_app_wraps(app_wrap_components, comp) + collect_var_app_wraps( + app_wrap_components, comp, hooks_internal, added_hooks + ) dynamic_import = comp._get_dynamic_imports() if dynamic_import is not None: @@ -412,15 +447,22 @@ def leave_component( return leave_component @staticmethod - def _collect_component_hooks( + def _apply_component_hooks( page_hooks: dict[str, VarData | None], component: Component, + hooks_internal: dict[str, VarData | None], + added_hooks: dict[str, VarData | None], ) -> None: - """Collect hooks for one structural-tree component in legacy order.""" - page_hooks.update(component._get_hooks_internal()) + """Add one structural-tree component's hooks in legacy order. + + ``hooks_internal`` and ``added_hooks`` are passed in (rather than + re-fetched) so the same dicts feed both this aggregation and the + app-wrap scan. + """ + page_hooks.update(hooks_internal) if (user_hooks := component._get_hooks()) is not None: page_hooks[user_hooks] = None - page_hooks.update(component._get_added_hooks()) + page_hooks.update(added_hooks) @staticmethod def _extend_imports( @@ -454,14 +496,12 @@ def _collect_app_wrap_components( page_app_wrap_components: dict[tuple[int, str], Component], component: Component, ) -> None: - """Collect app-wrap components for a structural-tree component.""" + """Collect subclass-declared app-wrap components for a component. + + Var-driven providers (including event-trigger ones) are collected by + :meth:`_collect_var_app_wraps`, which visits every component. + """ direct_wrappers = component._get_app_wrap_components() - # Event-trigger providers ride alongside subclass-declared wraps so - # subclass overrides of ``_get_app_wrap_components`` (e.g. radix - # color-mode) don't strip them. ``setdefault`` preserves a wrap a - # subclass already declared at the same key. - for key, wrapper in component._get_event_app_wraps().items(): - direct_wrappers.setdefault(key, wrapper) if not direct_wrappers: return @@ -482,10 +522,22 @@ def _collect_var_app_wraps( self, page_app_wrap_components: dict[tuple[int, str], Component], component: Component, + hooks_internal: dict[str, VarData | None], + added_hooks: dict[str, VarData | None], ) -> None: - """Collect app-wrap components declared by VarData on ``component``.""" - wraps_by_key = collect_var_app_wraps_for_component( - page_app_wrap_components, component + """Collect app-wrap components declared by VarData on ``component``. + + ``hooks_internal`` and ``added_hooks`` are the dicts already fetched for + page-hook aggregation, reused here to avoid a second uncached + ``_get_added_hooks`` per component. + """ + wraps_by_key: dict[tuple[int, str], Component] = {} + _ingest_component_var_app_wraps( + wraps_by_key, + page_app_wrap_components, + component, + hooks_internal, + added_hooks, ) if not wraps_by_key: return From beb3221230a9ade329964adb1ea07c81e47c9d5f Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 22:19:32 +0500 Subject: [PATCH 06/10] fix: mixed event/function dispatch must reach addEvents via module import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The mixed-dispatch path (_dispatch_mixed_event_var, used by on_click=lambda: rx.cond(..., function_var, event_spec)) still emitted the legacy hooks={Hooks.EVENTS: None} hook — const [addEvents, connectErrors] = useContext(EventLoopContext) — but Imports.EVENTS no longer imports EventLoopContext. The rendered component threw "ReferenceError: EventLoopContext is not defined", crashing the page and failing the test_event_chain_click integration tests. Match the other dispatch sites: reach addEvents through the module-level import and ride the state/event-loop providers on app_wraps via get_event_app_wraps(). Drop the now-unused Hooks import. Regression test covers the mixed-dispatch VarData. --- .../src/reflex_base/event/__init__.py | 7 ++- tests/units/test_event.py | 44 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/event/__init__.py b/packages/reflex-base/src/reflex_base/event/__init__.py index 007eb92da4f..842caeae089 100644 --- a/packages/reflex-base/src/reflex_base/event/__init__.py +++ b/packages/reflex-base/src/reflex_base/event/__init__.py @@ -29,7 +29,7 @@ from reflex_base import constants from reflex_base.components.field import BaseField -from reflex_base.constants.compiler import CompileVars, Hooks, Imports +from reflex_base.constants.compiler import CompileVars, Imports from reflex_base.utils import format from reflex_base.utils.decorator import once from reflex_base.utils.exceptions import ( @@ -2098,11 +2098,14 @@ def _dispatch_mixed_event_var(event_like_var: Var) -> FunctionVar: _js_expr=f'typeof {alias_name} === "function"', _var_type=bool, ) + # Lazy import: state_context → component → event (this module). + from reflex_base.components.state_context import get_event_app_wraps + add_events = FunctionStringVar.create( CompileVars.ADD_EVENTS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + app_wraps=get_event_app_wraps(), ), ) dispatch_expr = ternary_operation( diff --git a/tests/units/test_event.py b/tests/units/test_event.py index e80fb3403d5..72cc9a7da29 100644 --- a/tests/units/test_event.py +++ b/tests/units/test_event.py @@ -868,6 +868,50 @@ def return_conditional_mixed(v: Var[Any]) -> Any: assert "addEvents(" in rendered +def test_event_chain_mixed_dispatch_reaches_addevents_via_module_import(): + """The mixed function/event dispatcher must not emit a stale events hook. + + ``Imports.EVENTS`` no longer imports ``EventLoopContext``; the mixed + dispatcher reaches the module-level ``addEvents`` instead. Emitting the + legacy ``const [addEvents, connectErrors] = useContext(EventLoopContext)`` + hook here would render a component that throws ``ReferenceError: + EventLoopContext is not defined``. State/event-loop providers ride along + on ``app_wraps``. + """ + + class MixedState(BaseState): + @event + def do_a_thing(self, value: str): + pass + + log_after_timeout = make_timeout_logger() + + def return_conditional_mixed(v: Var[Any]) -> Any: + return rx.cond( + v == "foo", + log_after_timeout.partial("Input was foo!"), + MixedState.do_a_thing(v.to(str)), + ) + + chain = EventChain.create( + cast(LambdaEventCallback[Any], return_conditional_mixed), + args_spec=lambda e: [e], + ) + var_data = LiteralVar.create(chain)._get_all_var_data() + assert var_data is not None + + hook_text = "\n".join(str(hook) for hook in var_data.hooks) + assert "EventLoopContext" not in hook_text + + imported = {tag.tag for _lib, tags in var_data.imports for tag in tags} + assert "addEvents" in imported + + assert {wrapper.tag for _, wrapper in var_data.app_wraps} >= { + "StateProvider", + "EventLoopProvider", + } + + def test_event_chain_create_lambda_rejects_non_union_callable_var(): """Plain callable Vars should remain invalid event-lambda return values.""" From cfe2b90ac1afb2af82605310daa5c334303768ed Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 22:31:14 +0500 Subject: [PATCH 07/10] test: fix tab indentation in postcss config fixture string --- tests/units/test_app.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/units/test_app.py b/tests/units/test_app.py index a406eadd1c2..33a13dd50f3 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -2052,8 +2052,8 @@ def compilable_app( "postcss-import": {}, autoprefixer: {}, }, - }; - """, +}; +""", ) reload_state_module(__name__) app = App(theme=None) From d9398e690c00d2c8c7373f98cf41aee4f034a2ef Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 22:58:49 +0500 Subject: [PATCH 08/10] perf: share state/event-loop app-wrap providers as cached singletons Construct StateProvider/EventLoopProvider once and reuse them instead of rebuilding per state Var. This is now safe because consumers deep-copy before rebinding children and the render-cache no longer survives deepcopy, so the shared markers are never mutated; it lets the page-tree deepcopy and render hash memoize them. VarData.add_state now routes through get_event_app_wraps() for the single source of truth. --- news/6447.feature.md | 1 + packages/reflex-base/news/6447.feature.md | 1 + .../reflex_base/components/state_context.py | 27 ++++++++++++------- .../reflex-base/src/reflex_base/vars/base.py | 21 ++++----------- .../reflex-components-core/news/6447.misc.md | 1 + 5 files changed, 26 insertions(+), 25 deletions(-) create mode 100644 news/6447.feature.md create mode 100644 packages/reflex-base/news/6447.feature.md create mode 100644 packages/reflex-components-core/news/6447.misc.md diff --git a/news/6447.feature.md b/news/6447.feature.md new file mode 100644 index 00000000000..e3c8b383477 --- /dev/null +++ b/news/6447.feature.md @@ -0,0 +1 @@ +Event handlers attached to JSX literals built outside a component's render scope — such as an `ErrorBoundary`'s `onError` — can now dispatch events. `addEvents` is reached through a module-level import that `EventLoopProvider` populates on each render, so dispatch no longer depends on a `useContext` hook being hoisted into the calling scope. The state and event-loop providers, previously hard-coded in the layout template, are now injected around the app root by the compiler from the `app_wraps` declared on the `Var`s that use them. diff --git a/packages/reflex-base/news/6447.feature.md b/packages/reflex-base/news/6447.feature.md new file mode 100644 index 00000000000..be617a8a9a0 --- /dev/null +++ b/packages/reflex-base/news/6447.feature.md @@ -0,0 +1 @@ +`VarData` gained an `app_wraps` field so a `Var` can declare the app-level wrapper components it requires; the compiler injects them around the app root, deduped by `(priority, tag)`. This is how the state and event-loop providers now reach the React tree, since event dispatch reaches `addEvents` via a module-level import (`Imports.EVENTS`) rather than a hoisted hook. The still-reactive `connectErrors` value moves to its own `CONNECT_ERRORS` import/hook, and `Component` deep copies now drop the render cache so compile-time clones (e.g. the app-root wrapper chain) render their mutated children. diff --git a/packages/reflex-base/src/reflex_base/components/state_context.py b/packages/reflex-base/src/reflex_base/components/state_context.py index 4bccc1c88cd..16fe09dd6a6 100644 --- a/packages/reflex-base/src/reflex_base/components/state_context.py +++ b/packages/reflex-base/src/reflex_base/components/state_context.py @@ -29,22 +29,31 @@ class EventLoopContextProvider(Component): tag = "EventLoopProvider" +_event_app_wraps: tuple[tuple[int, Component], ...] | None = None + + def get_event_app_wraps() -> tuple[tuple[int, Component], ...]: """Return state/event-loop providers required when events are dispatched. - ``StateProvider`` (100) wraps further out than ``EventLoopProvider`` - (90) because the latter reads ``DispatchContext`` from the former. - Providers are constructed fresh per call — module-level caching - breaks because ``copy.deepcopy`` (used when assembling the app-root - chain) carries ``_cached_render_result`` across compile runs. + ``StateProvider`` (100) wraps further out than ``EventLoopProvider`` (90) + because the latter reads ``DispatchContext`` from the former. + + The two providers are created once and shared: they are immutable markers + deduped by ``(priority, tag)``, and every consumer (``App._app_root``) + deep-copies them before rebinding children, so the shared instances are + never mutated in place. Sharing also lets the page-tree deepcopy and the + render-hash memoize them instead of paying per state Var. Returns: ``(priority, provider)`` entries deduped by the compiler. """ - return ( - (100, StateContextProvider.create()), - (90, EventLoopContextProvider.create()), - ) + global _event_app_wraps + if _event_app_wraps is None: + _event_app_wraps = ( + (100, StateContextProvider.create()), + (90, EventLoopContextProvider.create()), + ) + return _event_app_wraps def get_events_hooks_var_data() -> VarData: diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 8e524491160..0a5997881d5 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -368,11 +368,8 @@ def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarDa Returns: The var with the set state. """ - # Lazy imports: state_context imports VarData from this module. - from reflex_base.components.state_context import ( - EventLoopContextProvider, - StateContextProvider, - ) + # Lazy import: state_context imports VarData from this module. + from reflex_base.components.state_context import get_event_app_wraps from reflex_base.utils import format state_name = state if isinstance(state, str) else state.get_full_name() @@ -388,17 +385,9 @@ def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarDa f"$/{constants.Dirs.CONTEXTS_PATH}": [ImportVar(tag="StateContexts")], "react": [ImportVar(tag="useContext")], }, - app_wraps=( - # Higher priority wraps further out. ``StateProvider`` must - # enclose ``EventLoopProvider`` because the latter reads - # ``DispatchContext`` (provided by StateProvider) at its top. - # Both must enclose the chain's other wraps so the hooks - # AppWrap hosts (e.g. ``useContext(EventLoopContext)``) see - # them as React-tree ancestors. The compiler dedupes by - # ``(priority, tag)`` so fresh per-call instances are fine. - (100, StateContextProvider.create()), - (90, EventLoopContextProvider.create()), - ), + # State Vars read ``StateContexts``/``EventLoopContext``, so the + # providers must enclose every component that uses them. + app_wraps=get_event_app_wraps(), ) diff --git a/packages/reflex-components-core/news/6447.misc.md b/packages/reflex-components-core/news/6447.misc.md new file mode 100644 index 00000000000..cb64015d0e8 --- /dev/null +++ b/packages/reflex-components-core/news/6447.misc.md @@ -0,0 +1 @@ +The connection-error banner now subscribes only to the dedicated `CONNECT_ERRORS` hook instead of the shared events hook, and the upload component declares its `UploadFilesProvider` through `VarData.app_wraps` rather than `Upload._get_app_wrap_components`. From b48f28c496cc0dd8ee64eeee5fe3c069a697be60 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Fri, 29 May 2026 23:04:19 +0500 Subject: [PATCH 09/10] test: assert upload provider stays out when no upload Vars are used --- tests/units/test_app.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 33a13dd50f3..bc20334d70e 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -2487,6 +2487,26 @@ def test_selected_files_collects_upload_provider_without_upload_component() -> N assert (90, "EventLoopProvider") not in page_ctx.app_wrap_components +def test_no_upload_usage_omits_upload_provider() -> None: + """A page that never references upload Vars omits ``UploadFilesProvider``. + + The provider rides only on ``selected_files`` / ``upload_files`` Var data + (or an ``Upload`` component). A page that uses state and events but no + upload must not pull it into the registry — the complement of + :func:`test_selected_files_collects_upload_provider_without_upload_component`. + """ + page_ctx = compile_page_context_for_app_wraps( + rx.button("ping", on_click=rx.console_log("ping")) + ) + keys = page_ctx.app_wrap_components.keys() + + # Event triggers pull in the state/event providers... + assert (100, "StateProvider") in keys + assert (90, "EventLoopProvider") in keys + # ...but nothing references upload, so its provider stays out. + assert (5, "UploadFilesProvider") not in keys + + def test_upload_root_collects_upload_and_event_providers() -> None: """Upload root requires both upload context and event-loop providers.""" page_ctx = compile_page_context_for_app_wraps( From 9ac52f13f41121446322157e20c900cad0026021 Mon Sep 17 00:00:00 2001 From: Farhan Ali Raza Date: Sat, 30 May 2026 23:15:22 +0500 Subject: [PATCH 10/10] refactor: centralize app-wrap merge dedup, raise on conflicting slots Extract the per-(priority, tag) app-wrap merge rule into a shared insert_app_wraps primitive used by both VarData.merge and the compiler's page-wide collection, so the dedup logic lives in one place. Two different wrappers claiming the same slot now raise instead of silently keeping the first. Hash app_wraps as a frozenset so merge order no longer affects VarData identity (a + b == b + a). --- .../src/reflex_base/components/component.py | 26 ++++--- .../reflex-base/src/reflex_base/vars/base.py | 67 ++++++++++++++----- reflex/compiler/plugins/builtin.py | 29 +++++--- tests/units/compiler/test_plugins.py | 19 ++++++ tests/units/test_var.py | 47 ++++++++++++- 5 files changed, 151 insertions(+), 37 deletions(-) diff --git a/packages/reflex-base/src/reflex_base/components/component.py b/packages/reflex-base/src/reflex_base/components/component.py index e04cce1108f..acc6b75fc48 100644 --- a/packages/reflex-base/src/reflex_base/components/component.py +++ b/packages/reflex-base/src/reflex_base/components/component.py @@ -2000,10 +2000,11 @@ def _get_vars_hooks(self) -> dict[str, VarData | None]: def _get_events_hooks(self) -> dict[str, VarData | None]: """Get the hooks required by events referenced in this component. - ``addEvents`` is reached via the module-level import in - ``Imports.EVENTS``, so no in-scope hook is needed for events. - State/event-loop providers ride along on the event invocation's - ``VarData.app_wraps`` and via :meth:`_get_event_app_wraps`. + Always empty: ``addEvents`` is reached via the module-level import + in ``Imports.EVENTS``, so events need no in-scope hook. The state/ + event-loop providers they still depend on are mounted as app wraps + instead — carried on the event invocation's ``VarData.app_wraps`` + and via :meth:`_get_event_app_wraps`. Returns: An empty dict. @@ -2166,12 +2167,17 @@ def _get_app_wrap_components() -> dict[tuple[int, str], Component]: def _get_event_app_wraps(self) -> dict[tuple[int, str], Component]: """Return state/event-loop providers required by event triggers. - Kept separate from :meth:`_get_app_wrap_components` so subclass - overrides of that method don't inadvertently strip these out — - the providers must be in the React tree for ``StateContexts``, - ``EventLoopContext``, and the websocket plumbing to stay alive - even though ``addEvents`` itself is reached via module-level - import rather than a hook closure. + A component with event triggers calls ``addEvents`` at runtime, + which only does anything if ``StateProvider`` (supplies the + dispatch context) and ``EventLoopProvider`` (runs the websocket) + are mounted as ancestors. ``addEvents`` now comes from a + module-level import rather than an in-scope hook, so nothing drags + those providers into the tree on its own — this method requests + them explicitly as app wraps. + + Kept separate from :meth:`_get_app_wrap_components` because + subclasses override that method to add their own app wraps; folding + these in would let such an override silently drop them. Returns: The state/event-loop provider entries (empty if no event diff --git a/packages/reflex-base/src/reflex_base/vars/base.py b/packages/reflex-base/src/reflex_base/vars/base.py index 0a5997881d5..f7212bef821 100644 --- a/packages/reflex-base/src/reflex_base/vars/base.py +++ b/packages/reflex-base/src/reflex_base/vars/base.py @@ -113,6 +113,48 @@ class VarSubclassEntry: _var_literal_subclasses: list[tuple[type[LiteralVar], VarSubclassEntry]] = [] +_AppWrap = TypeVar("_AppWrap", bound="BaseComponent") + + +def insert_app_wraps( + target: dict[tuple[int, str], _AppWrap], + sources: Iterable[tuple[int, _AppWrap]], + *, + existing: Mapping[tuple[int, str], _AppWrap] | None = None, +) -> None: + """Merge app-wrap requests into ``target`` keyed by ``(priority, tag)``. + + App wraps model a set of required wrapper roles: at most one wrapper per + ``(priority, tag)``. Requests resolving to an equal wrapper are deduped; + two different wrappers claiming one role is a conflict and raises. This is + the single place that rule lives, shared by ``VarData.merge`` (within one + Var) and the compiler's page-wide collection. + + Args: + target: Registry that receives newly seen wraps. + sources: ``(priority, wrapper)`` requests to merge in. + existing: Already-committed wraps to dedupe against without writing, + letting callers collect only the wraps they newly contribute. + + Raises: + ReflexError: If two different wrappers claim one ``(priority, tag)``. + """ + for priority, wrapper in sources: + key = (priority, wrapper.tag or type(wrapper).__name__) + seen = existing.get(key) if existing is not None else None + if seen is None: + seen = target.get(key) + if seen is not None: + if seen != wrapper: + msg = ( + f"Conflicting app wraps for {key!r}: two different " + "components claim the same (priority, tag) slot." + ) + raise exceptions.ReflexError(msg) + continue + target[key] = wrapper + + @dataclasses.dataclass( eq=True, frozen=True, @@ -269,15 +311,9 @@ def merge(*all: VarData | None) -> VarData | None: component for var_data in all_var_datas for component in var_data.components ) - app_wraps_seen: set[tuple[int, str]] = set() - app_wraps_list: list[tuple[int, BaseComponent]] = [] + app_wraps: dict[tuple[int, str], BaseComponent] = {} for var_data in all_var_datas: - for priority, wrapper in var_data.app_wraps: - key = (priority, wrapper.tag or type(wrapper).__name__) - if key in app_wraps_seen: - continue - app_wraps_seen.add(key) - app_wraps_list.append((priority, wrapper)) + insert_app_wraps(app_wraps, var_data.app_wraps) return VarData( state=state, @@ -287,7 +323,9 @@ def merge(*all: VarData | None) -> VarData | None: deps=deps, position=position, components=components, - app_wraps=tuple(app_wraps_list), + app_wraps=tuple( + (priority, wrapper) for (priority, _tag), wrapper in app_wraps.items() + ), ) def __bool__(self) -> bool: @@ -314,7 +352,9 @@ def _identity_key(self) -> tuple: ``__eq__`` override drops the default hash. Use component identity for embedded components because they can contribute hooks/imports, and use the compiler's app-wrap registry key for wrappers so fresh provider - instances with the same role still compare equal. + instances with the same role still compare equal. App wraps are a set + of required roles, so a ``frozenset`` keeps identity insensitive to the + order vars happened to merge in (``a + b`` and ``b + a`` stay equal). Returns: A hashable tuple uniquely identifying this VarData. @@ -327,11 +367,8 @@ def _identity_key(self) -> tuple: self.deps, self.position, tuple(id(component) for component in self.components), - tuple( - ( - priority, - component.tag or type(component).__name__, - ) + frozenset( + (priority, component.tag or type(component).__name__) for priority, component in self.app_wraps ), ) diff --git a/reflex/compiler/plugins/builtin.py b/reflex/compiler/plugins/builtin.py index 513da26cf83..de31f8c83c7 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -15,6 +15,7 @@ from reflex_base.utils.format import make_default_page_title from reflex_base.utils.imports import collapse_imports, merge_imports from reflex_base.vars import VarData +from reflex_base.vars.base import insert_app_wraps from reflex_components_core.base.fragment import Fragment from reflex.compiler import utils @@ -86,10 +87,14 @@ def _ingest_component_var_app_wraps( if hook_var_data is None: continue _ingest_var_data_app_wraps(wraps_by_key, existing, hook_var_data) - for key, wrapper in component._get_event_app_wraps().items(): - if key in existing or key in wraps_by_key: - continue - wraps_by_key[key] = wrapper + insert_app_wraps( + wraps_by_key, + ( + (priority, wrapper) + for (priority, _tag), wrapper in component._get_event_app_wraps().items() + ), + existing=existing, + ) def collect_var_app_wraps_for_component( @@ -143,13 +148,15 @@ def _ingest_app_wraps( app_wraps: tuple[tuple[int, BaseComponent], ...], ) -> None: """Insert app_wraps not already present in ``existing`` or ``wraps_by_key``.""" - for priority, wrapper in app_wraps: - if not isinstance(wrapper, Component): - continue - key = (priority, wrapper.tag or type(wrapper).__name__) - if key in existing or key in wraps_by_key: - continue - wraps_by_key[key] = wrapper + insert_app_wraps( + wraps_by_key, + ( + (priority, wrapper) + for priority, wrapper in app_wraps + if isinstance(wrapper, Component) + ), + existing=existing, + ) @dataclasses.dataclass(frozen=True, slots=True) diff --git a/tests/units/compiler/test_plugins.py b/tests/units/compiler/test_plugins.py index 03ff87b0a02..08943270a3b 100644 --- a/tests/units/compiler/test_plugins.py +++ b/tests/units/compiler/test_plugins.py @@ -23,6 +23,7 @@ ) from reflex_base.plugins.base import HookOrder from reflex_base.utils import format as format_utils +from reflex_base.utils.exceptions import ReflexError from reflex_base.utils.imports import ImportVar, collapse_imports, merge_imports from reflex_base.vars import VarData from reflex_base.vars.base import LiteralVar, Var @@ -839,6 +840,24 @@ def _get_app_wrap_components() -> dict[tuple[int, str], Component]: assert page_ctx.app_wrap_components[10, "WrapperComponent"] is component_wrap +def test_default_collector_raises_on_conflicting_var_app_wraps() -> None: + """The page walk surfaces a conflict between two Var-declared wraps.""" + # Same tag/priority, unequal wrappers, on two Vars of one component. + wrap_a = WrapperComponent.create(class_name="a") + wrap_b = WrapperComponent.create(class_name="b") + var_a = LiteralVar.create("a")._replace( + merge_var_data=VarData(app_wraps=((10, wrap_a),)) + ) + var_b = LiteralVar.create("b")._replace( + merge_var_data=VarData(app_wraps=((10, wrap_b),)) + ) + + component = RootComponent.create(ChildComponent.create(id=var_a, class_name=var_b)) + + with pytest.raises(ReflexError, match="Conflicting app wraps"): + collect_page_context(component, plugins=(DefaultCollectorPlugin(),)) + + def test_default_collector_collects_direct_events_hook_app_wraps() -> None: """Direct ``Hooks.EVENTS`` users collect the state/event providers.""" page_ctx = collect_page_context( diff --git a/tests/units/test_var.py b/tests/units/test_var.py index 5a4bc68b049..114e7cf8dff 100644 --- a/tests/units/test_var.py +++ b/tests/units/test_var.py @@ -12,6 +12,7 @@ from reflex_base.constants.state import FIELD_MARKER from reflex_base.utils.exceptions import ( PrimitiveUnserializableToJSONError, + ReflexError, UntypedComputedVarError, ) from reflex_base.utils.imports import ImportVar @@ -23,6 +24,7 @@ Var, _decode_var_immutable, computed_var, + insert_app_wraps, var_operation, var_operation_return, ) @@ -1937,7 +1939,8 @@ def test_var_data_app_wraps_merge(): assert (20, wrapper_b) in merged.app_wraps assert len(merged.app_wraps) == 2 - # Same priority/tag dedupes even when provider instances are fresh. + # Equal wrappers at the same priority/tag dedupe even when they are fresh + # instances (wrapper_a and wrapper_b are both empty, hence equal). vd_same_key = VarData(app_wraps=((10, wrapper_b),)) merged_same_key = VarData.merge(vd_a, vd_same_key) assert merged_same_key is not None @@ -1954,6 +1957,48 @@ def test_var_data_app_wraps_merge(): assert VarData(app_wraps=((10, wrapper_a),)) +def test_insert_app_wraps(): + """The shared app-wrap primitive dedupes equal wraps and rejects conflicts.""" + # Same tag ("Fragment") and priority; a/dup are equal, b is unequal. + wrap_a = rx.fragment(class_name="a") + wrap_a_dup = rx.fragment(class_name="a") + wrap_b = rx.fragment(class_name="b") + + # Equal wraps at one key collapse to a single entry. + target: dict[tuple[int, str], rx.Component] = {} + insert_app_wraps(target, [(10, wrap_a), (10, wrap_a_dup)]) + assert target == {(10, "Fragment"): wrap_a} + + # A different wrapper at the same key is a conflict. + with pytest.raises(ReflexError, match="Conflicting app wraps"): + insert_app_wraps(target, [(10, wrap_b)]) + + # ``existing`` is consulted but never written: equal is skipped... + fresh: dict[tuple[int, str], rx.Component] = {} + insert_app_wraps(fresh, [(10, wrap_a_dup)], existing=target) + assert fresh == {} + # ...and a conflict against ``existing`` still raises. + with pytest.raises(ReflexError, match="Conflicting app wraps"): + insert_app_wraps({}, [(10, wrap_b)], existing=target) + + # A different priority is a distinct slot. + insert_app_wraps(target, [(20, wrap_b)]) + assert target[20, "Fragment"] is wrap_b + + +def test_var_data_merge_raises_on_conflicting_app_wraps(): + """The merge walk surfaces a conflict instead of silently keeping the first.""" + # Same tag ("Fragment") and priority, unequal wrappers — a real conflict. + wrap_a = rx.fragment(class_name="a") + wrap_b = rx.fragment(class_name="b") + + with pytest.raises(ReflexError, match="Conflicting app wraps"): + VarData.merge( + VarData(app_wraps=((10, wrap_a),)), + VarData(app_wraps=((10, wrap_b),)), + ) + + def test_var_data_identity_hashes_component_metadata(): """VarData hashes component metadata without hashing components directly.""" wrapper_a = rx.fragment()