From 47130be6be3986c801431a1e6f20829b466aecd9 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 2 Apr 2021 16:47:07 -0700 Subject: [PATCH 01/20] EvenHandler should not serialize itself --- src/idom/core/events.py | 25 ++++++------------------- src/idom/core/layout.py | 11 +++++++++-- tests/test_core/test_events.py | 31 +++++++++++++++---------------- 3 files changed, 30 insertions(+), 37 deletions(-) diff --git a/src/idom/core/events.py b/src/idom/core/events.py index 6b3f49950..79d7e93f0 100644 --- a/src/idom/core/events.py +++ b/src/idom/core/events.py @@ -159,9 +159,9 @@ class EventHandler: "__weakref__", "_coro_handlers", "_func_handlers", - "_target_id", - "_prevent_default", - "_stop_propogation", + "target_id", + "prevent_default", + "stop_propogation", ) def __init__( @@ -170,16 +170,11 @@ def __init__( prevent_default: bool = False, target_id: Optional[str] = None, ) -> None: + self.target_id = target_id or str(id(self)) + self.stop_propogation = stop_propagation + self.prevent_default = prevent_default self._coro_handlers: List[Callable[..., Coroutine[Any, Any, Any]]] = [] self._func_handlers: List[Callable[..., Any]] = [] - self._target_id = target_id or str(id(self)) - self._stop_propogation = stop_propagation - self._prevent_default = prevent_default - - @property - def id(self) -> str: - """ID of the event handler.""" - return self._target_id def add(self, function: Callable[..., Any]) -> "EventHandler": """Add a callback to the event handler. @@ -207,14 +202,6 @@ def remove(self, function: Callable[..., Any]) -> None: else: self._func_handlers.remove(function) - def serialize(self) -> EventTarget: - """Serialize the event handler.""" - return { - "target": self._target_id, - "preventDefault": self._prevent_default, - "stopPropagation": self._stop_propogation, - } - async def __call__(self, data: List[Any]) -> Any: """Trigger all callbacks in the event handler.""" if self._coro_handlers: diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 403b12ef0..beca8da22 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -235,12 +235,19 @@ def _render_model_event_targets( h = attrs.pop(k) handlers[k] = h - event_handlers_by_id = {h.id: h for h in handlers.values()} + event_handlers_by_id = {h.target_id: h for h in handlers.values()} component_state.event_handler_ids.clear() component_state.event_handler_ids.update(event_handlers_by_id) self._event_handlers.update(event_handlers_by_id) - return {e: h.serialize() for e, h in handlers.items()} + return { + e: { + "target": h.target_id, + "preventDefault": h.prevent_default, + "stopPropagation": h.stop_propogation, + } + for e, h in handlers.items() + } def _get_component_state(self, component: AbstractComponent) -> ComponentState: return self._component_states[id(component)] diff --git a/tests/test_core/test_events.py b/tests/test_core/test_events.py index 821638602..730e3d7bb 100644 --- a/tests/test_core/test_events.py +++ b/tests/test_core/test_events.py @@ -39,22 +39,21 @@ async def handler_2(): assert calls == [1, 2] -def test_event_handler_serialization(): - assert EventHandler(target_id="uuid").serialize() == { - "target": "uuid", - "stopPropagation": False, - "preventDefault": False, - } - assert EventHandler(target_id="uuid", prevent_default=True).serialize() == { - "target": "uuid", - "stopPropagation": False, - "preventDefault": True, - } - assert EventHandler(target_id="uuid", stop_propagation=True).serialize() == { - "target": "uuid", - "stopPropagation": True, - "preventDefault": False, - } +def test_event_handler_props(): + handler_0 = EventHandler(target_id="uuid") + assert handler_0.target_id == "uuid" + assert handler_0.stop_propagation is False + assert handler_0.prevent_default is False + + handler_1 = EventHandler(target_id="uuid", prevent_default=True) + assert handler_1.target_id == "uuid" + assert handler_1.stop_propagation is False + assert handler_1.prevent_default is True + + handler_2 = EventHandler(target_id="uuid", stop_propagation=True) + assert handler_2.target_id == "uuid" + assert handler_2.stop_propagation is True + assert handler_2.prevent_default is False async def test_multiple_callbacks_per_event_handler(): From 1748bcee35db2abcc0d3cf63df34d1200e64b36a Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 2 Apr 2021 17:08:56 -0700 Subject: [PATCH 02/20] Rename validate_serialized_vdom to validate_vdom also add comment about why we inject extra validation in debug mode --- src/idom/core/layout.py | 8 ++++++-- src/idom/core/vdom.py | 2 +- tests/test_core/test_vdom.py | 6 +++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index beca8da22..fc3b9cde0 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -24,7 +24,7 @@ from .events import EventHandler, EventTarget from .hooks import LifeCycleHook from .utils import CannotAccessResource, HasAsyncResources, async_resource -from .vdom import validate_serialized_vdom +from .vdom import validate_vdom logger = getLogger(__name__) @@ -104,13 +104,17 @@ async def render(self) -> LayoutUpdate: return self._create_layout_update(component) if IDOM_DEBUG_MODE.get(): + # If in debug mode inject a function that ensures all returned updates + # contain valid VDOM models. We only do this in debug mode in order to + # avoid unnecessarily impacting performance. + _debug_render = render @wraps(_debug_render) async def render(self) -> LayoutUpdate: # Ensure that the model is valid VDOM on each render result = await self._debug_render() - validate_serialized_vdom(self._component_states[id(self.root)].model) + validate_vdom(self._component_states[id(self.root)].model) return result @async_resource diff --git a/src/idom/core/vdom.py b/src/idom/core/vdom.py index 3c5725649..70b72a89a 100644 --- a/src/idom/core/vdom.py +++ b/src/idom/core/vdom.py @@ -63,7 +63,7 @@ } -validate_serialized_vdom = compile_json_schema(VDOM_JSON_SCHEMA) +validate_vdom = compile_json_schema(VDOM_JSON_SCHEMA) class ImportSourceDict(TypedDict): diff --git a/tests/test_core/test_vdom.py b/tests/test_core/test_vdom.py index 735b46146..0b29c2063 100644 --- a/tests/test_core/test_vdom.py +++ b/tests/test_core/test_vdom.py @@ -2,7 +2,7 @@ from fastjsonschema import JsonSchemaException import idom -from idom.core.vdom import component, make_vdom_constructor, validate_serialized_vdom +from idom.core.vdom import component, make_vdom_constructor, validate_vdom fake_events = idom.Events() @@ -213,7 +213,7 @@ def MyComponentWithChildrenAndAttributes(children, x): ], ) def test_valid_vdom(value): - validate_serialized_vdom(value) + validate_vdom(value) @pytest.mark.parametrize( @@ -312,4 +312,4 @@ def test_valid_vdom(value): ) def test_invalid_vdom(value, error_message_pattern): with pytest.raises(JsonSchemaException, match=error_message_pattern): - validate_serialized_vdom(value) + validate_vdom(value) From 722d7fd1e64c7d9e6041473445cd17667cf13de0 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 3 Apr 2021 17:29:32 -0700 Subject: [PATCH 03/20] add 'key' to VDOM spec all makes event handler targets deterministic based on keys and child indices --- docs/source/core-concepts.rst | 15 +-- src/idom/core/component.py | 37 +++--- src/idom/core/events.py | 26 +--- src/idom/core/layout.py | 200 +++++++++++++++++------------ src/idom/core/vdom.py | 49 ++++--- src/idom/widgets/html.py | 21 ++- tests/general_utils.py | 21 ++- tests/test_core/test_dispatcher.py | 18 ++- tests/test_core/test_events.py | 9 +- tests/test_core/test_layout.py | 9 +- tests/test_core/test_vdom.py | 35 +---- 11 files changed, 207 insertions(+), 233 deletions(-) diff --git a/docs/source/core-concepts.rst b/docs/source/core-concepts.rst index b3aae76c1..9e338d870 100644 --- a/docs/source/core-concepts.rst +++ b/docs/source/core-concepts.rst @@ -77,27 +77,18 @@ have to re-render the layout and see what changed: from idom.core.layout import LayoutEvent - event_handler_id = "on-click" - - @idom.component def ClickCount(): count, set_count = idom.hooks.use_state(0) - - @idom.event(target_id=event_handler_id) # <-- trick to hard code event handler ID - def on_click(event): - set_count(count + 1) - return idom.html.button( - {"onClick": on_click}, + {"onClick": lambda event: set_count(count + 1)}, [f"Click count: {count}"], ) - - async with idom.Layout(ClickCount()) as layout: + async with idom.Layout(ClickCount(key="something")) as layout: patch_1 = await layout.render() - fake_event = LayoutEvent(event_handler_id, [{}]) + fake_event = LayoutEvent("something.onClick", [{}]) await layout.dispatch(fake_event) patch_2 = await layout.render() diff --git a/src/idom/core/component.py b/src/idom/core/component.py index 333e8fe86..3dfd2aaa0 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -1,15 +1,15 @@ +from __future__ import annotations + import abc import inspect from functools import wraps -from typing import TYPE_CHECKING, Any, Callable, Dict, Tuple, Union - +from typing import Any, Callable, Dict, Tuple, Union -if TYPE_CHECKING: # pragma: no cover - from .vdom import VdomDict # noqa +from .vdom import VdomDict ComponentConstructor = Callable[..., "AbstractComponent"] -ComponentRenderFunction = Callable[..., Union["AbstractComponent", "VdomDict"]] +ComponentRenderFunction = Callable[..., Union["AbstractComponent", VdomDict]] def component(function: ComponentRenderFunction) -> Callable[..., "Component"]: @@ -21,42 +21,47 @@ def component(function: ComponentRenderFunction) -> Callable[..., "Component"]: """ @wraps(function) - def constructor(*args: Any, **kwargs: Any) -> Component: - return Component(function, args, kwargs) + def constructor(*args: Any, key: str = "", **kwargs: Any) -> Component: + return Component(function, key, args, kwargs) return constructor class AbstractComponent(abc.ABC): - __slots__ = [] if hasattr(abc.ABC, "__weakref__") else ["__weakref__"] + __slots__ = ["key"] + if not hasattr(abc.ABC, "__weakref__"): + __slots__.append("__weakref__") # pragma: no cover + + key: str @abc.abstractmethod - def render(self) -> "VdomDict": + def render(self) -> VdomDict: """Render the component's :ref:`VDOM ` model.""" class Component(AbstractComponent): """An object for rending component models.""" - __slots__ = ( - "_function", - "_args", - "_kwargs", - ) + __slots__ = "_function", "_args", "_kwargs" def __init__( self, function: ComponentRenderFunction, + key: str, args: Tuple[Any, ...], kwargs: Dict[str, Any], ) -> None: + self.key = key self._function = function self._args = args self._kwargs = kwargs - def render(self) -> Any: - return self._function(*self._args, **self._kwargs) + def render(self) -> VdomDict: + model = self._function(*self._args, **self._kwargs) + if isinstance(model, AbstractComponent): + model = {"tagName": "div", "children": [model]} + return model def __repr__(self) -> str: sig = inspect.signature(self._function) diff --git a/src/idom/core/events.py b/src/idom/core/events.py index 79d7e93f0..3e23574bf 100644 --- a/src/idom/core/events.py +++ b/src/idom/core/events.py @@ -12,23 +12,15 @@ ) from anyio import create_task_group -from mypy_extensions import TypedDict EventsMapping = Union[Dict[str, Union["Callable[..., Any]", "EventHandler"]], "Events"] -class EventTarget(TypedDict): - target: str - preventDefault: bool # noqa - stopPropagation: bool # noqa - - def event( function: Optional[Callable[..., Any]] = None, stop_propagation: bool = False, prevent_default: bool = False, - target_id: Optional[str] = None, ) -> Union["EventHandler", Callable[[Callable[..., Any]], "EventHandler"]]: """Create an event handler function with extra functionality. @@ -49,12 +41,8 @@ def event( Block the event from propagating further up the DOM. prevent_default: Stops the default actional associate with the event from taking place. - target_id: - A unique ID used to locate this handler in the resulting VDOM. This is - automatically generated by default and is typically not set manually - except in testing. """ - handler = EventHandler(stop_propagation, prevent_default, target_id=target_id) + handler = EventHandler(stop_propagation, prevent_default) if function is not None: handler.add(function) return handler @@ -142,8 +130,6 @@ def __repr__(self) -> str: # pragma: no cover class EventHandler: """An object which defines an event handler. - Get a serialized reference to the handler via :meth:`Handler.serialize`. - The event handler object acts like a coroutine when called. Parameters: @@ -159,19 +145,16 @@ class EventHandler: "__weakref__", "_coro_handlers", "_func_handlers", - "target_id", "prevent_default", - "stop_propogation", + "stop_propagation", ) def __init__( self, stop_propagation: bool = False, prevent_default: bool = False, - target_id: Optional[str] = None, ) -> None: - self.target_id = target_id or str(id(self)) - self.stop_propogation = stop_propagation + self.stop_propagation = stop_propagation self.prevent_default = prevent_default self._coro_handlers: List[Callable[..., Coroutine[Any, Any, Any]]] = [] self._func_handlers: List[Callable[..., Any]] = [] @@ -216,6 +199,3 @@ def __contains__(self, function: Any) -> bool: return function in self._coro_handlers else: return function in self._func_handlers - - def __repr__(self) -> str: # pragma: no cover - return f"{type(self).__name__}({self.serialize()})" diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index fc3b9cde0..14ee78e5b 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -1,30 +1,21 @@ +from __future__ import annotations + import abc import asyncio from functools import wraps from logging import getLogger -from typing import ( - Any, - AsyncIterator, - Dict, - Iterator, - List, - Mapping, - NamedTuple, - Optional, - Set, - Tuple, - Union, -) +from typing import Any, AsyncIterator, Dict, Iterator, List, NamedTuple, Set, cast from jsonpatch import apply_patch, make_patch +from typing_extensions import TypedDict from idom.config import IDOM_DEBUG_MODE from .component import AbstractComponent -from .events import EventHandler, EventTarget +from .events import EventHandler from .hooks import LifeCycleHook from .utils import CannotAccessResource, HasAsyncResources, async_resource -from .vdom import validate_vdom +from .vdom import VdomDict, validate_vdom logger = getLogger(__name__) @@ -54,16 +45,6 @@ class LayoutEvent(NamedTuple): """A list of event data passed to the event handler.""" -class ComponentState(NamedTuple): - model: Dict[str, Any] - path: str - component_id: int - component_obj: AbstractComponent - event_handler_ids: Set[str] - child_component_ids: List[int] - life_cycle_hook: LifeCycleHook - - class Layout(HasAsyncResources): __slots__ = ["root", "_event_handlers"] @@ -124,8 +105,10 @@ async def _rendering_queue(self) -> AsyncIterator["_ComponentQueue"]: yield queue @async_resource - async def _component_states(self) -> AsyncIterator[Dict[int, ComponentState]]: - root_component_state = self._create_component_state(self.root, "", save=False) + async def _component_states(self) -> AsyncIterator[Dict[int, _ComponentState]]: + root_component_state = self._create_component_state( + self.root, "", "", save=False + ) try: yield {root_component_state.component_id: root_component_state} finally: @@ -155,105 +138,133 @@ def _create_layout_update(self, component: AbstractComponent) -> LayoutUpdate: ): state.life_cycle_hook.component_did_render() - return LayoutUpdate(path=component_state.path, changes=changes) + return LayoutUpdate(path=component_state.patch_path, changes=changes) - def _render_component(self, component_state: ComponentState) -> Dict[str, Any]: + def _render_component(self, component_state: _ComponentState) -> Dict[str, Any]: + component_obj = component_state.component_obj try: component_state.life_cycle_hook.set_current() try: - raw_model = component_state.component_obj.render() + raw_model = component_obj.render() finally: component_state.life_cycle_hook.unset_current() - if isinstance(raw_model, AbstractComponent): - raw_model = {"tagName": "div", "children": [raw_model]} - - resolved_model = self._render_model(component_state, raw_model) - component_state.model.clear() - component_state.model.update(resolved_model) + assert "key" not in raw_model, "Component must not return VDOM with a 'key'" + key = getattr(component_obj, "key", "") + if key: + raw_model = cast(VdomDict, {**raw_model, "key": key}) + + component_state.model.update( + self._render_model( + component_state, + raw_model, + component_state.patch_path, + component_state.key_path, + ) + ) except Exception as error: - logger.exception(f"Failed to render {component_state.component_obj}") + logger.exception(f"Failed to render {component_obj}") component_state.model.update({"tagName": "div", "__error__": str(error)}) # We need to return the model from the `component_state` so that the model - # between all `ComponentState` objects within a `Layout` are shared. + # between all `_ComponentState` objects within a `Layout` are shared. return component_state.model def _render_model( self, - component_state: ComponentState, - model: Mapping[str, Any], - path: Optional[str] = None, + component_state: _ComponentState, + model: VdomDict, + patch_path: str, + key_path: str, ) -> Dict[str, Any]: - if path is None: - path = component_state.path - serialized_model: Dict[str, Any] = {} - event_handlers = self._render_model_event_targets(component_state, model) + event_handlers = self._render_model_event_targets( + component_state, model, key_path + ) if event_handlers: serialized_model["eventHandlers"] = event_handlers if "children" in model: serialized_model["children"] = self._render_model_children( - component_state, model["children"], path + component_state, model, patch_path, key_path ) return {**model, **serialized_model} def _render_model_children( self, - component_state: ComponentState, - children: Union[List[Any], Tuple[Any, ...]], - path: str, + component_state: _ComponentState, + model: VdomDict, + patch_path: str, + key_path: str, ) -> List[Any]: + children = model["children"] resolved_children: List[Any] = [] for index, child in enumerate( children if isinstance(children, (list, tuple)) else [children] ): if isinstance(child, dict): - child_path = f"{path}/children/{index}" resolved_children.append( - self._render_model(component_state, child, child_path) + self._render_model( + component_state, + cast(VdomDict, child), + patch_path=f"{patch_path}/children/{index}", + key_path=f"{key_path}/{index}", + ) ) elif isinstance(child, AbstractComponent): - child_path = f"{path}/children/{index}" - child_state = self._create_component_state(child, child_path, save=True) - resolved_children.append(self._render_component(child_state)) + resolved_children.append( + self._render_component( + self._create_component_state( + child, + patch_path=f"{patch_path}/children/{index}", + parent_key_path=key_path, + save=True, + ) + ) + ) component_state.child_component_ids.append(id(child)) else: resolved_children.append(str(child)) return resolved_children def _render_model_event_targets( - self, component_state: ComponentState, model: Mapping[str, Any] - ) -> Dict[str, EventTarget]: - handlers: Dict[str, EventHandler] = {} + self, + component_state: _ComponentState, + model: VdomDict, + key_path: str, + ) -> Dict[str, _EventTarget]: + handlers_by_event: Dict[str, EventHandler] = {} if "eventHandlers" in model: - handlers.update(model["eventHandlers"]) + handlers_by_event.update(model["eventHandlers"]) + if "attributes" in model: attrs = model["attributes"] for k, v in list(attrs.items()): if callable(v): if not isinstance(v, EventHandler): - h = handlers[k] = EventHandler() + h = handlers_by_event[k] = EventHandler() h.add(attrs.pop(k)) else: h = attrs.pop(k) - handlers[k] = h + handlers_by_event[k] = h + + handlers_by_target: Dict[str, EventHandler] = {} + model_event_targets: Dict[str, _EventTarget] = {} + for event, handler in handlers_by_event.items(): + target = f"{key_path}.{event}" + handlers_by_target[target] = handler + model_event_targets[event] = { + "target": target, + "preventDefault": handler.prevent_default, + "stopPropagation": handler.stop_propagation, + } - event_handlers_by_id = {h.target_id: h for h in handlers.values()} component_state.event_handler_ids.clear() - component_state.event_handler_ids.update(event_handlers_by_id) - self._event_handlers.update(event_handlers_by_id) - - return { - e: { - "target": h.target_id, - "preventDefault": h.prevent_default, - "stopPropagation": h.stop_propogation, - } - for e, h in handlers.items() - } + component_state.event_handler_ids.update(handlers_by_target) + self._event_handlers.update(handlers_by_target) - def _get_component_state(self, component: AbstractComponent) -> ComponentState: + return model_event_targets + + def _get_component_state(self, component: AbstractComponent) -> _ComponentState: return self._component_states[id(component)] def _has_component_state(self, component: AbstractComponent) -> bool: @@ -262,13 +273,15 @@ def _has_component_state(self, component: AbstractComponent) -> bool: def _create_component_state( self, component: AbstractComponent, - path: str, + patch_path: str, + parent_key_path: str, save: bool, - ) -> ComponentState: + ) -> _ComponentState: component_id = id(component) - state = ComponentState( + state = _ComponentState( model={}, - path=path, + patch_path=patch_path, + key_path=f"{parent_key_path}/{_get_component_key(component)}", component_id=component_id, component_obj=component, event_handler_ids=set(), @@ -279,28 +292,30 @@ def _create_component_state( self._component_states[component_id] = state return state - def _delete_component_state(self, component_state: ComponentState) -> None: + def _delete_component_state(self, component_state: _ComponentState) -> None: self._clear_component_state_event_handlers(component_state) self._delete_component_state_children(component_state) del self._component_states[component_state.component_id] def _clear_component_state_event_handlers( - self, component_state: ComponentState + self, component_state: _ComponentState ) -> None: for handler_id in component_state.event_handler_ids: del self._event_handlers[handler_id] component_state.event_handler_ids.clear() - def _delete_component_state_children(self, component_state: ComponentState) -> None: + def _delete_component_state_children( + self, component_state: _ComponentState + ) -> None: for e_id in component_state.child_component_ids: self._delete_component_state(self._component_states[e_id]) component_state.child_component_ids.clear() def _iter_component_states_from_root( self, - root_component_state: ComponentState, + root_component_state: _ComponentState, include_root: bool, - ) -> Iterator[ComponentState]: + ) -> Iterator[_ComponentState]: if include_root: pending = [root_component_state] else: @@ -321,6 +336,27 @@ def __repr__(self) -> str: return f"{type(self).__name__}({self.root})" +class _ComponentState(NamedTuple): + model: Dict[str, Any] + patch_path: str + key_path: str + component_id: int + component_obj: AbstractComponent + event_handler_ids: Set[str] + child_component_ids: List[int] + life_cycle_hook: LifeCycleHook + + +class _EventTarget(TypedDict): + target: str + preventDefault: bool # noqa + stopPropagation: bool # noqa + + +def _get_component_key(component: AbstractComponent) -> str: + return getattr(component, "key", "") or hex(id(component))[2:] + + class _ComponentQueue: __slots__ = "_loop", "_queue", "_pending" diff --git a/src/idom/core/vdom.py b/src/idom/core/vdom.py index 70b72a89a..1cb511ffe 100644 --- a/src/idom/core/vdom.py +++ b/src/idom/core/vdom.py @@ -1,11 +1,12 @@ -from typing import Any, Callable, Dict, Iterable, List, Mapping, Optional, Tuple, Union +from __future__ import annotations + +from typing import Any, Dict, Iterable, List, Mapping, Optional, Sequence, Tuple, Union from fastjsonschema import compile as compile_json_schema from mypy_extensions import TypedDict from typing_extensions import Protocol -from .component import AbstractComponent -from .events import EventsMapping +from .events import EventHandler VDOM_JSON_SCHEMA = { @@ -16,6 +17,7 @@ "type": "object", "properties": { "tagName": {"type": "string"}, + "key": {"type": "string"}, "children": {"$ref": "#/definitions/elementChildren"}, "attributes": {"type": "object"}, "eventHandlers": {"$ref": "#/definitions/elementEventHandlers"}, @@ -72,9 +74,10 @@ class ImportSourceDict(TypedDict): class _VdomDictOptional(TypedDict, total=False): - children: Union[List[Any], Tuple[Any, ...]] # noqa + key: str # noqa + children: Sequence[Any] # noqa attributes: Dict[str, Any] # noqa - eventHandlers: EventsMapping # noqa + eventHandlers: Mapping[str, EventHandler] # noqa importSource: ImportSourceDict # noqa @@ -86,31 +89,15 @@ class VdomDict(_VdomDictRequired, _VdomDictOptional): """A VDOM dictionary""" -_TagArg = str -_ComponentFunc = Callable[..., Union[VdomDict, AbstractComponent]] _AttributesAndChildrenArg = Union[Mapping[str, Any], str, Iterable[Any], Any] -_EventHandlersArg = Optional[EventsMapping] +_EventHandlersArg = Optional[Mapping[str, EventHandler]] _ImportSourceArg = Optional[ImportSourceDict] -def component( - tag: Union[_TagArg, _ComponentFunc], - *attributes_and_children: _AttributesAndChildrenArg, -) -> Union[VdomDict, AbstractComponent]: - if isinstance(tag, str): - return vdom(tag, *attributes_and_children) - - attributes, children = _coalesce_attributes_and_children(attributes_and_children) - - if children: - return tag(children=children, **attributes) - else: - return tag(**attributes) - - def vdom( - tag: _TagArg, + tag: str, *attributes_and_children: _AttributesAndChildrenArg, + key: str = "", event_handlers: _EventHandlersArg = None, import_source: _ImportSourceArg = None, ) -> VdomDict: @@ -123,6 +110,11 @@ def vdom( The attributes **must** precede the children, though you may pass multiple sets of attributes, or children which will be merged into their respective parts of the model. + key: + A string idicating the identity of a particular element. This is significant + to preserve event handlers across updates - without a key, a re-render would + cause these handlers to be deleted, but with a key, they would be redirected + to any newly defined handlers. event_handlers: Maps event types to coroutines that are responsible for handling those events. import_source: @@ -131,7 +123,7 @@ def vdom( """ model: VdomDict = {"tagName": tag} - attributes, children = _coalesce_attributes_and_children(attributes_and_children) + attributes, children = coalesce_attributes_and_children(attributes_and_children) if attributes: model["attributes"] = attributes @@ -139,6 +131,9 @@ def vdom( if children: model["children"] = children + if key: + model["key"] = key + if event_handlers is not None: model["eventHandlers"] = event_handlers @@ -166,12 +161,14 @@ def make_vdom_constructor(tag: str, allow_children: bool = True) -> VdomDictCons def constructor( *attributes_and_children: _AttributesAndChildrenArg, + key: str = "", event_handlers: _EventHandlersArg = None, import_source: _ImportSourceArg = None, ) -> VdomDict: model = vdom( tag, *attributes_and_children, + key=key, event_handlers=event_handlers, import_source=import_source, ) @@ -189,7 +186,7 @@ def constructor( return constructor -def _coalesce_attributes_and_children( +def coalesce_attributes_and_children( attributes_and_children: _AttributesAndChildrenArg, ) -> Tuple[Dict[str, Any], List[Any]]: attributes: Dict[str, Any] = {} diff --git a/src/idom/widgets/html.py b/src/idom/widgets/html.py index 1faf00f2e..423276508 100644 --- a/src/idom/widgets/html.py +++ b/src/idom/widgets/html.py @@ -2,11 +2,13 @@ from typing import Any, Callable, Dict, Optional, Union import idom +from idom.core.component import AbstractComponent, ComponentConstructor from idom.core.vdom import ( VdomDict, VdomDictConstructor, - component, + coalesce_attributes_and_children, make_vdom_constructor, + vdom, ) @@ -69,8 +71,6 @@ class Html: All constructors return :class:`~idom.core.vdom.VdomDict`. """ - __call__ = staticmethod(component) - def __init__(self) -> None: # External sources self.link = make_vdom_constructor("link", allow_children=False) @@ -166,6 +166,21 @@ def __init__(self) -> None: self.menuitem = make_vdom_constructor("menuitem") self.summary = make_vdom_constructor("summary") + @staticmethod + def __call__( + tag: Union[str, ComponentConstructor], + *attributes_and_children: Any, + ) -> Union[VdomDict, AbstractComponent]: + if isinstance(tag, str): + return vdom(tag, *attributes_and_children) + + attributes, children = coalesce_attributes_and_children(attributes_and_children) + + if children: + return tag(children=children, **attributes) + else: + return tag(**attributes) + def __getattr__(self, tag: str) -> VdomDictConstructor: return make_vdom_constructor(tag) diff --git a/tests/general_utils.py b/tests/general_utils.py index f49a9b3ff..6d94737e2 100644 --- a/tests/general_utils.py +++ b/tests/general_utils.py @@ -1,3 +1,5 @@ +# dialect=pytest + from contextlib import contextmanager from functools import wraps from weakref import ref @@ -20,7 +22,7 @@ def patch_slots_object(obj, attr, new_value): class HookCatcher: """Utility for capturing a LifeCycleHook from a component - Example: + Eleftample: .. code-block:: component_hook = HookCatcher() @@ -55,15 +57,8 @@ def schedule_render(self) -> None: self.current.schedule_render() -def assert_same_items(x, y): - """Check that two unordered sequences are equal""" - - list_x = list(x) - list_y = list(y) - - assert len(x) == len(y), f"len({x}) != len({y})" - assert all( - # this is not very efficient unfortunately so don't compare anything large - list_x.count(value) == list_y.count(value) - for value in list_x - ), f"{x} != {y}" +def assert_same_items(left, right): + """Check that two unordered sequences are equal (only works if reprs are equal)""" + sorted_left = list(sorted(left, key=repr)) + sorted_right = list(sorted(right, key=repr)) + assert sorted_left == sorted_right diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index 5c3e62509..f7015ee49 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -17,7 +17,9 @@ async def test_shared_state_dispatcher(): done = asyncio.Event() changes_1 = [] changes_2 = [] - target_id = "an-event" + key = "test-element" + event_name = "onEvent" + target_id = f"/{key}.{event_name}" events_to_inject = [LayoutEvent(target=target_id, data=[])] * 4 @@ -42,14 +44,9 @@ async def recv_2(): @idom.component def Clickable(): count, set_count = idom.hooks.use_state(0) + return idom.html.div({event_name: lambda: set_count(count + 1), "count": count}) - @idom.event(target_id=target_id) - async def an_event(): - set_count(count + 1) - - return idom.html.div({"anEvent": an_event, "count": count}) - - async with SharedViewDispatcher(Layout(Clickable())) as dispatcher: + async with SharedViewDispatcher(Layout(Clickable(key=key))) as dispatcher: await dispatcher.run(send_1, recv_1, "1") await dispatcher.run(send_2, recv_2, "2") @@ -59,8 +56,8 @@ async def an_event(): "op": "add", "path": "/eventHandlers", "value": { - "anEvent": { - "target": "an-event", + event_name: { + "target": target_id, "preventDefault": False, "stopPropagation": False, } @@ -68,6 +65,7 @@ async def an_event(): }, {"op": "add", "path": "/attributes", "value": {"count": 0}}, {"op": "add", "path": "/tagName", "value": "div"}, + {"op": "add", "path": "/key", "value": key}, ], [{"op": "replace", "path": "/attributes/count", "value": 1}], [{"op": "replace", "path": "/attributes/count", "value": 2}], diff --git a/tests/test_core/test_events.py b/tests/test_core/test_events.py index 730e3d7bb..2cada485d 100644 --- a/tests/test_core/test_events.py +++ b/tests/test_core/test_events.py @@ -40,18 +40,15 @@ async def handler_2(): def test_event_handler_props(): - handler_0 = EventHandler(target_id="uuid") - assert handler_0.target_id == "uuid" + handler_0 = EventHandler() assert handler_0.stop_propagation is False assert handler_0.prevent_default is False - handler_1 = EventHandler(target_id="uuid", prevent_default=True) - assert handler_1.target_id == "uuid" + handler_1 = EventHandler(prevent_default=True) assert handler_1.stop_propagation is False assert handler_1.prevent_default is True - handler_2 = EventHandler(target_id="uuid", stop_propagation=True) - assert handler_2.target_id == "uuid" + handler_2 = EventHandler(stop_propagation=True) assert handler_2.stop_propagation is True assert handler_2.prevent_default is False diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index cf5c42e3f..6a5c30aad 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -178,14 +178,7 @@ def Outer(): component = idom.hooks.current_hook().component live_components.add(id(component)) finalize(component, live_components.remove, id(component)) - - hook = idom.hooks.current_hook() - - @idom.event(target_id="force-update") - async def force_update(): - hook.schedule_render() - - return idom.html.div({"onEvent": force_update}, Inner()) + return Inner() @idom.component def Inner(): diff --git a/tests/test_core/test_vdom.py b/tests/test_core/test_vdom.py index 0b29c2063..b5738d0cb 100644 --- a/tests/test_core/test_vdom.py +++ b/tests/test_core/test_vdom.py @@ -2,7 +2,7 @@ from fastjsonschema import JsonSchemaException import idom -from idom.core.vdom import component, make_vdom_constructor, validate_vdom +from idom.core.vdom import make_vdom_constructor, validate_vdom fake_events = idom.Events() @@ -118,39 +118,6 @@ def test_make_vdom_constructor(): assert no_children() == {"tagName": "no-children"} -def test_vdom_component(): - def MyComponentWithAttributes(x, y): - return idom.html.div({"x": x * 2, "y": y * 2}) - - assert component(MyComponentWithAttributes, {"x": 1}, {"y": 2}) == { - "tagName": "div", - "attributes": {"x": 2, "y": 4}, - } - - with pytest.raises(TypeError, match="unexpected keyword argument 'children'"): - assert component(MyComponentWithAttributes, "a-child") - - def MyComponentWithChildren(children): - return idom.html.div(children + ["world"]) - - assert component(MyComponentWithChildren, "hello") == { - "tagName": "div", - "children": ["hello", "world"], - } - - with pytest.raises(TypeError, match="unexpected keyword argument"): - assert component(MyComponentWithAttributes, {"an-attribute": 1}) - - def MyComponentWithChildrenAndAttributes(children, x): - return idom.html.div({"x": x * 2}, children + ["world"]) - - assert component(MyComponentWithChildrenAndAttributes, {"x": 2}, "hello") == { - "tagName": "div", - "attributes": {"x": 4}, - "children": ["hello", "world"], - } - - @pytest.mark.parametrize( "value", [ From ea645e057da0a07f9d59eb7c173d696e8507c5af Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 4 Apr 2021 00:26:27 -0700 Subject: [PATCH 04/20] add tests for callback identity preservation with keys --- docs/source/core-concepts.rst | 8 +-- src/idom/core/layout.py | 4 +- tests/test_core/test_dispatcher.py | 2 +- tests/test_core/test_layout.py | 84 ++++++++++++++++++++++++++++++ 4 files changed, 92 insertions(+), 6 deletions(-) diff --git a/docs/source/core-concepts.rst b/docs/source/core-concepts.rst index 9e338d870..9d00bdbe5 100644 --- a/docs/source/core-concepts.rst +++ b/docs/source/core-concepts.rst @@ -88,7 +88,7 @@ have to re-render the layout and see what changed: async with idom.Layout(ClickCount(key="something")) as layout: patch_1 = await layout.render() - fake_event = LayoutEvent("something.onClick", [{}]) + fake_event = LayoutEvent("/something/onClick", [{}]) await layout.dispatch(fake_event) patch_2 = await layout.render() @@ -129,7 +129,7 @@ callback that's called by the dispatcher to events it should execute. async def recv(): - event = LayoutEvent(event_handler_id, [{}]) + event = LayoutEvent("/my-component/onClick", [{}]) # We need this so we don't flood the render loop with events. # In practice this is never an issue since events won't arrive @@ -139,7 +139,9 @@ callback that's called by the dispatcher to events it should execute. return event - async with SingleViewDispatcher(idom.Layout(ClickCount())) as dispatcher: + async with SingleViewDispatcher( + idom.Layout(ClickCount(key="my-component")) + ) as dispatcher: context = None # see note below await dispatcher.run(send, recv, context) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 14ee78e5b..3d9327134 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -207,7 +207,7 @@ def _render_model_children( component_state, cast(VdomDict, child), patch_path=f"{patch_path}/children/{index}", - key_path=f"{key_path}/{index}", + key_path=f"{key_path}/{child.get('key') or hex(id(child))[2:]}", ) ) elif isinstance(child, AbstractComponent): @@ -250,7 +250,7 @@ def _render_model_event_targets( handlers_by_target: Dict[str, EventHandler] = {} model_event_targets: Dict[str, _EventTarget] = {} for event, handler in handlers_by_event.items(): - target = f"{key_path}.{event}" + target = f"{key_path}/{event}" handlers_by_target[target] = handler model_event_targets[event] = { "target": target, diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index f7015ee49..487c24a49 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -19,7 +19,7 @@ async def test_shared_state_dispatcher(): changes_2 = [] key = "test-element" event_name = "onEvent" - target_id = f"/{key}.{event_name}" + target_id = f"/{key}/{event_name}" events_to_inject = [LayoutEvent(target=target_id, data=[])] * 4 diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 6a5c30aad..cd13ca69e 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -275,3 +275,87 @@ def SomeComponent(): "Ignored event - handler 'missing' does not exist or its component unmounted", next(iter(caplog.records)).msg, ) + + +def use_toggle(init=False): + state, set_state = idom.hooks.use_state(init) + return state, lambda: set_state(lambda old: not old) + + +async def test_model_key_preserves_callback_identity_for_common_elements(): + called_good_trigger = idom.Ref(False) + + @idom.component + def MyComponent(): + reverse_children, set_reverse_children = use_toggle() + + def good_trigger(): + called_good_trigger.current = True + set_reverse_children() + + def bad_trigger(): + raise ValueError("Called bad trigger") + + children = [ + idom.html.button( + {"onClick": good_trigger, "id": "good"}, "good", key="good" + ), + idom.html.button({"onClick": bad_trigger, "id": "bad"}, "bad", key="bad"), + ] + + if reverse_children: + children.reverse() + + return idom.html.div(children) + + async with idom.Layout(MyComponent(key="component")) as layout: + await layout.render() + for i in range(3): + event = LayoutEvent("/component/good/onClick", []) + await layout.dispatch(event) + + assert called_good_trigger.current + # reset after checking + called_good_trigger.current = False + + await layout.render() + + +async def test_model_key_preserves_callback_identity_for_components(): + called_good_trigger = idom.Ref(False) + + @idom.component + def RootComponent(): + reverse_children, set_reverse_children = use_toggle() + + children = [ + Trigger(name, set_reverse_children, key=name) for name in ["good", "bad"] + ] + + if reverse_children: + children.reverse() + + return idom.html.div(children) + + @idom.component + def Trigger(name, set_reverse_children): + def callback(): + if name == "good": + called_good_trigger.current = True + set_reverse_children() + else: + raise ValueError("Called bad trigger") + + return idom.html.button({"onClick": callback, "id": "good"}, "good") + + async with idom.Layout(RootComponent(key="root")) as layout: + await layout.render() + for i in range(3): + event = LayoutEvent("/root/good/onClick", []) + await layout.dispatch(event) + + assert called_good_trigger.current + # reset after checking + called_good_trigger.current = False + + await layout.render() From 95addac7f2d82ba1a1a8352bc9d66160fe7bd4f2 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 4 Apr 2021 00:48:26 -0700 Subject: [PATCH 05/20] minor doc updates --- docs/source/core-concepts.rst | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/source/core-concepts.rst b/docs/source/core-concepts.rst index 9d00bdbe5..a67bd13c9 100644 --- a/docs/source/core-concepts.rst +++ b/docs/source/core-concepts.rst @@ -66,11 +66,11 @@ ever be removed from the model. Then you'll just need to call and await a async with idom.Layout(ClickCount()) as layout: patch = await layout.render() -The layout also handles the triggering of event handlers. Normally this is done -automatically by a :ref:`Dispatcher `, but for now we'll do it manually. -We can use a trick to hard-code the ``event_handler_id`` so we can pass it, and a fake -event, to the layout's :meth:`~idom.core.layout.Layout.dispatch` method. Then we just -have to re-render the layout and see what changed: +The layout also handles the triggering of event handlers. Normally these are +automatically sent to a :ref:`Dispatcher `, but for now we'll do it +manually. To do this we need to pass a fake event with its "target" (event handler +identifier), to the layout's :meth:`~idom.core.layout.Layout.dispatch` method, after +which we can re-render and see what changed: .. testcode:: @@ -85,10 +85,10 @@ have to re-render the layout and see what changed: [f"Click count: {count}"], ) - async with idom.Layout(ClickCount(key="something")) as layout: + async with idom.Layout(ClickCount(key="test-component")) as layout: patch_1 = await layout.render() - fake_event = LayoutEvent("/something/onClick", [{}]) + fake_event = LayoutEvent(target="/test-component/onClick", data=[{}]) await layout.dispatch(fake_event) patch_2 = await layout.render() @@ -98,6 +98,12 @@ have to re-render the layout and see what changed: assert count_did_increment +.. note:: + + Don't worry about the format of the layout event's ``target``. Its an internal + detail of the layout's implementation that is neither neccessary to understanding + how things work, nor is it part of the interface clients should rely on. + Layout Dispatcher ----------------- @@ -129,7 +135,7 @@ callback that's called by the dispatcher to events it should execute. async def recv(): - event = LayoutEvent("/my-component/onClick", [{}]) + event = LayoutEvent(target="/test-component/onClick", data=[{}]) # We need this so we don't flood the render loop with events. # In practice this is never an issue since events won't arrive @@ -140,7 +146,7 @@ callback that's called by the dispatcher to events it should execute. async with SingleViewDispatcher( - idom.Layout(ClickCount(key="my-component")) + idom.Layout(ClickCount(key="test-component")) ) as dispatcher: context = None # see note below await dispatcher.run(send, recv, context) From abfd2e77e04368496cbbaf5c07dd95f6b75067df Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 7 Apr 2021 19:19:31 -0700 Subject: [PATCH 06/20] add element and component identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At the moment, whenever a particular branch of a layout is updated, all the state (i.e. hooks and event handlers) that live within that branch are thrown away and reconstructed. Given IDOM’s current implementation for Layout this is unavoidable. To resolve this issue, we need to add a concept of “keys” which can be used to indicate the identity of an element within the layout structure. For example, if you have a list of elements that get re-ordered when a button is pressed, their state should be preserved, and reassigned to their new location in the layout. By default React requires keys to be used to communicate element identity whenever the order or number of simbling elements can change. While there are clear performance benefits for adhering to this stipulation, it’s often confusing for new developers. React has the further advantage of the JSX syntax, which makes it easier for the program to determine exactly when keys must be supplied by the user. To make the experience for new users simpler, and due to a lack of inforcement via JSX, IDOM will be to assume that any element without a key is new. Thus, for IDOM, keys will primarilly be a tool for optimization rather than a functional requirement. --- .github/workflows/test.yml | 2 +- docs/source/core-concepts.rst | 10 +- requirements/check-style.txt | 1 + src/idom/core/component.py | 7 +- src/idom/core/hooks.py | 7 +- src/idom/core/layout.py | 452 ++++++++++++++++------------- src/idom/core/vdom.py | 2 +- src/idom/widgets/html.py | 4 +- tests/test_core/test_dispatcher.py | 6 +- tests/test_core/test_hooks.py | 8 +- tests/test_core/test_layout.py | 97 ++++++- 11 files changed, 364 insertions(+), 232 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 93b003403..d1eec3883 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -11,7 +11,7 @@ on: - cron: "0 0 * * *" jobs: - coverage: + test-coverage: runs-on: ubuntu-latest steps: - uses: actions/checkout@v2 diff --git a/docs/source/core-concepts.rst b/docs/source/core-concepts.rst index a67bd13c9..76bec6b08 100644 --- a/docs/source/core-concepts.rst +++ b/docs/source/core-concepts.rst @@ -85,10 +85,10 @@ which we can re-render and see what changed: [f"Click count: {count}"], ) - async with idom.Layout(ClickCount(key="test-component")) as layout: + async with idom.Layout(ClickCount()) as layout: patch_1 = await layout.render() - fake_event = LayoutEvent(target="/test-component/onClick", data=[{}]) + fake_event = LayoutEvent(target="/onClick", data=[{}]) await layout.dispatch(fake_event) patch_2 = await layout.render() @@ -135,7 +135,7 @@ callback that's called by the dispatcher to events it should execute. async def recv(): - event = LayoutEvent(target="/test-component/onClick", data=[{}]) + event = LayoutEvent(target="/onClick", data=[{}]) # We need this so we don't flood the render loop with events. # In practice this is never an issue since events won't arrive @@ -145,9 +145,7 @@ callback that's called by the dispatcher to events it should execute. return event - async with SingleViewDispatcher( - idom.Layout(ClickCount(key="test-component")) - ) as dispatcher: + async with SingleViewDispatcher(idom.Layout(ClickCount())) as dispatcher: context = None # see note below await dispatcher.run(send, recv, context) diff --git a/requirements/check-style.txt b/requirements/check-style.txt index 9bd0ead39..1a15e1860 100644 --- a/requirements/check-style.txt +++ b/requirements/check-style.txt @@ -1,5 +1,6 @@ black flake8 +flake8-print pep8-naming flake8-idom-hooks >=0.4.0 isort >=5.7.0 diff --git a/src/idom/core/component.py b/src/idom/core/component.py index 3dfd2aaa0..ca68893be 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -21,8 +21,8 @@ def component(function: ComponentRenderFunction) -> Callable[..., "Component"]: """ @wraps(function) - def constructor(*args: Any, key: str = "", **kwargs: Any) -> Component: - return Component(function, key, args, kwargs) + def constructor(*args: Any, **kwargs: Any) -> Component: + return Component(function, args, kwargs) return constructor @@ -48,11 +48,10 @@ class Component(AbstractComponent): def __init__( self, function: ComponentRenderFunction, - key: str, args: Tuple[Any, ...], kwargs: Dict[str, Any], ) -> None: - self.key = key + self.key = kwargs.get("key", "") self._function = function self._args = args self._kwargs = kwargs diff --git a/src/idom/core/hooks.py b/src/idom/core/hooks.py index bfb062e4c..4a48a0b5a 100644 --- a/src/idom/core/hooks.py +++ b/src/idom/core/hooks.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import asyncio from logging import getLogger from threading import get_ident as get_thread_id @@ -20,6 +22,7 @@ from typing_extensions import Protocol +import idom from idom.utils import Ref from .component import AbstractComponent @@ -384,10 +387,10 @@ class LifeCycleHook: def __init__( self, component: AbstractComponent, - schedule_render: Callable[[AbstractComponent], None], + layout: idom.core.layout.Layout, ) -> None: self.component = component - self._schedule_render_callback = schedule_render + self._schedule_render_callback = layout.update self._schedule_render_later = False self._is_rendering = False self._rendered_atleast_once = False diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 3d9327134..ce55eb13a 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -4,7 +4,18 @@ import asyncio from functools import wraps from logging import getLogger -from typing import Any, AsyncIterator, Dict, Iterator, List, NamedTuple, Set, cast +from typing import ( + Any, + AsyncIterator, + Dict, + Iterator, + List, + NamedTuple, + Optional, + Set, + Tuple, +) +from weakref import ReferenceType, ref from jsonpatch import apply_patch, make_patch from typing_extensions import TypedDict @@ -15,7 +26,7 @@ from .events import EventHandler from .hooks import LifeCycleHook from .utils import CannotAccessResource, HasAsyncResources, async_resource -from .vdom import VdomDict, validate_vdom +from .vdom import validate_vdom logger = getLogger(__name__) @@ -81,7 +92,7 @@ async def dispatch(self, event: LayoutEvent) -> None: async def render(self) -> LayoutUpdate: while True: component = await self._rendering_queue.get() - if self._has_component_state(component): + if id(component) in self._model_state_by_component_id: return self._create_layout_update(component) if IDOM_DEBUG_MODE.get(): @@ -95,149 +106,96 @@ async def render(self) -> LayoutUpdate: async def render(self) -> LayoutUpdate: # Ensure that the model is valid VDOM on each render result = await self._debug_render() - validate_vdom(self._component_states[id(self.root)].model) + validate_vdom(self._model_state_by_component_id[id(self.root)].model) return result @async_resource - async def _rendering_queue(self) -> AsyncIterator["_ComponentQueue"]: + async def _rendering_queue(self) -> AsyncIterator[_ComponentQueue]: queue = _ComponentQueue() queue.put(self.root) yield queue @async_resource - async def _component_states(self) -> AsyncIterator[Dict[int, _ComponentState]]: - root_component_state = self._create_component_state( - self.root, "", "", save=False - ) - try: - yield {root_component_state.component_id: root_component_state} - finally: - self._delete_component_state(root_component_state) + async def _model_state_by_component_id( + self, + ) -> AsyncIterator[Dict[int, _ModelState]]: + root_state = _ModelState(None, -1, "", LifeCycleHook(self.root, self)) + yield {id(self.root): root_state} + self._unmount_model_states([root_state]) def _create_layout_update(self, component: AbstractComponent) -> LayoutUpdate: - component_state = self._get_component_state(component) - - component_state.life_cycle_hook.component_will_render() + old_state = self._model_state_by_component_id[id(component)] + new_state = old_state.new() - for state in self._iter_component_states_from_root( - component_state, - include_root=False, - ): - state.life_cycle_hook.component_will_unmount() + self._render_component(old_state, new_state, component) + changes = make_patch(getattr(old_state, "model", {}), new_state.model).patch - self._clear_component_state_event_handlers(component_state) - self._delete_component_state_children(component_state) + # replace model and state in parent + if hasattr(new_state, "parent_ref"): + parent = new_state.parent_ref() + if parent is not None: + parent.children_by_key[new_state.key] = new_state + parent.model["children"][new_state.index] = new_state.model - old_model = component_state.model.copy() # we copy because it will be mutated - new_model = self._render_component(component_state) - changes = make_patch(old_model, new_model).patch + # replace in global state + self._model_state_by_component_id[id(component)] = new_state - for state in self._iter_component_states_from_root( - component_state, - include_root=True, - ): - state.life_cycle_hook.component_did_render() + # hook effects must run after the update is complete + for state in new_state.iter_children(): + if hasattr(state, "life_cycle_hook"): + state.life_cycle_hook.component_did_render() - return LayoutUpdate(path=component_state.patch_path, changes=changes) + return LayoutUpdate(path=new_state.patch_path, changes=changes) - def _render_component(self, component_state: _ComponentState) -> Dict[str, Any]: - component_obj = component_state.component_obj + def _render_component( + self, + old_state: Optional[_ModelState], + new_state: _ModelState, + component: AbstractComponent, + ) -> None: + life_cycle_hook = new_state.life_cycle_hook + life_cycle_hook.component_will_render() try: - component_state.life_cycle_hook.set_current() + life_cycle_hook.set_current() try: - raw_model = component_obj.render() + raw_model = component.render() finally: - component_state.life_cycle_hook.unset_current() - - assert "key" not in raw_model, "Component must not return VDOM with a 'key'" - key = getattr(component_obj, "key", "") - if key: - raw_model = cast(VdomDict, {**raw_model, "key": key}) - - component_state.model.update( - self._render_model( - component_state, - raw_model, - component_state.patch_path, - component_state.key_path, - ) - ) + life_cycle_hook.unset_current() + self._render_model(old_state, new_state, raw_model) except Exception as error: - logger.exception(f"Failed to render {component_obj}") - component_state.model.update({"tagName": "div", "__error__": str(error)}) - - # We need to return the model from the `component_state` so that the model - # between all `_ComponentState` objects within a `Layout` are shared. - return component_state.model + logger.exception(f"Failed to render {component}") + new_state.model = {"tagName": "__error__", "children": [str(error)]} def _render_model( self, - component_state: _ComponentState, - model: VdomDict, - patch_path: str, - key_path: str, - ) -> Dict[str, Any]: - serialized_model: Dict[str, Any] = {} - event_handlers = self._render_model_event_targets( - component_state, model, key_path - ) - if event_handlers: - serialized_model["eventHandlers"] = event_handlers - if "children" in model: - serialized_model["children"] = self._render_model_children( - component_state, model, patch_path, key_path - ) - return {**model, **serialized_model} + old_state: Optional[_ModelState], + new_state: _ModelState, + raw_model: Any, + ) -> None: + new_state.model = {"tagName": raw_model["tagName"]} - def _render_model_children( - self, - component_state: _ComponentState, - model: VdomDict, - patch_path: str, - key_path: str, - ) -> List[Any]: - children = model["children"] - resolved_children: List[Any] = [] - for index, child in enumerate( - children if isinstance(children, (list, tuple)) else [children] - ): - if isinstance(child, dict): - resolved_children.append( - self._render_model( - component_state, - cast(VdomDict, child), - patch_path=f"{patch_path}/children/{index}", - key_path=f"{key_path}/{child.get('key') or hex(id(child))[2:]}", - ) - ) - elif isinstance(child, AbstractComponent): - resolved_children.append( - self._render_component( - self._create_component_state( - child, - patch_path=f"{patch_path}/children/{index}", - parent_key_path=key_path, - save=True, - ) - ) - ) - component_state.child_component_ids.append(id(child)) - else: - resolved_children.append(str(child)) - return resolved_children + self._render_model_attributes(old_state, new_state, raw_model) + self._render_model_children(old_state, new_state, raw_model.get("children", [])) + + if "key" in raw_model: + new_state.model["key"] = raw_model["key"] + if "importSource" in raw_model: + new_state.model["importSource"] = raw_model["importSource"] - def _render_model_event_targets( + def _render_model_attributes( self, - component_state: _ComponentState, - model: VdomDict, - key_path: str, - ) -> Dict[str, _EventTarget]: + old_state: Optional[_ModelState], + new_state: _ModelState, + raw_model: Dict[str, Any], + ) -> None: + # extract event handlers from 'eventHandlers' and 'attributes' handlers_by_event: Dict[str, EventHandler] = {} - if "eventHandlers" in model: - handlers_by_event.update(model["eventHandlers"]) - if "attributes" in model: - attrs = model["attributes"] + if "eventHandlers" in raw_model: + handlers_by_event.update(raw_model["eventHandlers"]) + + if "attributes" in raw_model: + attrs = new_state.model["attributes"] = raw_model["attributes"].copy() for k, v in list(attrs.items()): if callable(v): if not isinstance(v, EventHandler): @@ -248,9 +206,9 @@ def _render_model_event_targets( handlers_by_event[k] = h handlers_by_target: Dict[str, EventHandler] = {} - model_event_targets: Dict[str, _EventTarget] = {} + model_event_targets: Dict[str, _ModelEventTarget] = {} for event, handler in handlers_by_event.items(): - target = f"{key_path}/{event}" + target = f"{new_state.key_path}/{event}" handlers_by_target[target] = handler model_event_targets[event] = { "target": target, @@ -258,103 +216,205 @@ def _render_model_event_targets( "stopPropagation": handler.stop_propagation, } - component_state.event_handler_ids.clear() - component_state.event_handler_ids.update(handlers_by_target) - self._event_handlers.update(handlers_by_target) + if old_state is not None: + for old_target in set(old_state.event_targets).difference( + handlers_by_target + ): + del self._event_handlers[old_target] - return model_event_targets + if model_event_targets: + new_state.event_targets.update(handlers_by_target) + self._event_handlers.update(handlers_by_target) + new_state.model["eventHandlers"] = model_event_targets - def _get_component_state(self, component: AbstractComponent) -> _ComponentState: - return self._component_states[id(component)] - - def _has_component_state(self, component: AbstractComponent) -> bool: - return id(component) in self._component_states + return None - def _create_component_state( + def _render_model_children( self, - component: AbstractComponent, - patch_path: str, - parent_key_path: str, - save: bool, - ) -> _ComponentState: - component_id = id(component) - state = _ComponentState( - model={}, - patch_path=patch_path, - key_path=f"{parent_key_path}/{_get_component_key(component)}", - component_id=component_id, - component_obj=component, - event_handler_ids=set(), - child_component_ids=[], - life_cycle_hook=LifeCycleHook(component, self.update), - ) - if save: - self._component_states[component_id] = state - return state + old_state: Optional[_ModelState], + new_state: _ModelState, + raw_children: Any, + ) -> None: + if not isinstance(raw_children, (list, tuple)): + raw_children = [raw_children] - def _delete_component_state(self, component_state: _ComponentState) -> None: - self._clear_component_state_event_handlers(component_state) - self._delete_component_state_children(component_state) - del self._component_states[component_state.component_id] + if old_state is None: + if raw_children: + self._render_model_children_without_old_state(new_state, raw_children) + return None + elif not raw_children: + self._unmount_model_states(list(old_state.children_by_key.values())) + return None - def _clear_component_state_event_handlers( - self, component_state: _ComponentState - ) -> None: - for handler_id in component_state.event_handler_ids: - del self._event_handlers[handler_id] - component_state.event_handler_ids.clear() + DICT_TYPE, COMPONENT_TYPE, STRING_TYPE = 1, 2, 3 # noqa + + raw_typed_children_by_key: Dict[str, Tuple[int, Any]] = {} + for index, child in enumerate(raw_children): + if isinstance(child, dict): + child_type = DICT_TYPE + key = child.get("key") or hex(id(child))[2:] + elif isinstance(child, AbstractComponent): + child_type = COMPONENT_TYPE + key = getattr(child, "key", "") or hex(id(child))[2:] + else: + child = str(child) + child_type = STRING_TYPE + # The key doesn't matter since we won't look it up - all that matter is + # that the key is unique (which this approach guarantees) + key = object() + raw_typed_children_by_key[key] = (child_type, child) + + if len(raw_typed_children_by_key) != len(raw_children): + raise ValueError(f"Duplicate keys in {raw_children}") + + old_keys = set(old_state.children_by_key).difference(raw_typed_children_by_key) + old_child_states = {key: old_state.children_by_key[key] for key in old_keys} + if old_child_states: + self._unmount_model_states(list(old_child_states.values())) + + new_children = new_state.model["children"] = [] + for index, (key, (child_type, child)) in enumerate( + raw_typed_children_by_key.items() + ): + if child_type is DICT_TYPE: + child_state = _ModelState(ref(new_state), index, key, None) + self._render_model(old_child_states.get(key), child_state, child) + new_children.append(child_state.model) + new_state.children_by_key[key] = child_state + elif child_type is COMPONENT_TYPE: + key = getattr(child, "key", "") or hex(id(child)) + life_cycle_hook = LifeCycleHook(child, self) + child_state = _ModelState(ref(new_state), index, key, life_cycle_hook) + self._render_component(old_child_states.get(key), child_state, child) + new_children.append(child_state.model) + new_state.children_by_key[key] = child_state + self._model_state_by_component_id[id(child)] = child_state + else: + new_children.append(child) - def _delete_component_state_children( - self, component_state: _ComponentState + def _render_model_children_without_old_state( + self, new_state: _ModelState, raw_children: List[Any] ) -> None: - for e_id in component_state.child_component_ids: - self._delete_component_state(self._component_states[e_id]) - component_state.child_component_ids.clear() + new_children = new_state.model["children"] = [] + for index, child in enumerate(raw_children): + if isinstance(child, dict): + key = child.get("key") or hex(id(child)) + child_state = _ModelState(ref(new_state), index, key, None) + self._render_model(None, child_state, child) + new_children.append(child_state.model) + new_state.children_by_key[key] = child_state + elif isinstance(child, AbstractComponent): + key = getattr(child, "key", "") or hex(id(child)) + life_cycle_hook = LifeCycleHook(child, self) + child_state = _ModelState(ref(new_state), index, key, life_cycle_hook) + self._render_component(None, child_state, child) + new_children.append(child_state.model) + new_state.children_by_key[key] = child_state + self._model_state_by_component_id[id(child)] = child_state + else: + new_children.append(str(child)) + + def _unmount_model_states(self, old_states: List[_ModelState]) -> None: + to_unmount = old_states[::-1] + while to_unmount: + state = to_unmount.pop() + if hasattr(state, "life_cycle_hook"): + hook = state.life_cycle_hook + hook.component_will_unmount() + del self._model_state_by_component_id[id(hook.component)] + to_unmount.extend(state.children_by_key.values()) + + def __repr__(self) -> str: + return f"{type(self).__name__}({self.root})" + + +class _ModelState: - def _iter_component_states_from_root( + __slots__ = ( + "index", + "key", + "parent_ref", + "life_cycle_hook", + "patch_path", + "key_path", + "model", + "event_targets", + "children_by_key", + "__weakref__", + ) + + model: _ModelVdom + + def __init__( self, - root_component_state: _ComponentState, - include_root: bool, - ) -> Iterator[_ComponentState]: - if include_root: - pending = [root_component_state] + parent_ref: Optional[ReferenceType[_ModelState]], + index: int, + key: str, + life_cycle_hook: Optional[LifeCycleHook], + ) -> None: + self.index = index + self.key = key + + if parent_ref is not None: + self.parent_ref = parent_ref + # temporarilly hydrate for use below + parent = parent_ref() else: - pending = [ - self._component_states[i] - for i in root_component_state.child_component_ids - ] - - while pending: - visited_component_state = pending.pop(0) - yield visited_component_state - pending.extend( - self._component_states[i] - for i in visited_component_state.child_component_ids - ) + parent = None - def __repr__(self) -> str: - return f"{type(self).__name__}({self.root})" + if life_cycle_hook is not None: + self.life_cycle_hook = life_cycle_hook + if parent is None: + self.key_path = self.patch_path = "" + else: + self.key_path = f"{parent.key_path}/{key}" + self.patch_path = f"{parent.patch_path}/children/{index}" + + self.event_targets: Set[str] = set() + self.children_by_key: Dict[str, _ModelState] = {} + + def new(self) -> _ModelState: + return _ModelState( + getattr(self, "parent_ref", None), + self.index, + self.key, + getattr(self, "life_cycle_hook", None), + ) -class _ComponentState(NamedTuple): - model: Dict[str, Any] - patch_path: str - key_path: str - component_id: int - component_obj: AbstractComponent - event_handler_ids: Set[str] - child_component_ids: List[int] - life_cycle_hook: LifeCycleHook + def iter_children(self, include_self: bool = True) -> Iterator[_ModelState]: + to_yield = [self] if include_self else [] + while to_yield: + node = to_yield.pop() + yield node + to_yield.extend(node.children_by_key.values()) -class _EventTarget(TypedDict): +class _ModelEventTarget(TypedDict): target: str preventDefault: bool # noqa stopPropagation: bool # noqa -def _get_component_key(component: AbstractComponent) -> str: - return getattr(component, "key", "") or hex(id(component))[2:] +class _ModelImportSource(TypedDict): + source: str + fallback: Any + + +class _ModelVdomOptional(TypedDict, total=False): + key: str # noqa + children: List[Any] # noqa + attributes: Dict[str, Any] # noqa + eventHandlers: Dict[str, _ModelEventTarget] # noqa + importSource: _ModelImportSource # noqa + + +class _ModelVdomRequired(TypedDict, total=True): + tagName: str # noqa + + +class _ModelVdom(_ModelVdomRequired, _ModelVdomOptional): + """A VDOM dictionary model specifically for use with a :class:`Layout`""" class _ComponentQueue: diff --git a/src/idom/core/vdom.py b/src/idom/core/vdom.py index 1cb511ffe..2d9269bfc 100644 --- a/src/idom/core/vdom.py +++ b/src/idom/core/vdom.py @@ -76,7 +76,7 @@ class ImportSourceDict(TypedDict): class _VdomDictOptional(TypedDict, total=False): key: str # noqa children: Sequence[Any] # noqa - attributes: Dict[str, Any] # noqa + attributes: Mapping[str, Any] # noqa eventHandlers: Mapping[str, EventHandler] # noqa importSource: ImportSourceDict # noqa diff --git a/src/idom/widgets/html.py b/src/idom/widgets/html.py index 423276508..20bf894db 100644 --- a/src/idom/widgets/html.py +++ b/src/idom/widgets/html.py @@ -2,7 +2,7 @@ from typing import Any, Callable, Dict, Optional, Union import idom -from idom.core.component import AbstractComponent, ComponentConstructor +from idom.core.component import AbstractComponent, ComponentConstructor, component from idom.core.vdom import ( VdomDict, VdomDictConstructor, @@ -35,7 +35,7 @@ def image( return {"tagName": "img", "attributes": {"src": src, **(attributes or {})}} -@idom.core.component +@component def Input( callback: Callable[[str], None], type: str, diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index 487c24a49..e50934b36 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -17,9 +17,8 @@ async def test_shared_state_dispatcher(): done = asyncio.Event() changes_1 = [] changes_2 = [] - key = "test-element" event_name = "onEvent" - target_id = f"/{key}/{event_name}" + target_id = f"/{event_name}" events_to_inject = [LayoutEvent(target=target_id, data=[])] * 4 @@ -46,7 +45,7 @@ def Clickable(): count, set_count = idom.hooks.use_state(0) return idom.html.div({event_name: lambda: set_count(count + 1), "count": count}) - async with SharedViewDispatcher(Layout(Clickable(key=key))) as dispatcher: + async with SharedViewDispatcher(Layout(Clickable())) as dispatcher: await dispatcher.run(send_1, recv_1, "1") await dispatcher.run(send_2, recv_2, "2") @@ -65,7 +64,6 @@ def Clickable(): }, {"op": "add", "path": "/attributes", "value": {"count": 0}}, {"op": "add", "path": "/tagName", "value": "div"}, - {"op": "add", "path": "/key", "value": key}, ], [{"op": "replace", "path": "/attributes/count", "value": 1}], [{"op": "replace", "path": "/attributes/count", "value": 2}], diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index d1caaa3d7..c7d373b4f 100644 --- a/tests/test_core/test_hooks.py +++ b/tests/test_core/test_hooks.py @@ -354,18 +354,22 @@ def cleanup(): async def test_use_effect_cleanup_occurs_on_will_unmount(): outer_component_hook = HookCatcher() + component_did_render = idom.Ref(False) cleanup_triggered = idom.Ref(False) cleanup_triggered_before_next_render = idom.Ref(False) @idom.component @outer_component_hook.capture def OuterComponent(): - if cleanup_triggered.current: - cleanup_triggered_before_next_render.current = True return ComponentWithEffect() @idom.component def ComponentWithEffect(): + if component_did_render.current and cleanup_triggered.current: + cleanup_triggered_before_next_render.current = True + + component_did_render.current = True + @idom.hooks.use_effect def effect(): def cleanup(): diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index cd13ca69e..147c0d031 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -1,5 +1,4 @@ import asyncio -import gc import re from weakref import finalize @@ -75,10 +74,10 @@ async def test_nested_component_layout(): @idom.component def Parent(): state, parent_set_state.current = idom.hooks.use_state(0) - return idom.html.div(state, Child()) + return idom.html.div(state, Child(key="c")) @idom.component - def Child(): + def Child(key): state, child_set_state.current = idom.hooks.use_state(0) return idom.html.div(state) @@ -135,7 +134,10 @@ def BadChild(): "path": "/children", "value": [ {"tagName": "div", "children": ["hello"]}, - {"tagName": "div", "__error__": "Something went wrong :("}, + { + "tagName": "__error__", + "children": ["Something went wrong :("], + }, {"tagName": "div", "children": ["hello"]}, ], }, @@ -208,7 +210,6 @@ def Inner(): # the hook also contains a reference to the root component del outer_component_hook - gc.collect() assert not live_components @@ -308,10 +309,10 @@ def bad_trigger(): return idom.html.div(children) - async with idom.Layout(MyComponent(key="component")) as layout: + async with idom.Layout(MyComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent("/component/good/onClick", []) + event = LayoutEvent("/good/onClick", []) await layout.dispatch(event) assert called_good_trigger.current @@ -328,9 +329,7 @@ async def test_model_key_preserves_callback_identity_for_components(): def RootComponent(): reverse_children, set_reverse_children = use_toggle() - children = [ - Trigger(name, set_reverse_children, key=name) for name in ["good", "bad"] - ] + children = [Trigger(set_reverse_children, key=name) for name in ["good", "bad"]] if reverse_children: children.reverse() @@ -338,9 +337,9 @@ def RootComponent(): return idom.html.div(children) @idom.component - def Trigger(name, set_reverse_children): + def Trigger(set_reverse_children, key): def callback(): - if name == "good": + if key == "good": called_good_trigger.current = True set_reverse_children() else: @@ -348,10 +347,10 @@ def callback(): return idom.html.button({"onClick": callback, "id": "good"}, "good") - async with idom.Layout(RootComponent(key="root")) as layout: + async with idom.Layout(RootComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent("/root/good/onClick", []) + event = LayoutEvent("/good/onClick", []) await layout.dispatch(event) assert called_good_trigger.current @@ -359,3 +358,73 @@ def callback(): called_good_trigger.current = False await layout.render() + + +async def test_component_can_return_another_component_directly(): + @idom.component + def Outer(): + return Inner() + + @idom.component + def Inner(): + return idom.html.div("hello") + + async with idom.Layout(Outer()) as layout: + update = await layout.render() + assert_same_items( + update.changes, + [ + { + "op": "add", + "path": "/children", + "value": [{"children": ["hello"], "tagName": "div"}], + }, + {"op": "add", "path": "/tagName", "value": "div"}, + ], + ) + + +async def test_keyed_components_get_garbage_collected(): + pop_item = idom.Ref(None) + garbage_collect_items = [] + + @idom.component + def Outer(): + items, set_items = idom.hooks.use_state([1, 2, 3]) + pop_item.current = lambda: set_items(items[:-1]) + return idom.html.div(Inner(key=k) for k in items) + + @idom.component + def Inner(key): + component = idom.hooks.current_hook().component + finalize(component, lambda: garbage_collect_items.append(key)) + return idom.html.div(key) + + async with idom.Layout(Outer()) as layout: + await layout.render() + + pop_item.current() + await layout.render() + assert garbage_collect_items == [3] + + pop_item.current() + await layout.render() + assert garbage_collect_items == [3, 2] + + pop_item.current() + await layout.render() + assert garbage_collect_items == [3, 2, 1] + + +async def test_duplicate_sibling_keys_causes_error(caplog): + @idom.component + def ComponentReturnsDuplicateKeys(): + return idom.html.div( + idom.html.div(key="duplicate"), idom.html.div(key="duplicate") + ) + + async with idom.Layout(ComponentReturnsDuplicateKeys()) as layout: + await layout.render() + + with pytest.raises(ValueError, match="Duplicate keys in"): + raise next(iter(caplog.records)).exc_info[1] From 64b9a4c68c32d653128b6030dbfd6c1245a58b6c Mon Sep 17 00:00:00 2001 From: rmorshea Date: Fri, 9 Apr 2021 15:47:08 -0700 Subject: [PATCH 07/20] add tests for keyed hook preservation this needs to be cleaned up now that we know what the bugs were --- src/idom/core/component.py | 9 ++- src/idom/core/hooks.py | 9 ++- src/idom/core/layout.py | 136 +++++++++++++++++++-------------- tests/test_core/test_layout.py | 28 ++++++- 4 files changed, 115 insertions(+), 67 deletions(-) diff --git a/src/idom/core/component.py b/src/idom/core/component.py index ca68893be..22efbfdb6 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -21,8 +21,8 @@ def component(function: ComponentRenderFunction) -> Callable[..., "Component"]: """ @wraps(function) - def constructor(*args: Any, **kwargs: Any) -> Component: - return Component(function, args, kwargs) + def constructor(*args: Any, key: str = "", **kwargs: Any) -> Component: + return Component(function, key, args, kwargs) return constructor @@ -48,13 +48,16 @@ class Component(AbstractComponent): def __init__( self, function: ComponentRenderFunction, + key: str, args: Tuple[Any, ...], kwargs: Dict[str, Any], ) -> None: - self.key = kwargs.get("key", "") + self.key = key self._function = function self._args = args self._kwargs = kwargs + if key: + kwargs["key"] = key def render(self) -> VdomDict: model = self._function(*self._args, **self._kwargs) diff --git a/src/idom/core/hooks.py b/src/idom/core/hooks.py index 4a48a0b5a..168f69ea4 100644 --- a/src/idom/core/hooks.py +++ b/src/idom/core/hooks.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import weakref from logging import getLogger from threading import get_ident as get_thread_id from typing import ( @@ -374,7 +375,7 @@ class LifeCycleHook: __slots__ = ( "component", - "_schedule_render_callback", + "_layout", "_schedule_render_later", "_current_state_index", "_state", @@ -390,7 +391,7 @@ def __init__( layout: idom.core.layout.Layout, ) -> None: self.component = component - self._schedule_render_callback = layout.update + self._layout = weakref.ref(layout) self._schedule_render_later = False self._is_rendering = False self._rendered_atleast_once = False @@ -468,4 +469,6 @@ def unset_current(self) -> None: del _current_life_cycle_hook[get_thread_id()] def _schedule_render(self) -> None: - self._schedule_render_callback(self.component) + layout = self._layout() + assert layout is not None + layout.update(self.component) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index ce55eb13a..f19aca693 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -15,7 +15,7 @@ Set, Tuple, ) -from weakref import ReferenceType, ref +from weakref import ref from jsonpatch import apply_patch, make_patch from typing_extensions import TypedDict @@ -125,7 +125,7 @@ async def _model_state_by_component_id( def _create_layout_update(self, component: AbstractComponent) -> LayoutUpdate: old_state = self._model_state_by_component_id[id(component)] - new_state = old_state.new() + new_state = old_state.new(None, component) self._render_component(old_state, new_state, component) changes = make_patch(getattr(old_state, "model", {}), new_state.model).patch @@ -269,7 +269,7 @@ def _render_model_children( old_keys = set(old_state.children_by_key).difference(raw_typed_children_by_key) old_child_states = {key: old_state.children_by_key[key] for key in old_keys} - if old_child_states: + if old_keys: self._unmount_model_states(list(old_child_states.values())) new_children = new_state.model["children"] = [] @@ -277,18 +277,25 @@ def _render_model_children( raw_typed_children_by_key.items() ): if child_type is DICT_TYPE: - child_state = _ModelState(ref(new_state), index, key, None) - self._render_model(old_child_states.get(key), child_state, child) - new_children.append(child_state.model) - new_state.children_by_key[key] = child_state + old_child_state = old_state.children_by_key.get(key) + if old_child_state is not None: + new_child_state = old_child_state.new(new_state, None) + else: + new_child_state = _ModelState(new_state, index, key, None) + self._render_model(old_child_state, new_child_state, child) + new_children.append(new_child_state.model) + new_state.children_by_key[key] = new_child_state elif child_type is COMPONENT_TYPE: - key = getattr(child, "key", "") or hex(id(child)) - life_cycle_hook = LifeCycleHook(child, self) - child_state = _ModelState(ref(new_state), index, key, life_cycle_hook) - self._render_component(old_child_states.get(key), child_state, child) - new_children.append(child_state.model) - new_state.children_by_key[key] = child_state - self._model_state_by_component_id[id(child)] = child_state + old_child_state = old_state.children_by_key.get(key) + if old_child_state is not None: + new_child_state = old_child_state.new(new_state, child) + else: + hook = LifeCycleHook(child, self) + new_child_state = _ModelState(new_state, index, key, hook) + self._render_component(old_child_state, new_child_state, child) + new_children.append(new_child_state.model) + new_state.children_by_key[key] = new_child_state + self._model_state_by_component_id[id(child)] = new_child_state else: new_children.append(child) @@ -299,14 +306,14 @@ def _render_model_children_without_old_state( for index, child in enumerate(raw_children): if isinstance(child, dict): key = child.get("key") or hex(id(child)) - child_state = _ModelState(ref(new_state), index, key, None) + child_state = _ModelState(new_state, index, key, None) self._render_model(None, child_state, child) new_children.append(child_state.model) new_state.children_by_key[key] = child_state elif isinstance(child, AbstractComponent): key = getattr(child, "key", "") or hex(id(child)) life_cycle_hook = LifeCycleHook(child, self) - child_state = _ModelState(ref(new_state), index, key, life_cycle_hook) + child_state = _ModelState(new_state, index, key, life_cycle_hook) self._render_component(None, child_state, child) new_children.append(child_state.model) new_state.children_by_key[key] = child_state @@ -322,6 +329,10 @@ def _unmount_model_states(self, old_states: List[_ModelState]) -> None: hook = state.life_cycle_hook hook.component_will_unmount() del self._model_state_by_component_id[id(hook.component)] + import gc + + print(state) + print(gc.get_referrers(hook)) to_unmount.extend(state.children_by_key.values()) def __repr__(self) -> str: @@ -333,7 +344,7 @@ class _ModelState: __slots__ = ( "index", "key", - "parent_ref", + "_parent_ref", "life_cycle_hook", "patch_path", "key_path", @@ -347,7 +358,7 @@ class _ModelState: def __init__( self, - parent_ref: Optional[ReferenceType[_ModelState]], + parent: Optional[_ModelState], index: int, key: str, life_cycle_hook: Optional[LifeCycleHook], @@ -355,32 +366,41 @@ def __init__( self.index = index self.key = key - if parent_ref is not None: - self.parent_ref = parent_ref - # temporarilly hydrate for use below - parent = parent_ref() + if parent is not None: + self._parent_ref = ref(parent) + self.key_path = f"{parent.key_path}/{key}" + self.patch_path = f"{parent.patch_path}/children/{index}" else: - parent = None + self.key_path = self.patch_path = "" if life_cycle_hook is not None: self.life_cycle_hook = life_cycle_hook - if parent is None: - self.key_path = self.patch_path = "" - else: - self.key_path = f"{parent.key_path}/{key}" - self.patch_path = f"{parent.patch_path}/children/{index}" - self.event_targets: Set[str] = set() self.children_by_key: Dict[str, _ModelState] = {} - def new(self) -> _ModelState: - return _ModelState( - getattr(self, "parent_ref", None), - self.index, - self.key, - getattr(self, "life_cycle_hook", None), - ) + @property + def parent(self) -> _ModelState: + # An AttributeError here is ok. It's synonymous + # with the existance of 'parent' attribute + p = self._parent_ref() + assert p is not None, "detached model state" + return p + + def new( + self, + new_parent: Optional[_ModelState], + component: Optional[AbstractComponent], + ) -> _ModelState: + if new_parent is None: + new_parent = getattr(self, "parent", None) + if hasattr(self, "life_cycle_hook"): + assert component is not None + life_cycle_hook = self.life_cycle_hook + life_cycle_hook.component = component + else: + life_cycle_hook = None + return _ModelState(new_parent, self.index, self.key, life_cycle_hook) def iter_children(self, include_self: bool = True) -> Iterator[_ModelState]: to_yield = [self] if include_self else [] @@ -390,6 +410,28 @@ def iter_children(self, include_self: bool = True) -> Iterator[_ModelState]: to_yield.extend(node.children_by_key.values()) +class _ComponentQueue: + + __slots__ = "_loop", "_queue", "_pending" + + def __init__(self) -> None: + self._loop = asyncio.get_event_loop() + self._queue: "asyncio.Queue[AbstractComponent]" = asyncio.Queue() + self._pending: Set[int] = set() + + def put(self, component: AbstractComponent) -> None: + component_id = id(component) + if component_id not in self._pending: + self._pending.add(component_id) + self._loop.call_soon_threadsafe(self._queue.put_nowait, component) + return None + + async def get(self) -> AbstractComponent: + component = await self._queue.get() + self._pending.remove(id(component)) + return component + + class _ModelEventTarget(TypedDict): target: str preventDefault: bool # noqa @@ -415,25 +457,3 @@ class _ModelVdomRequired(TypedDict, total=True): class _ModelVdom(_ModelVdomRequired, _ModelVdomOptional): """A VDOM dictionary model specifically for use with a :class:`Layout`""" - - -class _ComponentQueue: - - __slots__ = "_loop", "_queue", "_pending" - - def __init__(self) -> None: - self._loop = asyncio.get_event_loop() - self._queue: "asyncio.Queue[AbstractComponent]" = asyncio.Queue() - self._pending: Set[int] = set() - - def put(self, component: AbstractComponent) -> None: - component_id = id(component) - if component_id not in self._pending: - self._pending.add(component_id) - self._loop.call_soon_threadsafe(self._queue.put_nowait, component) - return None - - async def get(self) -> AbstractComponent: - component = await self._queue.get() - self._pending.remove(id(component)) - return component diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 147c0d031..fc43759ca 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -384,7 +384,7 @@ def Inner(): ) -async def test_keyed_components_get_garbage_collected(): +async def test_hooks_for_keyed_components_get_garbage_collected(): pop_item = idom.Ref(None) garbage_collect_items = [] @@ -396,8 +396,7 @@ def Outer(): @idom.component def Inner(key): - component = idom.hooks.current_hook().component - finalize(component, lambda: garbage_collect_items.append(key)) + finalize(idom.hooks.current_hook(), lambda: garbage_collect_items.append(key)) return idom.html.div(key) async with idom.Layout(Outer()) as layout: @@ -428,3 +427,26 @@ def ComponentReturnsDuplicateKeys(): with pytest.raises(ValueError, match="Duplicate keys in"): raise next(iter(caplog.records)).exc_info[1] + + +async def test_keyed_components_preserve_hook_on_parent_update(): + outer_hook = HookCatcher() + inner_hook = HookCatcher() + + @idom.component + @outer_hook.capture + def Outer(): + return Inner(key=1) + + @idom.component + @inner_hook.capture + def Inner(key): + return idom.html.div(key) + + async with idom.Layout(Outer()) as layout: + await layout.render() + old_inner_hook = inner_hook.current + + outer_hook.current.schedule_render() + await layout.render() + assert old_inner_hook is inner_hook.current From 88124eff1d76ff885f36e98c6ec4e1d4c7817a87 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 11 Apr 2021 00:30:41 -0700 Subject: [PATCH 08/20] fix garbage collection issue --- src/idom/core/layout.py | 8 +++----- tests/test_core/test_layout.py | 6 +++++- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index f19aca693..c2078bfa6 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -255,7 +255,7 @@ def _render_model_children( key = child.get("key") or hex(id(child))[2:] elif isinstance(child, AbstractComponent): child_type = COMPONENT_TYPE - key = getattr(child, "key", "") or hex(id(child))[2:] + key = getattr(child, "key", None) or hex(id(child))[2:] else: child = str(child) child_type = STRING_TYPE @@ -288,6 +288,8 @@ def _render_model_children( elif child_type is COMPONENT_TYPE: old_child_state = old_state.children_by_key.get(key) if old_child_state is not None: + old_component = old_child_state.life_cycle_hook.component + del self._model_state_by_component_id[id(old_component)] new_child_state = old_child_state.new(new_state, child) else: hook = LifeCycleHook(child, self) @@ -329,10 +331,6 @@ def _unmount_model_states(self, old_states: List[_ModelState]) -> None: hook = state.life_cycle_hook hook.component_will_unmount() del self._model_state_by_component_id[id(hook.component)] - import gc - - print(state) - print(gc.get_referrers(hook)) to_unmount.extend(state.children_by_key.values()) def __repr__(self) -> str: diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index fc43759ca..5baec4727 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -387,6 +387,7 @@ def Inner(): async def test_hooks_for_keyed_components_get_garbage_collected(): pop_item = idom.Ref(None) garbage_collect_items = [] + registered_finalizers = set() @idom.component def Outer(): @@ -396,7 +397,10 @@ def Outer(): @idom.component def Inner(key): - finalize(idom.hooks.current_hook(), lambda: garbage_collect_items.append(key)) + if key not in registered_finalizers: + hook = idom.hooks.current_hook() + finalize(hook, lambda: garbage_collect_items.append(key)) + registered_finalizers.add(key) return idom.html.div(key) async with idom.Layout(Outer()) as layout: From e5d4fc10b65b12e345ab2019917c32684ce6b064 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 11:45:47 -0700 Subject: [PATCH 09/20] _render_component must update state pointers --- src/idom/core/layout.py | 44 ++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index c2078bfa6..de5c3e39a 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -130,16 +130,6 @@ def _create_layout_update(self, component: AbstractComponent) -> LayoutUpdate: self._render_component(old_state, new_state, component) changes = make_patch(getattr(old_state, "model", {}), new_state.model).patch - # replace model and state in parent - if hasattr(new_state, "parent_ref"): - parent = new_state.parent_ref() - if parent is not None: - parent.children_by_key[new_state.key] = new_state - parent.model["children"][new_state.index] = new_state.model - - # replace in global state - self._model_state_by_component_id[id(component)] = new_state - # hook effects must run after the update is complete for state in new_state.iter_children(): if hasattr(state, "life_cycle_hook"): @@ -166,6 +156,26 @@ def _render_component( logger.exception(f"Failed to render {component}") new_state.model = {"tagName": "__error__", "children": [str(error)]} + if old_state is not None and old_state.component is not component: + del self._model_state_by_component_id[id(old_state.component)] + self._model_state_by_component_id[id(component)] = new_state + + try: + parent = new_state.parent + except AttributeError: + pass + else: + key, index = new_state.key, new_state.index + if old_state is not None: + assert (key, index) == (old_state.key, old_state.index,), ( + "state mismatch during component update - " + f"key {key!r}!={old_state.key} " + f"or index {index}!={old_state.index}" + ) + parent.children_by_key[key] = new_state + # need to do insertion in case where old_state is None and we're appending + parent.model["children"][index : index + 1] = [new_state.model] + def _render_model( self, old_state: Optional[_ModelState], @@ -259,8 +269,8 @@ def _render_model_children( else: child = str(child) child_type = STRING_TYPE - # The key doesn't matter since we won't look it up - all that matter is - # that the key is unique (which this approach guarantees) + # The specific key doesn't matter here since we won't look it up - all + # we care about is that the key is unique, which object() can guarantee. key = object() raw_typed_children_by_key[key] = (child_type, child) @@ -288,16 +298,11 @@ def _render_model_children( elif child_type is COMPONENT_TYPE: old_child_state = old_state.children_by_key.get(key) if old_child_state is not None: - old_component = old_child_state.life_cycle_hook.component - del self._model_state_by_component_id[id(old_component)] new_child_state = old_child_state.new(new_state, child) else: hook = LifeCycleHook(child, self) new_child_state = _ModelState(new_state, index, key, hook) self._render_component(old_child_state, new_child_state, child) - new_children.append(new_child_state.model) - new_state.children_by_key[key] = new_child_state - self._model_state_by_component_id[id(child)] = new_child_state else: new_children.append(child) @@ -317,9 +322,6 @@ def _render_model_children_without_old_state( life_cycle_hook = LifeCycleHook(child, self) child_state = _ModelState(new_state, index, key, life_cycle_hook) self._render_component(None, child_state, child) - new_children.append(child_state.model) - new_state.children_by_key[key] = child_state - self._model_state_by_component_id[id(child)] = child_state else: new_children.append(str(child)) @@ -344,6 +346,7 @@ class _ModelState: "key", "_parent_ref", "life_cycle_hook", + "component", "patch_path", "key_path", "model", @@ -373,6 +376,7 @@ def __init__( if life_cycle_hook is not None: self.life_cycle_hook = life_cycle_hook + self.component = life_cycle_hook.component self.event_targets: Set[str] = set() self.children_by_key: Dict[str, _ModelState] = {} From 2a87a1e64d97bf9717411968d4a995cdedd4fd8c Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 20:43:25 -0700 Subject: [PATCH 10/20] use fixed length event target strings --- src/idom/core/layout.py | 64 +++++++++++++++++++++++++++-------------- 1 file changed, 43 insertions(+), 21 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index de5c3e39a..55454da02 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -215,27 +215,50 @@ def _render_model_attributes( h = attrs.pop(k) handlers_by_event[k] = h - handlers_by_target: Dict[str, EventHandler] = {} - model_event_targets: Dict[str, _ModelEventTarget] = {} + if old_state is None: + self._render_model_event_handlers_without_old_state( + new_state, handlers_by_event + ) + return None + + for old_event in set(old_state.targets_by_event).difference(handlers_by_event): + old_target = old_state.targets_by_event[old_event] + del self._event_handlers[old_target] + + if not handlers_by_event: + return None + + model_event_handlers = new_state.model["eventHandlers"] = {} for event, handler in handlers_by_event.items(): - target = f"{new_state.key_path}/{event}" - handlers_by_target[target] = handler - model_event_targets[event] = { + target = old_state.targets_by_event.get(event, id(handler)) + new_state.targets_by_event[event] = target + self._event_handlers[target] = handler + model_event_handlers[event] = { "target": target, "preventDefault": handler.prevent_default, "stopPropagation": handler.stop_propagation, } - if old_state is not None: - for old_target in set(old_state.event_targets).difference( - handlers_by_target - ): - del self._event_handlers[old_target] + return None + + def _render_model_event_handlers_without_old_state( + self, + new_state: _ModelState, + handlers_by_event: Dict[str, EventHandler], + ) -> None: + if not handlers_by_event: + return None - if model_event_targets: - new_state.event_targets.update(handlers_by_target) - self._event_handlers.update(handlers_by_target) - new_state.model["eventHandlers"] = model_event_targets + model_event_handlers = new_state.model["eventHandlers"] = {} + for event, handler in handlers_by_event.items(): + target = hex(id(handler))[2:] + new_state.targets_by_event[event] = target + self._event_handlers[target] = handler + model_event_handlers[event] = { + "target": target, + "preventDefault": handler.prevent_default, + "stopPropagation": handler.stop_propagation, + } return None @@ -283,8 +306,9 @@ def _render_model_children( self._unmount_model_states(list(old_child_states.values())) new_children = new_state.model["children"] = [] - for index, (key, (child_type, child)) in enumerate( - raw_typed_children_by_key.items() + for index, (key, (child_type, child)) in ( + # we can enumerate this because dict insertion order is preserved + enumerate(raw_typed_children_by_key.items()) ): if child_type is DICT_TYPE: old_child_state = old_state.children_by_key.get(key) @@ -348,9 +372,8 @@ class _ModelState: "life_cycle_hook", "component", "patch_path", - "key_path", "model", - "event_targets", + "targets_by_event", "children_by_key", "__weakref__", ) @@ -369,16 +392,15 @@ def __init__( if parent is not None: self._parent_ref = ref(parent) - self.key_path = f"{parent.key_path}/{key}" self.patch_path = f"{parent.patch_path}/children/{index}" else: - self.key_path = self.patch_path = "" + self.patch_path = "" if life_cycle_hook is not None: self.life_cycle_hook = life_cycle_hook self.component = life_cycle_hook.component - self.event_targets: Set[str] = set() + self.targets_by_event: Dict[str, str] = {} self.children_by_key: Dict[str, _ModelState] = {} @property From 75f7fe30fc74ca5c5586f5be8cedc59298efa569 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 20:49:03 -0700 Subject: [PATCH 11/20] use common hex_id() func --- src/idom/core/component.py | 5 +++-- src/idom/core/layout.py | 12 ++++++------ src/idom/core/utils.py | 4 ++++ tests/test_core/test_component.py | 3 ++- tests/test_core/test_layout.py | 3 ++- 5 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/idom/core/component.py b/src/idom/core/component.py index 22efbfdb6..137f0c58c 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -5,6 +5,7 @@ from functools import wraps from typing import Any, Callable, Dict, Tuple, Union +from .utils import hex_id from .vdom import VdomDict @@ -74,6 +75,6 @@ def __repr__(self) -> str: else: items = ", ".join(f"{k}={v!r}" for k, v in args.items()) if items: - return f"{self._function.__name__}({hex(id(self))}, {items})" + return f"{self._function.__name__}({hex_id(self)}, {items})" else: - return f"{self._function.__name__}({hex(id(self))})" + return f"{self._function.__name__}({hex_id(self)})" diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 55454da02..d1c0ee90b 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -25,7 +25,7 @@ from .component import AbstractComponent from .events import EventHandler from .hooks import LifeCycleHook -from .utils import CannotAccessResource, HasAsyncResources, async_resource +from .utils import CannotAccessResource, HasAsyncResources, async_resource, hex_id from .vdom import validate_vdom @@ -251,7 +251,7 @@ def _render_model_event_handlers_without_old_state( model_event_handlers = new_state.model["eventHandlers"] = {} for event, handler in handlers_by_event.items(): - target = hex(id(handler))[2:] + target = hex_id(handler) new_state.targets_by_event[event] = target self._event_handlers[target] = handler model_event_handlers[event] = { @@ -285,10 +285,10 @@ def _render_model_children( for index, child in enumerate(raw_children): if isinstance(child, dict): child_type = DICT_TYPE - key = child.get("key") or hex(id(child))[2:] + key = child.get("key") or hex_id(child) elif isinstance(child, AbstractComponent): child_type = COMPONENT_TYPE - key = getattr(child, "key", None) or hex(id(child))[2:] + key = getattr(child, "key", None) or hex_id(child) else: child = str(child) child_type = STRING_TYPE @@ -336,13 +336,13 @@ def _render_model_children_without_old_state( new_children = new_state.model["children"] = [] for index, child in enumerate(raw_children): if isinstance(child, dict): - key = child.get("key") or hex(id(child)) + key = child.get("key") or hex_id(child) child_state = _ModelState(new_state, index, key, None) self._render_model(None, child_state, child) new_children.append(child_state.model) new_state.children_by_key[key] = child_state elif isinstance(child, AbstractComponent): - key = getattr(child, "key", "") or hex(id(child)) + key = getattr(child, "key", "") or hex_id(child) life_cycle_hook = LifeCycleHook(child, self) child_state = _ModelState(new_state, index, key, life_cycle_hook) self._render_component(None, child_state, child) diff --git a/src/idom/core/utils.py b/src/idom/core/utils.py index bd069a6fc..09e05c5c6 100644 --- a/src/idom/core/utils.py +++ b/src/idom/core/utils.py @@ -22,6 +22,10 @@ from async_generator import asynccontextmanager +def hex_id(obj: Any) -> str: + return format(id(obj), "x") + + _Rsrc = TypeVar("_Rsrc") _Self = TypeVar("_Self", bound="HasAsyncResources") diff --git a/tests/test_core/test_component.py b/tests/test_core/test_component.py index a087f4e5f..0b4767dab 100644 --- a/tests/test_core/test_component.py +++ b/tests/test_core/test_component.py @@ -1,4 +1,5 @@ import idom +from idom.core.utils import hex_id def test_component_repr(): @@ -8,7 +9,7 @@ def MyComponent(a, *b, **c): mc1 = MyComponent(1, 2, 3, x=4, y=5) - expected = f"MyComponent({hex(id(mc1))}, a=1, b=(2, 3), c={{'x': 4, 'y': 5}})" + expected = f"MyComponent({hex_id(mc1)}, a=1, b=(2, 3), c={{'x': 4, 'y': 5}})" assert repr(mc1) == expected # not enough args supplied to function diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 5baec4727..c5ba5d92d 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -6,6 +6,7 @@ import idom from idom.core.layout import LayoutEvent, LayoutUpdate +from idom.core.utils import hex_id from tests.general_utils import HookCatcher, assert_same_items @@ -21,7 +22,7 @@ def MyComponent(): my_component = MyComponent() layout = idom.Layout(my_component) - assert str(layout) == f"Layout(MyComponent({hex(id(my_component))}))" + assert str(layout) == f"Layout(MyComponent({hex_id(my_component)}))" def test_layout_expects_abstract_component(): From 54ae5d9c325a8662cd79eba3078e177b18b5d5ac Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 21:47:53 -0700 Subject: [PATCH 12/20] fix tests using key path event targets --- src/idom/core/events.py | 26 ++++++++++++----------- src/idom/core/layout.py | 2 +- tests/general_utils.py | 34 +++++++++++++++++++++++++++++- tests/test_core/test_dispatcher.py | 15 ++++++++----- tests/test_core/test_layout.py | 24 +++++++++++++++------ 5 files changed, 76 insertions(+), 25 deletions(-) diff --git a/src/idom/core/events.py b/src/idom/core/events.py index 3e23574bf..c596bedbd 100644 --- a/src/idom/core/events.py +++ b/src/idom/core/events.py @@ -73,7 +73,7 @@ def on( stop_propagation: Block the event from propagating further up the DOM. prevent_default: - Stops the default actional associate with the event from taking place. + Stops the default action associate with the event from taking place. Returns: A decorator which accepts an event handler function as its first argument. @@ -133,12 +133,10 @@ class EventHandler: The event handler object acts like a coroutine when called. Parameters: - event_name: - The camel case name of the event. - target_id: - A unique identifier for the event handler. This is generally used if - an element has more than on event handler for the same event type. If - no ID is provided one will be generated automatically. + stop_propagation: + Block the event from propagating further up the DOM. + prevent_default: + Stops the default action associate with the event from taking place. """ __slots__ = ( @@ -160,13 +158,12 @@ def __init__( self._func_handlers: List[Callable[..., Any]] = [] def add(self, function: Callable[..., Any]) -> "EventHandler": - """Add a callback to the event handler. + """Add a callback function or coroutine to the event handler. Parameters: function: - The event handler function. Its parameters may indicate event attributes - which should be sent back from the fronend unless otherwise specified by - the ``properties`` parameter. + The event handler function accepting parameters sent by the client. + Typically this is a single ``event`` parameter that is a dictionary. """ if asyncio.iscoroutinefunction(function): self._coro_handlers.append(function) @@ -175,7 +172,7 @@ def add(self, function: Callable[..., Any]) -> "EventHandler": return self def remove(self, function: Callable[..., Any]) -> None: - """Remove the function from the event handler. + """Remove the given function or coroutine from this event handler. Raises: ValueError: if not found @@ -185,6 +182,11 @@ def remove(self, function: Callable[..., Any]) -> None: else: self._func_handlers.remove(function) + def clear(self) -> None: + """Remove all functions and coroutines from this event handler""" + self._coro_handlers.clear() + self._func_handlers.clear() + async def __call__(self, data: List[Any]) -> Any: """Trigger all callbacks in the event handler.""" if self._coro_handlers: diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index d1c0ee90b..1ccb2bed1 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -230,7 +230,7 @@ def _render_model_attributes( model_event_handlers = new_state.model["eventHandlers"] = {} for event, handler in handlers_by_event.items(): - target = old_state.targets_by_event.get(event, id(handler)) + target = old_state.targets_by_event.get(event, hex_id(handler)) new_state.targets_by_event[event] = target self._event_handlers[target] = handler model_event_handlers[event] = { diff --git a/tests/general_utils.py b/tests/general_utils.py index 6d94737e2..e2ea2cdca 100644 --- a/tests/general_utils.py +++ b/tests/general_utils.py @@ -5,6 +5,8 @@ from weakref import ref import idom +from idom.core.events import EventHandler +from idom.core.utils import hex_id @contextmanager @@ -19,11 +21,41 @@ def patch_slots_object(obj, attr, new_value): setattr(obj, attr, old_value) +class EventCatcher: + """Utility for capturing the target of an event handler + + Example: + .. code-block:: + + event_catcher = EventCatcher() + + @idom.component + def MyComponent(): + state, set_state = idom.hooks.use_state(0) + handler = event_catcher.capture(lambda event: set_state(state + 1)) + return idom.html.button({"onClick": handler}, "Click me!") + """ + + def __init__(self): + self._event_handler = EventHandler() + + @property + def target(self) -> str: + return hex_id(self._event_handler) + + def capture(self, function) -> EventHandler: + """Called within the body of a component to create a captured event handler""" + self._event_handler.clear() + self._event_handler.add(function) + return self._event_handler + + class HookCatcher: """Utility for capturing a LifeCycleHook from a component - Eleftample: + Example: .. code-block:: + component_hook = HookCatcher() @idom.component diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index e50934b36..c9176777f 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -10,22 +10,26 @@ SingleViewDispatcher, ) from idom.core.layout import Layout, LayoutEvent -from tests.general_utils import assert_same_items +from tests.general_utils import EventCatcher, assert_same_items async def test_shared_state_dispatcher(): done = asyncio.Event() changes_1 = [] changes_2 = [] + event_name = "onEvent" - target_id = f"/{event_name}" + event_catcher = EventCatcher() - events_to_inject = [LayoutEvent(target=target_id, data=[])] * 4 + events_to_inject = [LayoutEvent(target=event_catcher.target, data=[])] * 4 async def send_1(patch): changes_1.append(patch.changes) async def recv_1(): + # Need this to yield control back to event loop otherwise we block indefinitely + # for some reason. Realistically this await would be on some client event, so + # this isn't too contrived. await asyncio.sleep(0) try: return events_to_inject.pop(0) @@ -43,7 +47,8 @@ async def recv_2(): @idom.component def Clickable(): count, set_count = idom.hooks.use_state(0) - return idom.html.div({event_name: lambda: set_count(count + 1), "count": count}) + handler = event_catcher.capture(lambda: set_count(count + 1)) + return idom.html.div({event_name: handler, "count": count}) async with SharedViewDispatcher(Layout(Clickable())) as dispatcher: await dispatcher.run(send_1, recv_1, "1") @@ -56,7 +61,7 @@ def Clickable(): "path": "/eventHandlers", "value": { event_name: { - "target": target_id, + "target": event_catcher.target, "preventDefault": False, "stopPropagation": False, } diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index c5ba5d92d..7f8e4a87c 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -7,7 +7,7 @@ import idom from idom.core.layout import LayoutEvent, LayoutUpdate from idom.core.utils import hex_id -from tests.general_utils import HookCatcher, assert_same_items +from tests.general_utils import EventCatcher, HookCatcher, assert_same_items def test_layout_update_create_from_apply_to(): @@ -286,15 +286,19 @@ def use_toggle(init=False): async def test_model_key_preserves_callback_identity_for_common_elements(): called_good_trigger = idom.Ref(False) + good_event_catcher = EventCatcher() + bad_event_catcher = EventCatcher() @idom.component def MyComponent(): reverse_children, set_reverse_children = use_toggle() + @good_event_catcher.capture def good_trigger(): called_good_trigger.current = True set_reverse_children() + @bad_event_catcher.capture def bad_trigger(): raise ValueError("Called bad trigger") @@ -313,7 +317,7 @@ def bad_trigger(): async with idom.Layout(MyComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent("/good/onClick", []) + event = LayoutEvent(good_event_catcher.target, []) await layout.dispatch(event) assert called_good_trigger.current @@ -325,6 +329,8 @@ def bad_trigger(): async def test_model_key_preserves_callback_identity_for_components(): called_good_trigger = idom.Ref(False) + good_event_catcher = EventCatcher() + bad_event_catcher = EventCatcher() @idom.component def RootComponent(): @@ -339,11 +345,17 @@ def RootComponent(): @idom.component def Trigger(set_reverse_children, key): - def callback(): - if key == "good": + if key == "good": + + @good_event_catcher.capture + def callback(): called_good_trigger.current = True set_reverse_children() - else: + + else: + + @bad_event_catcher.capture + def callback(): raise ValueError("Called bad trigger") return idom.html.button({"onClick": callback, "id": "good"}, "good") @@ -351,7 +363,7 @@ def callback(): async with idom.Layout(RootComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent("/good/onClick", []) + event = LayoutEvent(good_event_catcher.target, []) await layout.dispatch(event) assert called_good_trigger.current From fb51589c7bf0495dfa947fc3d9de5d4fc3adbe29 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 23:08:44 -0700 Subject: [PATCH 13/20] make HookCatcher/StaticEventHandlers testing utils also fixes mypy typing issues --- src/idom/core/layout.py | 6 ++ src/idom/testing.py | 111 +++++++++++++++++++++++++++++ tests/general_utils.py | 74 ------------------- tests/test_core/test_dispatcher.py | 11 +-- tests/test_core/test_hooks.py | 41 +++++------ tests/test_core/test_layout.py | 37 +++++----- 6 files changed, 162 insertions(+), 118 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 1ccb2bed1..a7795e825 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -379,6 +379,9 @@ class _ModelState: ) model: _ModelVdom + life_cycle_hook: LifeCycleHook + patch_path: str + component: AbstractComponent def __init__( self, @@ -418,12 +421,15 @@ def new( ) -> _ModelState: if new_parent is None: new_parent = getattr(self, "parent", None) + + life_cycle_hook: Optional[LifeCycleHook] if hasattr(self, "life_cycle_hook"): assert component is not None life_cycle_hook = self.life_cycle_hook life_cycle_hook.component = component else: life_cycle_hook = None + return _ModelState(new_parent, self.index, self.key, life_cycle_hook) def iter_children(self, include_self: bool = True) -> Iterator[_ModelState]: diff --git a/src/idom/testing.py b/src/idom/testing.py index c98111cc7..bad261a91 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -1,5 +1,6 @@ import logging import re +from functools import wraps from types import TracebackType from typing import ( Any, @@ -12,12 +13,18 @@ Type, TypeVar, Union, + overload, ) from urllib.parse import urlencode, urlunparse +from weakref import ref from selenium.webdriver import Chrome from selenium.webdriver.remote.webdriver import WebDriver +from typing_extensions import Literal +from idom.core.events import EventHandler +from idom.core.hooks import LifeCycleHook, current_hook +from idom.core.utils import hex_id from idom.server.base import AbstractRenderServer from idom.server.prefab import hotswap_server from idom.server.utils import find_available_port, find_builtin_server_type @@ -167,3 +174,107 @@ def __init__(self) -> None: def handle(self, record: logging.LogRecord) -> None: self.records.append(record) + + +class HookCatcher: + """Utility for capturing a LifeCycleHook from a component + + Example: + .. code-block:: + + hooks = HookCatcher(index_by_kwarg="key") + + @idom.component + @hooks.capture + def MyComponent(key): + ... + + ... # render the component + + # grab the last render of where MyComponent(key='some_key') + hooks.index["some_key"] + # or grab the hook from the component's last render + hooks.latest + + After the first render of ``MyComponent`` the ``HookCatcher`` will have + captured the component's ``LifeCycleHook``. + """ + + latest: LifeCycleHook + + def __init__(self, index_by_kwarg: Optional[str] = None): + self.index_by_kwarg = index_by_kwarg + self.index: Dict[Any, LifeCycleHook] = {} + + def capture(self, render_function: Callable[..., Any]) -> Callable[..., Any]: + """Decorator for capturing a ``LifeCycleHook`` on each render of a component""" + + # The render function holds a reference to `self` and, via the `LifeCycleHook`, + # the component. Some tests check whether components are garbage collected, thus + # we must use a `ref` here to ensure these checks pass once the catcher itself + # has been collected. + self_ref = ref(self) + + @wraps(render_function) + def wrapper(*args: Any, **kwargs: Any) -> Any: + self = self_ref() + assert self is not None, "Hook catcher has been garbage collected" + + hook = current_hook() + if self.index_by_kwarg is not None: + self.index[kwargs[self.index_by_kwarg]] = hook + self.latest = hook + return render_function(*args, **kwargs) + + return wrapper + + +class StaticEventHandlers: + """Utility for capturing the target of a static set of event handlers + + Example: + .. code-block:: + + static_handlers = StaticEventHandlers("first", "second") + + @idom.component + def MyComponent(key): + state, set_state = idom.hooks.use_state(0) + handler = static_handlers.use(key, lambda event: set_state(state + 1)) + return idom.html.button({"onClick": handler}, "Click me!") + + # gives the target ID for onClick where MyComponent(key="first") + first_target = static_handlers.targets["first"] + """ + + def __init__(self, *index: Any) -> None: + if not index: + raise ValueError("Static set of index keys are required") + self._handlers: Dict[Any, EventHandler] = {i: EventHandler() for i in index} + self.targets: Dict[Any, str] = {i: hex_id(h) for i, h in self._handlers.items()} + + @overload + def use( + self, + index: Any, + function: Literal[None] = ..., + ) -> Callable[[Callable[..., Any]], EventHandler]: + ... + + @overload + def use(self, index: Any, function: Callable[..., Any]) -> EventHandler: + ... + + def use(self, index: Any, function: Optional[Callable[..., Any]] = None) -> Any: + """Decorator for capturing an event handler function""" + + def setup(function: Callable[..., Any]) -> EventHandler: + handler = self._handlers[index] + handler.clear() + handler.add(function) + return handler + + if function is not None: + return setup(function) + else: + return setup diff --git a/tests/general_utils.py b/tests/general_utils.py index e2ea2cdca..978dc6a2f 100644 --- a/tests/general_utils.py +++ b/tests/general_utils.py @@ -1,12 +1,6 @@ # dialect=pytest from contextlib import contextmanager -from functools import wraps -from weakref import ref - -import idom -from idom.core.events import EventHandler -from idom.core.utils import hex_id @contextmanager @@ -21,74 +15,6 @@ def patch_slots_object(obj, attr, new_value): setattr(obj, attr, old_value) -class EventCatcher: - """Utility for capturing the target of an event handler - - Example: - .. code-block:: - - event_catcher = EventCatcher() - - @idom.component - def MyComponent(): - state, set_state = idom.hooks.use_state(0) - handler = event_catcher.capture(lambda event: set_state(state + 1)) - return idom.html.button({"onClick": handler}, "Click me!") - """ - - def __init__(self): - self._event_handler = EventHandler() - - @property - def target(self) -> str: - return hex_id(self._event_handler) - - def capture(self, function) -> EventHandler: - """Called within the body of a component to create a captured event handler""" - self._event_handler.clear() - self._event_handler.add(function) - return self._event_handler - - -class HookCatcher: - """Utility for capturing a LifeCycleHook from a component - - Example: - .. code-block:: - - component_hook = HookCatcher() - - @idom.component - @component_hook.capture - def MyComponent(): - ... - - After the first render of ``MyComponent`` the ``HookCatcher`` will have - captured the component's ``LifeCycleHook``. - """ - - current: idom.hooks.LifeCycleHook - - def capture(self, render_function): - """Decorator for capturing a ``LifeCycleHook`` on the first render of a component""" - - # The render function holds a reference to `self` and, via the `LifeCycleHook`, - # the component. Some tests check whether components are garbage collected, thus we - # must use a `ref` here to ensure these checks pass. - self_ref = ref(self) - - @wraps(render_function) - def wrapper(*args, **kwargs): - self_ref().current = idom.hooks.current_hook() - return render_function(*args, **kwargs) - - return wrapper - - def schedule_render(self) -> None: - """Useful alias of ``HookCatcher.current.schedule_render``""" - self.current.schedule_render() - - def assert_same_items(left, right): """Check that two unordered sequences are equal (only works if reprs are equal)""" sorted_left = list(sorted(left, key=repr)) diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index c9176777f..34f32704c 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -10,7 +10,8 @@ SingleViewDispatcher, ) from idom.core.layout import Layout, LayoutEvent -from tests.general_utils import EventCatcher, assert_same_items +from idom.testing import StaticEventHandlers +from tests.general_utils import assert_same_items async def test_shared_state_dispatcher(): @@ -19,9 +20,9 @@ async def test_shared_state_dispatcher(): changes_2 = [] event_name = "onEvent" - event_catcher = EventCatcher() + event_handlers = StaticEventHandlers(event_name) - events_to_inject = [LayoutEvent(target=event_catcher.target, data=[])] * 4 + events_to_inject = [LayoutEvent(event_handlers.targets[event_name], [])] * 4 async def send_1(patch): changes_1.append(patch.changes) @@ -47,7 +48,7 @@ async def recv_2(): @idom.component def Clickable(): count, set_count = idom.hooks.use_state(0) - handler = event_catcher.capture(lambda: set_count(count + 1)) + handler = event_handlers.use(event_name, lambda: set_count(count + 1)) return idom.html.div({event_name: handler, "count": count}) async with SharedViewDispatcher(Layout(Clickable())) as dispatcher: @@ -61,7 +62,7 @@ def Clickable(): "path": "/eventHandlers", "value": { event_name: { - "target": event_catcher.target, + "target": event_handlers.targets[event_name], "preventDefault": False, "stopPropagation": False, } diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index c7d373b4f..bcce896ef 100644 --- a/tests/test_core/test_hooks.py +++ b/tests/test_core/test_hooks.py @@ -4,7 +4,8 @@ import pytest import idom -from tests.general_utils import HookCatcher, assert_same_items +from idom.testing import HookCatcher +from tests.general_utils import assert_same_items async def test_must_be_rendering_in_layout_to_use_hooks(): @@ -345,7 +346,7 @@ def cleanup(): assert not cleanup_triggered.current - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert cleanup_triggered.current @@ -384,7 +385,7 @@ def cleanup(): assert not cleanup_triggered.current - outer_component_hook.schedule_render() + outer_component_hook.latest.schedule_render() await layout.render() assert cleanup_triggered.current @@ -415,7 +416,7 @@ def effect(): assert effect_run_count.current == 1 - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert effect_run_count.current == 1 @@ -425,7 +426,7 @@ def effect(): assert effect_run_count.current == 2 - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert effect_run_count.current == 2 @@ -458,7 +459,7 @@ def cleanup(): assert cleanup_trigger_count.current == 0 - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert cleanup_trigger_count.current == 0 @@ -504,7 +505,7 @@ async def effect(): await layout.render() await effect_ran.wait() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() @@ -536,7 +537,7 @@ async def effect(): await layout.render() await effect_ran.wait() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() @@ -583,7 +584,7 @@ def bad_cleanup(): async with idom.Layout(ComponentWithEffect()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() # no error first_log_line = next(iter(caplog.records)).msg.split("\n", 1)[0] @@ -611,7 +612,7 @@ def bad_cleanup(): async with idom.Layout(OuterComponent()) as layout: await layout.render() - outer_component_hook.schedule_render() + outer_component_hook.latest.schedule_render() await layout.render() # no error first_log_line = next(iter(caplog.records)).msg.split("\n", 1)[0] @@ -689,7 +690,7 @@ def ComponentWithRef(): async with idom.Layout(ComponentWithRef()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_callbacks[0] is used_callbacks[1] @@ -717,7 +718,7 @@ def cb(): await layout.render() set_state_hook.current(1) await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_callbacks[0] is not used_callbacks[1] @@ -745,7 +746,7 @@ def ComponentWithMemo(): await layout.render() set_state_hook.current(1) await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_values[0] is not used_values[1] @@ -768,9 +769,9 @@ def ComponentWithMemo(): async with idom.Layout(ComponentWithMemo()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_values == [1, 2, 3] @@ -795,10 +796,10 @@ def ComponentWithMemo(): async with idom.Layout(ComponentWithMemo()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() args_used_in_memo.current = None await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() args_used_in_memo.current = () await layout.render() @@ -820,9 +821,9 @@ def ComponentWithMemo(): async with idom.Layout(ComponentWithMemo()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_values == [1, 1, 1] @@ -840,7 +841,7 @@ def ComponentWithRef(): async with idom.Layout(ComponentWithRef()) as layout: await layout.render() - component_hook.schedule_render() + component_hook.latest.schedule_render() await layout.render() assert used_refs[0] is used_refs[1] diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 7f8e4a87c..5f52288d5 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -7,7 +7,8 @@ import idom from idom.core.layout import LayoutEvent, LayoutUpdate from idom.core.utils import hex_id -from tests.general_utils import EventCatcher, HookCatcher, assert_same_items +from idom.testing import HookCatcher, StaticEventHandlers +from tests.general_utils import assert_same_items def test_layout_update_create_from_apply_to(): @@ -200,7 +201,7 @@ def Inner(): # the the old `Inner` component should be deleted. Thus there should be one # changed component in the set of `live_components` the old `Inner` deleted and new # `Inner` added. - outer_component_hook.schedule_render() + outer_component_hook.latest.schedule_render() await layout.render() assert len(live_components - last_live_components) == 1 @@ -229,8 +230,8 @@ def AnyComponent(): assert run_count.current == 1 - hook.schedule_render() - hook.schedule_render() + hook.latest.schedule_render() + hook.latest.schedule_render() await layout.render() try: @@ -259,7 +260,7 @@ def Child(): async with idom.Layout(Parent()) as layout: await layout.render() - hook.current.schedule_render() + hook.latest.schedule_render() update = await layout.render() assert update.path == "/children/0/children/0" @@ -286,19 +287,18 @@ def use_toggle(init=False): async def test_model_key_preserves_callback_identity_for_common_elements(): called_good_trigger = idom.Ref(False) - good_event_catcher = EventCatcher() - bad_event_catcher = EventCatcher() + event_handlers = StaticEventHandlers("good", "bad") @idom.component def MyComponent(): reverse_children, set_reverse_children = use_toggle() - @good_event_catcher.capture + @event_handlers.use("good") def good_trigger(): called_good_trigger.current = True set_reverse_children() - @bad_event_catcher.capture + @event_handlers.use("bad") def bad_trigger(): raise ValueError("Called bad trigger") @@ -317,7 +317,7 @@ def bad_trigger(): async with idom.Layout(MyComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent(good_event_catcher.target, []) + event = LayoutEvent(event_handlers.targets["good"], []) await layout.dispatch(event) assert called_good_trigger.current @@ -329,8 +329,7 @@ def bad_trigger(): async def test_model_key_preserves_callback_identity_for_components(): called_good_trigger = idom.Ref(False) - good_event_catcher = EventCatcher() - bad_event_catcher = EventCatcher() + event_handlers = StaticEventHandlers("good", "bad") @idom.component def RootComponent(): @@ -347,14 +346,14 @@ def RootComponent(): def Trigger(set_reverse_children, key): if key == "good": - @good_event_catcher.capture + @event_handlers.use("good") def callback(): called_good_trigger.current = True set_reverse_children() else: - @bad_event_catcher.capture + @event_handlers.use("bad") def callback(): raise ValueError("Called bad trigger") @@ -362,8 +361,8 @@ def callback(): async with idom.Layout(RootComponent()) as layout: await layout.render() - for i in range(3): - event = LayoutEvent(good_event_catcher.target, []) + for _ in range(3): + event = LayoutEvent(event_handlers.targets["good"], []) await layout.dispatch(event) assert called_good_trigger.current @@ -462,8 +461,8 @@ def Inner(key): async with idom.Layout(Outer()) as layout: await layout.render() - old_inner_hook = inner_hook.current + old_inner_hook = inner_hook.latest - outer_hook.current.schedule_render() + outer_hook.latest.schedule_render() await layout.render() - assert old_inner_hook is inner_hook.current + assert old_inner_hook is inner_hook.latest From 4b10098cdfdc2c40d07d84e55498af02e7df0c82 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 23:27:25 -0700 Subject: [PATCH 14/20] simplify to singular StaticEventHandler --- docs/source/core-concepts.rst | 15 ++++---- src/idom/testing.py | 56 +++++++++--------------------- tests/test_core/test_dispatcher.py | 10 +++--- tests/test_core/test_layout.py | 20 ++++++----- 4 files changed, 42 insertions(+), 59 deletions(-) diff --git a/docs/source/core-concepts.rst b/docs/source/core-concepts.rst index 76bec6b08..5eb10b5e1 100644 --- a/docs/source/core-concepts.rst +++ b/docs/source/core-concepts.rst @@ -75,20 +75,23 @@ which we can re-render and see what changed: .. testcode:: from idom.core.layout import LayoutEvent + from idom.testing import StaticEventHandler + static_handler = StaticEventHandler() @idom.component def ClickCount(): count, set_count = idom.hooks.use_state(0) - return idom.html.button( - {"onClick": lambda event: set_count(count + 1)}, - [f"Click count: {count}"], - ) + + # we do this in order to capture the event handler's target ID + handler = static_handler.use(lambda event: set_count(count + 1)) + + return idom.html.button({"onClick": handler}, [f"Click count: {count}"]) async with idom.Layout(ClickCount()) as layout: patch_1 = await layout.render() - fake_event = LayoutEvent(target="/onClick", data=[{}]) + fake_event = LayoutEvent(target=static_handler.target, data=[{}]) await layout.dispatch(fake_event) patch_2 = await layout.render() @@ -135,7 +138,7 @@ callback that's called by the dispatcher to events it should execute. async def recv(): - event = LayoutEvent(target="/onClick", data=[{}]) + event = LayoutEvent(target=static_handler.target, data=[{}]) # We need this so we don't flood the render loop with events. # In practice this is never an issue since events won't arrive diff --git a/src/idom/testing.py b/src/idom/testing.py index bad261a91..0ebffb2f7 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -13,14 +13,12 @@ Type, TypeVar, Union, - overload, ) from urllib.parse import urlencode, urlunparse from weakref import ref from selenium.webdriver import Chrome from selenium.webdriver.remote.webdriver import WebDriver -from typing_extensions import Literal from idom.core.events import EventHandler from idom.core.hooks import LifeCycleHook, current_hook @@ -229,52 +227,32 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: return wrapper -class StaticEventHandlers: - """Utility for capturing the target of a static set of event handlers +class StaticEventHandler: + """Utility for capturing the target of one event handler Example: .. code-block:: - static_handlers = StaticEventHandlers("first", "second") + static_handler = StaticEventHandler() @idom.component - def MyComponent(key): + def MyComponent(): state, set_state = idom.hooks.use_state(0) - handler = static_handlers.use(key, lambda event: set_state(state + 1)) + handler = static_handler.use(lambda event: set_state(state + 1)) return idom.html.button({"onClick": handler}, "Click me!") - # gives the target ID for onClick where MyComponent(key="first") - first_target = static_handlers.targets["first"] + # gives the target ID for onClick where from the last render of MyComponent + static_handlers.target """ - def __init__(self, *index: Any) -> None: - if not index: - raise ValueError("Static set of index keys are required") - self._handlers: Dict[Any, EventHandler] = {i: EventHandler() for i in index} - self.targets: Dict[Any, str] = {i: hex_id(h) for i, h in self._handlers.items()} + def __init__(self) -> None: + self._handler = EventHandler() - @overload - def use( - self, - index: Any, - function: Literal[None] = ..., - ) -> Callable[[Callable[..., Any]], EventHandler]: - ... - - @overload - def use(self, index: Any, function: Callable[..., Any]) -> EventHandler: - ... - - def use(self, index: Any, function: Optional[Callable[..., Any]] = None) -> Any: - """Decorator for capturing an event handler function""" - - def setup(function: Callable[..., Any]) -> EventHandler: - handler = self._handlers[index] - handler.clear() - handler.add(function) - return handler - - if function is not None: - return setup(function) - else: - return setup + @property + def target(self) -> str: + return hex_id(self._handler) + + def use(self, function: Callable[..., Any]) -> EventHandler: + self._handler.clear() + self._handler.add(function) + return self._handler diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_dispatcher.py index 34f32704c..3858336d4 100644 --- a/tests/test_core/test_dispatcher.py +++ b/tests/test_core/test_dispatcher.py @@ -10,7 +10,7 @@ SingleViewDispatcher, ) from idom.core.layout import Layout, LayoutEvent -from idom.testing import StaticEventHandlers +from idom.testing import StaticEventHandler from tests.general_utils import assert_same_items @@ -20,9 +20,9 @@ async def test_shared_state_dispatcher(): changes_2 = [] event_name = "onEvent" - event_handlers = StaticEventHandlers(event_name) + event_handler = StaticEventHandler() - events_to_inject = [LayoutEvent(event_handlers.targets[event_name], [])] * 4 + events_to_inject = [LayoutEvent(event_handler.target, [])] * 4 async def send_1(patch): changes_1.append(patch.changes) @@ -48,7 +48,7 @@ async def recv_2(): @idom.component def Clickable(): count, set_count = idom.hooks.use_state(0) - handler = event_handlers.use(event_name, lambda: set_count(count + 1)) + handler = event_handler.use(lambda: set_count(count + 1)) return idom.html.div({event_name: handler, "count": count}) async with SharedViewDispatcher(Layout(Clickable())) as dispatcher: @@ -62,7 +62,7 @@ def Clickable(): "path": "/eventHandlers", "value": { event_name: { - "target": event_handlers.targets[event_name], + "target": event_handler.target, "preventDefault": False, "stopPropagation": False, } diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 5f52288d5..d0fd98184 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -7,7 +7,7 @@ import idom from idom.core.layout import LayoutEvent, LayoutUpdate from idom.core.utils import hex_id -from idom.testing import HookCatcher, StaticEventHandlers +from idom.testing import HookCatcher, StaticEventHandler from tests.general_utils import assert_same_items @@ -287,18 +287,19 @@ def use_toggle(init=False): async def test_model_key_preserves_callback_identity_for_common_elements(): called_good_trigger = idom.Ref(False) - event_handlers = StaticEventHandlers("good", "bad") + good_handler = StaticEventHandler() + bad_handler = StaticEventHandler() @idom.component def MyComponent(): reverse_children, set_reverse_children = use_toggle() - @event_handlers.use("good") + @good_handler.use def good_trigger(): called_good_trigger.current = True set_reverse_children() - @event_handlers.use("bad") + @bad_handler.use def bad_trigger(): raise ValueError("Called bad trigger") @@ -317,7 +318,7 @@ def bad_trigger(): async with idom.Layout(MyComponent()) as layout: await layout.render() for i in range(3): - event = LayoutEvent(event_handlers.targets["good"], []) + event = LayoutEvent(good_handler.target, []) await layout.dispatch(event) assert called_good_trigger.current @@ -329,7 +330,8 @@ def bad_trigger(): async def test_model_key_preserves_callback_identity_for_components(): called_good_trigger = idom.Ref(False) - event_handlers = StaticEventHandlers("good", "bad") + good_handler = StaticEventHandler() + bad_handler = StaticEventHandler() @idom.component def RootComponent(): @@ -346,14 +348,14 @@ def RootComponent(): def Trigger(set_reverse_children, key): if key == "good": - @event_handlers.use("good") + @good_handler.use def callback(): called_good_trigger.current = True set_reverse_children() else: - @event_handlers.use("bad") + @bad_handler.use def callback(): raise ValueError("Called bad trigger") @@ -362,7 +364,7 @@ def callback(): async with idom.Layout(RootComponent()) as layout: await layout.render() for _ in range(3): - event = LayoutEvent(event_handlers.targets["good"], []) + event = LayoutEvent(good_handler.target, []) await layout.dispatch(event) assert called_good_trigger.current From ac945279ed0d593980a47d6e74a7d10ee6231378 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 12 Apr 2021 23:35:14 -0700 Subject: [PATCH 15/20] improve StaticEventHandler docs --- src/idom/testing.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/src/idom/testing.py b/src/idom/testing.py index 0ebffb2f7..c800f560c 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -243,6 +243,30 @@ def MyComponent(): # gives the target ID for onClick where from the last render of MyComponent static_handlers.target + + If you need to capture event handlers from different instances of a component + the you should create multiple ``StaticEventHandler`` instances. + + .. code-block:: + + static_handlers_by_key = { + "first": StaticEventHandler(), + "second": StaticEventHandler(), + } + + @idom.component + def Parent(): + return idom.html.div(Child(key="first"), Child(key="second")) + + @idom.component + def Child(key): + state, set_state = idom.hooks.use_state(0) + handler = static_handlers_by_key[key].use(lambda event: set_state(state + 1)) + return idom.html.button({"onClick": handler}, "Click me!") + + # grab the individual targets for each instance above + first_target = static_handlers_by_key["first"].target + second_target = static_handlers_by_key["second"].target """ def __init__(self) -> None: From 6fdc9a73c4fb12dfdf2f8d06a61dfce3a6bb072a Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 17 Apr 2021 13:08:28 -0700 Subject: [PATCH 16/20] clean up index as default key logic --- src/idom/core/component.py | 11 +++-- src/idom/core/layout.py | 87 ++++++++++++++++++++-------------- tests/test_core/test_layout.py | 2 +- 3 files changed, 59 insertions(+), 41 deletions(-) diff --git a/src/idom/core/component.py b/src/idom/core/component.py index 137f0c58c..e8e3cedd8 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -3,7 +3,7 @@ import abc import inspect from functools import wraps -from typing import Any, Callable, Dict, Tuple, Union +from typing import Any, Callable, Dict, Optional, Tuple, Union from .utils import hex_id from .vdom import VdomDict @@ -22,7 +22,7 @@ def component(function: ComponentRenderFunction) -> Callable[..., "Component"]: """ @wraps(function) - def constructor(*args: Any, key: str = "", **kwargs: Any) -> Component: + def constructor(*args: Any, key: Optional[Any] = None, **kwargs: Any) -> Component: return Component(function, key, args, kwargs) return constructor @@ -49,7 +49,7 @@ class Component(AbstractComponent): def __init__( self, function: ComponentRenderFunction, - key: str, + key: Optional[Any], args: Tuple[Any, ...], kwargs: Dict[str, Any], ) -> None: @@ -57,7 +57,7 @@ def __init__( self._function = function self._args = args self._kwargs = kwargs - if key: + if key is not None: kwargs["key"] = key def render(self) -> VdomDict: @@ -66,6 +66,9 @@ def render(self) -> VdomDict: model = {"tagName": "div", "children": [model]} return model + def __eq__(self, other: Any) -> bool: + return isinstance(other, Component) and other._func == self._func + def __repr__(self) -> str: sig = inspect.signature(self._function) try: diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index a7795e825..5a82660d3 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -2,6 +2,7 @@ import abc import asyncio +from collections import Counter from functools import wraps from logging import getLogger from typing import ( @@ -279,38 +280,25 @@ def _render_model_children( self._unmount_model_states(list(old_state.children_by_key.values())) return None - DICT_TYPE, COMPONENT_TYPE, STRING_TYPE = 1, 2, 3 # noqa + child_type_key_tuples = list(_process_child_type_and_key(raw_children)) - raw_typed_children_by_key: Dict[str, Tuple[int, Any]] = {} - for index, child in enumerate(raw_children): - if isinstance(child, dict): - child_type = DICT_TYPE - key = child.get("key") or hex_id(child) - elif isinstance(child, AbstractComponent): - child_type = COMPONENT_TYPE - key = getattr(child, "key", None) or hex_id(child) - else: - child = str(child) - child_type = STRING_TYPE - # The specific key doesn't matter here since we won't look it up - all - # we care about is that the key is unique, which object() can guarantee. - key = object() - raw_typed_children_by_key[key] = (child_type, child) - - if len(raw_typed_children_by_key) != len(raw_children): - raise ValueError(f"Duplicate keys in {raw_children}") - - old_keys = set(old_state.children_by_key).difference(raw_typed_children_by_key) - old_child_states = {key: old_state.children_by_key[key] for key in old_keys} + 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) + duplicate_keys = [key for key, count in key_counter.items() if count > 1] + raise ValueError( + f"Duplicate keys {duplicate_keys} at {new_state.patch_path or '/'!r}" + ) + + old_keys = set(old_state.children_by_key).difference(new_keys) if old_keys: - self._unmount_model_states(list(old_child_states.values())) + self._unmount_model_states( + [old_state.children_by_key[key] for key in old_keys] + ) new_children = new_state.model["children"] = [] - for index, (key, (child_type, child)) in ( - # we can enumerate this because dict insertion order is preserved - enumerate(raw_typed_children_by_key.items()) - ): - if child_type is DICT_TYPE: + for index, (child, child_type, key) in enumerate(child_type_key_tuples): + if child_type is _DICT_TYPE: old_child_state = old_state.children_by_key.get(key) if old_child_state is not None: new_child_state = old_child_state.new(new_state, None) @@ -319,7 +307,7 @@ def _render_model_children( self._render_model(old_child_state, new_child_state, child) new_children.append(new_child_state.model) new_state.children_by_key[key] = new_child_state - elif child_type is COMPONENT_TYPE: + elif child_type is _COMPONENT_TYPE: old_child_state = old_state.children_by_key.get(key) if old_child_state is not None: new_child_state = old_child_state.new(new_state, child) @@ -334,20 +322,20 @@ def _render_model_children_without_old_state( self, new_state: _ModelState, raw_children: List[Any] ) -> None: new_children = new_state.model["children"] = [] - for index, child in enumerate(raw_children): - if isinstance(child, dict): - key = child.get("key") or hex_id(child) + for index, (child, child_type, key) in enumerate( + _process_child_type_and_key(raw_children) + ): + if child_type is _DICT_TYPE: child_state = _ModelState(new_state, index, key, None) self._render_model(None, child_state, child) new_children.append(child_state.model) new_state.children_by_key[key] = child_state - elif isinstance(child, AbstractComponent): - key = getattr(child, "key", "") or hex_id(child) + elif child_type is _COMPONENT_TYPE: life_cycle_hook = LifeCycleHook(child, self) child_state = _ModelState(new_state, index, key, life_cycle_hook) self._render_component(None, child_state, child) else: - new_children.append(str(child)) + new_children.append(child) def _unmount_model_states(self, old_states: List[_ModelState]) -> None: to_unmount = old_states[::-1] @@ -387,7 +375,7 @@ def __init__( self, parent: Optional[_ModelState], index: int, - key: str, + key: Any, life_cycle_hook: Optional[LifeCycleHook], ) -> None: self.index = index @@ -487,3 +475,30 @@ class _ModelVdomRequired(TypedDict, total=True): class _ModelVdom(_ModelVdomRequired, _ModelVdomOptional): """A VDOM dictionary model specifically for use with a :class:`Layout`""" + + +def _process_child_type_and_key( + raw_children: List[Any], +) -> Iterator[Tuple[Any, int, Any]]: + for index, child in enumerate(raw_children): + if isinstance(child, dict): + child_type = _DICT_TYPE + key = child.get("key") + elif isinstance(child, AbstractComponent): + child_type = _COMPONENT_TYPE + key = getattr(child, "key", None) + else: + child = f"{child}" + child_type = _STRING_TYPE + key = None + + if key is None: + key = index + + yield (child, child_type, key) + + +# used in _process_child_type_and_key +_DICT_TYPE = 1 +_COMPONENT_TYPE = 2 +_STRING_TYPE = 3 diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index d0fd98184..523812842 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -443,7 +443,7 @@ def ComponentReturnsDuplicateKeys(): async with idom.Layout(ComponentReturnsDuplicateKeys()) as layout: await layout.render() - with pytest.raises(ValueError, match="Duplicate keys in"): + with pytest.raises(ValueError, match=r"Duplicate keys \['duplicate'\] at '/'"): raise next(iter(caplog.records)).exc_info[1] From cc23ad0afcd160b65471c830bc864bca390fc157 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 17 Apr 2021 13:15:49 -0700 Subject: [PATCH 17/20] fix up types --- src/idom/core/component.py | 16 ++++++++-------- src/idom/widgets/html.py | 14 +++++++++++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/idom/core/component.py b/src/idom/core/component.py index e8e3cedd8..1678c4cc2 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -34,7 +34,7 @@ class AbstractComponent(abc.ABC): if not hasattr(abc.ABC, "__weakref__"): __slots__.append("__weakref__") # pragma: no cover - key: str + key: Optional[Any] @abc.abstractmethod def render(self) -> VdomDict: @@ -44,7 +44,7 @@ def render(self) -> VdomDict: class Component(AbstractComponent): """An object for rending component models.""" - __slots__ = "_function", "_args", "_kwargs" + __slots__ = "_func", "_args", "_kwargs" def __init__( self, @@ -54,14 +54,14 @@ def __init__( kwargs: Dict[str, Any], ) -> None: self.key = key - self._function = function + self._func = function self._args = args self._kwargs = kwargs if key is not None: kwargs["key"] = key def render(self) -> VdomDict: - model = self._function(*self._args, **self._kwargs) + model = self._func(*self._args, **self._kwargs) if isinstance(model, AbstractComponent): model = {"tagName": "div", "children": [model]} return model @@ -70,14 +70,14 @@ def __eq__(self, other: Any) -> bool: return isinstance(other, Component) and other._func == self._func def __repr__(self) -> str: - sig = inspect.signature(self._function) + sig = inspect.signature(self._func) try: args = sig.bind(*self._args, **self._kwargs).arguments except TypeError: - return f"{self._function.__name__}(...)" + return f"{self._func.__name__}(...)" else: items = ", ".join(f"{k}={v!r}" for k, v in args.items()) if items: - return f"{self._function.__name__}({hex_id(self)}, {items})" + return f"{self._func.__name__}({hex_id(self)}, {items})" else: - return f"{self._function.__name__}({hex_id(self)})" + return f"{self._func.__name__}({hex_id(self)})" diff --git a/src/idom/widgets/html.py b/src/idom/widgets/html.py index 20bf894db..1fc4b1287 100644 --- a/src/idom/widgets/html.py +++ b/src/idom/widgets/html.py @@ -1,5 +1,5 @@ from base64 import b64encode -from typing import Any, Callable, Dict, Optional, Union +from typing import Any, Callable, Dict, Optional, Union, overload import idom from idom.core.component import AbstractComponent, ComponentConstructor, component @@ -166,6 +166,18 @@ def __init__(self) -> None: self.menuitem = make_vdom_constructor("menuitem") self.summary = make_vdom_constructor("summary") + @overload + @staticmethod + def __call__( + tag: ComponentConstructor, *attributes_and_children: Any + ) -> AbstractComponent: + ... + + @overload + @staticmethod + def __call__(tag: str, *attributes_and_children: Any) -> VdomDict: + ... + @staticmethod def __call__( tag: Union[str, ComponentConstructor], From 427a2ba60ea3245330a1c410512d036b1ba47e02 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 17 Apr 2021 13:23:25 -0700 Subject: [PATCH 18/20] use unique object instead of index as default key we can change this in a later pull request --- src/idom/core/layout.py | 54 +++++++++++++++++----------------- tests/test_core/test_layout.py | 4 +-- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 5a82660d3..4ded5e1f4 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -450,6 +450,33 @@ async def get(self) -> AbstractComponent: return component +def _process_child_type_and_key( + children: List[Any], +) -> Iterator[Tuple[Any, int, Any]]: + for child in children: + if isinstance(child, dict): + child_type = _DICT_TYPE + key = child.get("key") + elif isinstance(child, AbstractComponent): + child_type = _COMPONENT_TYPE + key = getattr(child, "key", None) + else: + child = f"{child}" + child_type = _STRING_TYPE + key = None + + if key is None: + key = object() + + yield (child, child_type, key) + + +# used in _process_child_type_and_key +_DICT_TYPE = 1 +_COMPONENT_TYPE = 2 +_STRING_TYPE = 3 + + class _ModelEventTarget(TypedDict): target: str preventDefault: bool # noqa @@ -475,30 +502,3 @@ class _ModelVdomRequired(TypedDict, total=True): class _ModelVdom(_ModelVdomRequired, _ModelVdomOptional): """A VDOM dictionary model specifically for use with a :class:`Layout`""" - - -def _process_child_type_and_key( - raw_children: List[Any], -) -> Iterator[Tuple[Any, int, Any]]: - for index, child in enumerate(raw_children): - if isinstance(child, dict): - child_type = _DICT_TYPE - key = child.get("key") - elif isinstance(child, AbstractComponent): - child_type = _COMPONENT_TYPE - key = getattr(child, "key", None) - else: - child = f"{child}" - child_type = _STRING_TYPE - key = None - - if key is None: - key = index - - yield (child, child_type, key) - - -# used in _process_child_type_and_key -_DICT_TYPE = 1 -_COMPONENT_TYPE = 2 -_STRING_TYPE = 3 diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 523812842..d61152756 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -181,14 +181,14 @@ async def test_components_are_garbage_collected(): def Outer(): component = idom.hooks.current_hook().component live_components.add(id(component)) - finalize(component, live_components.remove, id(component)) + finalize(component, live_components.discard, id(component)) return Inner() @idom.component def Inner(): component = idom.hooks.current_hook().component live_components.add(id(component)) - finalize(component, live_components.remove, id(component)) + finalize(component, live_components.discard, id(component)) return idom.html.div() async with idom.Layout(Outer()) as layout: From 5d7aabc1565f8a87272535952dfe6b79e928873e Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 17 Apr 2021 13:50:32 -0700 Subject: [PATCH 19/20] add feature flag for default key behavior --- src/idom/config.py | 26 ++++++++++++++++++++++++++ src/idom/core/layout.py | 18 +++++++++++++++--- 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/src/idom/config.py b/src/idom/config.py index c1ec85ebe..a7e952f4a 100644 --- a/src/idom/config.py +++ b/src/idom/config.py @@ -52,3 +52,29 @@ Absolute URL ``ABSOLUTE_PATH/my-module.js`` """ + +IDOM_FEATURE_INDEX_AS_DEFAULT_KEY = _option.Option( + "IDOM_FEATURE_INDEX_AS_DEFAULT_KEY", + default=False, + mutable=False, + validator=lambda x: bool(int(x)), +) +"""A feature flag for using the index of a sibling element as its default key + +In a future release this flag's default value will be set to true, and after that, this +flag willbe removed entirely and the indices will always be the default key. + +For more information on changes to this feature flag see: https://github.com/idom-team/idom/issues/351 +""" + +if not IDOM_FEATURE_INDEX_AS_DEFAULT_KEY.get(): # pragma: no cover + from warnings import warn + + warn( + ( + "In a future release 'IDOM_FEATURE_INDEX_AS_DEFAULT_KEY' will be turned on " + "by default. For more information on changes to this feature flag, see: " + "https://github.com/idom-team/idom/issues/351" + ), + UserWarning, + ) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 4ded5e1f4..4873d6796 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -21,7 +21,7 @@ from jsonpatch import apply_patch, make_patch from typing_extensions import TypedDict -from idom.config import IDOM_DEBUG_MODE +from idom.config import IDOM_DEBUG_MODE, IDOM_FEATURE_INDEX_AS_DEFAULT_KEY from .component import AbstractComponent from .events import EventHandler @@ -453,7 +453,7 @@ async def get(self) -> AbstractComponent: def _process_child_type_and_key( children: List[Any], ) -> Iterator[Tuple[Any, int, Any]]: - for child in children: + for index, child in enumerate(children): if isinstance(child, dict): child_type = _DICT_TYPE key = child.get("key") @@ -466,11 +466,23 @@ def _process_child_type_and_key( key = None if key is None: - key = object() + key = _default_key(index) yield (child, child_type, key) +if IDOM_FEATURE_INDEX_AS_DEFAULT_KEY.get(): + + def _default_key(index: int) -> Any: # pragma: no cover + return index + + +else: + + def _default_key(index: int) -> Any: + return object() + + # used in _process_child_type_and_key _DICT_TYPE = 1 _COMPONENT_TYPE = 2 From d91baac0a0ce97cbe9a7c770a7516a0e80a96f79 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 17 Apr 2021 13:50:53 -0700 Subject: [PATCH 20/20] checking component equivalence can be added later --- src/idom/core/component.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/idom/core/component.py b/src/idom/core/component.py index 1678c4cc2..08a1990ed 100644 --- a/src/idom/core/component.py +++ b/src/idom/core/component.py @@ -66,9 +66,6 @@ def render(self) -> VdomDict: model = {"tagName": "div", "children": [model]} return model - def __eq__(self, other: Any) -> bool: - return isinstance(other, Component) and other._func == self._func - def __repr__(self) -> str: sig = inspect.signature(self._func) try: