From 238b03a8c753e27e51581de384f005fcf855b192 Mon Sep 17 00:00:00 2001 From: Khaleel Al-Adhami Date: Fri, 31 Jan 2025 18:36:35 -0800 Subject: [PATCH] disable react strict mode for event loop (#4720) * disable react strict mode for event loop * oops * pyi oui * separate socket connection from event loop * prettier state.js * disable react strict mode * didn't work sadge * socket connect/disconnect depends on new isBackendDisabled state * only start the event loop when the socket is set or we're not stateful * Always drain the queue unless backend is disabled --------- Co-authored-by: Masen Furer --- .../.templates/jinja/web/pages/_app.js.jinja2 | 14 ++-- reflex/.templates/web/utils/state.js | 69 +++++++++++-------- reflex/app.py | 10 ++- reflex/components/base/strict_mode.py | 10 +++ reflex/components/base/strict_mode.pyi | 57 +++++++++++++++ reflex/utils/prerequisites.py | 1 - tests/units/test_app.py | 38 +++++++--- tests/units/test_prerequisites.py | 12 ++-- 8 files changed, 157 insertions(+), 54 deletions(-) create mode 100644 reflex/components/base/strict_mode.py create mode 100644 reflex/components/base/strict_mode.pyi diff --git a/reflex/.templates/jinja/web/pages/_app.js.jinja2 b/reflex/.templates/jinja/web/pages/_app.js.jinja2 index 40e31dee6..ee3e24540 100644 --- a/reflex/.templates/jinja/web/pages/_app.js.jinja2 +++ b/reflex/.templates/jinja/web/pages/_app.js.jinja2 @@ -38,13 +38,13 @@ export default function MyApp({ Component, pageProps }) { }, []); return ( - - - - - - - + + + + + + + ); } diff --git a/reflex/.templates/web/utils/state.js b/reflex/.templates/web/utils/state.js index 009910a32..2f09ac2de 100644 --- a/reflex/.templates/web/utils/state.js +++ b/reflex/.templates/web/utils/state.js @@ -227,8 +227,8 @@ export const applyEvent = async (event, socket) => { a.href = eval?.( event.payload.url.replace( "getBackendURL(env.UPLOAD)", - `"${getBackendURL(env.UPLOAD)}"` - ) + `"${getBackendURL(env.UPLOAD)}"`, + ), ); } a.download = event.payload.filename; @@ -341,7 +341,7 @@ export const applyRestEvent = async (event, socket) => { event.payload.files, event.payload.upload_id, event.payload.on_upload_progress, - socket + socket, ); return false; } @@ -408,7 +408,7 @@ export const connect = async ( dispatch, transports, setConnectErrors, - client_storage = {} + client_storage = {}, ) => { // Get backend URL object from the endpoint. const endpoint = getBackendURL(EVENTURL); @@ -499,7 +499,7 @@ export const uploadFiles = async ( files, upload_id, on_upload_progress, - socket + socket, ) => { // return if there's no file to upload if (files === undefined || files.length === 0) { @@ -604,7 +604,7 @@ export const Event = ( name, payload = {}, event_actions = {}, - handler = null + handler = null, ) => { return { name, payload, handler, event_actions }; }; @@ -631,7 +631,7 @@ export const hydrateClientStorage = (client_storage) => { for (const state_key in client_storage.local_storage) { const options = client_storage.local_storage[state_key]; const local_storage_value = localStorage.getItem( - options.name || state_key + options.name || state_key, ); if (local_storage_value !== null) { client_storage_values[state_key] = local_storage_value; @@ -642,7 +642,7 @@ export const hydrateClientStorage = (client_storage) => { for (const state_key in client_storage.session_storage) { const session_options = client_storage.session_storage[state_key]; const session_storage_value = sessionStorage.getItem( - session_options.name || state_key + session_options.name || state_key, ); if (session_storage_value != null) { client_storage_values[state_key] = session_storage_value; @@ -667,7 +667,7 @@ export const hydrateClientStorage = (client_storage) => { const applyClientStorageDelta = (client_storage, delta) => { // find the main state and check for is_hydrated const unqualified_states = Object.keys(delta).filter( - (key) => key.split(".").length === 1 + (key) => key.split(".").length === 1, ); if (unqualified_states.length === 1) { const main_state = delta[unqualified_states[0]]; @@ -701,7 +701,7 @@ const applyClientStorageDelta = (client_storage, delta) => { const session_options = client_storage.session_storage[state_key]; sessionStorage.setItem( session_options.name || state_key, - delta[substate][key] + delta[substate][key], ); } } @@ -721,7 +721,7 @@ const applyClientStorageDelta = (client_storage, delta) => { export const useEventLoop = ( dispatch, initial_events = () => [], - client_storage = {} + client_storage = {}, ) => { const socket = useRef(null); const router = useRouter(); @@ -735,7 +735,7 @@ export const useEventLoop = ( event_actions = events.reduce( (acc, e) => ({ ...acc, ...e.event_actions }), - event_actions ?? {} + event_actions ?? {}, ); const _e = args.filter((o) => o?.preventDefault !== undefined)[0]; @@ -763,7 +763,7 @@ export const useEventLoop = ( debounce( combined_name, () => queueEvents(events, socket), - event_actions.debounce + event_actions.debounce, ); } else { queueEvents(events, socket); @@ -782,7 +782,7 @@ export const useEventLoop = ( query, asPath, }))(router), - })) + })), ); sentHydrate.current = true; } @@ -817,13 +817,9 @@ export const useEventLoop = ( }; }, []); - // Main event loop. + // Handle socket connect/disconnect. useEffect(() => { - // Skip if the router is not ready. - if (!router.isReady) { - return; - } - // only use websockets if state is present + // only use websockets if state is present and backend is not disabled (reflex cloud). if (Object.keys(initialState).length > 1 && !isBackendDisabled()) { // Initialize the websocket connection. if (!socket.current) { @@ -832,16 +828,31 @@ export const useEventLoop = ( dispatch, ["websocket"], setConnectErrors, - client_storage + client_storage, ); } - (async () => { - // Process all outstanding events. - while (event_queue.length > 0 && !event_processing) { - await processEvent(socket.current); - } - })(); } + + // Cleanup function. + return () => { + if (socket.current) { + socket.current.disconnect(); + } + }; + }, []); + + // Main event loop. + useEffect(() => { + // Skip if the router is not ready. + if (!router.isReady || isBackendDisabled()) { + return; + } + (async () => { + // Process all outstanding events. + while (event_queue.length > 0 && !event_processing) { + await processEvent(socket.current); + } + })(); }); // localStorage event handling @@ -865,7 +876,7 @@ export const useEventLoop = ( vars[storage_to_state_map[e.key]] = e.newValue; const event = Event( `${state_name}.reflex___state____update_vars_internal_state.update_vars_internal`, - { vars: vars } + { vars: vars }, ); addEvents([event], e); } @@ -958,7 +969,7 @@ export const getRefValues = (refs) => { return refs.map((ref) => ref.current ? ref.current.value || ref.current.getAttribute("aria-valuenow") - : null + : null, ); }; diff --git a/reflex/app.py b/reflex/app.py index f681addc5..247977e7e 100644 --- a/reflex/app.py +++ b/reflex/app.py @@ -54,6 +54,7 @@ from reflex.compiler.compiler import ExecutorSafeFunctions, compile_theme from reflex.components.base.app_wrap import AppWrap from reflex.components.base.error_boundary import ErrorBoundary from reflex.components.base.fragment import Fragment +from reflex.components.base.strict_mode import StrictMode from reflex.components.component import ( Component, ComponentStyle, @@ -956,6 +957,12 @@ class App(MiddlewareMixin, LifespanMixin): # If a theme component was provided, wrap the app with it app_wrappers[(20, "Theme")] = self.theme + # Get the env mode. + config = get_config() + + if config.react_strict_mode: + app_wrappers[(200, "StrictMode")] = StrictMode.create() + should_compile = self._should_compile() if not should_compile: @@ -1001,9 +1008,6 @@ class App(MiddlewareMixin, LifespanMixin): progress.advance(task) - # Get the env mode. - config = get_config() - # Store the compile results. compile_results = [] diff --git a/reflex/components/base/strict_mode.py b/reflex/components/base/strict_mode.py new file mode 100644 index 000000000..46b01ad87 --- /dev/null +++ b/reflex/components/base/strict_mode.py @@ -0,0 +1,10 @@ +"""Module for the StrictMode component.""" + +from reflex.components.component import Component + + +class StrictMode(Component): + """A React strict mode component to enable strict mode for its children.""" + + library = "react" + tag = "StrictMode" diff --git a/reflex/components/base/strict_mode.pyi b/reflex/components/base/strict_mode.pyi new file mode 100644 index 000000000..9005c0222 --- /dev/null +++ b/reflex/components/base/strict_mode.pyi @@ -0,0 +1,57 @@ +"""Stub file for reflex/components/base/strict_mode.py""" + +# ------------------- DO NOT EDIT ---------------------- +# This file was generated by `reflex/utils/pyi_generator.py`! +# ------------------------------------------------------ +from typing import Any, Dict, Optional, Union, overload + +from reflex.components.component import Component +from reflex.event import BASE_STATE, EventType +from reflex.style import Style +from reflex.vars.base import Var + +class StrictMode(Component): + @overload + @classmethod + def create( # type: ignore + cls, + *children, + style: Optional[Style] = None, + key: Optional[Any] = None, + id: Optional[Any] = None, + class_name: Optional[Any] = None, + autofocus: Optional[bool] = None, + custom_attrs: Optional[Dict[str, Union[Var, Any]]] = None, + on_blur: Optional[EventType[[], BASE_STATE]] = None, + on_click: Optional[EventType[[], BASE_STATE]] = None, + on_context_menu: Optional[EventType[[], BASE_STATE]] = None, + on_double_click: Optional[EventType[[], BASE_STATE]] = None, + on_focus: Optional[EventType[[], BASE_STATE]] = None, + on_mount: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_down: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_enter: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_leave: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_move: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_out: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_over: Optional[EventType[[], BASE_STATE]] = None, + on_mouse_up: Optional[EventType[[], BASE_STATE]] = None, + on_scroll: Optional[EventType[[], BASE_STATE]] = None, + on_unmount: Optional[EventType[[], BASE_STATE]] = None, + **props, + ) -> "StrictMode": + """Create the component. + + Args: + *children: The children of the component. + style: The style of the component. + key: A unique key for the component. + id: The id for the component. + class_name: The class name for the component. + autofocus: Whether the component should take the focus once the page is loaded + custom_attrs: custom attribute + **props: The props of the component. + + Returns: + The component. + """ + ... diff --git a/reflex/utils/prerequisites.py b/reflex/utils/prerequisites.py index f78908a84..4741400f8 100644 --- a/reflex/utils/prerequisites.py +++ b/reflex/utils/prerequisites.py @@ -912,7 +912,6 @@ def _update_next_config( next_config = { "basePath": config.frontend_path or "", "compress": config.next_compression, - "reactStrictMode": config.react_strict_mode, "trailingSlash": True, "staticPageGenerationTimeout": config.static_page_generation_timeout, } diff --git a/tests/units/test_app.py b/tests/units/test_app.py index bf1a8a313..058174a1b 100644 --- a/tests/units/test_app.py +++ b/tests/units/test_app.py @@ -1274,12 +1274,23 @@ def compilable_app(tmp_path) -> Generator[tuple[App, Path], None, None]: yield app, web_dir -def test_app_wrap_compile_theme(compilable_app: tuple[App, Path]): +@pytest.mark.parametrize( + "react_strict_mode", + [True, False], +) +def test_app_wrap_compile_theme( + react_strict_mode: bool, compilable_app: tuple[App, Path], mocker +): """Test that the radix theme component wraps the app. Args: + react_strict_mode: Whether to use React Strict Mode. compilable_app: compilable_app fixture. + mocker: pytest mocker object. """ + conf = rx.Config(app_name="testing", react_strict_mode=react_strict_mode) + mocker.patch("reflex.config._get_config", return_value=conf) + app, web_dir = compilable_app app.theme = rx.theme(accent_color="plum") app._compile() @@ -1290,24 +1301,37 @@ def test_app_wrap_compile_theme(compilable_app: tuple[App, Path]): assert ( "function AppWrap({children}) {" "return (" - "" + + ("" if react_strict_mode else "") + + "" "" "" "{children}" "" "" "" - ")" + + ("" if react_strict_mode else "") + + ")" "}" ) in "".join(app_js_lines) -def test_app_wrap_priority(compilable_app: tuple[App, Path]): +@pytest.mark.parametrize( + "react_strict_mode", + [True, False], +) +def test_app_wrap_priority( + react_strict_mode: bool, compilable_app: tuple[App, Path], mocker +): """Test that the app wrap components are wrapped in the correct order. Args: + react_strict_mode: Whether to use React Strict Mode. compilable_app: compilable_app fixture. + mocker: pytest mocker object. """ + conf = rx.Config(app_name="testing", react_strict_mode=react_strict_mode) + mocker.patch("reflex.config._get_config", return_value=conf) + app, web_dir = compilable_app class Fragment1(Component): @@ -1339,8 +1363,7 @@ def test_app_wrap_priority(compilable_app: tuple[App, Path]): ] assert ( "function AppWrap({children}) {" - "return (" - "" + "return (" + ("" if react_strict_mode else "") + "" '' "" "" @@ -1350,8 +1373,7 @@ def test_app_wrap_priority(compilable_app: tuple[App, Path]): "" "" "" - "" - ")" + "" + ("" if react_strict_mode else "") + ")" "}" ) in "".join(app_js_lines) diff --git a/tests/units/test_prerequisites.py b/tests/units/test_prerequisites.py index 3bd029077..4723d8648 100644 --- a/tests/units/test_prerequisites.py +++ b/tests/units/test_prerequisites.py @@ -32,7 +32,7 @@ runner = CliRunner() app_name="test", ), False, - 'module.exports = {basePath: "", compress: true, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 60};', + 'module.exports = {basePath: "", compress: true, trailingSlash: true, staticPageGenerationTimeout: 60};', ), ( Config( @@ -40,7 +40,7 @@ runner = CliRunner() static_page_generation_timeout=30, ), False, - 'module.exports = {basePath: "", compress: true, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 30};', + 'module.exports = {basePath: "", compress: true, trailingSlash: true, staticPageGenerationTimeout: 30};', ), ( Config( @@ -48,7 +48,7 @@ runner = CliRunner() next_compression=False, ), False, - 'module.exports = {basePath: "", compress: false, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 60};', + 'module.exports = {basePath: "", compress: false, trailingSlash: true, staticPageGenerationTimeout: 60};', ), ( Config( @@ -56,7 +56,7 @@ runner = CliRunner() frontend_path="/test", ), False, - 'module.exports = {basePath: "/test", compress: true, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 60};', + 'module.exports = {basePath: "/test", compress: true, trailingSlash: true, staticPageGenerationTimeout: 60};', ), ( Config( @@ -65,14 +65,14 @@ runner = CliRunner() next_compression=False, ), False, - 'module.exports = {basePath: "/test", compress: false, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 60};', + 'module.exports = {basePath: "/test", compress: false, trailingSlash: true, staticPageGenerationTimeout: 60};', ), ( Config( app_name="test", ), True, - 'module.exports = {basePath: "", compress: true, reactStrictMode: true, trailingSlash: true, staticPageGenerationTimeout: 60, output: "export", distDir: "_static"};', + 'module.exports = {basePath: "", compress: true, trailingSlash: true, staticPageGenerationTimeout: 60, output: "export", distDir: "_static"};', ), ], )