diff --git a/docs/library/dynamic-rendering/foreach.md b/docs/library/dynamic-rendering/foreach.md index 0a4e6e60934..874d3c8ed5c 100644 --- a/docs/library/dynamic-rendering/foreach.md +++ b/docs/library/dynamic-rendering/foreach.md @@ -220,8 +220,146 @@ def foreach_complex_dict_example(): ) ``` +## Per-Item State And Event Handlers + +Each rendered item gets its own scope, so the item and index are available +anywhere in that item's subtree -- including in event handlers and in +components the compiler splits out on its own: + +```python +class TodoState(rx.State): + items: list[str] = ["write docs", "ship it"] + + @rx.event + def done(self, item: str, index: int): ... + + +def todo_row(item: rx.Var[str], index: rx.Var[int]) -> rx.Component: + return rx.hstack( + rx.text(item), + rx.button("done", on_click=TodoState.done(item, index)), + ) + + +def todo_list(): + return rx.vstack(rx.foreach(TodoState.items, todo_row)) +``` + +Client state works the same way: an unnamed `rx.client_state` var in a +`foreach` body is per item, the way `useState` would be in a React list. + +```python +def expandable_row(item: rx.Var[str]) -> rx.Component: + expanded = rx.client_state(False) # one per rendered row + return rx.vstack( + rx.button(item, on_click=expanded.set(lambda prev: ~prev)), + rx.cond(expanded.value, rx.text(f"details for {item}")), + ) +``` + +One exception: the body of an explicit `@rx.memo` component is compiled as a +single component and not split up, so a `foreach` inside it renders its item +inline and an `rx.client_state` var declared in that item function is shared +by every item. To keep per-item state inside a memo, make the item function an +`@rx.memo` component of its own; each rendered instance then owns its state: + +```python +@rx.memo +def expandable_row(item: rx.Var[str]) -> rx.Component: + expanded = rx.client_state(False) # one per rendered instance + return rx.vstack( + rx.button(item, on_click=expanded.set(lambda prev: ~prev)), + rx.cond(expanded.value, rx.text(f"details for {item}")), + ) + + +@rx.memo +def expandable_list(items: rx.Var[list[str]]) -> rx.Component: + return rx.vstack(rx.foreach(items, lambda item: expandable_row(item=item))) +``` + + +The default can be the loop item or index, which seeds each row from its own +value: + +```python +def counter_row(item: rx.Var[str], index: rx.Var[int]) -> rx.Component: + count = rx.client_state(index) # row N starts at N + return rx.hstack( + rx.text(item), + rx.heading(count.value), + rx.button("+", on_click=count.set(lambda prev: prev + 1)), + ) +``` + +A default is a *seed*, read once when the row first claims the slot. It is not a +binding: a later change to the var does not reset a row, which is what keeps a +row from losing what the user typed into it every time the list re-renders. + +That has a consequence worth knowing before you reach for it, and it is the same +one `useState(props.value)` has in React. Rows are keyed by position by default +(see below), so replacing the list's contents re-renders the existing rows +instead of mounting new ones -- and a state seeded from the item keeps the *old* +item's seed: + +```python +rx.foreach(State.items, lambda item: rx.text(rx.client_state(item).value)) +# State.items: ["a", "b", "c"] -> ["d", "e", "f"] +# rx.text(item) renders d, e, f <- follows the data +# rx.client_state(item).value renders a, b, c <- seeded once, per position +``` + +The item itself always follows the data; only the seeded state lags. Pass `key=` +on the item when you want the state to belong to the item, so replacing the list +unmounts the old rows and the new ones seed themselves: + +```python +rx.foreach(State.items, lambda item: row(item, key=item)) +``` + +If you want a row's state to track a var while staying editable in between, seed +it and then push updates explicitly with `on_mount=count.set(index)`, or on a +`rx.fragment(key=..., on_mount=...)` to tie the reset to something of your own +choosing. + +Loops nest, and each level gets its own scope. A nested body can read an +*enclosing* loop's item and index, as long as it does not reuse their names -- +the same rule Python already imposes, since an inner argument of the same name +shadows the outer one: + +```python +rx.foreach( + State.rows, + lambda row: rx.foreach(row, lambda cell: rx.text(f"{row[0]}/{cell}")), +) +``` + +By default each item is keyed by its position in the list. Pass `key=` on the +item to key by identity instead: + +```python +rx.foreach(TodoState.items, lambda item: todo_row(item, key=item)) +``` + +The key decides what a row's state belongs to. Under the default positional +keys, changing the list re-renders the existing rows in place rather than +mounting new ones, so anything a row is holding -- a typed-in value, focus, an +in-flight animation, a `rx.client_state` var -- stays with the *position*. Row 3 +of the old list keeps its expanded/selected state as row 3 of the new one. Key +by identity and the old rows unmount instead, releasing their state, and a row +that reappears starts fresh. + +Neither is the right default for every list, which is why the choice is yours. +Positional keys suit a list whose rows are interchangeable slots, where you want +"row 3 is expanded" to persist as the data flows through. Identity keys suit a +list of distinct things each carrying its own state, where a row's state should +travel with its item across reorders and disappear with it. Identity keys need +the key expression to be unique within the list -- duplicate keys make React +reconcile the wrong rows together. + ## API Reference + ### `rx.foreach` ```python diff --git a/docs/wrapping-react/overview.md b/docs/wrapping-react/overview.md index af9160c6c44..498a97b700f 100644 --- a/docs/wrapping-react/overview.md +++ b/docs/wrapping-react/overview.md @@ -172,19 +172,28 @@ def counter(): rx.vstack(counter(), counter(), counter()) # three independent counters ``` -Inside `rx.foreach`, give the item its own state by making the item function an -`@rx.memo` component; each rendered instance then owns its unnamed vars: +Each item rendered by `rx.foreach` is its own scope too, so an unnamed var used in a +loop body is per item, and can be seeded from the loop item or index: ```python -@rx.memo -def row(item: rx.Var[str]) -> rx.Component: +def row(item: rx.Var[str], index: rx.Var[int]) -> rx.Component: expanded = rx.client_state(False) # one per rendered row + count = rx.client_state(index) # row N starts at N ... -rx.foreach(State.items, lambda item: row(item=item)) +rx.foreach(State.items, row) ``` +A default is read once, when the scope first claims the name, so it seeds the state +rather than tracking the var. That means a row seeded from its item keeps the *old* +seed when the list's contents change, because `rx.foreach` keys rows by position and +a row that does not unmount never re-claims its slot. The item itself still follows +the data -- only the seeded state lags. Pass `key=` on the item to tie a row's state +to the item instead, or set the value explicitly (`on_mount=count.set(index)`) when +you want it to follow. See +[foreach](/docs/library/dynamic-rendering/foreach) for the worked example. + Pass `prefix=` to make generated names readable in the compiled output: `rx.client_state(0, prefix="counter")`. diff --git a/news/+foreach-scoped-loop-vars.bugfix.md b/news/+foreach-scoped-loop-vars.bugfix.md new file mode 100644 index 00000000000..ecf2b8f2f87 --- /dev/null +++ b/news/+foreach-scoped-loop-vars.bugfix.md @@ -0,0 +1 @@ +`rx.foreach` item and index values are now available to event handlers and memoized components rendered inside the loop, and each item renders as a component of its own, so an unnamed `rx.client_state` var declared in the item function is per item. diff --git a/packages/reflex-base/news/+foreach-scoped-loop-vars.bugfix.md b/packages/reflex-base/news/+foreach-scoped-loop-vars.bugfix.md new file mode 100644 index 00000000000..ecf2b8f2f87 --- /dev/null +++ b/packages/reflex-base/news/+foreach-scoped-loop-vars.bugfix.md @@ -0,0 +1 @@ +`rx.foreach` item and index values are now available to event handlers and memoized components rendered inside the loop, and each item renders as a component of its own, so an unnamed `rx.client_state` var declared in the item function is per item. diff --git a/packages/reflex-base/src/reflex_base/.templates/web/utils/client_state.js b/packages/reflex-base/src/reflex_base/.templates/web/utils/client_state.js index ed3a810288f..6d0bbb4ddba 100644 --- a/packages/reflex-base/src/reflex_base/.templates/web/utils/client_state.js +++ b/packages/reflex-base/src/reflex_base/.templates/web/utils/client_state.js @@ -255,6 +255,62 @@ export const withClientStateScope = (Component) => { return Wrapped; }; +/** + * Per-render values provided down the tree, chained like the slot scopes. + * + * Distinct from the slot scopes on purpose: these are read-only values that + * change every render (a loop's item and index), so their context identity has + * to be free to change, while a slot scope must stay stable or mounted hooks + * would rebind. Null outside any provider. + */ +export const ScopedValuesContext = createContext(null); + +/** + * Provide read-only values to a subtree, keyed by name. + * + * This is how a loop hands its item and index to descendants that compile into + * their own components: they read by name from context instead of closing over + * a variable that only exists inside the loop callback. + * + * It also opens a client state scope, because it marks a component instance the + * same way a memo boundary does -- one rendered item. That makes an unnamed + * client state var used in a loop body per item, which is what a reader of the + * Python expects and what React's `useState` would do. + * @param props The component props. + * @param props.children The children to render. + * @param props.values Mapping of name to value for this subtree. + * @returns The provider element. + */ +export function ScopedValues({ children, values }) { + const parent = useContext(ScopedValuesContext); + // A fresh object each render is correct here -- the values themselves change + // per render, and nothing subscribes to them. + const chained = { parent, values }; + return createElement( + ScopedValuesContext.Provider, + { value: chained }, + createElement(ClientStateScope, null, children), + ); +} + +/** + * Read a value provided by an enclosing `ScopedValues`. + * + * Walks outward, so a nested loop's descendants can still reach the outer loop's + * values. Names are generated at compile time, so nested loops never collide. + * @param name The value's name. + * @returns The value, or undefined when nothing provides it. + */ +export function useScopedValue(name) { + const scope = useContext(ScopedValuesContext); + for (let current = scope; current !== null; current = current.parent) { + if (name in current.values) { + return current.values[name]; + } + } + return undefined; +} + /** * Subscribe to a piece of client state. * @param defaultValue The initial value. diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index 4899a9230ad..4c67b089056 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -53,7 +53,15 @@ def render_iterable_tag(component: Any) -> str: children_rendered = "".join([ _RenderUtils.render(child) for child in component.get("children", []) ]) - return f"Array.prototype.map.call({component['iterable_state']} ?? [],(({component['arg_name']},{component['arg_index']})=>({children_rendered})))" + arg = component["arg_name"] + index = component["arg_index"] + # Provide the item and index to the subtree by name, so descendants + # that compile into their own components read them from context. + values = f"{{{arg}:{arg},{index}:{index}}}" + # The provider is the element the map yields, so it carries the key. + key = component["item_key"] + wrapped = f"jsx(ScopedValues,{{key:{key},values:{values}}},{children_rendered})" + return f"Array.prototype.map.call({component['iterable_state']} ?? [],(({arg},{index})=>({wrapped})))" @staticmethod def render_match_tag(component: Any) -> str: diff --git a/packages/reflex-base/src/reflex_base/components/memo.py b/packages/reflex-base/src/reflex_base/components/memo.py index eb5e74999fe..1ac46c297d0 100644 --- a/packages/reflex-base/src/reflex_base/components/memo.py +++ b/packages/reflex-base/src/reflex_base/components/memo.py @@ -2042,7 +2042,7 @@ def component_hash(component: Component, *, recursive: bool) -> str: return digest -def memo_tag(component: Component) -> str: +def memo_tag(component: Component, *, recursive: bool | None = None) -> str: """Compute a stable tag name for the memo wrapping ``component``. The class qualname is in the tag prefix so distinct classes that render @@ -2053,11 +2053,16 @@ def memo_tag(component: Component) -> str: Args: component: The component being memoized. + recursive: Whether the memo body carries the whole subtree, so its + descendants' artifacts identify the body too. Defaults to whether + the component's own strategy is a snapshot; a caller that renders + a snapshot of a component whose strategy is not says so here. Returns: The stable tag name. """ - recursive = get_memoization_strategy(component) is MemoizationStrategy.SNAPSHOT + if recursive is None: + recursive = get_memoization_strategy(component) is MemoizationStrategy.SNAPSHOT prefix, tag = type(component).__qualname__, component.tag or "Comp" # Memo component classes are already named ``MemoComponent_``. if prefix != f"MemoComponent_{tag}": @@ -2084,6 +2089,8 @@ def memo_tag(component: Component) -> str: def create_passthrough_component_memo( component: Component, source_module: str | None = None, + *, + snapshot: bool = False, ) -> tuple[ Callable[..., MemoComponent], MemoComponentDefinition, @@ -2104,6 +2111,10 @@ def create_passthrough_component_memo( component: The component to wrap. source_module: The user-app Python module that triggered creation of this memo (typically the page that contained the wrapped subtree). + snapshot: Render the whole component as the memo body, children + included, instead of substituting a ``{children}`` hole. A loop + item is wrapped this way so every hook in its subtree runs inside + the item component, below the scope the loop provides around it. Returns: The callable memo wrapper and its component definition. @@ -2116,7 +2127,7 @@ def create_passthrough_component_memo( # body renders the full snapshot rather than a ``{children}``-holed # template. render_snapshot = ( - get_memoization_strategy(component) is MemoizationStrategy.SNAPSHOT + snapshot or get_memoization_strategy(component) is MemoizationStrategy.SNAPSHOT ) captured_hole_child: list[Component] = [] @@ -2158,7 +2169,7 @@ def passthrough(children: Var[Component]) -> Component: params = _PASSTHROUGH_PARAMS rest_target_fields: set[str] = set() preview = _evaluate_component_body(passthrough, params, rest_target_fields) - tag = memo_tag(preview) + tag = memo_tag(preview, recursive=render_snapshot) passthrough.__name__ = format.to_snake_case(tag) passthrough.__qualname__ = passthrough.__name__ diff --git a/packages/reflex-base/src/reflex_base/components/tags/iter_tag.py b/packages/reflex-base/src/reflex_base/components/tags/iter_tag.py index 99f66ecb006..240e7f1887f 100644 --- a/packages/reflex-base/src/reflex_base/components/tags/iter_tag.py +++ b/packages/reflex-base/src/reflex_base/components/tags/iter_tag.py @@ -8,14 +8,54 @@ from typing import TYPE_CHECKING from reflex_base.components.tags.tag import CommonTag +from reflex_base.constants import Dirs +from reflex_base.utils.imports import ImportVar from reflex_base.utils.types import GenericType from reflex_base.vars import LiteralArrayVar, Var, get_unique_variable_name +from reflex_base.vars.base import LiteralVar, VarData from reflex_base.vars.sequence import _determine_value_of_array_index if TYPE_CHECKING: from reflex_base.components.component import Component +_SCOPED_VALUE_IMPORT = { + f"$/{Dirs.CLIENT_STATE_PATH}": [ImportVar(tag="useScopedValue")] +} + + +def scoped_loop_var(name: str, var_type: GenericType) -> Var: + """Build a loop var that reads its value from the enclosing scope. + + The loop publishes its item and index by name around each rendered item, + so a consumer that compiles into its own function -- an event handler in a + ``useCallback``, a subtree in its own memo module -- reads them from context + rather than from the map callback's parameter. + + The hook declares the *same* identifier as the map callback's parameter, on + purpose. Hooks float to the top of whichever component they land in, so in + the module that renders the loop itself the declaration sits above the + ``.map`` and reads nothing; the parameter shadows it for everything inside + the callback, which is exactly the scope where the parameter is the real + value. Anywhere else there is no parameter, and the context read wins. + + Args: + name: The name the value is provided under. + var_type: The type of the value. + + Returns: + A Var carrying the hook that reads the value. + """ + return Var( + _js_expr=name, + _var_type=var_type, + _var_data=VarData( + hooks={f"const {name} = useScopedValue({LiteralVar.create(name)!s})": None}, + imports=_SCOPED_VALUE_IMPORT, + ), + ).guess_type() + + @dataclasses.dataclass(frozen=True) class IterTag(CommonTag): """An iterator tag.""" @@ -50,10 +90,7 @@ def get_index_var(self) -> Var: Returns: The index var. """ - return Var( - _js_expr=self.index_var_name, - _var_type=int, - ).guess_type() + return scoped_loop_var(self.index_var_name, int) def get_arg_var(self) -> Var: """Get the arg var for the tag (with curly braces). @@ -63,10 +100,7 @@ def get_arg_var(self) -> Var: Returns: The arg var. """ - return Var( - _js_expr=self.arg_var_name, - _var_type=self.get_iterable_var_type(), - ).guess_type() + return scoped_loop_var(self.arg_var_name, self.get_iterable_var_type()) def render_component(self) -> Component: """Render the component. @@ -110,8 +144,4 @@ def render_component(self) -> Component: msg = "The render function must return a component." raise ValueError(msg) - # Set the component key. - if component.key is None: - component.key = index - return component diff --git a/packages/reflex-components-core/news/+foreach-scoped-loop-vars.bugfix.md b/packages/reflex-components-core/news/+foreach-scoped-loop-vars.bugfix.md new file mode 100644 index 00000000000..437c9840099 --- /dev/null +++ b/packages/reflex-components-core/news/+foreach-scoped-loop-vars.bugfix.md @@ -0,0 +1 @@ +`rx.foreach` provides each item's value and index to the components rendered inside it, so event handlers and memoized descendants can use them. diff --git a/packages/reflex-components-core/pyproject.toml b/packages/reflex-components-core/pyproject.toml index 1d109aeb775..9b61e64513e 100644 --- a/packages/reflex-components-core/pyproject.toml +++ b/packages/reflex-components-core/pyproject.toml @@ -16,7 +16,7 @@ classifiers = [ "Programming Language :: Python :: 3.15", ] dependencies = [ - "reflex-base >= 0.10.1", + "reflex-base >= 0.10.2.dev0", "reflex-components-lucide >= 1.1.0", "reflex-components-sonner >= 0.10.0", "python_multipart >= 0.0.21", diff --git a/packages/reflex-components-core/src/reflex_components_core/core/foreach.py b/packages/reflex-components-core/src/reflex_components_core/core/foreach.py index 83d41fdbb48..a84b531970e 100644 --- a/packages/reflex-components-core/src/reflex_components_core/core/foreach.py +++ b/packages/reflex-components-core/src/reflex_components_core/core/foreach.py @@ -10,7 +10,7 @@ from reflex_base.components.component import Component, field from reflex_base.components.tags import IterTag -from reflex_base.constants import MemoizationMode +from reflex_base.constants import Dirs from reflex_base.constants.state import FIELD_MARKER from reflex_base.utils import types from reflex_base.utils.exceptions import UntypedVarError @@ -47,10 +47,16 @@ class ForeachRenderError(TypeError): class Foreach(Component): """A component that takes in an iterable and a render function and renders a list of components.""" - _memoization_mode = MemoizationMode(recursive=False) - iterable: Var[Iterable] = field(doc="The iterable to create components from.") + def add_imports(self) -> dict[str, str]: + """Import the provider each item's subtree is wrapped in. + + Returns: + The imports for the component. + """ + return {f"$/{Dirs.CLIENT_STATE_PATH}": "ScopedValues"} + render_fn: Callable = field( doc="A function from the render args to the component.", default=Fragment.create, @@ -184,12 +190,25 @@ def render(self): The dictionary for template of component. """ tag = self._render() + # The per-item provider is the element the map yields, so it is what + # React reconciles the list by and therefore what carries the key. An + # explicit key on the item is lifted up to it; otherwise the loop index + # keys by position, as it always has. + item_key = next( + ( + str(LiteralVar.create(child.key)) + for child in self.children + if isinstance(child, Component) and child.key is not None + ), + tag.index_var_name, + ) return dict( tag, iterable_state=str(tag.iterable), arg_name=tag.arg_var_name, arg_index=tag.index_var_name, + item_key=item_key, ) diff --git a/pyi_hashes.json b/pyi_hashes.json index 99003b2297a..6bafacf7a31 100644 --- a/pyi_hashes.json +++ b/pyi_hashes.json @@ -120,5 +120,5 @@ "packages/reflex-components-sonner/src/reflex_components_sonner/toast.pyi": "f170ac685b6ba5892370166c80684db3", "reflex/__init__.pyi": "067a60e22fb825072f2f0b690662da5a", "reflex/components/__init__.pyi": "9facd05a776d0641432696bbf8e34388", - "reflex/experimental/memo.pyi": "71e62a97412c73225fac44658ca0e06c" + "reflex/experimental/memo.pyi": "23a4ebb09a45ce79bb594035dd0c25c7" } diff --git a/pyproject.toml b/pyproject.toml index f10560bbfe4..399949a3cbf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,7 +34,7 @@ dependencies = [ "wrapt >=1.17.0,<2.6", "reflex-base >= 0.10.2.dev0", "reflex-components-code >= 0.10.1", - "reflex-components-core >= 0.10.0", + "reflex-components-core >= 0.10.2.dev0", "reflex-components-dataeditor >= 0.10.1", "reflex-components-gridjs >= 0.10.0", "reflex-components-lucide >= 1.1.1", diff --git a/reflex/compiler/plugins/memoize.py b/reflex/compiler/plugins/memoize.py index 86b6b53a3e4..0e73632f73f 100644 --- a/reflex/compiler/plugins/memoize.py +++ b/reflex/compiler/plugins/memoize.py @@ -21,6 +21,7 @@ from __future__ import annotations import dataclasses +import functools from typing import Any from reflex_base.components.app_wraps import ( @@ -39,6 +40,7 @@ from reflex_base.constants.compiler import MemoizationDisposition from reflex_base.plugins import ComponentAndChildren, PageContext from reflex_base.plugins.base import Plugin +from reflex_base.plugins.compiler import CompilerHooks from reflex_components_core.base.bare import Bare from reflex_components_core.core.cond import Cond from reflex_components_core.core.match import Match @@ -203,6 +205,21 @@ def _should_memoize(component: Component) -> bool: return bool(component.event_triggers) +@functools.cache +def _memoize_only_hooks() -> CompilerHooks: + """Return a hook chain that runs auto-memoization and nothing else. + + Used to walk a structural snapshot child's subtree: it must keep memoizing + so descendants get their own modules, but no page-level collector may see + it -- the subtree is compiled into the snapshot's own memo body. The plugin + holds no state, so one chain is shared across compiles. + + Returns: + A single-plugin hook chain. + """ + return CompilerHooks(plugins=(MemoizeStatefulPlugin(),)) + + @dataclasses.dataclass(frozen=True, slots=True) class MemoizeStatefulPlugin(Plugin): """Auto-memoize stateful components with experimental-memo wrappers. @@ -212,18 +229,22 @@ class MemoizeStatefulPlugin(Plugin): wrappers (see ``get_memoization_strategy``): - Snapshot wrappers (``MemoizationLeaf``-style boundaries and structural - ``Foreach`` wrappers): wrapped in ``enter_component`` - and returned with empty structural children. The walker skips descent, so - hooks attached to the captured body are compiled into the memo body only. + ``Foreach`` wrappers): wrapped in ``enter_component`` and returned with + empty structural children, so hooks attached to the captured body are + compiled into the memo body only. - Passthrough wrappers are wrapped in ``leave_component`` after descendants have already compiled, so any inner memo wrappers flow into this wrapper's children. - Descendants of a snapshot boundary are never independently memoized; the + Descendants of a snapshot *boundary* are never independently memoized; the boundary owns the wrapping decision for its whole subtree. This is tracked via ``PageContext.memoize_suppressor_stack`` — a stack of component ids that pushed suppression, popped in ``leave_component`` when the matching component leaves. + + A structural snapshot child is the one case in between: its subtree is user + content, so it keeps memoizing, but under a memoize-only hook chain that no + page-level collector sees (``_memoize_structural_child``). """ def enter_component( @@ -235,7 +256,7 @@ def enter_component( compile_context: Any, in_prop_tree: bool = False, ) -> BaseComponent | ComponentAndChildren | None: - """Memoize snapshot-boundary subtrees before descent. + """Memoize snapshot subtrees before descent. Snapshot boundaries (``MemoizationLeaf``-style, see ``is_snapshot_boundary``) stash state-referencing hooks inside @@ -247,7 +268,10 @@ def enter_component( entirely — the boundary's full snapshot lives only in the memo component definition compiled separately. - Non-boundary components are handled in ``leave_component`` so their + Structural snapshot children (``Foreach``) seal the same way, but their + subtree is memoized on the way in rather than skipped. + + Everything else is handled in ``leave_component`` so its already-compiled children flow into the wrapper. Args: @@ -266,16 +290,20 @@ def enter_component( return None if page_context.memoize_suppressor_stack: return None - strategy = get_memoization_strategy(comp) - if strategy is not MemoizationStrategy.SNAPSHOT: - return None - snapshot_boundary = is_snapshot_boundary(comp) + if not is_snapshot_boundary(comp): + if get_memoization_strategy(comp) is not MemoizationStrategy.SNAPSHOT: + return None + # A structural snapshot child (``Foreach``) also renders its whole + # subtree into its own memo body, but unlike a boundary that + # subtree is user content that must keep memoizing: a descendant + # needs its own module so its hooks land below the per-item scope + # the loop provides. Memoize it here, sealed from the page walk. + return self._memoize_structural_child(comp, page_context, compile_context) if not _should_memoize(comp): # Boundary not worth wrapping — still suppress descendants so # they don't memoize independently of the boundary's subtree. - if snapshot_boundary: - page_context.memoize_suppressor_stack.append(id(comp)) + page_context.memoize_suppressor_stack.append(id(comp)) return None wrapper = self._build_wrapper( @@ -303,7 +331,7 @@ def leave_component( compile_context: Any, in_prop_tree: bool = False, ) -> BaseComponent | ComponentAndChildren | None: - """Wrap non-boundary memoizables and pop any suppression this component pushed. + """Wrap memoizables handled after descent, and pop this component's suppression. Args: comp: The component being visited. @@ -337,8 +365,8 @@ def leave_component( comp = page_context.own(comp) comp.children = list(children) - strategy = get_memoization_strategy(comp) - if strategy is MemoizationStrategy.SNAPSHOT: + if is_snapshot_boundary(comp): + # Already handled (and sealed) in ``enter_component``. return None if not _should_memoize(comp): @@ -356,6 +384,101 @@ def leave_component( return self._build_wrapper(comp, page_context, compile_context) + def _memoize_structural_child( + self, + comp: Component, + page_context: PageContext, + compile_context: Any, + ) -> ComponentAndChildren | None: + """Memoize a structural snapshot child's subtree without exposing it. + + The item is first rendered through its own memo component (see + :meth:`_isolate_loop_item`), whose body was walked with this plugin + alone, so descendants still get their own memo modules while the page + collector never sees them: their hooks, imports, refs and custom code + belong to the memo body that renders them. + + Args: + comp: The structural snapshot child. + page_context: The active page context. + compile_context: The active compile context. + + Returns: + A ``(wrapper, ())`` replacement, or ``None`` if not worth wrapping. + """ + comp = self._isolate_loop_item(comp, page_context, compile_context) + # The loop stays inline when it is not wrapped itself; the walker + # applies the returned children, so the item component must be kept. + inline = (comp, tuple(comp.children)) + if not _should_memoize(comp): + return inline + + wrapper = self._build_wrapper(comp, page_context, compile_context) + if wrapper is None: + return inline + # Var-declared app wraps still have to reach the page registry; the + # collector that normally surfaces them never walks this subtree. + collect_var_app_wraps_in_subtree(page_context.app_wrap_components, comp) + return (wrapper, ()) + + @staticmethod + def _isolate_loop_item( + comp: Component, + page_context: PageContext, + compile_context: Any, + ) -> Component: + """Render a loop's item through a memo component of its own. + + The loop publishes the item and index around each rendered item with + ``ScopedValues``, but a hook only reads that scope if it runs *below* + the provider. Hooks float to the top of whichever component they land + in, so an item body rendered inline in the ``.map`` callback has its + ``useScopedValue`` and ``useClientState`` hoisted above the loop, where + every item resolves one shared slot. Wrapping the item in its own + component fixes the placement wherever the walk reaches the loop -- a + page or another loop's item -- and keeps the item's subtree memoizing + recursively, since the sealed walk runs over the body first. An + explicit ``@rx.memo`` body is never walked, so a loop inside one keeps + its item inline; there, an ``@rx.memo`` item function is the way to + give each item its own scope. + + Args: + comp: The loop component. + page_context: The active page context. + compile_context: The active compile context. + + Returns: + A page-local clone whose only child is the item's memo component, + or ``comp`` itself when it rendered no item. + """ + if not comp.children: + return comp + (item,) = comp.children + item = _memoize_only_hooks().compile_component( + item, + page_context=page_context, + compile_context=compile_context, + ) + if not isinstance(item, Component): + return comp + item = fix_event_triggers_for_memo(item, page_context) + wrapper_factory, definition = create_passthrough_component_memo( + item, source_module=page_context.source_module, snapshot=True + ) + tag = definition.export_name + compile_context.memoize_wrappers[tag] = None + compile_context.auto_memo_components[tag, definition.source_module] = definition + wrapper = wrapper_factory() + # ``Foreach.render`` lifts the item's key onto the provider it yields, + # so the stand-in has to carry it; refs stay reachable for form + # handlers walking the loop, as with any other wrapper. + wrapper.key = item.key + object.__setattr__(wrapper, "_get_all_refs", item._get_all_refs) + object.__setattr__(wrapper, "_form_control_source", item) + comp = page_context.own(comp) + comp.children = [wrapper] + return comp + @staticmethod def _build_wrapper( comp: Component, @@ -397,6 +520,10 @@ def _build_wrapper( compile_context.auto_memo_components[tag, definition.source_module] = definition wrapper = wrapper_factory() + # The wrapper takes the wrapped component's place in the tree, so it has + # to take its key too: the key belongs to the element the parent renders, + # and a key left behind on the memo body does nothing. + wrapper.key = comp.key # The wrapper has no structural children at the page level, but parents # walking ``_get_all_refs`` (e.g. ``Form._get_form_refs`` collecting # ref_ mappings into ``handleSubmit``) need to see refs from the diff --git a/tests/integration/tests_playwright/test_memo.py b/tests/integration/tests_playwright/test_memo.py index 4af2d3844df..5fb0dc01c14 100644 --- a/tests/integration/tests_playwright/test_memo.py +++ b/tests/integration/tests_playwright/test_memo.py @@ -31,6 +31,8 @@ class MemoState(rx.State): last_value: str = "" numbers: rx.Field[list[int]] = rx.field(default_factory=lambda: [1, 2, 3, 4]) order: list[str] = ["row-a", "row-b", "row-c"] + grid: list[list[str]] = [["a0", "a1"], ["b0", "b1"]] + mutable: list[str] = ["a", "b", "c"] tree: TreeNode = TreeNode( name="root", children=[ @@ -57,6 +59,14 @@ def replace_tree(self): def reverse_order(self): self.order = list(reversed(self.order)) + @rx.event + def replace_mutable(self): + self.mutable = ["d", "e", "f"] + + @rx.event + def record_submit(self, item: str, position: int): + self.last_value = f"{item}@{position}" + @rx.event def replace_numbers(self): """Replace the source array for the frontend calculation.""" @@ -104,6 +114,97 @@ def unwrapped_label(value: rx.Var[str]) -> rx.Component: # component that must still render and follow its prop. return rx.text(value, id="unwrapped-label") + def scoped_row(item: rx.Var[str], position: rx.Var[int]) -> rx.Component: + # No ``rx.memo``: an inline foreach body, which is where loop vars used + # to fall out of scope. Every consumer here compiles into its own + # module -- the submit handler into a ``useCallback``, the client state + # read into its own memo -- so each one only works if it can reach the + # loop item from the scope the loop provides around the item. + opened = rx.client_state(False, prefix="opened") + # Seeded from a loop var: the default is a cast Var whose declaration + # has to travel into every module that reads the slot. + count = rx.client_state(position, prefix="seeded") + return rx.hstack( + rx.form( + rx.el.button("submit", type="submit"), + on_submit=lambda _form_data: MemoState.record_submit(item, position), + id=f"scoped-form-{position}", + ), + rx.el.button( + "toggle", + id=f"scoped-toggle-{position}", + on_click=opened.set(~opened.value), + ), + rx.text( + rx.cond(opened.value, f"open:{item}", f"closed:{item}"), + id=f"scoped-status-{position}", + ), + rx.text(count.value, id=f"scoped-count-{position}"), + rx.el.button( + "bump", + id=f"scoped-bump-{position}", + on_click=count.set(lambda prev: prev + 1), + ), + ) + + def nested_grid() -> rx.Component: + # Distinct names, so the leaf reads the outer item by walking out past + # the inner loop's provider. + return rx.box( + rx.foreach( + MemoState.grid, + lambda row: rx.foreach( + row, + lambda cell: rx.text(f"{row[0]}/{cell}", class_name="nested-cell"), + ), + ), + id="nested-grid", + ) + + def mutable_row(item: rx.Var[str], index: rx.Var[int]) -> rx.Component: + # Three things that could go stale when the list content changes, with + # the loop keyed by position (the default -- no `key=` here). + seeded = rx.client_state(item, prefix="seeded") + fixed = rx.client_state("untouched", prefix="fixed") + return rx.hstack( + rx.text(item, id=f"mut-item-{index}"), + rx.text(index, id=f"mut-index-{index}"), + rx.text(seeded.value, id=f"mut-seeded-{index}"), + rx.text(fixed.value, id=f"mut-fixed-{index}"), + rx.el.button( + "mark", + id=f"mut-mark-{index}", + on_click=fixed.set("marked"), + ), + ) + + def mutable_list() -> rx.Component: + return rx.box(rx.foreach(MemoState.mutable, mutable_row), id="mutable-list") + + def identity_keyed_row(item: rx.Var[str], index: rx.Var[int]) -> rx.Component: + seeded = rx.client_state(item, prefix="keyedseed") + return rx.text(seeded.value, id=f"keyed-seeded-{index}", key=item) + + def keyed_list() -> rx.Component: + return rx.box( + rx.foreach(MemoState.mutable, identity_keyed_row), id="keyed-list" + ) + + def shadowed_grid() -> rx.Component: + # Both loops name their arg the same. Python shadows the outer binding + # inside the inner lambda, and the compiled output has to shadow it the + # same way: each level renders its own value. + return rx.box( + rx.foreach( + MemoState.grid, + lambda v: rx.box( + rx.text(v[0], class_name="shadowed-head"), + rx.foreach(v, lambda v: rx.text(v, class_name="shadowed-cell")), + ), + ), + id="shadowed-grid", + ) + @rx.memo def memo_row( item: rx.Var[str], @@ -143,8 +244,9 @@ def memo_rows( ) -> rx.Component: """Loop over the order inside an explicitly memoized container. - Per-item client state inside the loop lives in an ``@rx.memo`` item - function, whose instances each own their state. + An explicit memo body compiles as one component and is not walked, so + a loop inside it keeps its item inline. Per-item client state in that + position needs an ``@rx.memo`` item function, which owns its state. Args: label: A prefix every row renders. @@ -213,7 +315,18 @@ def index() -> rx.Component: rx.text(MemoState.last_value, id="framed-child"), title=MemoState.last_value, ), + rx.box( + rx.foreach(MemoState.order, scoped_row), + id="scoped-rows", + ), memo_rows(label=MemoState.last_value, on_pick=MemoState.set_last_value), + nested_grid(), + shadowed_grid(), + rx.el.button( + "replace", id="replace-mutable", on_click=MemoState.replace_mutable + ), + mutable_list(), + keyed_list(), ) app = rx.App() @@ -416,14 +529,63 @@ def test_memo_wrapper_none_renders_and_updates( expect(page.locator("#unwrapped-label")).to_have_text("unwrapped_update") +def test_foreach_item_handler_receives_its_own_loop_vars( + memo_app: AppHarness, page: Page +) -> None: + """A submit handler inside an inline foreach body sees its item and index. + + Regression for reflex-dev/reflex#3210: the handler compiles into a + ``useCallback`` that the compiler lifts out of the ``.map`` body, so the + loop vars it referenced were not in scope and the page threw + ``ReferenceError``. Submitting each row must report that row's own values. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + for position, item in enumerate(("row-a", "row-b", "row-c")): + page.locator(f"#scoped-form-{position} button").click() + expect(page.locator("#memo-last-value")).to_have_text(f"{item}@{position}") + + +def test_foreach_item_client_state_is_per_item( + memo_app: AppHarness, page: Page +) -> None: + """An unnamed client state var in an inline foreach body is per item. + + The var is constructed once at compile time, so all three rows resolve the + same generated name -- against the scope the loop opens around each item, + which is what makes them independent. The rendered text also interpolates + the loop item, so this covers the item reaching a memoized reader. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + expect(page.locator("#scoped-status-0")).to_have_text("closed:row-a") + expect(page.locator("#scoped-status-1")).to_have_text("closed:row-b") + + page.locator("#scoped-toggle-1").click() + + expect(page.locator("#scoped-status-1")).to_have_text("open:row-b") + # The other rows are untouched: each item owns its own slot. + expect(page.locator("#scoped-status-0")).to_have_text("closed:row-a") + expect(page.locator("#scoped-status-2")).to_have_text("closed:row-c") + + def test_foreach_inside_an_explicit_memo_keeps_client_state_per_item( memo_app: AppHarness, page: Page ) -> None: - """An ``@rx.memo`` item function scopes client state per loop item. + """An ``@rx.memo`` item function scopes client state per item in a memo. - Each rendered row is its own memo instance, so its unnamed client state is - its own. The rows also use the outer memo's parameters, passed down as - ordinary props. + The memo body compiles as one component that is never walked, so a loop + inside it renders its item inline; an ``@rx.memo`` item function is the + supported way to give each row its own state there. The rows also use the + outer memo's parameters, passed down as ordinary props. Args: memo_app: Running app harness. @@ -450,6 +612,163 @@ def test_foreach_inside_an_explicit_memo_keeps_client_state_per_item( expect(page.locator("#memo-status-1")).to_have_text("row-c:open:row-b") +def test_foreach_item_client_state_seeded_from_the_loop_index( + memo_app: AppHarness, page: Page +) -> None: + """A client state var defaulting to a loop var is seeded per item. + + The default is a cast ``Var`` whose ``useScopedValue`` declaration lives on + the var it wraps; dropping it compiled every consumer to + ``useClientState(, …)`` with nothing declaring ````, so each row + seeded from ``undefined``. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + for position in range(3): + expect(page.locator(f"#scoped-count-{position}")).to_have_text(str(position)) + + page.locator("#scoped-bump-1").click() + + expect(page.locator("#scoped-count-1")).to_have_text("2") + # Seeded per item, and independent of each other. + expect(page.locator("#scoped-count-0")).to_have_text("0") + expect(page.locator("#scoped-count-2")).to_have_text("2") + + +def test_nested_foreach_leaf_reads_both_loop_scopes( + memo_app: AppHarness, page: Page +) -> None: + """A leaf in a nested loop reaches the outer item by walking outward. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + cells = page.locator("#nested-grid .nested-cell") + expect(cells).to_have_count(4) + expect(cells).to_have_text(["a0/a0", "a0/a1", "b0/b0", "b0/b1"]) + + +def test_nested_foreach_with_shadowed_names_renders_each_level( + memo_app: AppHarness, page: Page +) -> None: + """Nested loops reusing one parameter name each render their own values. + + Reusing the name is only expressible in Python by shadowing the outer + binding, and the scope chain has to resolve it the same way: a read inside + the inner loop binds to the inner provider, and the outer row head -- which + sits above that provider -- keeps binding to the outer one. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + expect(page.locator("#shadowed-grid .shadowed-head")).to_have_text(["a0", "b0"]) + expect(page.locator("#shadowed-grid .shadowed-cell")).to_have_text([ + "a0", + "a1", + "b0", + "b1", + ]) + + +def test_mutating_a_list_updates_the_rendered_loop_values( + memo_app: AppHarness, page: Page +) -> None: + """Replacing a list's contents updates what each row renders. + + The loop keys by position here (no ``key=``), so React re-renders the + existing rows rather than mounting new ones. The item and index still have + to follow the data: they resolve through the scope the loop provides on + every render, not from anything captured at mount. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + for index, item in enumerate(("a", "b", "c")): + expect(page.locator(f"#mut-item-{index}")).to_have_text(item) + expect(page.locator(f"#mut-index-{index}")).to_have_text(str(index)) + + page.click("#replace-mutable") + + for index, item in enumerate(("d", "e", "f")): + expect(page.locator(f"#mut-item-{index}")).to_have_text(item) + expect(page.locator(f"#mut-index-{index}")).to_have_text(str(index)) + + +def test_positionally_keyed_row_keeps_its_client_state_across_a_list_change( + memo_app: AppHarness, page: Page +) -> None: + """A row's client state belongs to the position when the loop keys by position. + + This is ``useState`` semantics: the default seeds the slot when the row + first claims it and is not re-read afterwards, and a positional key means + the row never unmounts, so nothing re-claims. Both the state a row was + given (``fixed``) and a state seeded from the item (``seeded``) therefore + survive the list being replaced. + + Asserted rather than assumed, because the alternative -- re-seeding when the + default changes -- would silently discard whatever the user had put in the + row. ``key=`` is the way to tie state to the item instead; the next test + covers that. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + for index, item in enumerate(("a", "b", "c")): + expect(page.locator(f"#mut-seeded-{index}")).to_have_text(item) + expect(page.locator(f"#mut-fixed-{index}")).to_have_text("untouched") + + page.click("#mut-mark-1") + expect(page.locator("#mut-fixed-1")).to_have_text("marked") + + page.click("#replace-mutable") + + # The rows now show d/e/f (previous test), but their state stayed put. + expect(page.locator("#mut-item-1")).to_have_text("e") + expect(page.locator("#mut-fixed-1")).to_have_text("marked") + for index, seed in enumerate(("a", "b", "c")): + expect(page.locator(f"#mut-seeded-{index}")).to_have_text(seed) + + +def test_identity_keyed_row_reseeds_its_client_state_from_the_new_item( + memo_app: AppHarness, page: Page +) -> None: + """``key=`` on the item ties a row's state to the item, not the position. + + Keying by identity unmounts the old rows when the list is replaced, which + releases their scopes, so the new rows claim fresh slots and a state seeded + from the item reflects the new item. + + Args: + memo_app: Running app harness. + page: Playwright page. + """ + _load_page(page, memo_app) + + for index, item in enumerate(("a", "b", "c")): + expect(page.locator(f"#keyed-seeded-{index}")).to_have_text(item) + + page.click("#replace-mutable") + + for index, item in enumerate(("d", "e", "f")): + expect(page.locator(f"#keyed-seeded-{index}")).to_have_text(item) + + def test_memo_derived_array_updates(memo_app: AppHarness, page: Page) -> None: """Derived values stay current after unrelated writes and array replacement. diff --git a/tests/js/client_state.test.js b/tests/js/client_state.test.js index eb539729295..9bfde6dde47 100644 --- a/tests/js/client_state.test.js +++ b/tests/js/client_state.test.js @@ -16,11 +16,13 @@ import { CLIENT_STATE_REF, ClientStateProvider, ClientStateScope, + ScopedValues, createClientStateStore, getClientState, getClientStore, setClientState, useClientState, + useScopedValue, withClientStateScope, } from "$/utils/client_state"; @@ -502,6 +504,171 @@ describe("withClientStateScope", () => { }); }); +describe("scoped values", () => { + /** Render a component that reads one scoped value by name. */ + const reader = (name) => { + const seen = []; + const Reader = () => { + seen.push(useScopedValue(name)); + return null; + }; + return { seen, element: createElement(Reader) }; + }; + + test("a descendant component reads a value it never received as a prop", () => { + // The shape a loop emits: the value lives in context, so a descendant that + // compiled into its own component can still see it. + const item = reader("item0"); + const { unmount } = mount( + createElement(ScopedValues, { values: { item0: "a" } }, item.element), + ); + + expect(item.seen.at(-1)).toBe("a"); + + unmount(); + }); + + test("a nested provider still exposes the outer values", () => { + const outer = reader("outer0"); + const inner = reader("inner0"); + const { unmount } = mount( + createElement( + ScopedValues, + { values: { outer0: "out" } }, + createElement( + ScopedValues, + { values: { inner0: "in" } }, + outer.element, + inner.element, + ), + ), + ); + + expect(outer.seen.at(-1)).toBe("out"); + expect(inner.seen.at(-1)).toBe("in"); + + unmount(); + }); + + test("a nearer provider shadows the same name", () => { + const item = reader("item0"); + const { unmount } = mount( + createElement( + ScopedValues, + { values: { item0: "outer" } }, + createElement( + ScopedValues, + { values: { item0: "inner" } }, + item.element, + ), + ), + ); + + expect(item.seen.at(-1)).toBe("inner"); + + unmount(); + }); + + test("an unprovided name reads undefined rather than throwing", () => { + const item = reader("missing"); + const { unmount } = mount( + createElement(ScopedValues, { values: {} }, item.element), + ); + + expect(item.seen.at(-1)).toBeUndefined(); + + unmount(); + }); + + test("reading outside any provider is undefined", () => { + const item = reader("orphan"); + const { unmount } = mount(item.element); + + expect(item.seen.at(-1)).toBeUndefined(); + + unmount(); + }); + + test("a re-render with new values is seen by descendants", () => { + // A loop re-renders with a new item on every list change, so the provided + // value must not be frozen at first render. + const item = reader("item0"); + const container = document.createElement("div"); + document.body.appendChild(container); + const root = createRoot(container); + const render = (value) => + act(() => { + root.render( + createElement( + ScopedValues, + { values: { item0: value } }, + item.element, + ), + ); + }); + + render("first"); + expect(item.seen.at(-1)).toBe("first"); + + render("second"); + expect(item.seen.at(-1)).toBe("second"); + + act(() => root.unmount()); + container.remove(); + }); + + test("each provided subtree owns its unnamed client state", () => { + // One rendered item is one component instance, so an unnamed var used in a + // loop body must not be shared between items. + const stateProbe = () => { + const renders = { value: undefined, set: undefined }; + const Probe = () => { + const [value, set] = useClientState("", "cs0"); + renders.value = value; + renders.set = set; + return null; + }; + return { renders, element: createElement(Probe) }; + }; + const first = stateProbe(); + const second = stateProbe(); + const { unmount } = mount( + createElement( + ClientStateProvider, + null, + createElement(ScopedValues, { values: { item0: "a" } }, first.element), + createElement(ScopedValues, { values: { item0: "b" } }, second.element), + ), + ); + + act(() => first.renders.set("typed into the first")); + + expect(first.renders.value).toBe("typed into the first"); + expect(second.renders.value).toBe(""); + + unmount(); + }); + + test("sibling providers give each subtree its own value", () => { + // One loop, two items: each item's subtree sees only its own value. + const first = reader("item0"); + const second = reader("item0"); + const { unmount } = mount( + createElement( + "div", + null, + createElement(ScopedValues, { values: { item0: "a" } }, first.element), + createElement(ScopedValues, { values: { item0: "b" } }, second.element), + ), + ); + + expect(first.seen.at(-1)).toBe("a"); + expect(second.seen.at(-1)).toBe("b"); + + unmount(); + }); +}); + describe("slot lifetime", () => { test("a consumer unsubscribes from its slot when it unmounts", () => { // Root-scope slots live for the page, so a listener that outlived its @@ -533,6 +700,90 @@ describe("slot lifetime", () => { expect(live).toBe(0); slot.subscribe = realSubscribe; }); + + test("per-item state is claimed in the item's scope, never at the root", () => { + // Otherwise a loop would append a root entry per rendered item and the + // store would grow without bound as the list churned. + const root = getClientStore().root; + const before = root.owned.size; + const rows = (list) => + createElement( + ClientStateProvider, + null, + ...list.map((item) => + createElement( + ScopedValues, + { key: item, values: { item0: item } }, + createElement(ItemState), + ), + ), + ); + const { rerender, unmount } = mount(rows(["a", "b", "c"])); + + expect(root.owned.size).toBe(before); + + rerender(rows(["d", "e", "f"])); + expect(root.owned.size).toBe(before); + + unmount(); + expect(root.owned.size).toBe(before); + }); + + test("an item's state is released once that item unmounts", () => { + // Keying by identity means a changed list unmounts the old rows, so their + // scopes -- and the slots those scopes own -- become unreachable. A new row + // reaching the same name has to start from the default, not inherit. + const rows = (list) => + createElement( + ClientStateProvider, + null, + ...list.map((item) => + createElement( + ScopedValues, + { key: item, values: { item0: item } }, + createElement(ItemState), + ), + ), + ); + const { rerender, unmount } = mount(rows(["a", "b", "c"])); + + act(() => lastItemSetter("written")); + expect(itemValues.at(-1)).toBe("written"); + + itemValues.length = 0; + rerender(rows(["d", "e", "f"])); + + expect(itemValues).toEqual(["default", "default", "default"]); + + unmount(); + }); + + test("under positional keys an item keeps its state across a list change", () => { + // The counterpart, and the reason `key=` matters now that a loop item can + // hold state: index keys reuse the component, so nothing unmounts and the + // state stays with the position rather than the item. + const rows = (list) => + createElement( + ClientStateProvider, + null, + ...list.map((item, index) => + createElement( + ScopedValues, + { key: index, values: { item0: item } }, + createElement(ItemState), + ), + ), + ); + const { rerender, unmount } = mount(rows(["a", "b", "c"])); + + act(() => lastItemSetter("written")); + itemValues.length = 0; + rerender(rows(["d", "e", "f"])); + + expect(itemValues).toEqual(["default", "default", "written"]); + + unmount(); + }); }); test("CLIENT_STATE_REF matches the Python constant", () => { diff --git a/tests/units/compiler/test_memoize_plugin.py b/tests/units/compiler/test_memoize_plugin.py index 3076349e3eb..10a4185cf5d 100644 --- a/tests/units/compiler/test_memoize_plugin.py +++ b/tests/units/compiler/test_memoize_plugin.py @@ -50,6 +50,7 @@ from reflex_components_core.el.elements.typography import Hr from reflex_components_moment.moment import Moment from reflex_components_radix.themes.layout.box import Box +from reflex_components_radix.themes.typography.text import Text import reflex as rx import reflex.compiler.plugins.memoize as memoize_plugin @@ -409,6 +410,20 @@ def special_child() -> Component: assert body_marker not in page_output +def _loop_item_export(loop_code: str) -> str: + """Pick the component a compiled loop mounts inside each item's provider. + + Args: + loop_code: A module (or body) containing the rendered ``.map``. + + Returns: + The export name of the item component. + """ + match = re.search(r"jsx\(ScopedValues,\{key:.*?\}\},jsx\((\w+),", loop_code) + assert match is not None, loop_code + return match.group(1) + + def test_foreach_snapshot_memo_applies_component_styles() -> None: """Foreach memo bodies must render styled children after preview rendering. @@ -442,10 +457,15 @@ def accordion() -> Component: foreach_code = next( code for path, code in memo_files if "/Foreach" in path or "\\Foreach" in path ) + # The loop mounts each item through its own component, which is where + # the item's subtree (and its styles) now renders. + item_code = next( + code for path, code in memo_files if _loop_item_export(foreach_code) in path + ) - assert '["width"] : "400px"' in foreach_code - assert '["borderRadius"] : "var(--radius-4)"' in foreach_code - assert '["justifyContent"] : "space-between"' in foreach_code + assert '["width"] : "400px"' in item_code + assert '["borderRadius"] : "var(--radius-4)"' in item_code + assert '["justifyContent"] : "space-between"' in item_code def test_foreach_parent_does_not_absorb_sibling_into_snapshot() -> None: @@ -456,6 +476,10 @@ def test_foreach_parent_does_not_absorb_sibling_into_snapshot() -> None: reactive content into the same wide memo body. The parent should now render on the page side, with Foreach and any reactive sibling each getting their own independent wrapper. + + The Foreach snapshot is not opaque: the walker descends into the item body, + so the item's own loop-var consumer gets a third, independent wrapper, and + the item itself renders through a fourth, its own component. """ ctx, _page_ctx = _compile_single_page( lambda: rx.box( @@ -474,8 +498,9 @@ def test_foreach_parent_does_not_absorb_sibling_into_snapshot() -> None: ] wrapped_types = {type(definition.component) for definition in wrapped_definitions} - assert len(wrapped_definitions) == 2 + assert len(wrapped_definitions) == 4 assert Box not in wrapped_types + assert Text in wrapped_types foreach_definition = next( definition @@ -487,16 +512,25 @@ def test_foreach_parent_does_not_absorb_sibling_into_snapshot() -> None: is MemoizationStrategy.SNAPSHOT ) - bare_definition = next( + bare_definitions = [ definition for definition in wrapped_definitions if isinstance(definition.component, Bare) - ) - assert ( - get_memoization_strategy(bare_definition.component) + ] + assert len(bare_definitions) == 2 + assert all( + get_memoization_strategy(definition.component) is MemoizationStrategy.PASSTHROUGH + for definition in bare_definitions ) - assert bare_definition is not foreach_definition + # One reads app state on the page side, the other reads the loop item from + # the scope the Foreach provides around each rendered item. + bare_contents = { + str(cast(Bare, definition.component).contents) + for definition in bare_definitions + } + assert any("items_rx_state_.length" in contents for contents in bare_contents) + assert any(contents == f"item{FIELD_MARKER}" for contents in bare_contents) def test_common_memoization_snapshot_helper_classifies_snapshot_cases() -> None: @@ -733,6 +767,75 @@ def slot_card(label: rx.Var[str], children: rx.Var[Component]) -> Component: assert "static child" not in wrapper_code +def test_user_memo_inside_foreach_reads_its_loop_var_from_the_scope() -> None: + """A user memo with a loop-var prop is wrapped, and the wrapper reads the scope. + + Foreach owns the snapshot for the item subtree, so the wrapper module lands + beside it rather than on the page, and it resolves ``item`` through + ``useScopedValue`` instead of closing over the callback parameter. + """ + from reflex.compiler.compiler import compile_memo_components + + @rx.memo + def row(label: rx.Var[str]) -> Component: + return WithProp.create(label=label) + + ctx, page_ctx = _compile_single_page( + lambda: rx.box( + rx.foreach(SpecialFormMemoState.items, lambda item: row(label=item)) + ) + ) + + definitions = list(ctx.auto_memo_components.values()) + (foreach_definition,) = ( + definition + for definition in definitions + if isinstance(definition.component, Foreach) + ) + # The user memo instance is wrapped twice over: a passthrough wrapper + # reads the loop var for it, and the item component renders that wrapper + # below the per-item provider. + (wrapper_definition, item_definition) = ( + definition + for definition in definitions + if isinstance(definition.component, MemoComponent) + ) + assert isinstance(foreach_definition, MemoComponentDefinition) + assert isinstance(wrapper_definition, MemoComponentDefinition) + assert isinstance(item_definition, MemoComponentDefinition) + assert wrapper_definition.auto_memo_wrapper + assert item_definition.component is not wrapper_definition.component + + memo_files, _memo_imports = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + code_by_export = { + definition.export_name: next( + code for path, code in memo_files if definition.export_name in path + ) + for definition in (foreach_definition, wrapper_definition, item_definition) + } + + wrapper_code = code_by_export[wrapper_definition.export_name] + assert 'useScopedValue("item_rx_state_")' in wrapper_code + # The wrapper is transparent to injected props, so the loop var reaches the + # user memo through the ``mergeSlotProps`` merge rather than a bare literal. + assert re.search( + rf"jsx\({row(label='x').tag},\{{\.\.\.mergeSlotProps\(rest, \(\{{[^)]*label:item_rx_state_", + wrapper_code, + ), wrapper_code + + # The foreach body mounts the item component inside the per-item provider, + # which mounts the wrapper; the page module is left with neither the loop + # var nor the item subtree. + foreach_code = code_by_export[foreach_definition.export_name] + assert _loop_item_export(foreach_code) == item_definition.export_name + item_code = code_by_export[item_definition.export_name] + assert f"jsx({wrapper_definition.export_name}," in item_code + assert f"jsx({wrapper_definition.export_name}," not in foreach_code + assert not any("useScopedValue" in hook for hook in page_ctx.hooks) + + def test_passthrough_memo_skips_hole_for_childless_component() -> None: """Childless components own their JSX output, so the wrapper must not inject a ``{children}`` hole. @@ -2815,6 +2918,135 @@ def page() -> Component: ) +def test_foreach_item_event_handler_reaches_the_loop_index() -> None: + """A hoisted item handler reads the loop index from the item's scope. + + Regression for reflex-dev/reflex#3210: the ``on_submit`` callback of a form + rendered inside an ``rx.foreach`` compiles into a ``useCallback`` that the + compiler lifts out of the ``.map`` body, so the callback parameter the loop + index used to render as was not in scope and the page threw + ``ReferenceError: index is not defined``. The index now renders as a + ``useScopedValue`` read inside the handler's own memo module. + """ + from reflex.compiler.compiler import compile_memo_components + + ctx, page_ctx = _compile_single_page( + lambda: rx.vstack( + rx.foreach( + Var.range(3), + lambda index: rx.form( + rx.input(name="input"), + on_submit=lambda form_data: SpecialFormMemoState.record( + index, form_data + ), + ), + ) + ) + ) + + files, _imports = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + code = "\n".join(memo_code for _path, memo_code in files) + + handler = next( + block for block in code.split("export const ") if "handleSubmit" in block + ) + index_read = re.search(r'const (\w+) = useScopedValue\("(index\w*)"\)', handler) + assert index_read is not None, f"no scoped index read in the handler\n{handler}" + local, provided = index_read.groups() + # The handler sends the scoped read, not the map callback parameter. + assert f'["index"] : {local}' in handler + + # ... and the loop provides that exact name around each rendered item. + foreach_block = next( + block for block in code.split("export const ") if "Array.prototype.map" in block + ) + assert f"{provided}:{provided}" in foreach_block + # The read has to happen below the provider, which means in a module of its + # own: hooks are hoisted to the top of whichever component they land in, so + # a read in the module that *renders* the provider would sit above it. + assert handler is not foreach_block, "the handler must be its own memo module" + # Inside the loop body the callback parameter of the same name shadows any + # hoisted read, so inline uses see the real per-item value. + assert f"(({provided}," in foreach_block + + # The page itself stays free of the loop scope. + assert "useScopedValue" not in (page_ctx.output_code or "") + + +def test_memo_wrapper_carries_the_wrapped_component_key() -> None: + """An auto-memo wrapper takes the key of the component it replaces. + + The key belongs to the element the parent renders. Left on the memo body it + does nothing, so a keyed item inside a ``rx.foreach`` would silently fall + back to positional identity once its root became a wrapper. + """ + from reflex.compiler.compiler import compile_memo_components + + ctx, _page_ctx = _compile_single_page( + lambda: rx.box( + rx.foreach( + SpecialFormMemoState.items, + lambda item: rx.el.div( + Bare.create(SpecialFormMemoState.value), key=item + ), + ) + ) + ) + + files, _imports = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + code = "\n".join(memo_code for _path, memo_code in files) + + assert f"jsx(ScopedValues,{{key:item{FIELD_MARKER}," in code + + +def test_client_state_seeded_from_a_loop_var_declares_it_in_every_consumer() -> None: + """A ``Var`` client state default reaches each consumer module it seeds. + + A loop var is a cast wrapper whose hooks live on the var it wraps, so the + default used to compile into a bare identifier with nothing declaring it -- + ``useClientState(ix_rx_state_, "cs0")`` above no ``useScopedValue`` line. + """ + from reflex.compiler.compiler import compile_memo_components + + def counter(initial: Any) -> Component: + count = rx.client_state(initial, prefix="seeded") + return rx.hstack( + rx.el.button("-", on_click=count.set(lambda v: v - 1)), + Bare.create(count.value), + ) + + ctx, page_ctx = _compile_single_page( + lambda: rx.box( + rx.foreach(SpecialFormMemoState.items, lambda _x, ix: counter(ix)) + ) + ) + + files, _imports = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()), + ) + consumers = [ + block + for _path, code in files + for block in code.split("export const ") + if "useClientState(" in block + ] + assert consumers, "no memo module read the client state var" + for block in consumers: + read = re.search(r'const (\w+) = useScopedValue\("(\w+)"\)', block) + assert read is not None, f"nothing declares the seed\n{block}" + local, provided = read.groups() + assert local == provided + assert f"useClientState({local}," in block + # Declared before it is read, since hooks emit in order. + assert block.index(local) < block.index("useClientState(") + + assert "useScopedValue" not in (page_ctx.output_code or "") + + def test_passthrough_memo_forwards_ref_and_props_to_root() -> None: """Passthrough wrappers are transparent to their parent. @@ -3192,14 +3424,54 @@ def _module_named(modules: dict[str, str], python_name: str) -> str: return block +def test_loop_items_differing_only_in_descendant_hooks_get_their_own_modules() -> None: + """Item components are told apart by artifacts the JSX does not show. + + Regression: the item component captures the item's whole subtree, but its + tag hashed the root alone, so two loops whose items rendered identical + JSX with different descendant hooks shared one module and both ran the + hooks registered last. + """ + from reflex.compiler.compiler import compile_memo_components + + def hooked(effect: str) -> type[Component]: + class Hooked(rx.el.Div): + def add_hooks(self) -> list[str]: + return [f"useEffect(() => console.log('{effect}'), [])"] + + return Hooked + + first, second = hooked("first"), hooked("second") + ctx, _page_ctx = _compile_single_page( + lambda: rx.box( + rx.foreach( + SpecialFormMemoState.items, lambda _item: rx.box(first.create("x")) + ), + rx.foreach( + SpecialFormMemoState.items, lambda _item: rx.box(second.create("x")) + ), + ) + ) + files, _imports = compile_memo_components( + memos=tuple(ctx.auto_memo_components.values()) + ) + item_modules = [ + block for block in _module_by_export(files).values() if "console.log(" in block + ] + assert len(item_modules) == 2 + assert {"'first'" in block for block in item_modules} == {True, False} + + def test_memo_item_function_inside_an_explicit_memo_scopes_client_state_per_item() -> ( None ): - """An ``@rx.memo`` item function keeps client state per loop item. + """An ``@rx.memo`` item function keeps client state per item inside a memo. - Each rendered instance of the item memo owns its unnamed client state, so - the slot binding lives in the item module and the loop body only mounts - the item with its props. + An explicit memo body is compiled as one component and never walked, so a + loop inside it renders its item inline and any ``useClientState`` in the + item hoists above the ``.map``, where every item shares one slot. The + supported way to scope it per item is an ``@rx.memo`` item function: the + loop then mounts an instance per item, and the instance owns its state. """ from reflex.compiler.compiler import compile_memo_components @@ -3227,7 +3499,4 @@ def rows(label: rx.Var[str]) -> Component: assert f"useClientState(item{FIELD_MARKER}," not in body assert "useClientState(itemRxMemo," in row # The loop mounts the row with its item and the memo's own parameter. - mount = re.search(rf"jsx\({row_export},\{{([^}}]*)\}}", body) - assert mount is not None, body - assert f"item:item{FIELD_MARKER}" in mount.group(1) - assert "label:labelRxMemo" in mount.group(1) + assert f"jsx({row_export},{{item:item{FIELD_MARKER},label:labelRxMemo}}" in body diff --git a/tests/units/components/core/test_foreach.py b/tests/units/components/core/test_foreach.py index a1a7f92b109..d1b37079e82 100644 --- a/tests/units/components/core/test_foreach.py +++ b/tests/units/components/core/test_foreach.py @@ -333,3 +333,79 @@ def test_optional_list(): ForEachState.optional_dict_value, lambda color: text(color[0], color[1]), ) + + +def test_foreach_wraps_each_item_in_a_scope_provider(): + """Each rendered item is wrapped in ``ScopedValues``, keyed by index. + + The provider is the element the ``.map`` yields, so it -- not the item root + -- carries the React key, and it publishes the item and index by name for + descendants that compile into their own components. + """ + component = foreach(ForEachState.colors_list, lambda color: text(color)) + rendered = str(component) + + arg_name = f"color{FIELD_MARKER}" + assert "jsx(ScopedValues,{key:index_" in rendered + assert f"values:{{{arg_name}:{arg_name},index_" in rendered + # The provider sits between the map callback and the item's subtree. + assert rendered.index("jsx(ScopedValues,") < rendered.index("RadixThemesText") + + +def test_foreach_loop_vars_read_from_the_enclosing_scope(): + """Loop vars render as a context read, not as the map callback parameter. + + Regression for reflex-dev/reflex#3210: a loop var used to compile to the + ``.map`` callback's parameter name, which went out of scope the moment + anything referencing it was hoisted into its own function -- a + ``useCallback``'d event handler, or a subtree lifted into its own memo + module. Reading by name from context works wherever the compiler puts the + consumer. + """ + tag = foreach( + ForEachState.colors_list, lambda color, index: text(color, index) + )._render() + + arg_var = tag.get_arg_var() + index_var = tag.get_index_var() + + # The hook declares the same identifier the map callback binds, so the + # parameter shadows it inside the loop body and the context read applies + # everywhere else. + for var, name in ( + (arg_var, f"color{FIELD_MARKER}"), + (index_var, f"index{FIELD_MARKER}"), + ): + assert str(var) == name + var_data = var._get_all_var_data() + assert var_data is not None + assert list(var_data.hooks) == [f'const {name} = useScopedValue("{name}")'] + assert dict(var_data.imports).keys() == {"$/utils/client_state"} + + +@pytest.mark.parametrize( + ("key", "expected"), + [ + (lambda color: color, f"color{FIELD_MARKER}"), + (lambda _color: "literal", '"literal"'), + (lambda _color: 7, "7"), + ], + ids=["var", "string", "int"], +) +def test_foreach_lifts_an_explicit_item_key_to_the_provider(key, expected): + """An explicit ``key`` on the item becomes the provider's key. + + The provider is what React reconciles the list by, so a key left on the + item root would give the list positional identity and the explicit key + would do nothing. It is rendered as a JS value, not pasted in as source: a + plain string key emitted bare would be a reference to an undefined name. + + Args: + key: Builds the key to pass, from the loop var. + expected: The JS the provider's key must render as. + """ + component = foreach( + ForEachState.colors_list, lambda color: text(color, key=key(color)) + ) + + assert f"jsx(ScopedValues,{{key:{expected}," in str(component) diff --git a/tests/units/reflex_base/components/test_memo.py b/tests/units/reflex_base/components/test_memo.py index 9d521156872..a146ef99f88 100644 --- a/tests/units/reflex_base/components/test_memo.py +++ b/tests/units/reflex_base/components/test_memo.py @@ -46,6 +46,41 @@ def test_auto_memo_evaluates_body_once(snapshot: bool, has_children: bool): assert isinstance(definition.fn(Var(_js_expr="children", _var_type=Component)), Div) +def test_forced_snapshot_tag_hashes_descendant_artifacts(): + """A forced snapshot body is identified by its whole subtree. + + ``snapshot=True`` bakes the children into the body of a component whose + own strategy is passthrough, so the tag has to cover descendants' hooks + and other artifacts that never show up in the rendered JSX; a root-only + hash would let two such bodies collapse into one module. + """ + + def hooked(effect: str) -> type[Div]: + class Hooked(Div): + def add_hooks(self) -> list[str]: + return [f"useEffect(() => console.log('{effect}'), [])"] + + return Hooked + + bodies = [Div.create(hooked(effect).create("x")) for effect in ("A", "B")] + for body in bodies: + body._memoization_mode = MemoizationMode(recursive=True) + tags = { + memo.create_passthrough_component_memo(body, snapshot=True)[1].export_name + for body in bodies + } + assert len(tags) == 2 + # Without the snapshot the children sit behind the hole, so the bodies are + # the same and the tags rightly coincide. + assert ( + len({ + memo.create_passthrough_component_memo(body)[1].export_name + for body in bodies + }) + == 1 + ) + + def test_auto_memo_snapshot_renders_lifted_rest_props(): """The retained memo body must render props lifted out of its children.""" component = Div.create(Bare.create(memo._rest_placeholder("rest"))) diff --git a/tests/units/reflex_base/test_client_state.py b/tests/units/reflex_base/test_client_state.py index 100e29d629d..b40d6e0e293 100644 --- a/tests/units/reflex_base/test_client_state.py +++ b/tests/units/reflex_base/test_client_state.py @@ -541,31 +541,23 @@ def test_var_default_is_used_directly() -> None: def test_derived_var_default_brings_its_hook_and_import() -> None: """A default whose var data is only reachable through the operation graph. - ``Var.guess_type()`` returns a cast wrapper whose own ``_var_data`` is - ``None``; the hook and import live on the var it wraps and must still be - collected, or the default compiles to a dangling identifier. + ``scoped_loop_var(...).guess_type()`` returns a cast wrapper whose own + ``_var_data`` is ``None`` -- the hook and import live on the var it wraps. + Reading the field directly dropped them, compiling the default's identifier + into a dangling reference. """ - from reflex_base.utils.imports import ImportVar - from reflex_base.vars.base import Var, VarData - - seed = Var( - _js_expr="seed", - _var_type=int, - _var_data=VarData( - hooks={"const seed = useSeed()": None}, - imports={"$/utils/seed": [ImportVar(tag="useSeed")]}, - ), - ).guess_type() - cs = client_state(seed, name="seeded") + from reflex_base.components.tags.iter_tag import scoped_loop_var + + cs = client_state(scoped_loop_var("ix_rx_state_", int), name="seeded") # The declaration comes first; ``useClientState`` reading it comes last. assert _hooks_ending_with_client_state(cs) == [ - "const seed = useSeed()", - 'const [seededRxClientState, setSeeded] = useClientState(seed, "seeded", true)', + 'const ix_rx_state_ = useScopedValue("ix_rx_state_")', + 'const [seededRxClientState, setSeeded] = useClientState(ix_rx_state_, "seeded", true)', ] var_data = cs._get_all_var_data() assert var_data is not None - assert "useSeed" in str(dict(var_data.imports)) + assert "useScopedValue" in str(dict(var_data.imports)) def test_state_var_default_brings_its_state_wiring() -> None: