-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[REF-2273] Implement .setvar special EventHandler #3163
Conversation
When an EventHandler is called with an incomplete set of args it creates a partial EventSpec. This change allows Component._create_event_chain to apply remaining args from an args_spec to an existing EventSpec to make it functional. Instead of requiring the use of `lambda` functions to pass arguments to an EventHandler, they can now be passed directly and any remaining args defined in the event trigger will be applied after those.
All State subclasses will now have a special `setvar` EventHandler which appears in the autocomplete drop down, passes static analysis, and canbe used to set State Vars in response to event triggers. Before: rx.input(value=State.a, on_change=State.set_a) After: rx.input(value=State.a, on_change=State.setvar("a")) This reduces the "magic" because `setvar` is statically defined on all State subclasses.
When using setvar with a non-existing state var, this is actually a runtime error instead of a compile time error. That may be non-ideal behavior. Maybe instead of setvar, we should expose a |
@masenf how about this? rx.input(value=State.a, on_change=State.setvar(State.a)) |
@benedikt-bartscher we considered that, but it's actually kind of ambiguous... is it setting class State(rx.State):
a: str = "b"
b: str = "foo"
...
rx.input(value=State.a, on_change=State.setvar(State.a)) Does this set |
You may be right, it is kind of ambiguous... I wanted to find a typed alternative to
I like this one. It is not that ambiguous, and "typed". |
diff --git a/reflex/state.py b/reflex/state.py
index a84ea7f8..2845d280 100644
--- a/reflex/state.py
+++ b/reflex/state.py
@@ -909,6 +909,7 @@ class BaseState(Base, ABC, extra=pydantic.Extra.allow):
if setter_name not in cls.__dict__:
event_handler = cls._create_event_handler(prop.get_setter())
cls.event_handlers[setter_name] = event_handler
+ prop._set = event_handler
setattr(cls, setter_name, event_handler)
@classmethod
diff --git a/reflex/vars.py b/reflex/vars.py
index 4a8e6b30..8268d003 100644
--- a/reflex/vars.py
+++ b/reflex/vars.py
@@ -40,6 +40,7 @@ from reflex.utils import console, format, imports, serializers, types
from reflex.utils.imports import ImportDict, ImportVar
if TYPE_CHECKING:
+ from reflex.event import EventHandler
from reflex.state import BaseState
# Set of unique variable names.
@@ -1739,6 +1740,9 @@ class BaseVar(Var):
# Extra metadata associated with the Var
_var_data: Optional[VarData] = dataclasses.field(default=None)
+ # An EventHandler for setting the Var from an event trigger
+ _set: "EventHandler" | None = dataclasses.field(default=None)
+
def __hash__(self) -> int:
"""Define a hash function for a var.
@@ -1835,6 +1839,19 @@ class BaseVar(Var):
return setter
+ @property
+ def set(self) -> "EventHandler":
+ """Get the EventHandler for setting the var.
+
+ Returns:
+ An EventHandler for setting the var.
+ """
+ if self._set is None:
+ raise ValueError(
+ f"Var {self._var_full_name} does not have a setter defined."
+ )
+ return self._set
+
@dataclasses.dataclass(init=False, eq=False)
class ComputedVar(Var, property):
diff --git a/reflex/vars.pyi b/reflex/vars.pyi
index fb2ed465..51d68b98 100644
--- a/reflex/vars.pyi
+++ b/reflex/vars.pyi
@@ -6,6 +6,7 @@ from dataclasses import dataclass
from _typeshed import Incomplete
from reflex import constants as constants
from reflex.base import Base as Base
+from reflex.event import EventHandler as EventHandler
from reflex.state import State as State
from reflex.state import BaseState as BaseState
from reflex.utils import console as console, format as format, types as types
@@ -126,10 +127,13 @@ class BaseVar(Var):
_var_is_string: bool = False
_var_full_name_needs_state_prefix: bool = False
_var_data: VarData | None = None
+ _set: EventHandler | None = None
def __hash__(self) -> int: ...
def get_default_value(self) -> Any: ...
def get_setter_name(self, include_state: bool = ...) -> str: ...
def get_setter(self) -> Callable[[BaseState, Any], None]: ...
+ @property
+ def set(self) -> EventHandler: ...
@dataclass(init=False)
class ComputedVar(Var):
diff --git a/tests/test_state.py b/tests/test_state.py
index ce62e9c6..c13f3a64 100644
--- a/tests/test_state.py
+++ b/tests/test_state.py
@@ -2883,3 +2883,29 @@ async def test_setvar(mock_app: rx.App, token: str):
# Cannot setvar with non-string
with pytest.raises(ValueError):
TestState.setvar(42, 42)
+
+
+@pytest.mark.asyncio
+async def test_base_var_set(mock_app: rx.App, token: str):
+ """Test that Var.set works correctly.
+
+ Args:
+ mock_app: An app that will be returned by `get_app()`
+ token: A token.
+ """
+ state = await mock_app.state_manager.get_state(_substate_key(token, TestState))
+
+ # Set Var in same state (with Var type casting)
+ for event in rx.event.fix_events(
+ [TestState.num1.set(42), TestState.num2.set("4.2")], token
+ ):
+ async for update in state._process(event):
+ print(update)
+ assert state.num1 == 42
+ assert state.num2 == 4.2
+
+ # Set Var in parent state
+ for event in rx.event.fix_events([GrandchildState.array.set([43])], token):
+ async for update in state._process(event):
+ print(update)
+ assert state.array == [43] /Users/masenf/code/reflex-dev/reflex/tests/test_state.py
/Users/masenf/code/reflex-dev/reflex/tests/test_state.py:2900:25 - error: Cannot access member "set" for type "int"
Member "set" is unknown (reportGeneralTypeIssues)
/Users/masenf/code/reflex-dev/reflex/tests/test_state.py:2900:49 - error: Cannot access member "set" for type "float"
Member "set" is unknown (reportGeneralTypeIssues)
/Users/masenf/code/reflex-dev/reflex/tests/test_state.py:2908:61 - error: Cannot access member "set" for type "List[float]"
Member "set" is unknown (reportGeneralTypeIssues) Ultimately, we need to make Probably will take this change as-is for now, and circle back around to the typing improvements once we have real descriptors for these fields. This week, the focus is on getting a 0.5.0 release out with radix 3.0 and some other Component API changes, so any descriptor work would have to wait a few weeks at minimum. |
Yes, I know that it won't pass pyright for base_vars, but (as you explained) this is another issue not related to your I am really looking forward to descriptor based event handlers and vars 🚀 |
|
||
# Set Var in same state (with Var type casting) | ||
for event in rx.event.fix_events( | ||
[TestState.setvar("num1", 42), TestState.setvar("num2", "4.2")], token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is the way to set the num2 to a string value of "4.2"
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
num2 is typed as a float, you can't (shouldn't) set it to a str
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh yeah, should've looked at the hidden part of the code. That's what the type casting is. Sounds good.
All State subclasses will now have a special
setvar
EventHandler which appears in the autocomplete drop down, passes static analysis, and can be used to set State Vars in response to event triggers.Before:
After:
This reduces the "magic" because
setvar
is statically defined on all State subclasses.To make this actually work nicely, this PR also enables partial application of
EventHandler
.When an
EventHandler
is called with an incomplete set of args it creates a partialEventSpec
. This change allowsComponent._create_event_chain
to apply remaining args from anargs_spec
to an existingEventSpec
to make it functional.Instead of requiring the use of
lambda
functions to pass arguments to anEventHandler
, they can now be passed directly and any remaining args defined in the event trigger will be applied after those.Deprecations: this removes the old-style args_spec which hasn't been used since 0.2.x and was slated for removal in 0.5.0.