From 3a3ad3f706477642133190c08bbe1e43a991bfa3 Mon Sep 17 00:00:00 2001 From: Ryan Morshead Date: Wed, 27 Dec 2023 20:02:16 -0700 Subject: [PATCH] Skip rendering None in all situations (#1171) * skip rendering none * add changelog * conditional render none should not reset state for sibling components * minor renaming + better changelog * misc fixes * raises exceptiongroup * skipif * handle egroup in starlette * final nocov --- docs/source/about/changelog.rst | 17 +++++++ src/py/reactpy/pyproject.toml | 1 + src/py/reactpy/reactpy/__init__.py | 1 - src/py/reactpy/reactpy/backend/starlette.py | 15 ++++-- src/py/reactpy/reactpy/core/layout.py | 48 +++++++++-------- src/py/reactpy/reactpy/core/serve.py | 12 ++++- src/py/reactpy/reactpy/core/types.py | 11 +--- src/py/reactpy/tests/test_core/test_layout.py | 51 +++++++++++++++---- src/py/reactpy/tests/test_core/test_serve.py | 8 ++- 9 files changed, 116 insertions(+), 48 deletions(-) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index d874a470f..feecbd1f0 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -35,6 +35,23 @@ Unreleased the overall responsiveness of your app, particularly when handling larger renders that would otherwise block faster renders from being processed. +**Changed** + +- :pull:`1171` - Previously ``None``, when present in an HTML element, would render as + the string ``"None"``. Now ``None`` will not render at all. This is consistent with + how ``None`` is handled when returned from components. It also makes it easier to + conditionally render elements. For example, previously you would have needed to use a + fragment to conditionally render an element by writing + ``something if condition else html._()``. Now you can simply write + ``something if condition else None``. + +**Deprecated** + +- :pull:`1171` - The ``Stop`` exception. Recent releases of ``anyio`` have made this + exception difficult to use since it now raises an ``ExceptionGroup``. This exception + was primarily used for internal testing purposes and so is now deprecated. + + v1.0.2 ------ diff --git a/src/py/reactpy/pyproject.toml b/src/py/reactpy/pyproject.toml index 67189808b..309248507 100644 --- a/src/py/reactpy/pyproject.toml +++ b/src/py/reactpy/pyproject.toml @@ -25,6 +25,7 @@ classifiers = [ "Programming Language :: Python :: Implementation :: PyPy", ] dependencies = [ + "exceptiongroup >=1.0", "typing-extensions >=3.10", "mypy-extensions >=0.4.3", "anyio >=3", diff --git a/src/py/reactpy/reactpy/__init__.py b/src/py/reactpy/reactpy/__init__.py index 63a8550cc..49e357441 100644 --- a/src/py/reactpy/reactpy/__init__.py +++ b/src/py/reactpy/reactpy/__init__.py @@ -16,7 +16,6 @@ use_state, ) from reactpy.core.layout import Layout -from reactpy.core.serve import Stop from reactpy.core.vdom import vdom from reactpy.utils import Ref, html_to_vdom, vdom_to_html diff --git a/src/py/reactpy/reactpy/backend/starlette.py b/src/py/reactpy/reactpy/backend/starlette.py index 2953b97b3..9bc68db47 100644 --- a/src/py/reactpy/reactpy/backend/starlette.py +++ b/src/py/reactpy/reactpy/backend/starlette.py @@ -7,6 +7,7 @@ from dataclasses import dataclass from typing import Any, Callable +from exceptiongroup import BaseExceptionGroup from starlette.applications import Starlette from starlette.middleware.cors import CORSMiddleware from starlette.requests import Request @@ -137,8 +138,6 @@ async def serve_index(request: Request) -> HTMLResponse: def _setup_single_view_dispatcher_route( options: Options, app: Starlette, component: RootComponentConstructor ) -> None: - @app.websocket_route(str(STREAM_PATH)) - @app.websocket_route(f"{STREAM_PATH}/{{path:path}}") async def model_stream(socket: WebSocket) -> None: await socket.accept() send, recv = _make_send_recv_callbacks(socket) @@ -162,8 +161,16 @@ async def model_stream(socket: WebSocket) -> None: send, recv, ) - except WebSocketDisconnect as error: - logger.info(f"WebSocket disconnect: {error.code}") + except BaseExceptionGroup as egroup: + for e in egroup.exceptions: + if isinstance(e, WebSocketDisconnect): + logger.info(f"WebSocket disconnect: {e.code}") + break + else: # nocov + raise + + app.add_websocket_route(str(STREAM_PATH), model_stream) + app.add_websocket_route(f"{STREAM_PATH}/{{path:path}}", model_stream) def _make_send_recv_callbacks( diff --git a/src/py/reactpy/reactpy/core/layout.py b/src/py/reactpy/reactpy/core/layout.py index d59ab31eb..70bdbbbff 100644 --- a/src/py/reactpy/reactpy/core/layout.py +++ b/src/py/reactpy/reactpy/core/layout.py @@ -11,7 +11,7 @@ wait, ) from collections import Counter -from collections.abc import Iterator +from collections.abc import Sequence from contextlib import AsyncExitStack from logging import getLogger from typing import ( @@ -27,6 +27,7 @@ from weakref import ref as weakref from anyio import Semaphore +from typing_extensions import TypeAlias from reactpy.config import ( REACTPY_ASYNC_RENDERING, @@ -37,8 +38,10 @@ from reactpy.core.types import ( ComponentType, EventHandlerDict, + Key, LayoutEventMessage, LayoutUpdateMessage, + VdomChild, VdomDict, VdomJson, ) @@ -189,9 +192,7 @@ async def _render_component( # wrap the model in a fragment (i.e. tagName="") to ensure components have # a separate node in the model state tree. This could be removed if this # components are given a node in the tree some other way - wrapper_model: VdomDict = {"tagName": ""} - if raw_model is not None: - wrapper_model["children"] = [raw_model] + wrapper_model: VdomDict = {"tagName": "", "children": [raw_model]} await self._render_model(exit_stack, old_state, new_state, wrapper_model) except Exception as error: logger.exception(f"Failed to render {component}") @@ -329,11 +330,11 @@ async def _render_model_children( await self._unmount_model_states(list(old_state.children_by_key.values())) return None - child_type_key_tuples = list(_process_child_type_and_key(raw_children)) + children_info = _get_children_info(raw_children) - new_keys = {item[2] for item in child_type_key_tuples} - if len(new_keys) != len(raw_children): - key_counter = Counter(item[2] for item in child_type_key_tuples) + new_keys = {k for _, _, k in children_info} + if len(new_keys) != len(children_info): + key_counter = Counter(item[2] for item in children_info) duplicate_keys = [key for key, count in key_counter.items() if count > 1] msg = f"Duplicate keys {duplicate_keys} at {new_state.patch_path or '/'!r}" raise ValueError(msg) @@ -345,7 +346,7 @@ async def _render_model_children( ) new_state.model.current["children"] = [] - for index, (child, child_type, key) in enumerate(child_type_key_tuples): + for index, (child, child_type, key) in enumerate(children_info): old_child_state = old_state.children_by_key.get(key) if child_type is _DICT_TYPE: old_child_state = old_state.children_by_key.get(key) @@ -420,17 +421,17 @@ async def _render_model_children_without_old_state( new_state: _ModelState, raw_children: list[Any], ) -> None: - child_type_key_tuples = list(_process_child_type_and_key(raw_children)) + children_info = _get_children_info(raw_children) - new_keys = {item[2] for item in child_type_key_tuples} - if len(new_keys) != len(raw_children): - key_counter = Counter(item[2] for item in child_type_key_tuples) + new_keys = {k for _, _, k in children_info} + if len(new_keys) != len(children_info): + key_counter = Counter(k for _, _, k in children_info) duplicate_keys = [key for key, count in key_counter.items() if count > 1] msg = f"Duplicate keys {duplicate_keys} at {new_state.patch_path or '/'!r}" raise ValueError(msg) new_state.model.current["children"] = [] - for index, (child, child_type, key) in enumerate(child_type_key_tuples): + for index, (child, child_type, key) in enumerate(children_info): if child_type is _DICT_TYPE: child_state = _make_element_model_state(new_state, index, key) await self._render_model(exit_stack, None, child_state, child) @@ -609,7 +610,7 @@ def __init__( key: Any, model: Ref[VdomJson], patch_path: str, - children_by_key: dict[str, _ModelState], + children_by_key: dict[Key, _ModelState], targets_by_event: dict[str, str], life_cycle_state: _LifeCycleState | None = None, ): @@ -720,16 +721,17 @@ async def get(self) -> _Type: return value -def _process_child_type_and_key( - children: list[Any], -) -> Iterator[tuple[Any, _ElementType, Any]]: +def _get_children_info(children: list[VdomChild]) -> Sequence[_ChildInfo]: + infos: list[_ChildInfo] = [] for index, child in enumerate(children): - if isinstance(child, dict): + if child is None: + continue + elif isinstance(child, dict): child_type = _DICT_TYPE key = child.get("key") elif isinstance(child, ComponentType): child_type = _COMPONENT_TYPE - key = getattr(child, "key", None) + key = child.key else: child = f"{child}" child_type = _STRING_TYPE @@ -738,8 +740,12 @@ def _process_child_type_and_key( if key is None: key = index - yield (child, child_type, key) + infos.append((child, child_type, key)) + return infos + + +_ChildInfo: TypeAlias = tuple[Any, "_ElementType", Key] # used in _process_child_type_and_key _ElementType = NewType("_ElementType", int) diff --git a/src/py/reactpy/reactpy/core/serve.py b/src/py/reactpy/reactpy/core/serve.py index 3a530e854..3a540af59 100644 --- a/src/py/reactpy/reactpy/core/serve.py +++ b/src/py/reactpy/reactpy/core/serve.py @@ -3,6 +3,7 @@ from collections.abc import Awaitable from logging import getLogger from typing import Callable +from warnings import warn from anyio import create_task_group from anyio.abc import TaskGroup @@ -24,7 +25,9 @@ class Stop(BaseException): - """Stop serving changes and events + """Deprecated + + Stop serving changes and events Raising this error will tell dispatchers to gracefully exit. Typically this is called by code running inside a layout to tell it to stop rendering. @@ -42,7 +45,12 @@ async def serve_layout( async with create_task_group() as task_group: task_group.start_soon(_single_outgoing_loop, layout, send) task_group.start_soon(_single_incoming_loop, task_group, layout, recv) - except Stop: + except Stop: # nocov + warn( + "The Stop exception is deprecated and will be removed in a future version", + UserWarning, + stacklevel=1, + ) logger.info(f"Stopped serving {layout}") diff --git a/src/py/reactpy/reactpy/core/types.py b/src/py/reactpy/reactpy/core/types.py index e5a81814f..39a9b3534 100644 --- a/src/py/reactpy/reactpy/core/types.py +++ b/src/py/reactpy/reactpy/core/types.py @@ -91,7 +91,7 @@ async def __aexit__( VdomAttributes = Mapping[str, Any] """Describes the attributes of a :class:`VdomDict`""" -VdomChild: TypeAlias = "ComponentType | VdomDict | str" +VdomChild: TypeAlias = "ComponentType | VdomDict | str | None | Any" """A single child element of a :class:`VdomDict`""" VdomChildren: TypeAlias = "Sequence[VdomChild] | VdomChild" @@ -100,14 +100,7 @@ async def __aexit__( class _VdomDictOptional(TypedDict, total=False): key: Key | None - children: Sequence[ - # recursive types are not allowed yet: - # https://github.com/python/mypy/issues/731 - ComponentType - | dict[str, Any] - | str - | Any - ] + children: Sequence[ComponentType | VdomChild] attributes: VdomAttributes eventHandlers: EventHandlerDict importSource: ImportSourceDict diff --git a/src/py/reactpy/tests/test_core/test_layout.py b/src/py/reactpy/tests/test_core/test_layout.py index 9f27727df..6eec7a8d2 100644 --- a/src/py/reactpy/tests/test_core/test_layout.py +++ b/src/py/reactpy/tests/test_core/test_layout.py @@ -102,15 +102,6 @@ def SimpleComponent(): ) -async def test_component_can_return_none(): - @reactpy.component - def SomeComponent(): - return None - - async with reactpy.Layout(SomeComponent()) as layout: - assert (await layout.render())["model"] == {"tagName": ""} - - async def test_nested_component_layout(): parent_set_state = reactpy.Ref(None) child_set_state = reactpy.Ref(None) @@ -1310,3 +1301,45 @@ def child_2(): assert child_1_render_count.current == 1 assert child_2_render_count.current == 1 + + +async def test_none_does_not_render(): + @component + def Root(): + return html.div(None, Child()) + + @component + def Child(): + return None + + async with layout_runner(Layout(Root())) as runner: + tree = await runner.render() + assert tree == { + "tagName": "", + "children": [ + {"tagName": "div", "children": [{"tagName": "", "children": []}]} + ], + } + + +async def test_conditionally_render_none_does_not_trigger_state_change_in_siblings(): + toggle_condition = Ref() + effect_run_count = Ref(0) + + @component + def Root(): + condition, toggle_condition.current = use_toggle(True) + return html.div("text" if condition else None, Child()) + + @component + def Child(): + @reactpy.use_effect + def effect(): + effect_run_count.current += 1 + + async with layout_runner(Layout(Root())) as runner: + await runner.render() + poll(lambda: effect_run_count.current).until_equals(1) + toggle_condition.current() + await runner.render() + assert effect_run_count.current == 1 diff --git a/src/py/reactpy/tests/test_core/test_serve.py b/src/py/reactpy/tests/test_core/test_serve.py index 9b22ee866..bae3c1e01 100644 --- a/src/py/reactpy/tests/test_core/test_serve.py +++ b/src/py/reactpy/tests/test_core/test_serve.py @@ -1,7 +1,9 @@ import asyncio +import sys from collections.abc import Sequence from typing import Any +import pytest from jsonpointer import set_pointer import reactpy @@ -31,7 +33,7 @@ async def send(patch): changes.append(patch) sem.release() if not events_to_inject: - raise reactpy.Stop() + raise Exception("Stop running") async def recv(): await sem.acquire() @@ -90,10 +92,12 @@ def Counter(): return reactpy.html.div({EVENT_NAME: handler, "count": count}) +@pytest.mark.skipif(sys.version_info < (3, 11), reason="ExceptionGroup not available") async def test_dispatch(): events, expected_model = make_events_and_expected_model() changes, send, recv = make_send_recv_callbacks(events) - await asyncio.wait_for(serve_layout(Layout(Counter()), send, recv), 1) + with pytest.raises(ExceptionGroup): + await asyncio.wait_for(serve_layout(Layout(Counter()), send, recv), 1) assert_changes_produce_expected_model(changes, expected_model)