diff --git a/packages/reflex-base/news/7369.bugfix.md b/packages/reflex-base/news/7369.bugfix.md new file mode 100644 index 00000000000..8a7668f630d --- /dev/null +++ b/packages/reflex-base/news/7369.bugfix.md @@ -0,0 +1 @@ +Pages in apps with many substates no longer intermittently fail to server-render with a 500 such as `SyntaxError: Invalid regular expression: /[^a-z-]/: Stack overflow`. diff --git a/packages/reflex-base/src/reflex_base/compiler/templates.py b/packages/reflex-base/src/reflex_base/compiler/templates.py index cef9dba2221..be06aa1a487 100644 --- a/packages/reflex-base/src/reflex_base/compiler/templates.py +++ b/packages/reflex-base/src/reflex_base/compiler/templates.py @@ -388,8 +388,8 @@ def context_template( """ ) - create_state_contexts_str = "\n".join( - rf"createElement(SubstateProvider, {{substateName: '{state_name}', contextName: '{format_state_name(state_name)}'}}," + substates_str = "".join( + f"\n ['{state_name}', '{format_state_name(state_name)}']," for state_name in initial_state ) @@ -554,16 +554,40 @@ def context_template( ); }}; -export function StateProvider({{ children }}) {{ - return useMemo( - () => ( - createElement(DispatchProvider, {{}}, - {create_state_contexts_str}children - {")" * len(initial_state)} - )), - [children], - ); -}}""" +// ``[substateName, contextName]`` for every substate, outermost first. +const SUBSTATES = [{substates_str} +]; + +function ClientStateProvider({{ children }}) {{ + return useMemo(() => {{ + let tree = children; + for (let i = SUBSTATES.length - 1; i >= 0; i--) {{ + const [substateName, contextName] = SUBSTATES[i]; + tree = createElement(SubstateProvider, {{ substateName, contextName }}, tree); + }} + return createElement(DispatchProvider, {{}}, tree); + }}, [children]); +}} + +// The server renders once and never applies a delta, so it provides the +// initial state through bare context providers. ``SubstateProvider`` would add +// a second render level per substate, and the server renderer recurses once per +// level, so with many substates rendering a page can exhaust the stack. +function ServerStateProvider({{ children }}) {{ + let tree = children; + for (let i = SUBSTATES.length - 1; i >= 0; i--) {{ + const [substateName, contextName] = SUBSTATES[i]; + tree = createElement( + StateContexts[contextName], + {{ value: initialState[substateName] }}, + tree, + ); + }} + return createElement(DispatchContext, {{ value: {{}} }}, tree); +}} + +export const StateProvider = + typeof document === "undefined" ? ServerStateProvider : ClientStateProvider;""" def component_template(component: Component): diff --git a/tests/units/compiler/test_compiler.py b/tests/units/compiler/test_compiler.py index cdc6a8b4fed..c3c1f9a3901 100644 --- a/tests/units/compiler/test_compiler.py +++ b/tests/units/compiler/test_compiler.py @@ -1734,16 +1734,15 @@ def test_context_template_owner_stack_pin(disable_owner_stacks: bool): assert "captureOwnerStack" in rendered -def test_context_template_one_provider_per_substate(): - """Each substate gets its own provider so one delta re-renders one context. +def _render_two_substate_context() -> str: + """Render the context template for a state with one substate. - A single provider owning every reducer means any delta recreates every - ``StateContexts`` element; nesting one ``SubstateProvider`` per substate - keeps the untouched providers memoized. + Returns: + The rendered context module source. """ from reflex_base.compiler.templates import context_template - rendered = context_template( + return context_template( is_dev_mode=True, default_color_mode='"light"', initial_state={ @@ -1753,18 +1752,56 @@ def test_context_template_one_provider_per_substate(): state_name="reflex___state____state", ) + +def test_context_template_one_provider_per_substate(): + """Each substate gets its own provider so one delta re-renders one context. + + A single provider owning every reducer means any delta recreates every + ``StateContexts`` element; nesting one ``SubstateProvider`` per substate + keeps the untouched providers memoized. + """ + rendered = _render_two_substate_context() + assert ( - "createElement(SubstateProvider, {substateName: 'reflex___state____state', " - "contextName: 'reflex___state____state'}," in rendered + "const SUBSTATES = [\n" + " ['reflex___state____state', 'reflex___state____state'],\n" + " ['reflex___state____state__sub', 'reflex___state____state__sub'],\n" + "];" in rendered + ) + # The reducers live in SubstateProvider; the client provider only composes. + client = rendered[ + rendered.index("function ClientStateProvider") : rendered.index( + "function ServerStateProvider" + ) + ] + assert "useReducer" not in client + assert "createElement(SubstateProvider, { substateName, contextName }, tree)" in ( + client ) + assert "createElement(DispatchProvider, {}, tree)" in client + + +def test_context_template_server_state_provider_is_flat(): + """The server provides initial state without a component per substate. + + Server rendering recurses once per element level, so a ``SubstateProvider`` + around every context doubled the depth of every page render and overflowed + the stack of apps with many substates. + """ + rendered = _render_two_substate_context() + + server = rendered[rendered.index("function ServerStateProvider") :] + assert "SubstateProvider" not in server + assert "DispatchProvider" not in server + assert "useReducer" not in server assert ( - "createElement(SubstateProvider, {substateName: 'reflex___state____state__sub', " - "contextName: 'reflex___state____state__sub'}," in rendered + "StateContexts[contextName],\n" + " { value: initialState[substateName] }," in server + ) + assert rendered.rstrip().endswith( + "export const StateProvider =\n" + ' typeof document === "undefined" ? ServerStateProvider : ClientStateProvider;' ) - # The reducers moved into SubstateProvider; StateProvider only composes. - provider_body = rendered[rendered.index("export function StateProvider") :] - assert "useReducer" not in provider_body - assert "createElement(DispatchProvider, {}," in provider_body def test_context_template_client_side_component_is_named(): diff --git a/tests/units/compiler/test_context_js_template.py b/tests/units/compiler/test_context_js_template.py index b290577f461..16c181bc0a6 100644 --- a/tests/units/compiler/test_context_js_template.py +++ b/tests/units/compiler/test_context_js_template.py @@ -1,9 +1,82 @@ """Regression tests for the generated context.js frontend template.""" +import json import re +import shutil +import subprocess +from pathlib import Path +import pytest from reflex_base.compiler.templates import context_template +requires_node = pytest.mark.skipif(shutil.which("node") is None, reason="node missing") + +# Stand-ins for the modules context.js imports, keyed by the specifier they +# replace. ``createElement`` returns a plain tree to walk, and every hook throws: +# the server provider must not need React's dispatcher at all. +_STUB_MODULES = { + "react": """ +const hook = (name) => () => { + throw new Error(`${name} called while rendering on the server`); +}; +export const useContext = hook("useContext"); +export const useMemo = hook("useMemo"); +export const useReducer = hook("useReducer"); +export const useRef = hook("useRef"); +export const useState = hook("useState"); +export const useEffect = hook("useEffect"); +export const useLayoutEffect = hook("useLayoutEffect"); +export const createElement = (type, props, ...children) => ({ + type, + props: { ...props, children: children.length === 1 ? children[0] : children }, +}); +""", + "$/utils/state": """ +export const applyDelta = () => {}; +export const ReflexEvent = () => ({}); +export const hydrateClientStorage = () => ({}); +export const useEventLoop = () => []; +export const refs = {}; +""", + "$/utils/context-registry": """ +export const ColorModeContext = {}; +export const UploadFilesContext = {}; +export const DispatchContext = {}; +export const EventLoopContext = {}; +export const getStateContext = () => ({}); +export const registerApp = () => {}; +export const eventLoop = {}; +""", + "@emotion/react": "export const jsx = () => null;\n", +} + +# Renders ``StateProvider`` around a leaf and reports every level between them, +# outermost first: a component by name, or a context with whether its value is +# the one the server should provide. +_SERVER_RENDER_DRIVER = """ +import { pathToFileURL } from "node:url"; +const mod = await import(pathToFileURL(process.argv[2]).href); +const leaf = { leaf: true }; +const levels = []; +let node = mod.StateProvider({ children: leaf }); +while (node !== leaf) { + if (typeof node.type === "function") { + levels.push({ component: node.type.name }); + break; + } + if (node.type === mod.DispatchContext) { + levels.push({ context: "DispatchContext" }); + } else { + const name = Object.keys(mod.StateContexts).find( + (key) => mod.StateContexts[key] === node.type, + ); + levels.push({ context: name, value: node.props.value === mod.initialState[name] }); + } + node = node.props.children; +} +process.stdout.write(JSON.stringify(levels)); +""" + def _event_loop_provider_body() -> str: """Render context.js and return the ``EventLoopProvider`` function body. @@ -18,7 +91,7 @@ def _event_loop_provider_body() -> str: state_name="state", ) start = rendered.index("export function EventLoopProvider") - end = rendered.index("export function StateProvider", start) + end = rendered.index("const useIsomorphicLayoutEffect", start) return rendered[start:end] @@ -56,3 +129,42 @@ def test_event_loop_provider_still_publishes_module_dispatchers() -> None: "module-level dispatchers must be published before the memoized return." ) assert "eventLoop.connectErrors = connectErrors;" in body[:memo_index] + + +@requires_node +def test_server_state_provider_renders_one_context_per_substate(tmp_path: Path): + """On the server, each substate adds exactly one level to the page render. + + React's server renderer recurses once per element level, so wrapping every + substate's context in its own component doubled the depth of every page and + overflowed the stack of apps with many substates. + """ + names = ["reflex___state____state"] + [ + f"reflex___state____state__sub_{i}" for i in range(199) + ] + rendered = context_template( + is_dev_mode=True, + default_color_mode='"light"', + initial_state={name: {"n": i} for i, name in enumerate(names)}, + state_name=names[0], + ) + for specifier, source in _STUB_MODULES.items(): + stub = tmp_path / (specifier.replace("/", "_").replace("$", "") + ".mjs") + stub.write_text(source) + rendered = rendered.replace(f'from "{specifier}"', f'from "./{stub.name}"') + module = tmp_path / "context.mjs" + module.write_text(rendered) + driver = tmp_path / "driver.mjs" + driver.write_text(_SERVER_RENDER_DRIVER) + + result = subprocess.run( + ["node", str(driver), str(module)], + capture_output=True, + encoding="utf-8", + ) + assert result.returncode == 0, result.stderr + levels = json.loads(result.stdout) + + assert levels == [{"context": "DispatchContext"}] + [ + {"context": name, "value": True} for name in names + ]