From 141e21ba3d2cf15a3910fb92297eec675b4734e8 Mon Sep 17 00:00:00 2001 From: Dev Chiniwala Date: Thu, 29 Oct 2026 02:25:41 +0530 Subject: [PATCH] fix: capture client-state setter expressions in event scope Do not turn a value expression into a JavaScript function parameter. Optional chaining and property access are not identifiers, and shadowing the enclosing event can also read the wrong value. Bind explicit values in a zero-argument closure while leaving the no-value setter unchanged. Execute 18 generated callback cases, including 14 failing regressions on unchanged source. Cover raw and typed event arguments and direct setter bindings in the existing browser app in development and production. All 164 affected tests, 14 browser cases and nine presubmit hooks pass on the refreshed main branch. The full unit suite remains in progress. Fixes #5741. --- .../+client-state-event-expressions.bugfix.md | 1 + reflex/experimental/client_state.py | 11 +-- .../tests_playwright/test_client_state.py | 62 ++++++++++++++- tests/units/experimental/test_client_state.py | 76 ++++++++++++++++++- 4 files changed, 140 insertions(+), 10 deletions(-) create mode 100644 news/+client-state-event-expressions.bugfix.md 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.