Skip to content

Commit

Permalink
Skip rendering None in all situations (#1171)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
rmorshea authored Dec 28, 2023
1 parent 43009e4 commit 3a3ad3f
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 48 deletions.
17 changes: 17 additions & 0 deletions docs/source/about/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------

Expand Down
1 change: 1 addition & 0 deletions src/py/reactpy/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 0 additions & 1 deletion src/py/reactpy/reactpy/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
15 changes: 11 additions & 4 deletions src/py/reactpy/reactpy/backend/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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(
Expand Down
48 changes: 27 additions & 21 deletions src/py/reactpy/reactpy/core/layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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,
Expand All @@ -37,8 +38,10 @@
from reactpy.core.types import (
ComponentType,
EventHandlerDict,
Key,
LayoutEventMessage,
LayoutUpdateMessage,
VdomChild,
VdomDict,
VdomJson,
)
Expand Down Expand Up @@ -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}")
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
):
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
12 changes: 10 additions & 2 deletions src/py/reactpy/reactpy/core/serve.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand All @@ -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}")


Expand Down
11 changes: 2 additions & 9 deletions src/py/reactpy/reactpy/core/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down
51 changes: 42 additions & 9 deletions src/py/reactpy/tests/test_core/test_layout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
8 changes: 6 additions & 2 deletions src/py/reactpy/tests/test_core/test_serve.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)


Expand Down

0 comments on commit 3a3ad3f

Please sign in to comment.