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/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 838d414b964..9541eab5a24 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'; @@ -218,11 +218,7 @@ def app_root_template( }}, []); return jsx(ThemeProvider, {{defaultTheme: defaultColorMode, attribute: "class"}}, - jsx(StateProvider, {{}}, - jsx(EventLoopProvider, {{}}, - jsx(AppWrap, {{}}, children) - ) - ) + jsx(AppWrap, {{}}, children) ); }} @@ -373,6 +369,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 => {{ @@ -403,14 +417,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 9f68ee46908..acc6b75fc48 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: @@ -1970,14 +2000,16 @@ 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. + 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: - The hooks for the events. + An empty dict. """ - return ( - {Hooks.EVENTS: VarData(position=Hooks.HookPosition.INTERNAL)} - if self.event_triggers - else {} - ) + return {} def _get_hooks_internal(self) -> dict[str, VarData | None]: """Get the React hooks for this component managed by the framework. @@ -2132,6 +2164,35 @@ 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. + + 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 + 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 new file mode 100644 index 00000000000..16fe09dd6a6 --- /dev/null +++ b/packages/reflex-base/src/reflex_base/components/state_context.py @@ -0,0 +1,68 @@ +"""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" + + +_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. + + 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. + """ + 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: + """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=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 a5dd51b3925..036dfe1799a 100644 --- a/packages/reflex-base/src/reflex_base/constants/compiler.py +++ b/packages/reflex-base/src/reflex_base/constants/compiler.py @@ -148,20 +148,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 97a8af96ba1..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 ( @@ -1080,14 +1080,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 = [ ( @@ -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( @@ -2418,11 +2421,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_event_app_wraps + invocation = FunctionStringVar.create( CompileVars.ADD_EVENTS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + app_wraps=get_event_app_wraps(), ), ) else: @@ -2463,11 +2469,14 @@ 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_event_app_wraps + apply_event_actions = FunctionStringVar.create( CompileVars.APPLY_EVENT_ACTIONS, _var_data=VarData( imports=Imports.EVENTS, - hooks={Hooks.EVENTS: None}, + app_wraps=get_event_app_wraps(), ), ) 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 0f25e55d163..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, @@ -141,6 +183,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 +198,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 +210,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 +226,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 +239,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 +311,10 @@ def merge(*all: VarData | None) -> VarData | None: component for var_data in all_var_datas for component in var_data.components ) + app_wraps: dict[tuple[int, str], BaseComponent] = {} + for var_data in all_var_datas: + insert_app_wraps(app_wraps, var_data.app_wraps) + return VarData( state=state, field_name=field_name, @@ -267,6 +323,9 @@ def merge(*all: VarData | None) -> VarData | None: deps=deps, position=position, components=components, + app_wraps=tuple( + (priority, wrapper) for (priority, _tag), wrapper in app_wraps.items() + ), ) def __bool__(self) -> bool: @@ -283,8 +342,58 @@ 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. 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. + """ + return ( + self.state, + self.field_name, + self.imports, + self.hooks, + self.deps, + self.position, + tuple(id(component) for component in self.components), + frozenset( + (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 +405,8 @@ def from_state(cls, state: type[BaseState] | str, field_name: str = "") -> VarDa Returns: The var with the set state. """ + # 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() @@ -311,6 +422,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")], }, + # 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`. 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..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 @@ -7,7 +7,13 @@ class AppWrap(Fragment): - """Top-level component that wraps the entire app.""" + """Innermost (priority 0) element of the python app-wrap chain. + + 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 9bbb28de64d..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,8 +13,9 @@ field, ) from reflex_base.components.memoize_helpers import get_memoized_event_triggers +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, @@ -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}, + app_wraps=get_event_app_wraps(), ), 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 b4498b3581b..1189da44d4b 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -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": "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/compiler.py b/reflex/compiler/compiler.py index b4b562e1121..7ce838968cf 100644 --- a/reflex/compiler/compiler.py +++ b/reflex/compiler/compiler.py @@ -39,6 +39,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.state import BaseState, code_uses_state_contexts from reflex.utils import console, frontend_skeleton, path_ops, prerequisites from reflex.utils.exec import get_compile_context, is_prod_mode @@ -925,6 +926,21 @@ def memoized_toast_provider() -> Component: 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..de31f8c83c7 100644 --- a/reflex/compiler/plugins/builtin.py +++ b/reflex/compiler/plugins/builtin.py @@ -7,17 +7,158 @@ 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 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 +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 _ingest_component_var_app_wraps( + wraps_by_key: dict[tuple[int, str], Component], + existing: dict[tuple[int, str], Component], + component: Component, + 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. + """ + 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, 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, 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, existing, hook_var_data) + 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( + 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 + + +def _ingest_var_data_app_wraps( + wraps_by_key: dict[tuple[int, str], Component], + existing: dict[tuple[int, str], Component], + var_data: VarData, +) -> 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``.""" + 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) class DefaultPagePlugin(Plugin): """Evaluate an unevaluated page into a mutable page context.""" @@ -192,8 +333,15 @@ 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 @@ -204,6 +352,10 @@ def leave_component( 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) @@ -250,9 +402,10 @@ 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 base_get_app_wrap_components = Component._get_app_wrap_components seen_app_wrap_methods: set[object] = set() @@ -270,8 +423,13 @@ 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 if ( @@ -281,6 +439,10 @@ 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, hooks_internal, added_hooks + ) + dynamic_import = comp._get_dynamic_imports() if dynamic_import is not None: dynamic_imports.add(dynamic_import) @@ -292,15 +454,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( @@ -334,7 +503,11 @@ 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() if not direct_wrappers: return @@ -352,6 +525,43 @@ 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, + hooks_internal: dict[str, VarData | None], + added_hooks: dict[str, VarData | None], + ) -> None: + """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 + + 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 62878ef222a..3eb4bc51873 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -35,15 +35,23 @@ from reflex_base.plugins import ComponentAndChildren, PageContext from reflex_base.plugins.base import Plugin +from reflex.compiler.plugins.builtin import ( + collect_var_app_wraps_for_component, + collect_var_app_wraps_in_subtree, +) + def _subtree_has_reactive_data( component: Component, _cache: dict[int, bool] | None = None ) -> 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 @@ -58,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 = {} @@ -270,7 +278,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, @@ -323,6 +339,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_dynamic_components_codegen.py b/tests/units/compiler/test_dynamic_components_codegen.py index 14eafa4de90..0e8ce9bded2 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,pyOr} = 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,pyOr} = 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/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index 4061d0470f1..19c25c42fb7 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -75,6 +75,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 = component_field(default=None) + + +class MemoAppWrapProvider(Component): + tag = "MemoAppWrapProvider" + library = "memo-app-wrap-provider-lib" + + class ChildrenViaProp(Component): """Stub mirroring ``CodeBlock`` — injects its content as ``children`` prop.""" @@ -238,6 +251,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 + + def test_memoize_wrappers_distinct_for_different_on_mount() -> None: """Two components differing only in ``on_mount`` must NOT dedupe. @@ -1411,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. +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``) 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. + 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: diff --git a/tests/units/compiler/test_plugins.py b/tests/units/compiler/test_plugins.py index 26eb1f39c99..08943270a3b 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, @@ -22,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 @@ -769,6 +771,119 @@ 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_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( + 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/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 diff --git a/tests/units/test_app.py b/tests/units/test_app.py index 2461ce18afc..bc20334d70e 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -22,6 +22,7 @@ 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 @@ -29,6 +30,7 @@ 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 @@ -40,6 +42,8 @@ from reflex import AdminDash, constants from reflex._upload import upload from reflex.app import App, ComponentCallable +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 @@ -2020,11 +2024,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) @@ -2047,12 +2055,61 @@ def compilable_app(tmp_path: Path) -> Generator[tuple[App, Path], None, None]: }; """, ) + 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], @@ -2078,17 +2135,22 @@ def test_app_wrap_compile_theme( app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() + # 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() - expected = ( - "function AppWrap({children}) {\n" - "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" + "function AppWrap({children}) {\n\n\n\n\n" "return (" + ("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], ({ }))))""" @@ -2101,8 +2163,9 @@ def test_app_wrap_compile_theme( + "jsx(DefaultOverlayComponents,{},)," + "jsx(Fragment,{}," + "children" - "))))))" + (")" if react_strict_mode else "") + ")" - "\n}" + + "))))))))" + + (")" if react_strict_mode else "") + + ")\n}" ) assert expected.split(",") == function_app_definition.split(",") @@ -2275,6 +2338,186 @@ 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 chain components with event triggers pull state/event providers. + + 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) + app = App(theme=None, enable_state=False) + + root_contents = compile_app_root_from_page_wraps(app, {}) + + assert EVENT_LOOP_CONTEXT_HOOK not 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) + + # The provider chain is rendered inside the ``AppWrap`` function body; + # ``ReflexProviders`` references ``jsx(AppWrap, {}, children)`` earlier in + # the file, so scope the search to the chain to avoid matching that. + chain = root_contents[root_contents.index("function AppWrap({children})") :] + state_index = chain.index("jsx(StateProvider") + event_loop_index = chain.index("jsx(EventLoopProvider") + children_index = chain.rindex("children") + assert state_index < event_loop_index < children_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_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( + 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], @@ -2322,19 +2565,22 @@ def page(): app_js_contents = ( web_dir / constants.Dirs.PAGES / constants.PageNames.APP_ROOT ).read_text() + # 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() - expected = ( - "function AppWrap({children}) {\n" - "const [addEvents, connectErrors] = useContext(EventLoopContext);\n\n\n\n" + "function AppWrap({children}) {\n\n\n\n\n" "return (" + ("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], ({ }))))""" + "}," @@ -2347,7 +2593,9 @@ def page(): + "jsx(DefaultOverlayComponents,{},)," + "jsx(Fragment,{}," + "children" - ")))))))" + (")" if react_strict_mode else "") + "))\n}" + + "))))))))))" + + (")" if react_strict_mode else "") + + ")\n}" ) assert expected.split(",") == function_app_definition.split(",") diff --git a/tests/units/test_event.py b/tests/units/test_event.py index 500dd542097..72cc9a7da29 100644 --- a/tests/units/test_event.py +++ b/tests/units/test_event.py @@ -18,7 +18,7 @@ ) from reflex_base.utils import format from reflex_base.utils.exceptions import EventHandlerValueError -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 @@ -501,10 +501,21 @@ 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 (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 == () + 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(): @@ -530,7 +541,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: @@ -853,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.""" diff --git a/tests/units/test_var.py b/tests/units/test_var.py index 6971cdffcd4..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 @@ -21,7 +22,9 @@ ComputedVar, LiteralVar, Var, + _decode_var_immutable, computed_var, + insert_app_wraps, var_operation, var_operation_return, ) @@ -1921,6 +1924,117 @@ 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 + + # 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 + 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_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() + 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