From 69e4bbc301e25557e9a593c28eae45ae0a41b866 Mon Sep 17 00:00:00 2001 From: Elijah Ahianyo Date: Fri, 14 Jun 2024 09:48:53 -0700 Subject: [PATCH] [REF-2830] server side events and stateless components should not require not require a backend (#3475) * `rx.color_mode.icon`, `rx.color_mode.button` and `rx.color_mode.switch` should not require a backend` * remove print statement * unit tests and precommit fix * add unit tests * change logic to check if event handlers actually contain state. Also delay websocket object check in state.js so server side events can get executed for stateless apps * make sure events are not queued for server side events particularly ones that call queueEvents(clear_local_storage, clear_cookies, remove_local_storage, remove_cookies) when the app is stateless(no ws) * fix unit tests * fix broken unit tests in test_app * modify socket check in processEvent to only return if socket exists and theres any event in the queue that requires state * Apply suggestions from code review make queueEvent call async Co-authored-by: Masen Furer * await queueEventIfSocketExists * Revert "await queueEventIfSocketExists" This reverts commit 9ef8070b87f1f4f55c0176fb44ff73cf6fbaa532. --------- Co-authored-by: Masen Furer --- reflex/.templates/web/utils/state.js | 40 ++++++++++++--- reflex/app.py | 2 +- reflex/components/component.py | 28 +++++++++-- tests/components/test_component.py | 73 ++++++++++++++++++++++++++++ tests/test_app.py | 6 ++- 5 files changed, 135 insertions(+), 14 deletions(-) diff --git a/reflex/.templates/web/utils/state.js b/reflex/.templates/web/utils/state.js index 35a129337..5c9634d08 100644 --- a/reflex/.templates/web/utils/state.js +++ b/reflex/.templates/web/utils/state.js @@ -107,6 +107,18 @@ export const getBackendURL = (url_str) => { return endpoint; }; +/** + * Determine if any event in the event queue is stateful. + * + * @returns True if there's any event that requires state and False if none of them do. + */ +export const isStateful = () => { + if (event_queue.length === 0) { + return false; + } + return event_queue.some(event => event.name.startsWith("state")); +} + /** * Apply a delta to the state. * @param state The state to apply the delta to. @@ -116,6 +128,20 @@ export const applyDelta = (state, delta) => { return { ...state, ...delta }; }; +/** + * Only Queue and process events when websocket connection exists. + * @param event The event to queue. + * @param socket The socket object to send the event on. + * + * @returns Adds event to queue and processes it if websocket exits, does nothing otherwise. + */ +export const queueEventIfSocketExists = async (events, socket) => { + if (!socket) { + return; + } + await queueEvents(events, socket); +} + /** * Handle frontend event or send the event to the backend via Websocket. * @param event The event to send. @@ -143,19 +169,19 @@ export const applyEvent = async (event, socket) => { if (event.name == "_remove_cookie") { cookies.remove(event.payload.key, { ...event.payload.options }); - queueEvents(initialEvents(), socket); + queueEventIfSocketExists(initialEvents(), socket); return false; } if (event.name == "_clear_local_storage") { localStorage.clear(); - queueEvents(initialEvents(), socket); + queueEventIfSocketExists(initialEvents(), socket); return false; } if (event.name == "_remove_local_storage") { localStorage.removeItem(event.payload.key); - queueEvents(initialEvents(), socket); + queueEventIfSocketExists(initialEvents(), socket); return false; } @@ -249,7 +275,7 @@ export const applyRestEvent = async (event, socket) => { let eventSent = false; if (event.handler === "uploadFiles") { - if (event.payload.files === undefined || event.payload.files.length === 0){ + if (event.payload.files === undefined || event.payload.files.length === 0) { // Submit the event over the websocket to trigger the event handler. return await applyEvent(Event(event.name), socket) } @@ -282,8 +308,8 @@ export const queueEvents = async (events, socket) => { * @param socket The socket object to send the event on. */ export const processEvent = async (socket) => { - // Only proceed if the socket is up, otherwise we throw the event into the void - if (!socket) { + // Only proceed if the socket is up and no event in the queue uses state, otherwise we throw the event into the void + if (!socket && isStateful()) { return; } @@ -684,7 +710,7 @@ export const useEventLoop = ( const change_start = () => { const main_state_dispatch = dispatch["state"] if (main_state_dispatch !== undefined) { - main_state_dispatch({is_hydrated: false}) + main_state_dispatch({ is_hydrated: false }) } } const change_complete = () => addEvents(onLoadInternalEvent()); diff --git a/reflex/app.py b/reflex/app.py index 65bc32438..5441ce987 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -541,7 +541,7 @@ class App(LifespanMixin, Base): # Ensure state is enabled if this page uses state. if self.state is None: - if on_load or component._has_event_triggers(): + if on_load or component._has_stateful_event_triggers(): self._enable_state() else: for var in component._get_vars(include_children=True): diff --git a/reflex/components/component.py b/reflex/components/component.py index 370ed5693..cb481504e 100644 --- a/reflex/components/component.py +++ b/reflex/components/component.py @@ -1119,17 +1119,35 @@ class Component(BaseComponent, ABC): return vars - def _has_event_triggers(self) -> bool: - """Check if the component or children have any event triggers. + def _event_trigger_values_use_state(self) -> bool: + """Check if the values of a component's event trigger use state. Returns: - True if the component or children have any event triggers. + True if any of the component's event trigger values uses State. """ - if self.event_triggers: + for trigger in self.event_triggers.values(): + if isinstance(trigger, EventChain): + for event in trigger.events: + if event.handler.state_full_name: + return True + elif isinstance(trigger, Var) and trigger._var_state: + return True + return False + + def _has_stateful_event_triggers(self): + """Check if component or children have any event triggers that use state. + + Returns: + True if the component or children have any event triggers that uses state. + """ + if self.event_triggers and self._event_trigger_values_use_state(): return True else: for child in self.children: - if isinstance(child, Component) and child._has_event_triggers(): + if ( + isinstance(child, Component) + and child._has_stateful_event_triggers() + ): return True return False diff --git a/tests/components/test_component.py b/tests/components/test_component.py index 76a75a67a..b014088ba 100644 --- a/tests/components/test_component.py +++ b/tests/components/test_component.py @@ -2069,3 +2069,76 @@ def test_add_style_foreach(): # Expect only one instance of this CSS dict in the rendered page assert str(page).count('css={{"color": "red"}}') == 1 + + +class TriggerState(rx.State): + """Test state with event handlers.""" + + def do_something(self): + """Sample event handler.""" + pass + + +@pytest.mark.parametrize( + "component, output", + [ + (rx.box(rx.text("random text")), False), + ( + rx.box(rx.text("random text", on_click=rx.console_log("log"))), + False, + ), + ( + rx.box( + rx.text("random text", on_click=TriggerState.do_something), + rx.text( + "random text", + on_click=BaseVar(_var_name="toggleColorMode", _var_type=EventChain), + ), + ), + True, + ), + ( + rx.box( + rx.text("random text", on_click=rx.console_log("log")), + rx.text( + "random text", + on_click=BaseVar(_var_name="toggleColorMode", _var_type=EventChain), + ), + ), + False, + ), + ( + rx.box(rx.text("random text", on_click=TriggerState.do_something)), + True, + ), + ( + rx.box( + rx.text( + "random text", + on_click=[rx.console_log("log"), rx.window_alert("alert")], + ), + ), + False, + ), + ( + rx.box( + rx.text( + "random text", + on_click=[rx.console_log("log"), TriggerState.do_something], + ), + ), + True, + ), + ( + rx.box( + rx.text( + "random text", + on_blur=lambda: TriggerState.do_something, + ), + ), + True, + ), + ], +) +def test_has_state_event_triggers(component, output): + assert component._has_stateful_event_triggers() == output diff --git a/tests/test_app.py b/tests/test_app.py index 5709d93a9..536b88575 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -1392,8 +1392,12 @@ def test_app_state_determination(): a4 = App() assert a4.state is None - # Referencing an event handler enables state. a4.add_page(rx.box(rx.button("Click", on_click=rx.console_log(""))), route="/") + assert a4.state is None + + a4.add_page( + rx.box(rx.button("Click", on_click=DynamicState.on_counter)), route="/page2" + ) assert a4.state is not None