diff --git a/docs/library/other/memo.md b/docs/library/other/memo.md index a3415b6cd00..1d78413c237 100644 --- a/docs/library/other/memo.md +++ b/docs/library/other/memo.md @@ -15,7 +15,7 @@ Every parameter must be annotated with `rx.Var[...]` or `rx.RestProp`. The compi 3. **`rx.Var[rx.Component]` for slot children** — a parameter named `children` annotated as `rx.Var[rx.Component]` accepts children rendered by the caller. 4. **Keyword arguments at the call site** — pass props by name, not by position. -Defaults need to be `rx.Var` values. For the common empty cases use the module-level constants `rx.EMPTY_VAR_STR` (an empty string) and `rx.EMPTY_VAR_INT` (zero): `class_name: rx.Var[str] = rx.EMPTY_VAR_STR` falls back to `""` when the caller omits the prop. +Defaults need to be `rx.Var` values. For the common empty cases use the module-level constants `rx.EMPTY_VAR_STR` (an empty string), `rx.EMPTY_VAR_INT` (zero), and `rx.EMPTY_VAR_COMPONENT` (an empty component): `class_name: rx.Var[str] = rx.EMPTY_VAR_STR` falls back to `""` when the caller omits the prop, and `children: rx.Var[rx.Component] = rx.EMPTY_VAR_COMPONENT` makes a slot optional. ## Basic Usage @@ -79,7 +79,7 @@ def primary_button( *, label: rx.Var[str], ) -> rx.Component: - return rx.button(label, class_name="bg-primary-9 text-white", **rest) + return rx.button(label, rest, class_name="bg-primary-9 text-white") def index(): @@ -92,6 +92,30 @@ def index(): At most one `rx.RestProp` parameter is allowed per memo. +The `rest` parameter should be treated as an opaque value and passed +positionally to any component which will use it. + +You may use the `.merge` var operation to combine the arbitrary props with +another object Var or python dict. The memo body can read placeholders like +`rest.get("class_name", "")`, but the actual value will be unavailable at +compile time, so you can't branch on it or do python operations with the values, +only var operations which will be translated to Javascript expressions. + +The same example as above, but now allowing the caller to optionally pass a +`class_name` that gets merged with the default styles: + +```python +@rx.memo +def primary_button( + rest: rx.RestProp, + *, + label: rx.Var[str], +) -> rx.Component: + class_name = rest.get("class_name", "") + " bg-primary-9 text-white" + return rx.button(label, rest.merge({"class_name": class_name})) +``` + + ## Accepting Children Declare a parameter named `children` typed as `rx.Var[rx.Component]` to receive a child subtree. diff --git a/news/6598.deprecation.md b/news/6598.deprecation.md new file mode 100644 index 00000000000..e967f0216ce --- /dev/null +++ b/news/6598.deprecation.md @@ -0,0 +1 @@ +`@rx.memo` now expects each parameter to be annotated as `rx.Var[...]` (or `rx.RestProp`/`rx.EventHandler`) and the function to declare an `rx.Component` or `rx.Var[...]` return type. Memos that still use bare Python types (e.g. `name: str`) or omit the return annotation keep working — the values are coerced to `rx.Var[...]`/`rx.Component` and a deprecation warning points at the parameters and return type that need explicit annotations — but this fallback will be removed in 1.0. diff --git a/news/6598.feature.1.md b/news/6598.feature.1.md new file mode 100644 index 00000000000..4e5a5038549 --- /dev/null +++ b/news/6598.feature.1.md @@ -0,0 +1 @@ +Added `rx.EMPTY_VAR_COMPONENT`, an empty-component `rx.Var[rx.Component]` sentinel for use as a default on `@rx.memo` `children` slots (and any `rx.Var[rx.Component]` prop) — the component counterpart to `rx.EMPTY_VAR_STR` and `rx.EMPTY_VAR_INT`. diff --git a/news/6598.feature.md b/news/6598.feature.md new file mode 100644 index 00000000000..b9e67fb847b --- /dev/null +++ b/news/6598.feature.md @@ -0,0 +1 @@ +`@rx.memo` now evaluates the decorated function body lazily — on first use (component instantiation) or at compile time — instead of at import time. This speeds up startup and lets a memo reference modules that aren't fully imported yet, sidestepping circular-import errors during decoration. Body-dependent errors (e.g. a var-returning memo that uses hooks or non-bundled imports) now surface when the memo is first used or compiled rather than at import. diff --git a/packages/reflex-base/news/+memo-base-prop-passthrough.deprecation.md b/packages/reflex-base/news/+memo-base-prop-passthrough.deprecation.md new file mode 100644 index 00000000000..3c667d9a940 --- /dev/null +++ b/packages/reflex-base/news/+memo-base-prop-passthrough.deprecation.md @@ -0,0 +1 @@ +Component-returning `@rx.memo` again accepts `key` without an `rx.RestProp` (with a deprecation warning), so `rx.foreach` call sites that set the react `key` keep working; this fallback is removed in 1.0. Other base props (`id`, `class_name`, `style`, `custom_attrs`, `ref`) and identity fields like `tag`/`library` still raise — declare an `rx.RestProp` to forward them. diff --git a/packages/reflex-base/src/reflex_base/components/memo.py b/packages/reflex-base/src/reflex_base/components/memo.py index 0f8b0218023..c5011454e51 100644 --- a/packages/reflex-base/src/reflex_base/components/memo.py +++ b/packages/reflex-base/src/reflex_base/components/memo.py @@ -2,19 +2,21 @@ from __future__ import annotations -import contextlib import dataclasses -import importlib import inspect -from collections.abc import Callable, Iterator, Mapping, Sequence +import sys +from collections.abc import Callable, Mapping, Sequence from copy import copy from enum import Enum from functools import cache, update_wrapper +from types import UnionType from typing import ( Annotated, Any, ClassVar, + Generic, TypeVar, + Union, get_args, get_origin, get_type_hints, @@ -51,6 +53,28 @@ ) from reflex_base.vars.object import RestProp +# A `Var[Component]` default for memo `children` slots (and any prop typed +# `rx.Var[rx.Component]`), mirroring `EMPTY_VAR_STR` / `EMPTY_VAR_INT`. It lives +# here rather than in ``component.py`` because materializing a component var +# eagerly imports ``Bare`` (and thus ``reflex_base.environment``); defining it +# in the always-early ``component.py`` would cycle when ``environment`` is the +# entry point. ``memo.py`` is imported lazily, after ``environment`` is ready. +EMPTY_VAR_COMPONENT: Var[Component] = LiteralVar.create(Component.create()) + +# Base ``Component`` props a memo accepts without an ``rx.RestProp`` (with a +# deprecation warning). Only ``key`` qualifies: React consumes it at the +# reconciliation layer, so it takes effect on the rendered element even though +# the compiled memo function destructures only its declared params. The legacy +# custom-component use case this restores is setting ``key`` under ``rx.foreach``. +# +# Other base props (``id``, ``class_name``, ``style``, ``custom_attrs``, +# ``ref``) are deliberately NOT forwardable: without a ``RestProp`` the memo +# function emits no ``...rest`` spread, so they would be silently dropped rather +# than reaching the root. They raise like any unknown prop, and the error points +# at ``rx.RestProp`` — which compiles to a ``...rest`` spread that genuinely +# forwards them. +_FORWARDABLE_BASE_PROPS: frozenset[str] = frozenset({"key"}) + class MemoParamKind(str, Enum): """The role a memo parameter plays in the compiled component. @@ -143,6 +167,85 @@ class _MemoParamSpec: signature_field: Callable[[MemoParam], str | None] +_BodyT = TypeVar("_BodyT") + + +class _LazyBody(Generic[_BodyT]): + """A memo body computed once, on first read. + + ``@rx.memo`` registers a definition without running the decorated body; the + body is built by ``thunk`` on the first :meth:`get` and cached thereafter, + so decoration has no import-time side effects. A re-entrant read during that + evaluation — a component memo that instantiates itself via recursive + ``rx.foreach`` — returns the ``placeholder`` instead of recursing. Var memos + never re-enter (recursion resolves through the imported function var), so + they pass no placeholder. Eagerly built definitions use :meth:`ready`. + + Not thread-safe: the re-entrancy guard is a plain flag, which is sufficient + because memo bodies are only ever evaluated during single-threaded compile. + """ + + __slots__ = ("_busy", "_placeholder", "_ready", "_thunk", "_value") + _value: _BodyT + + def __init__( + self, thunk: Callable[[], _BodyT], placeholder: _BodyT | None = None + ) -> None: + """Defer ``thunk`` until first read. + + Args: + thunk: Builds and returns the body on first :meth:`get`. + placeholder: Stand-in returned if the body is read while ``thunk`` + is still running (the recursive component-memo case). + """ + self._thunk = thunk + self._placeholder = placeholder + self._ready = False + self._busy = False + + @classmethod + def ready(cls, value: _BodyT) -> _LazyBody[_BodyT]: + """Wrap an already-computed body. + + Args: + value: The precomputed body. + + Returns: + A lazy body that yields ``value`` without running a thunk. + """ + body = cls(lambda: value) + body._value = value + body._ready = True + return body + + def get(self) -> _BodyT: + """Return the body, running and caching ``thunk`` on first read. + + Returns: + The cached body, or the placeholder when read mid-evaluation. + + Raises: + RuntimeError: If the body re-enters its own evaluation but carries + no placeholder (only component memos re-enter, and they always + provide one — so this signals a broken invariant, not a missing + body). + """ + if self._ready: + return self._value + if self._busy: + if self._placeholder is None: + msg = "Re-entrant memo body read before its evaluation finished." + raise RuntimeError(msg) + return self._placeholder + self._busy = True + try: + self._value = self._thunk() + self._ready = True + finally: + self._busy = False + return self._value + + @dataclasses.dataclass(frozen=True, slots=True) class MemoDefinition: """Base metadata for a memo.""" @@ -156,16 +259,25 @@ class MemoDefinition: class MemoFunctionDefinition(MemoDefinition): """A memo that compiles to a JavaScript function.""" - function: ArgsFunctionOperation + _function: _LazyBody[ArgsFunctionOperation] imported_var: FunctionVar + @property + def function(self) -> ArgsFunctionOperation: + """The compiled function body, evaluated on first access. + + Returns: + The compiled ``ArgsFunctionOperation`` for this memo. + """ + return self._function.get() + @dataclasses.dataclass(frozen=True, slots=True) class MemoComponentDefinition(MemoDefinition): """A memo that compiles to a React component.""" export_name: str - component: Component + _component: _LazyBody[Component] # For passthrough wrappers built by the auto-memoize plugin: the # ``Bare``-wrapped ``{children}`` placeholder used when rendering the memo # body. The ``component`` keeps its ORIGINAL children so compile-time @@ -175,6 +287,15 @@ class MemoComponentDefinition(MemoDefinition): # page scope rather than being duplicated inside the memo body. passthrough_hole_child: Component | None = None + @property + def component(self) -> Component: + """The compiled component body, evaluated on first access. + + Returns: + The compiled ``Component`` for this memo. + """ + return self._component.get() + class MemoComponent(Component): """A rendered instance of a memo component.""" @@ -362,6 +483,37 @@ def _strip_annotated(annotation: Any) -> Any: return annotation +_UNION_ORIGINS = (Union, UnionType) + +# Python <=3.10's ``get_type_hints`` rewrites a parameter with a ``= None`` +# default into ``Optional[...]``, hiding the real annotation from the param +# classifiers; 3.11 dropped that behavior. Only those versions need the +# ``_strip_optional`` normalization, so newer interpreters skip it entirely +# rather than pay ``get_origin`` per parameter for a problem they don't have. +_GET_TYPE_HINTS_WRAPS_NONE_DEFAULT = sys.version_info < (3, 11) + + +def _strip_optional(annotation: Any) -> Any: + """Unwrap ``Optional[X]`` / ``X | None`` down to ``X``. + + Restores the annotation the classifiers expect after Python <=3.10's + ``get_type_hints`` wraps a ``= None``-defaulted parameter in ``Optional`` + (see ``_GET_TYPE_HINTS_WRAPS_NONE_DEFAULT``). + + Args: + annotation: The annotation to normalize. + + Returns: + The sole non-``None`` member of an ``Optional`` union, else the + annotation unchanged. + """ + if get_origin(annotation) in _UNION_ORIGINS: + non_none = [arg for arg in get_args(annotation) if arg is not type(None)] + if len(non_none) == 1: + return non_none[0] + return annotation + + def _is_rest_annotation(annotation: Any) -> bool: """Check whether an annotation is a RestProp. @@ -436,6 +588,23 @@ def _is_component_annotation(annotation: Any) -> bool: ) +def _is_memo_annotation(annotation: Any) -> bool: + """Check whether an annotation is already a recognized memo annotation. + + Recognized annotations are ``rx.Var[...]`` (including ``rx.RestProp``, a + ``Var`` subclass) and ``rx.EventHandler[...]``. Anything else is a legacy + bare Python type that the public :func:`memo` decorator coerces into + ``rx.Var[...]`` for backwards compatibility. + + Args: + annotation: The annotation to check. + + Returns: + Whether the annotation is already a valid memo parameter annotation. + """ + return _is_var_annotation(annotation) or _is_event_handler_annotation(annotation)[0] + + def _children_annotation_is_valid(annotation: Any) -> bool: """Check whether an annotation is valid for children. @@ -921,10 +1090,12 @@ def _analyze_params( for_component: Whether the memo returns a component. hints: Pre-computed type hints with ``include_extras=True``; computed from ``fn`` when omitted. - defaulted_params: When provided, parameters missing an annotation are - defaulted (``Var[Component]`` for ``children``, otherwise - ``Var[Any]``) and their names appended; when ``None``, a missing - annotation raises ``TypeError``. + defaulted_params: When provided, parameters that are missing an + annotation or carry a legacy bare-type annotation are coerced to + ``Var[...]`` (``Var[Component]`` for ``children``, ``Var[Any]`` for + a missing annotation, otherwise ``Var[]``) and their + names appended; when ``None`` (strict mode, used by internal + callers) either case raises ``TypeError``. Returns: The analyzed parameters. @@ -943,14 +1114,30 @@ def _analyze_params( _check_parameter_kind(parameter, fn.__name__) annotation = hints.get(parameter.name, parameter.annotation) - if annotation is inspect.Parameter.empty: + if _GET_TYPE_HINTS_WRAPS_NONE_DEFAULT and parameter.default is None: + annotation = _strip_optional(annotation) + is_missing = annotation is inspect.Parameter.empty + # Legacy `@rx.memo` (the old `custom_component`) accepted missing and + # bare Python-type annotations and auto-wrapped them in a `Var`. Coerce + # both into `rx.Var[...]` and flag the parameter, so one deprecation + # warning points the user at the params that still need an explicit + # annotation. Strict callers (`defaulted_params is None`) reject a + # missing annotation here; a bare type falls through to + # `_classify_parameter`, which rejects it. + is_legacy = defaulted_params is not None and not _is_memo_annotation(annotation) + if is_missing or is_legacy: if defaulted_params is None: msg = ( f"All parameters of `{fn.__name__}` must be annotated as `rx.Var[...]` " f"or `rx.RestProp`. Missing annotation for `{parameter.name}`." ) raise TypeError(msg) - annotation = Var[Component] if parameter.name == "children" else Var[Any] + if parameter.name == "children": + annotation = Var[Component] + elif is_missing: + annotation = Var[Any] + else: + annotation = Var[annotation] defaulted_params.append(parameter.name) # Children parameters by name must match the children kind exactly — @@ -1079,6 +1266,48 @@ def _build_args_function( ) +def _evaluate_component_body( + fn: Callable[..., Any], params: tuple[MemoParam, ...] +) -> Component: + """Run a component memo's body and return its compiled component. + + Args: + fn: The decorated function. + params: The analyzed memo parameters. + + Returns: + The wrapped component the body returned. + + Raises: + TypeError: If the body does not return a component. + """ + body = _normalize_component_return(_evaluate_memo_function(fn, params)) + if body is None: + msg = ( + f"Component-returning `@rx.memo` `{fn.__name__}` must return an " + "`rx.Component` or `rx.Var[rx.Component]`." + ) + raise TypeError(msg) + return _lift_rest_props(body) + + +def _evaluate_function_body( + fn: Callable[..., Any], params: tuple[MemoParam, ...] +) -> ArgsFunctionOperation: + """Run a var memo's body and build its compiled function. + + Args: + fn: The decorated function. + params: The analyzed memo parameters. + + Returns: + The compiled ``ArgsFunctionOperation`` for the memo body. + """ + return_expr = Var.create(_evaluate_memo_function(fn, params)) + _validate_var_return_expr(return_expr, fn.__name__) + return _build_args_function(params, return_expr) + + def _create_component_definition( fn: Callable[..., Any], return_annotation: Any, @@ -1096,20 +1325,12 @@ def _create_component_definition( TypeError: If the function does not return a component. """ params = _analyze_params(fn, for_component=True) - component = _normalize_component_return(_evaluate_memo_function(fn, params)) - if component is None: - msg = ( - f"Component-returning `@rx.memo` `{fn.__name__}` must return an " - "`rx.Component` or `rx.Var[rx.Component]`." - ) - raise TypeError(msg) - return MemoComponentDefinition( fn=fn, python_name=fn.__name__, params=params, export_name=format.to_title_case(fn.__name__), - component=_lift_rest_props(component), + _component=_LazyBody.ready(_evaluate_component_body(fn, params)), ) @@ -1300,6 +1521,11 @@ def __init__(self, definition: MemoComponentDefinition): for param in definition.params if param.kind not in (MemoParamKind.CHILDREN, MemoParamKind.REST) ] + # Forwardable-base-prop name sets already warned about, keyed per + # wrapper. ``console.deprecate`` walks and path-resolves the call stack + # before its own dedupe check, so a keyed memo under ``rx.foreach`` + # would pay that walk on every row. Gating here keeps it to the first. + self._warned_base_props: set[frozenset[str]] = set() update_wrapper(self, definition.fn) def __call__(self, *children: Any, **props: Any) -> MemoComponent: @@ -1347,16 +1573,26 @@ def __call__(self, *children: Any, **props: Any) -> MemoComponent: msg = f"`{definition.python_name}` is missing required prop `{param.name}`." raise TypeError(msg) - # Reject unknown props unless a rest prop is declared. + # Reject unknown props unless a rest prop is declared. ``key`` is the + # one exception (see ``_FORWARDABLE_BASE_PROPS``); every other undeclared + # prop raises with a message that points at ``rx.RestProp``. if remaining_props and rest_param is None: - unexpected_prop = next(iter(remaining_props)) - msg = ( - f"`{definition.python_name}` does not accept prop `{unexpected_prop}`. " - "Only declared props may be passed when no `rx.RestProp` is present." - ) - raise TypeError(msg) + unknown = [ + name for name in remaining_props if name not in _FORWARDABLE_BASE_PROPS + ] + if unknown: + msg = ( + f"`{definition.python_name}` does not accept prop `{unknown[0]}`. " + "Only declared props may be passed when no `rx.RestProp` is present." + ) + raise TypeError(msg) + warned_key = frozenset(remaining_props) + if warned_key not in self._warned_base_props: + self._warned_base_props.add(warned_key) + _warn_legacy_base_props(definition.python_name, list(remaining_props)) - # Build the component props passed into the memo wrapper. + # Reading ``component`` materializes the deferred body, so ``type(...)`` + # reflects the real wrapped class rather than the placeholder. return _get_memo_component_class( definition.export_name, type(definition.component) )._create( @@ -1502,157 +1738,58 @@ def passthrough(children: Var[Component]) -> Component: return _create_component_wrapper(definition), definition -@contextlib.contextmanager -def _bind_self_reference(fn: Callable[..., Any], wrapper: Any) -> Iterator[None]: - """Bind ``wrapper`` to ``fn.__name__`` so the body can self-reference. - - Python only assigns the decorated name after the decorator returns, but - memo bodies are evaluated during decoration (and ``rx.foreach`` eagerly - invokes its render function once). The binding is installed at both the - module-global slot and the matching free-variable cell so recursion works - for module-level memos and for memos defined inside another function. - """ - fn_name = fn.__name__ - fn_globals = fn.__globals__ - sentinel = object() - previous_global = fn_globals.get(fn_name, sentinel) - fn_globals[fn_name] = wrapper - - cell = None - previous_cell_value: Any = sentinel - free_vars = fn.__code__.co_freevars - if fn_name in free_vars and fn.__closure__: - cell = fn.__closure__[free_vars.index(fn_name)] - # An unset cell stays in the ``sentinel`` state; the decorator's - # eventual return assigns the wrapper to the same cell anyway, so - # leaving our temporary write in place is a no-op. - with contextlib.suppress(ValueError): - previous_cell_value = cell.cell_contents - cell.cell_contents = wrapper - - try: - yield - finally: - if previous_global is sentinel: - fn_globals.pop(fn_name, None) - else: - fn_globals[fn_name] = previous_global - if cell is not None and previous_cell_value is not sentinel: - cell.cell_contents = previous_cell_value - - _MemoVarT = TypeVar("_MemoVarT") -_PUBLIC_NAMESPACES: tuple[tuple[str, str], ...] = ( - # (display prefix, dotted attribute path to walk). Order matters — the - # shortest user-facing name wins. ``rxe`` only resolves when the optional - # ``reflex_enterprise`` package is installed. - ("rx.el", "reflex.el"), - ("rx", "reflex"), - ("rxe.dnd", "reflex_enterprise.dnd"), - ("rxe.flow", "reflex_enterprise.flow"), - ("rxe.components.dnd", "reflex_enterprise.components.dnd"), - ("rxe.components.flow", "reflex_enterprise.components.flow"), - ("rxe", "reflex_enterprise"), -) - - -def _resolve_namespace(dotted: str) -> Any: - """Walk a dotted path of attribute accesses rooted at an importable module. - - Args: - dotted: e.g. ``"reflex.el"`` or ``"reflex_enterprise.components.flow"``. - - Returns: - The resolved namespace object, or ``None`` if any step fails. - """ - head, *rest = dotted.split(".") - try: - ns: Any = importlib.import_module(head) - except ImportError: - return None - for attr in rest: - ns = getattr(ns, attr, None) - if ns is None: - return None - return ns - - -def _resolve_component_qualname(cls: type) -> str | None: - """Find the shortest public ``rx``/``rxe`` qualname under which ``cls`` lives. - - Args: - cls: The class to resolve. - - Returns: - The qualname (e.g. ``"rxe.dnd.Draggable"``), or ``None`` when no public - path is found. - """ - name = cls.__name__ - for display_prefix, dotted in _PUBLIC_NAMESPACES: - ns = _resolve_namespace(dotted) - if ns is not None and getattr(ns, name, None) is cls: - return f"{display_prefix}.{name}" - return None - - -def _suggest_return_annotation(result: Any, is_component: bool) -> str | None: - """Infer a copy-pasteable return annotation from a memo body's eval result. - - Args: - result: The value the body returned during memo eval. - is_component: Whether the memo was treated as component-returning. - - Returns: - A suggestion like ``"rxe.dnd.Draggable"`` or ``"rx.Var[str]"``, or - ``None`` when the result doesn't map cleanly to a public name. - """ - if is_component: - body = _normalize_component_return(result) - if body is None: - return None - return _resolve_component_qualname(type(body)) - if isinstance(result, Var): - inner = result._var_type - if isinstance(inner, type): - qual = _resolve_component_qualname(inner) - if qual is not None: - return f"rx.Var[{qual}]" - if inner.__module__ == "builtins": - return f"rx.Var[{inner.__name__}]" - return None - - def _warn_missing_annotations( fn_name: str, missing_return: bool, defaulted_params: Sequence[str], - suggested_return: str | None = None, ) -> None: """Emit a deprecation warning for ``@rx.memo`` without explicit annotations. Args: fn_name: Name of the decorated function (for the warning text). missing_return: Whether the return annotation was missing. - defaulted_params: Names of parameters whose annotation was defaulted. - suggested_return: Inferred return type (e.g. ``"rxe.dnd.Draggable"``) - to surface in the message. When ``None``, the generic hint is used. + defaulted_params: Names of parameters whose annotation was missing or a + legacy bare type and so was coerced to ``rx.Var[...]``. """ parts: list[str] = [] if missing_return: - if suggested_return is not None: - parts.append(f"a return annotation `-> {suggested_return}`") - else: - parts.append("a return annotation (`-> rx.Component` or `-> rx.Var[...]`)") + parts.append("a return annotation `-> rx.Component` (or `-> rx.Var[...]`)") if defaulted_params: joined = ", ".join(f"`{name}`" for name in defaulted_params) - parts.append(f"annotations on parameter(s) {joined} (`rx.Var[...]`)") + parts.append(f"`rx.Var[...]` annotations on parameter(s) {joined}") console.deprecate( feature_name=f"`@rx.memo` on `{fn_name}` without explicit annotations", reason=( - f"Add {' and '.join(parts)}. Missing annotations now default to " - "`rx.Component` / `rx.Var[Any]`" + f"Add {' and '.join(parts)}. Until removal, a missing return " + "defaults to `rx.Component` and missing or bare-type parameters are " + "coerced to `rx.Var[...]`" + ), + deprecation_version="0.9.3", + removal_version="1.0", + ) + + +def _warn_legacy_base_props(fn_name: str, prop_names: Sequence[str]) -> None: + """Warn that base-``Component`` props on a ``RestProp``-less memo are deprecated. + + ``prop_names`` is effectively always ``["key"]`` — the only forwardable prop. + + Args: + fn_name: Name of the memo (for the warning text). + prop_names: The base-component prop names passed at the call site. + """ + joined = ", ".join(f"`{name}`" for name in prop_names) + console.deprecate( + feature_name=( + f"Passing base-component prop(s) {joined} to `@rx.memo` `{fn_name}` " + "without an `rx.RestProp`" + ), + reason=( + "Declare an `rx.RestProp` parameter to keep passing base props like " + "`key` to the rendered component" ), deprecation_version="0.9.3", removal_version="1.0", @@ -1666,6 +1803,16 @@ def memo(fn: Callable[..., Var[_MemoVarT]]) -> _MemoFunctionWrapper: ... def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper: """Create a memo from a function. + The decorated function's body is **not** executed here. Only signature-level + analysis — return annotation, parameter kinds, name-collision registration, + and the deprecation warning for missing annotations — runs at decoration + time. The body is compiled lazily on first read of ``.component`` / + ``.function`` — when the component wrapper is instantiated, or when the + compiler reads the memo (see ``_LazyBody``). Deferring the body keeps + ``@rx.memo`` free of import-time side effects, so a memo whose body + references another module no longer forces that module to load during + import — sidestepping circular-import ordering issues. + Args: fn: The function to memoize. @@ -1673,7 +1820,7 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper The wrapped function or component factory. Raises: - TypeError: If the return type is not supported. + TypeError: If the return annotation is not supported. """ hints = get_type_hints(fn, include_extras=True) return_annotation = hints.get("return", inspect.Signature.empty) @@ -1698,9 +1845,14 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper defaulted_params=defaulted_params, ) - # Construct the wrapper against a placeholder body so the user's body can - # self-reference the memo during eager evaluation; the real body is patched - # in after eval completes (see `_bind_self_reference`). + if missing_return or defaulted_params: + _warn_missing_annotations(fn.__name__, missing_return, defaulted_params) + + # Build the definition with a deferred body: the decorated function runs on + # first read of ``.component`` / ``.function`` (see ``_LazyBody``), not here, + # so decoration has no import-time side effects. The component placeholder + # stands in for re-entrant reads during a recursive memo's own evaluation, + # where the name resolves to ``wrapper`` (already bound by first use). definition: MemoComponentDefinition | MemoFunctionDefinition if is_component: definition = MemoComponentDefinition( @@ -1708,7 +1860,10 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper python_name=fn.__name__, params=params, export_name=format.to_title_case(fn.__name__), - component=Fragment.create(), + _component=_LazyBody( + lambda: _evaluate_component_body(fn, params), + placeholder=Fragment.create(), + ), ) wrapper = _create_component_wrapper(definition) else: @@ -1716,49 +1871,19 @@ def memo(fn: Callable[..., Any]) -> _MemoComponentWrapper | _MemoFunctionWrapper fn=fn, python_name=fn.__name__, params=params, - function=ArgsFunctionOperation.create( - args_names=(), return_expr=LiteralVar.create(None) - ), + _function=_LazyBody(lambda: _evaluate_function_body(fn, params)), imported_var=_imported_function_var( fn.__name__, _annotation_inner_type(return_annotation) ), ) wrapper = _create_function_wrapper(definition) - with _bind_self_reference(fn, wrapper): - result = _evaluate_memo_function(fn, params) - - if missing_return or defaulted_params: - _warn_missing_annotations( - fn.__name__, - missing_return, - defaulted_params, - suggested_return=_suggest_return_annotation(result, is_component) - if missing_return - else None, - ) - - if is_component: - body = _normalize_component_return(result) - if body is None: - msg = ( - f"Component-returning `@rx.memo` `{fn.__name__}` must return an " - "`rx.Component` or `rx.Var[rx.Component]`." - ) - raise TypeError(msg) - object.__setattr__(definition, "component", _lift_rest_props(body)) - else: - return_expr = Var.create(result) - _validate_var_return_expr(return_expr, fn.__name__) - object.__setattr__( - definition, "function", _build_args_function(params, return_expr) - ) - _register_memo_definition(definition) return wrapper __all__ = [ + "EMPTY_VAR_COMPONENT", "MEMOS", "MemoComponent", "MemoComponentDefinition", diff --git a/packages/reflex-base/src/reflex_base/vars/function.py b/packages/reflex-base/src/reflex_base/vars/function.py index 85a311c58b5..878678925ac 100644 --- a/packages/reflex-base/src/reflex_base/vars/function.py +++ b/packages/reflex-base/src/reflex_base/vars/function.py @@ -552,6 +552,6 @@ def create( "Array.isArray", _var_type=ReflexCallable[[Any], bool] ) PROTOTYPE_TO_STRING = FunctionStringVar.create( - "((__to_string) => __to_string.toString())", + "((__to_string) => __to_string ? __to_string.toString() : '')", _var_type=ReflexCallable[[Any], str], ) diff --git a/packages/reflex-base/src/reflex_base/vars/object.py b/packages/reflex-base/src/reflex_base/vars/object.py index 307240f7186..521771272de 100644 --- a/packages/reflex-base/src/reflex_base/vars/object.py +++ b/packages/reflex-base/src/reflex_base/vars/object.py @@ -364,6 +364,29 @@ def contains(self, key: Var | Any) -> BooleanVar: class RestProp(ObjectVar[dict[str, Any]]): """A special object var representing forwarded rest props.""" + def merge(self, other: ObjectVar | Mapping[str, Any]): + """Merge another object into these RestProps. + + Args: + other: The other object (or plain mapping) to merge. + + Returns: + The merged RestProp-typed value. + """ + other_var = ( + other + if isinstance(other, ObjectVar) + else LiteralVar.create(other).to(ObjectVar) + ) + merged_var = object_merge_operation(self, other_var) + return type(self)( + _js_expr=str(merged_var), + _var_type=self._var_type, + _var_data=VarData.merge( + self._get_all_var_data(), merged_var._get_all_var_data() + ), + ) + @dataclasses.dataclass( eq=False, diff --git a/packages/reflex-components-internal/src/reflex_components_internal/components/base/skeleton.py b/packages/reflex-components-internal/src/reflex_components_internal/components/base/skeleton.py index 3207225b0f5..c24fcfa6df6 100644 --- a/packages/reflex-components-internal/src/reflex_components_internal/components/base/skeleton.py +++ b/packages/reflex-components-internal/src/reflex_components_internal/components/base/skeleton.py @@ -3,7 +3,7 @@ from reflex_components_core.el.elements.typography import Div from reflex.components.component import Component -from reflex.components.memo import memo +from reflex.components.memo import EMPTY_VAR_COMPONENT, memo from reflex.vars.base import EMPTY_VAR_STR, Var from reflex_components_internal.utils.twmerge import cn @@ -16,6 +16,7 @@ class ClassNames: @memo def skeleton_component( + children: Var[Component] = EMPTY_VAR_COMPONENT, class_name: Var[str] = EMPTY_VAR_STR, ) -> Component: """Skeleton component. @@ -23,7 +24,7 @@ def skeleton_component( Returns: The component. """ - return Div.create(class_name=cn(ClassNames.ROOT, class_name)) + return Div.create(children, class_name=cn(ClassNames.ROOT, class_name)) skeleton = skeleton_component diff --git a/pyi_hashes.json b/pyi_hashes.json index b4498b3581b..75a264871d8 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -118,7 +118,7 @@ "packages/reflex-components-recharts/src/reflex_components_recharts/polar.pyi": "1979bb6c22bb7a0d3342b2d63fb19d74", "packages/reflex-components-recharts/src/reflex_components_recharts/recharts.pyi": "c5288f311fe37b23539518ba2a3d4482", "packages/reflex-components-sonner/src/reflex_components_sonner/toast.pyi": "2c5fadcc014056f041cd4d916137d9e7", - "reflex/__init__.pyi": "12a863ddbcac050c702a3ec6092ae17c", + "reflex/__init__.pyi": "674cc55e646deb97c0e414e1d0e850ef", "reflex/components/__init__.pyi": "f39a2af77f438fa243c58c965f19d42e", - "reflex/experimental/memo.pyi": "5bfbbd60585132d7a76840a0dbacbdd2" + "reflex/experimental/memo.pyi": "97f96db9b67acdd103f71f9da3548600" } diff --git a/reflex/__init__.py b/reflex/__init__.py index 6e711166871..6b23b495d3e 100644 --- a/reflex/__init__.py +++ b/reflex/__init__.py @@ -142,7 +142,7 @@ "NoSSRComponent", "ComponentNamespace", ], - "reflex_base.components.memo": ["memo"], + "reflex_base.components.memo": ["memo", "EMPTY_VAR_COMPONENT"], "reflex_components_core.el.elements.media": ["image"], "reflex_components_lucide": ["icon"], **_COMPONENTS_BASE_MAPPING, diff --git a/reflex/compiler/utils.py b/reflex/compiler/utils.py index bafb2290fb0..6daf1390a44 100644 --- a/reflex/compiler/utils.py +++ b/reflex/compiler/utils.py @@ -526,7 +526,9 @@ def compile_experimental_function_memo( A tuple of the compiled function definition and its imports. """ imports: ParsedImportDict = {} - if var_data := definition.function._get_all_var_data(): + # Reading ``.function`` evaluates a deferred function-memo body on first use. + function = definition.function + if var_data := function._get_all_var_data(): # Per-file memo modules live at ``$/utils/components/``; strip # only a self-import to this function memo's own module. self_module = f"$/{constants.Dirs.COMPONENTS_PATH}/{definition.python_name}" @@ -540,7 +542,7 @@ def compile_experimental_function_memo( { "kind": "function", "name": definition.python_name, - "function": str(definition.function), + "function": str(function), }, imports, ) diff --git a/tests/integration/tests_playwright/test_memo.py b/tests/integration/tests_playwright/test_memo.py index 67f06fa4243..0d43ef526e5 100644 --- a/tests/integration/tests_playwright/test_memo.py +++ b/tests/integration/tests_playwright/test_memo.py @@ -28,6 +28,7 @@ class TreeNode(TypedDict): class MemoState(rx.State): last_value: str = "" + order: list[str] = ["row-a", "row-b", "row-c"] tree: TreeNode = TreeNode( name="root", children=[ @@ -50,6 +51,10 @@ def replace_tree(self): children=[TreeNode(name="only-child", children=[])], ) + @rx.event + def reverse_order(self): + self.order = list(reversed(self.order)) + @rx.memo def my_memoed_component( some_value: rx.Var[str], @@ -68,6 +73,14 @@ def tree_node(data: rx.vars.ObjectVar[TreeNode]) -> rx.Component: class_name="pl-4 border-l", ) + @rx.memo + def keyed_row(label: rx.Var[str]) -> rx.Component: + # Uncontrolled input: its typed value lives in the DOM, not in state, + # so React only preserves it across a reorder when the element keeps + # its identity — i.e. when ``key`` is honored. ``label`` doubles as the + # element id so each row is locatable after reordering. + return rx.input(id=label) + def index() -> rx.Component: return rx.vstack( rx.text(MemoState.last_value, id="memo-last-value"), @@ -78,6 +91,15 @@ def index() -> rx.Component: "replace-tree", id="replace-tree", on_click=MemoState.replace_tree ), rx.box(tree_node(data=MemoState.tree), id="tree-root"), + rx.button( + "reverse-order", id="reverse-order", on_click=MemoState.reverse_order + ), + rx.box( + rx.foreach( + MemoState.order, lambda item: keyed_row(label=item, key=item) + ), + id="keyed-rows", + ), ) app = rx.App() @@ -167,3 +189,36 @@ def test_memo_recursive_tree_reacts_to_state(memo_app: AppHarness, page: Page) - expect(node_names).to_have_count(2) expect(node_names).to_have_text(["root2", "only-child"]) + + +def test_memo_key_preserves_identity_across_reorder( + memo_app: AppHarness, page: Page +) -> None: + """``key`` on a memo under ``rx.foreach`` drives React's reconciliation. + + Each row is a memo with an uncontrolled input keyed by its label. Type a + distinct value into each, reverse the list, and the values must follow + their labels rather than their positions — which only happens if the + ``key`` reaches React. (``rx.foreach`` would otherwise key by index, giving + positional identity, so this asserts the explicit ``key`` is honored.) + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + assert memo_app.frontend_url is not None + page.goto(memo_app.frontend_url) + + rows = page.locator("#keyed-rows input") + expect(rows).to_have_count(3) + for row_id in ("row-a", "row-b", "row-c"): + page.locator(f"#{row_id}").fill(row_id.upper()) + + page.click("#reverse-order") + + # Order reversed (positional proof the reorder happened) ... + expect(rows.first).to_have_attribute("id", "row-c") + expect(rows.last).to_have_attribute("id", "row-a") + # ... while each row kept the value typed into it, by key, not by slot. + for row_id in ("row-a", "row-b", "row-c"): + expect(page.locator(f"#{row_id}")).to_have_value(row_id.upper()) diff --git a/tests/units/components/core/test_colors.py b/tests/units/components/core/test_colors.py index 48b074b3103..3ef74e2582b 100644 --- a/tests/units/components/core/test_colors.py +++ b/tests/units/components/core/test_colors.py @@ -37,19 +37,19 @@ def create_color_var(color): (create_color_var(rx.color("mint", 3, True)), '"var(--mint-a3)"', Color), ( create_color_var(rx.color(ColorState.color, ColorState.shade)), - f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+(((__to_string) => __to_string.toString())({color_state_name!s}.shade{FIELD_MARKER}))+")")', + f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+(((__to_string) => __to_string ? __to_string.toString() : \'\')({color_state_name!s}.shade{FIELD_MARKER}))+")")', Color, ), ( create_color_var( rx.color(ColorState.color, ColorState.shade, ColorState.alpha) ), - f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+({color_state_name!s}.alpha{FIELD_MARKER} ? "a" : "")+(((__to_string) => __to_string.toString())({color_state_name!s}.shade{FIELD_MARKER}))+")")', + f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+({color_state_name!s}.alpha{FIELD_MARKER} ? "a" : "")+(((__to_string) => __to_string ? __to_string.toString() : \'\')({color_state_name!s}.shade{FIELD_MARKER}))+")")', Color, ), ( create_color_var(color_with_fstring), - f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+(((__to_string) => __to_string.toString())({color_state_name!s}.shade{FIELD_MARKER}))+")")', + f'("var(--"+{color_state_name!s}.color{FIELD_MARKER}+"-"+(((__to_string) => __to_string ? __to_string.toString() : \'\')({color_state_name!s}.shade{FIELD_MARKER}))+")")', Color, ), ( @@ -59,7 +59,7 @@ def create_color_var(color): ColorState.shade, ) ), - f'("var(--"+({color_state_name!s}.color_part{FIELD_MARKER}+"ato")+"-"+(((__to_string) => __to_string.toString())({color_state_name!s}.shade{FIELD_MARKER}))+")")', + f'("var(--"+({color_state_name!s}.color_part{FIELD_MARKER}+"ato")+"-"+(((__to_string) => __to_string ? __to_string.toString() : \'\')({color_state_name!s}.shade{FIELD_MARKER}))+")")', Color, ), ( diff --git a/tests/units/components/test_memo.py b/tests/units/components/test_memo.py index 0881e2ad075..9276bf248c2 100644 --- a/tests/units/components/test_memo.py +++ b/tests/units/components/test_memo.py @@ -11,6 +11,7 @@ from reflex_base.components.component import Component from reflex_base.components.memo import ( _SPECS, + EMPTY_VAR_COMPONENT, MEMOS, MemoComponent, MemoComponentDefinition, @@ -18,7 +19,9 @@ MemoParam, MemoParamKind, _analyze_params, + _LazyBody, _MemoCallBinding, + _strip_optional, ) from reflex_base.event import EventChain, EventHandler, no_args_event_spec from reflex_base.style import Style @@ -183,6 +186,37 @@ def hover_trigger(rest: rx.RestProp) -> rx.Component: assert "({," not in code +def test_component_memo_rest_prop_merge_is_forwarded_as_rest_prop(): + """A merged ``RestProp`` stays a ``RestProp``. + + Passing ``rest.merge({...})`` to another component must lift it onto that + component's ``special_props`` (a JSX spread), exactly like the bare ``rest`` + — not render it as a literal child. + """ + + @rx.memo + def primary_button(rest: rx.RestProp, *, label: rx.Var[str]) -> rx.Component: + return rx.button(label, rest.merge({"className": "btn"})) + + definition = MEMOS["PrimaryButton"] + assert isinstance(definition, MemoComponentDefinition) + + # The merged value is accepted as a RestProp: lifted onto special_props + # rather than wrapped as a child. + merged_specials = [ + prop + for prop in definition.component.special_props + if isinstance(prop, rx.RestProp) + ] + assert len(merged_specials) == 1 + assert "...rest" in str(merged_specials[0]) + + files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) + code = "\n".join(c for _, c in files) + # Spread into the button props, not emitted as a jsx child. + assert '{...({...rest, ...({ ["className"] : "btn" })})}' in code + + def test_var_returning_memo_with_only_rest(): """Var-returning memos with only RestProp should emit valid JS (#6443).""" @@ -223,14 +257,254 @@ def label_slot( assert "export const label_slot = (({children, label, ...rest}) => label);" in code -def test_memo_requires_var_annotations(): - """Memos should reject non-Var annotations on parameters.""" - with pytest.raises(TypeError, match="must be annotated"): +def test_memo_munges_legacy_bare_type_param(): + """Legacy bare-type params should coerce to ``rx.Var[...]`` with a warning.""" + with patch.object(console, "deprecate") as mock_deprecate: @rx.memo def bad_annotation(value: int) -> rx.Var[str]: return rx.Var.create("x") + mock_deprecate.assert_called_once() + kwargs = mock_deprecate.call_args.kwargs + assert "bad_annotation" in kwargs["feature_name"] + assert "`value`" in kwargs["reason"] + + definition = MEMOS["bad_annotation"] + assert isinstance(definition, MemoFunctionDefinition) + (value_param,) = definition.params + assert value_param.kind is MemoParamKind.VALUE + # The bare ``int`` annotation is coerced into ``Var[int]``. + assert value_param.annotation == rx.Var[int] + + +def test_memo_munges_legacy_bare_type_params_for_component(): + """Component memos coerce legacy bare-type params and keep their defaults.""" + with patch.object(console, "deprecate") as mock_deprecate: + + @rx.memo + def legacy_card(title: str, count: int = 3) -> rx.Component: + return rx.box(rx.heading(title), rx.text(count)) + + mock_deprecate.assert_called_once() + reason = mock_deprecate.call_args.kwargs["reason"] + assert "`title`" in reason + assert "`count`" in reason + + definition = MEMOS["LegacyCard"] + assert isinstance(definition, MemoComponentDefinition) + assert {p.name: p.kind for p in definition.params} == { + "title": MemoParamKind.VALUE, + "count": MemoParamKind.VALUE, + } + count_param = next(p for p in definition.params if p.name == "count") + assert count_param.default == 3 + + # The munged props bind at instantiation; ``count`` falls back to its default. + component = legacy_card(title="Hi") + assert isinstance(component, MemoComponent) + + +def test_memo_does_not_warn_for_event_handler_param(): + """``rx.EventHandler`` params are recognized and must not be munged/warned.""" + with patch.object(console, "deprecate") as mock_deprecate: + + @rx.memo + def eh_only(event: rx.EventHandler) -> rx.Component: + return rx.button("click", on_click=event()) + + mock_deprecate.assert_not_called() + + +def test_memo_component_forwards_key_without_rest(): + """``key`` passes through a ``RestProp``-less memo and reaches the element. + + ``key`` is the one base ``Component`` prop that takes effect without an + ``rx.RestProp``: React consumes it at the reconciliation layer, so the + legacy custom-component use case (notably setting ``key`` under + ``rx.foreach``) keeps working. It is set as a real base field while a + deprecation warning points at ``rx.RestProp``. Props that only matter once + spread onto the rendered root (``id``, ``class_name``, ...) are *not* + forwardable here — see ``test_memo_nonkey_base_props_require_rest_prop``. + """ + + @rx.memo + def keyed_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + with patch.object(console, "deprecate") as mock_deprecate: + component = keyed_card(title="hi", key="row-1") + + mock_deprecate.assert_called_once() + feature_name = mock_deprecate.call_args.kwargs["feature_name"] + assert "keyed_card" in feature_name + assert "`key`" in feature_name + + assert isinstance(component, MemoComponent) + # ``key`` lands as a real base field, not as a declared memo prop ... + assert component.key == "row-1" + assert component.get_props() == ("title",) + # ... and reaches the rendered element, where React reads it for list + # reconciliation. + assert 'key:"row-1"' in component.render()["props"] + + +def test_memo_component_key_deprecation_warns_once_across_instances(): + """Repeated ``key=`` instantiations warn once, without re-walking the stack. + + Under ``rx.foreach`` a keyed memo is instantiated once per row. The warning + is deduped, but ``console.deprecate`` walks and path-resolves the call stack + *before* its dedupe check, so an ungated call site would pay that walk on + every row. The wrapper gates the call so only the first row reaches + ``console.deprecate`` at all. + """ + + @rx.memo + def row_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + with patch.object(console, "deprecate") as mock_deprecate: + for i in range(5): + row_card(title="hi", key=f"row-{i}") + + mock_deprecate.assert_called_once() + + +def test_memo_nonkey_base_props_require_rest_prop(): + """Non-``key`` base props raise without a ``RestProp`` rather than silently dropping. + + Without a ``RestProp`` the compiled memo function destructures only its + declared params and emits no ``...rest`` spread, so ``id``/``class_name``/ + ``style``/``custom_attrs``/``ref`` set on the wrapper never reach the + rendered root — they would be silently discarded. Reject them and point at + ``rx.RestProp``, which genuinely forwards them (see + ``test_memo_base_props_forward_to_root_via_rest_prop``). + """ + + @rx.memo + def plain_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + for prop, value in ( + ("id", "card-id"), + ("class_name", "c"), + ("style", {"color": "red"}), + ("custom_attrs", {"data-x": "y"}), + ("ref", "myref"), + ): + with pytest.raises(TypeError, match=f"does not accept prop `{prop}`"): + plain_card(title="hi", **{prop: value}) + + +def test_memo_nonkey_base_prop_dropped_from_render_without_rest(): + """Guard the *reason* non-``key`` base props are rejected: they don't render. + + Bypass the call-site gate by setting ``class_name`` directly on a built memo + wrapper, then compile. The base prop shows up on the page-level element but + the memo's own function body neither destructures nor spreads it onto the + root — proving a ``RestProp``-less memo cannot forward it, which is why the + call site rejects it. + """ + + @rx.memo + def dropper(title: rx.Var[str]) -> rx.Component: + return rx.box(rx.text(title)) + + component = dropper(title="hi") + component.class_name = Var.create("leaks") # set past the call-site gate + + files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) + code = next(c for path, c in files if path.endswith("Dropper.jsx")) + # No rest capture, and the root Box gets an empty props object. + assert "...rest" not in code + assert "className" not in code + + +def test_memo_base_props_forward_to_root_via_rest_prop(): + """With an ``rx.RestProp``, base props reach the rendered root via JS ``...rest``. + + This is the supported forwarding path the rejection message points users at. + """ + + @rx.memo + def rest_card(rest: rx.RestProp, *, title: rx.Var[str]) -> rx.Component: + return rx.box(rx.text(title), rest) + + component = rest_card(title="hi", class_name="c", id="card-id") + assert isinstance(component, MemoComponent) + + files, _ = compiler.compile_memo_components(tuple(MEMOS.values())) + code = next(c for path, c in files if path.endswith("RestCard.jsx")) + # Undeclared props are captured in ``...rest`` and spread onto the root, so + # ``className``/``id`` actually reach the rendered element. + assert "...rest" in code + assert "{...rest}" in code + + +def test_memo_component_still_rejects_unknown_props_without_rest(): + """Props that are not base ``Component`` fields still raise without a ``RestProp``.""" + + @rx.memo + def plain_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + with pytest.raises(TypeError, match="does not accept prop `bogus`"): + plain_card(title="hi", bogus="x") + + +def test_memo_component_rejects_unknown_even_alongside_base_props(): + """A genuinely-unknown prop raises even when a base prop is also present.""" + + @rx.memo + def mixed_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + with pytest.raises(TypeError, match="does not accept prop `bogus`"): + mixed_card(title="hi", key="row-1", bogus="x") + + +def test_memo_component_rejects_structural_base_fields_without_rest(): + """Identity/internal base fields (``tag``, ``library``, ...) are not forwardable. + + Overriding them would corrupt the memo's render, so they keep raising like + any other unknown prop rather than passing through. + """ + + @rx.memo + def struct_card(title: rx.Var[str]) -> rx.Component: + return rx.text(title) + + for prop in ("tag", "library", "event_triggers", "special_props"): + with pytest.raises(TypeError, match=f"does not accept prop `{prop}`"): + struct_card(title="hi", **{prop: "x"}) + + +def test_analyze_params_strict_mode_rejects_bare_type(): + """Strict callers (``defaulted_params=None``) must still reject bare types.""" + + def bare(value: int) -> rx.Component: + return rx.text("x") + + with pytest.raises(TypeError, match="must be annotated"): + _analyze_params(bare, for_component=True) + + +def test_is_memo_annotation_recognizes_supported_kinds(): + """``_is_memo_annotation`` gates which annotations are coerced to ``Var``.""" + from reflex_base.components.memo import _is_memo_annotation + + assert _is_memo_annotation(rx.Var[int]) is True + assert _is_memo_annotation(rx.RestProp) is True + assert _is_memo_annotation(rx.EventHandler) is True + assert ( + _is_memo_annotation(rx.EventHandler[rx.event.passthrough_event_spec(str)]) + is True + ) + # Legacy bare types are not recognized -> they get munged + warned. + assert _is_memo_annotation(int) is False + assert _is_memo_annotation(str) is False + assert _is_memo_annotation(list[str]) is False + def test_memo_warns_on_missing_param_annotation(): """Unannotated parameters should fall back to ``rx.Var[Any]`` with a warning.""" @@ -260,16 +534,139 @@ def soft_return(): assert "return annotation" in kwargs["reason"] -def test_memo_warning_suggests_inferred_return_type(): - """The warning should surface the inferred public qualname of the body's return.""" +def test_memo_warning_suggests_component_return(): + """A missing return annotation warns with a constant `-> rx.Component` hint. + + The suggestion no longer inspects the body's return value, so the warning + fires eagerly at decoration time even though the body itself runs lazily. + """ + evaluated = [] with patch.object(console, "deprecate") as mock_deprecate: @rx.memo def fragment_memo(): + evaluated.append(1) return rx.fragment(rx.text("x")) + mock_deprecate.assert_called_once() reason = mock_deprecate.call_args.kwargs["reason"] - assert "-> rx.Fragment" in reason + assert "-> rx.Component" in reason + # Emitting the warning did not require evaluating the body. + assert evaluated == [] + + +def test_memo_component_body_not_evaluated_until_used(): + """A component memo's body must not run until the wrapper is instantiated.""" + evaluated = [] + + @rx.memo + def lazy_box(value: rx.Var[str]) -> rx.Component: + evaluated.append(1) + return rx.box(value) + + # Decoration registers the memo without running the body. + assert "LazyBox" in MEMOS + assert evaluated == [] + + # First instantiation triggers a single evaluation... + component = lazy_box(value="hi") + assert isinstance(component, MemoComponent) + assert evaluated == [1] + + # ...and subsequent uses reuse the cached body. + lazy_box(value="bye") + assert evaluated == [1] + + +def test_memo_function_body_not_evaluated_until_compiled(): + """A var memo's body must not run at decoration or when merely called.""" + evaluated = [] + + @rx.memo + def lazy_join(value: rx.Var[str]) -> rx.Var[str]: + evaluated.append(1) + return value + + assert "lazy_join" in MEMOS + assert evaluated == [] + + # Calling a function memo references the imported var, not the body. + lazy_join(value=Var(_js_expr="x", _var_type=str)) + assert evaluated == [] + + # The compiler (reading ``.function``) triggers a single evaluation. + definition = MEMOS["lazy_join"] + assert isinstance(definition, MemoFunctionDefinition) + _ = definition.function + assert evaluated == [1] + _ = definition.function + assert evaluated == [1] + + +def test_lazy_body_placeholder_stands_in_for_reentrant_read(): + """A re-entrant read returns the placeholder, then caches the real body.""" + cell: _LazyBody[str] + seen = [] + + def thunk() -> str: + seen.append(cell.get()) # re-enters while the thunk is running + return "real" + + cell = _LazyBody(thunk, placeholder="placeholder") + assert cell.get() == "real" + assert seen == ["placeholder"] + # Cached afterwards; the thunk does not run again (``seen`` stays unchanged). + assert cell.get() == "real" + assert seen == ["placeholder"] + + +def test_lazy_body_reentrant_read_without_placeholder_raises(): + """A placeholder-less body that re-enters its own evaluation fails loudly.""" + cell: _LazyBody[str] + + def thunk() -> str: + return cell.get() + + cell = _LazyBody(thunk) + with pytest.raises(RuntimeError, match="Re-entrant"): + cell.get() + + +@pytest.mark.parametrize( + ("attr_name", "expected_type", "expected_render"), + [ + ("EMPTY_VAR_STR", str, '""'), + ("EMPTY_VAR_INT", int, "0"), + ("EMPTY_VAR_COMPONENT", Component, "(jsx(Fragment, ({})))"), + ], +) +def test_empty_var_sentinels_are_public_typed_vars( + attr_name: str, expected_type: type, expected_render: str +): + """`rx.EMPTY_VAR_*` defaults are public, correctly-typed empty Vars. + + These back the documented `rx.Var[...]` memo prop defaults; + `EMPTY_VAR_COMPONENT` lives in `memo` (not `component`) to avoid a circular + import, but must still be reachable as `rx.EMPTY_VAR_COMPONENT`. + """ + sentinel = getattr(rx, attr_name) + assert isinstance(sentinel, Var) + assert sentinel._var_type is expected_type + assert str(sentinel) == expected_render + + +def test_empty_var_component_default_for_memo_children_slot(): + """`EMPTY_VAR_COMPONENT` works as the default for a memo `children` slot.""" + + @rx.memo + def slot( + children: rx.Var[rx.Component] = EMPTY_VAR_COMPONENT, + ) -> rx.Component: + return rx.box(children) + + # Omitting children falls back to the empty-component default. + assert isinstance(slot(), MemoComponent) + assert isinstance(slot(rx.text("hi")), MemoComponent) def test_memo_warns_once_when_return_and_param_both_missing(): @@ -438,29 +835,39 @@ def child_label( def test_var_returning_memo_rejects_hooks(): - """Var-returning memos should reject hook-bearing expressions.""" - with pytest.raises(TypeError, match="cannot depend on hooks"): + """Var-returning memos should reject hook-bearing expressions (lazily).""" - @rx.memo - def bad_hook(value: rx.Var[str]) -> rx.Var[str]: - return Var( - _js_expr="value", - _var_type=str, - _var_data=VarData(hooks={"const badHook = 1": None}), - ) + @rx.memo + def bad_hook(value: rx.Var[str]) -> rx.Var[str]: + return Var( + _js_expr="value", + _var_type=str, + _var_data=VarData(hooks={"const badHook = 1": None}), + ) + + # Decoration defers the body; reading ``.function`` surfaces the error. + definition = MEMOS["bad_hook"] + assert isinstance(definition, MemoFunctionDefinition) + with pytest.raises(TypeError, match="cannot depend on hooks"): + _ = definition.function def test_var_returning_memo_rejects_non_bundled_imports(): - """Var-returning memos should reject non-bundled imports.""" - with pytest.raises(TypeError, match="not bundled"): + """Var-returning memos should reject non-bundled imports (lazily).""" - @rx.memo - def bad_import(value: rx.Var[str]) -> rx.Var[str]: - return Var( - _js_expr="value", - _var_type=str, - _var_data=VarData(imports={"some-lib": [ImportVar(tag="x")]}), - ) + @rx.memo + def bad_import(value: rx.Var[str]) -> rx.Var[str]: + return Var( + _js_expr="value", + _var_type=str, + _var_data=VarData(imports={"some-lib": [ImportVar(tag="x")]}), + ) + + # Decoration defers the body; reading ``.function`` surfaces the error. + definition = MEMOS["bad_import"] + assert isinstance(definition, MemoFunctionDefinition) + with pytest.raises(TypeError, match="not bundled"): + _ = definition.function def test_compile_memo_components_includes_functions_and_components(): @@ -500,7 +907,7 @@ def noop() -> None: python_name=f"memo_{idx}", params=(), export_name=f"Memo{idx}", - component=rx.fragment(), + _component=_LazyBody.ready(rx.fragment()), passthrough_hole_child=None, ) for idx in range(5) @@ -580,6 +987,7 @@ def wrapper() -> rx.Component: definition = MEMOS["Wrapper"] assert isinstance(definition, MemoComponentDefinition) + # Reading ``.component`` triggers the deferred body evaluation. assert definition.component.style == Style() monkeypatch.setattr( @@ -745,6 +1153,41 @@ def bad_eh_default( return rx.button("hi") +def test_strip_optional_unwraps_none_union(): + """`_strip_optional` collapses a ``X | None`` union to ``X``; any other + annotation passes through unchanged. + """ + assert _strip_optional(int | None) is int + var = rx.Var[str] + assert _strip_optional(var) is var + + +def test_analyze_params_unwraps_optional_event_handler_default( + monkeypatch: pytest.MonkeyPatch, +): + """Regression: on Python 3.10 ``get_type_hints`` rewrites ``event: EH = None`` + to ``Optional[EH]``. With that shim active, ``_analyze_params`` must still see + the ``EventHandler`` underneath and reject the default (it silently passed on + 3.10 before the fix, since ``Optional[...]`` is not recognized as an EH). + + Force the shim on so this exercises the path on every Python version, not + only the <=3.10 interpreters that actually wrap the annotation. + """ + monkeypatch.setattr( + "reflex_base.components.memo._GET_TYPE_HINTS_WRAPS_NONE_DEFAULT", True + ) + + def fn(event=None) -> rx.Component: + return rx.button("hi") + + # Python <=3.10 wraps a ``= None`` param into a union with ``None`` (its + # ``get_type_hints`` adds ``Optional``); the ``EventHandler`` underneath + # must still be recognized so the default is rejected. + wrapped_hints = {"event": EventHandler | None} + with pytest.raises(TypeError, match="default"): + _analyze_params(fn, for_component=True, hints=wrapped_hints) + + def test_component_memo_rejects_event_handler_named_children(): """A `children` parameter must not be an EventHandler.""" with pytest.raises(TypeError, match="children"): diff --git a/tests/units/vars/test_object.py b/tests/units/vars/test_object.py index c2a479294c3..e9ab66c82dc 100644 --- a/tests/units/vars/test_object.py +++ b/tests/units/vars/test_object.py @@ -1,10 +1,12 @@ import dataclasses from collections.abc import Sequence +from typing import Any import pytest +from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import GenericType -from reflex_base.vars.base import Var -from reflex_base.vars.object import LiteralObjectVar, ObjectVar +from reflex_base.vars.base import Var, VarData +from reflex_base.vars.object import LiteralObjectVar, ObjectVar, RestProp from reflex_base.vars.sequence import ArrayVar from typing_extensions import assert_type @@ -164,3 +166,46 @@ def test_typing() -> None: _ = assert_type(list_var, ArrayVar[Sequence[Base]]) list_var_0 = list_var[0] _ = assert_type(list_var_0, ObjectVar[Base]) + + +def test_rest_prop_merge_with_dict_preserves_type_and_spreads(): + """`RestProp.merge` accepts a dict, spreads both, and stays a RestProp. + + The result must remain a `RestProp` so a memo body can keep forwarding it + (e.g. `rest.merge({...})` passed positionally still lifts to a `{...}` spread). + """ + rest = RestProp(_js_expr="rest", _var_type=dict[str, Any]) + + # A plain mapping merges like an object var (the signature accepts Mapping). + merged = rest.merge({"color": "red"}) + + assert isinstance(merged, RestProp) + assert merged._var_type == dict[str, Any] + assert str(merged) == '({...rest, ...({ ["color"] : "red" })})' + + +def test_rest_prop_merge_with_object_var(): + """`RestProp.merge` spreads another object Var after the rest props.""" + rest = RestProp(_js_expr="rest", _var_type=dict[str, Any]) + other = Var(_js_expr="extra", _var_type=dict).to(ObjectVar) + + merged = rest.merge(other) + + assert isinstance(merged, RestProp) + assert str(merged) == "({...rest, ...extra})" + + +def test_rest_prop_merge_propagates_var_data(): + """`RestProp.merge` carries the merged object's VarData (deps/imports).""" + rest = RestProp(_js_expr="rest", _var_type=dict[str, Any]) + other = Var( + _js_expr="extra", + _var_type=dict, + _var_data=VarData(imports={"some-lib": [ImportVar(tag="thing")]}), + ).to(ObjectVar) + + merged = rest.merge(other) + + var_data = merged._get_all_var_data() + assert var_data is not None + assert "some-lib" in dict(var_data.imports)