diff --git a/news/+client-state-event-expressions.bugfix.md b/news/+client-state-event-expressions.bugfix.md new file mode 100644 index 00000000000..5980597ff64 --- /dev/null +++ b/news/+client-state-event-expressions.bugfix.md @@ -0,0 +1 @@ +Fix client state setters generating invalid JavaScript when event arguments use optional chaining, property access, or other expressions. diff --git a/reflex/experimental/client_state.py b/reflex/experimental/client_state.py index 1d0b69c1bc1..6cb2c4b2771 100644 --- a/reflex/experimental/client_state.py +++ b/reflex/experimental/client_state.py @@ -3,7 +3,6 @@ from __future__ import annotations import dataclasses -import re from collections.abc import Callable from typing import Any @@ -257,15 +256,11 @@ def set_value(self, value: Any = NoValue) -> Var: ).to(FunctionVar) if value is not NoValue: - # This is a hack to make it work like an EventSpec taking an arg value_var = LiteralVar.create(value) - value_str = str(value_var) - + # The value can be an expression in the enclosing event callback. + # Capture it rather than treating its JavaScript as a parameter name. setter = ArgsFunctionOperationBuilder.create( - # remove patterns of ["*"] from the value_str using regex - args_names=(re.sub(r"(\?\.)?\[\".*\"\]", "", value_str),) - if value_str.startswith("_") - else (), + args_names=(), return_expr=setter.call(value_var), ) diff --git a/tests/integration/tests_playwright/test_client_state.py b/tests/integration/tests_playwright/test_client_state.py index 706a95635a0..7e32093974f 100644 --- a/tests/integration/tests_playwright/test_client_state.py +++ b/tests/integration/tests_playwright/test_client_state.py @@ -10,6 +10,8 @@ def ClientStateApp(): """App sharing a global client state var between sibling components.""" + from reflex_components_radix.themes.components.button import Button + import reflex as rx pushed = rx._x.client_state(default="initial") @@ -32,6 +34,38 @@ def push_none(self): shared = rx._x.client_state(default="initial") backend_default = rx._x.client_state(default=ClientStateAppState.default_text) + raw_x = rx._x.client_state(default=0) + typed_x = rx._x.client_state(default=0) + + def raw_client_x( + event: rx.vars.ObjectVar[rx.event.JavascriptMouseEvent], + ) -> tuple[rx.Var]: + return (rx.Var(f"{event}?.clientX"),) + + def typed_client_x( + event: rx.vars.ObjectVar[rx.event.JavascriptMouseEvent], + ) -> tuple[rx.Var]: + return (event.clientX,) + + class RawXButton(Button): + on_click: rx.EventHandler[raw_client_x] + + class TypedXButton(Button): + on_click: rx.EventHandler[typed_client_x] + + def event_expressions(): + return rx.box( + RawXButton.create( + "raw", on_click=lambda x: rx.call_function(raw_x.set_value(x)), id="raw" + ), + rx.text(raw_x.value.to_string(), id="raw-value"), + TypedXButton.create( + "typed", on_click=lambda x: typed_x.set_value(x), id="typed" + ), + rx.text(typed_x.value.to_string(), id="typed-value"), + rx.input(on_change=shared.set, id="direct-input"), + rx.text(shared.value, id="direct-value"), + ) def index(): return rx.box( @@ -83,27 +117,53 @@ def push_before_mount(): app.add_page(siblings) app.add_page(backend_default_page, route="/backend-default") app.add_page(push_before_mount, route="/push-before-mount") + app.add_page(event_expressions, route="/event-expressions") @pytest.fixture(scope="module") def client_state_app( tmp_path_factory: pytest.TempPathFactory, + app_harness_env: type[AppHarness], ) -> Generator[AppHarness, None, None]: """Run the client state app under an AppHarness. Args: tmp_path_factory: Pytest fixture for creating temporary directories. + app_harness_env: Development or production harness class. Yields: The running harness. """ - with AppHarness.create( + with app_harness_env.create( root=tmp_path_factory.mktemp("client_state"), app_source=ClientStateApp, ) as harness: yield harness +def test_event_expressions_update_client_state( + client_state_app: AppHarness, page: Page +) -> None: + """Raw and typed event expressions work alongside direct setter bindings. + + Args: + client_state_app: Running app harness. + page: Playwright page. + """ + assert client_state_app.frontend_url is not None + errors = _collect_page_errors(page) + script_errors = _collect_script_errors(page) + page.goto(client_state_app.frontend_url.removesuffix("/") + "/event-expressions") + page.locator("#raw").dispatch_event("click", {"clientX": 123}) + expect(page.locator("#raw-value")).to_have_text("123") + page.locator("#typed").dispatch_event("click", {"clientX": 456}) + expect(page.locator("#typed-value")).to_have_text("456") + page.locator("#direct-input").fill("changed") + expect(page.locator("#direct-value")).to_have_text("changed") + assert errors == [] + assert script_errors == [] + + def _collect_page_errors(page: Page) -> list[str]: """Record uncaught frontend errors raised on the page. diff --git a/tests/units/experimental/test_client_state.py b/tests/units/experimental/test_client_state.py index 460bbad7dd8..0d9dba1019b 100644 --- a/tests/units/experimental/test_client_state.py +++ b/tests/units/experimental/test_client_state.py @@ -1,15 +1,89 @@ """Tests for reflex.experimental.client_state.""" +import json import re -from typing import cast +import shutil +import subprocess +from typing import Any, cast import pytest +from reflex_base.event import EventChain, LambdaEventCallback, call_function, run_script from reflex_base.vars.base import Var +from reflex_base.vars.function import FunctionVar from reflex.experimental.client_state import ClientStateVar, NoValue from reflex.state import BaseState +@pytest.mark.skipif(shutil.which("node") is None, reason="Node.js is unavailable") +@pytest.mark.parametrize("wrapper", ["direct", "call_function", "run_script"]) +@pytest.mark.parametrize( + ("expression", "expected"), + [ + ("{event}?.clientX", 42), + ("{event}.clientX", 42), + ('{event}["clientX"]', 42), + ("{event}?.target?.value", "changed"), + ("{event}?.formData?.answer", 7), + ("({event}?.clientX ?? 0) + 1", 43), + ], +) +def test_set_value_captures_event_expression(expression, expected, wrapper) -> None: + """Event expressions remain values when a client state callback runs. + + Args: + expression: JavaScript expression returned by the trigger's args spec. + expected: Value the client state setter should receive. + wrapper: Optional event wrapper around the client state setter. + """ + cs = ClientStateVar.create("captured", default=0) + + def handler(value): + """Bind the client state expression through the selected event wrapper. + + Args: + value: Expression produced by the event's args spec. + + Returns: + The callback to execute on the frontend. + """ + setter = cs.set_value(value) + if wrapper == "call_function": + return call_function(setter) + if wrapper == "run_script": + return run_script(setter.to(FunctionVar).call()) + return setter + + chain = EventChain.create( + value=cast("LambdaEventCallback[Any]", handler), + args_spec=lambda event: (Var(expression.format(event=event)),), + key="on_click", + ) + result = subprocess.run( + [ + "node", + "-e", + """ +const assert = require('node:assert/strict'); +const input = JSON.parse(require('node:fs').readFileSync(0, 'utf8')); +const values = []; +const refs = { _client_state_setCaptured: value => values.push(value) }; +const ReflexEvent = (name, payload) => ({ name, payload }); +const addEvents = events => events.forEach(event => event.payload.function()); +const callback = eval(input.callback); +callback({ clientX: 42, target: { value: 'changed' }, formData: { answer: 7 } }); +assert.deepEqual(values, [input.expected]); +""", + ], + input=json.dumps({"callback": str(Var.create(chain)), "expected": expected}), + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stdout + result.stderr + + @pytest.mark.parametrize("use_set_property", [True, False]) def test_global_setter_carries_client_state_hooks(use_set_property: bool) -> None: """A global setter Var must carry the hooks that initialize the client state.