From f318624829e7922e54393028422ebc8ffcef33f6 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 17:31:57 -0800 Subject: [PATCH 1/6] make IDOM_DEBUG_MODE mutable + add Option.subscribe subscribe() allows users to listen when a mutable option is changed --- src/idom/_option.py | 27 ++++++++++++++-- src/idom/config.py | 4 +-- src/idom/core/layout.py | 23 ++++---------- src/idom/core/vdom.py | 70 ++++++++++++++++------------------------- src/idom/logging.py | 14 +++++---- src/idom/web/module.py | 12 ++++--- tests/test__option.py | 22 +++++++++++++ tests/test_config.py | 7 +++++ 8 files changed, 103 insertions(+), 76 deletions(-) create mode 100644 tests/test_config.py diff --git a/src/idom/_option.py b/src/idom/_option.py index 1367e6bd9..85fbaa142 100644 --- a/src/idom/_option.py +++ b/src/idom/_option.py @@ -16,16 +16,25 @@ class Option(Generic[_O]): def __init__( self, name: str, - default: _O, + default: _O | Option[_O], mutable: bool = True, validator: Callable[[Any], _O] = lambda x: cast(_O, x), ) -> None: self._name = name - self._default = default self._mutable = mutable self._validator = validator + self._subscribers: list[Callable[[_O], None]] = [] + if name in os.environ: self._current = validator(os.environ[name]) + + self._default: _O + if isinstance(default, Option): + self._default = default.default + default.subscribe(lambda value: setattr(self, "_default", value)) + else: + self._default = default + logger.debug(f"{self._name}={self.current}") @property @@ -55,6 +64,14 @@ def current(self, new: _O) -> None: self.set_current(new) return None + def subscribe(self, handler: Callable[[_O], None]) -> Callable[[_O], None]: + """Register a callback that will be triggered when this option changes""" + if not self.mutable: + raise TypeError("Immutable options cannot be subscribed to.") + self._subscribers.append(handler) + handler(self.current) + return handler + def is_set(self) -> bool: """Whether this option has a value other than its default.""" return hasattr(self, "_current") @@ -66,8 +83,12 @@ def set_current(self, new: Any) -> None: """ if not self._mutable: raise TypeError(f"{self} cannot be modified after initial load") - self._current = self._validator(new) + old = self.current + new = self._current = self._validator(new) logger.debug(f"{self._name}={self._current}") + if new != old: + for sub_func in self._subscribers: + sub_func(new) def set_default(self, new: _O) -> _O: """Set the value of this option if not :meth:`Option.is_set` diff --git a/src/idom/config.py b/src/idom/config.py index 950b7402c..ab65feea6 100644 --- a/src/idom/config.py +++ b/src/idom/config.py @@ -13,7 +13,6 @@ IDOM_DEBUG_MODE = _Option( "IDOM_DEBUG_MODE", default=False, - mutable=False, validator=lambda x: bool(int(x)), ) """This immutable option turns on/off debug mode @@ -27,8 +26,7 @@ IDOM_CHECK_VDOM_SPEC = _Option( "IDOM_CHECK_VDOM_SPEC", - default=IDOM_DEBUG_MODE.current, - mutable=False, + default=IDOM_DEBUG_MODE, validator=lambda x: bool(int(x)), ) """This immutable option turns on/off checks which ensure VDOM is rendered to spec diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 4a2561190..16f51ba25 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -135,23 +135,12 @@ async def render(self) -> LayoutUpdate: f"{model_state_id!r} - component already unmounted" ) else: - return self._create_layout_update(model_state) - - if IDOM_CHECK_VDOM_SPEC.current: - # If in debug mode inject a function that ensures all returned updates - # contain valid VDOM models. We only do this in debug mode or when this check - # is explicitely turned in order to avoid unnecessarily impacting performance. - - _debug_render = render - - @wraps(_debug_render) - async def render(self) -> LayoutUpdate: - result = await self._debug_render() - # Ensure that the model is valid VDOM on each render - root_id = self._root_life_cycle_state_id - root_model = self._model_states_by_life_cycle_state_id[root_id] - validate_vdom_json(root_model.model.current) - return result + update = self._create_layout_update(model_state) + if IDOM_CHECK_VDOM_SPEC.current: + root_id = self._root_life_cycle_state_id + root_model = self._model_states_by_life_cycle_state_id[root_id] + validate_vdom_json(root_model.model.current) + return update def _create_layout_update(self, old_state: _ModelState) -> LayoutUpdate: new_state = _copy_component_model_state(old_state) diff --git a/src/idom/core/vdom.py b/src/idom/core/vdom.py index 4af5bb009..78bfb3725 100644 --- a/src/idom/core/vdom.py +++ b/src/idom/core/vdom.py @@ -13,6 +13,7 @@ to_event_handler_function, ) from idom.core.types import ( + ComponentType, EventHandlerDict, EventHandlerMapping, EventHandlerType, @@ -295,47 +296,30 @@ def _is_attributes(value: Any) -> bool: return isinstance(value, Mapping) and "tagName" not in value -if IDOM_DEBUG_MODE.current: - - _debug_is_attributes = _is_attributes - - def _is_attributes(value: Any) -> bool: - result = _debug_is_attributes(value) - if result and "children" in value: - logger.error(f"Reserved key 'children' found in attributes {value}") - return result - - def _is_single_child(value: Any) -> bool: - return isinstance(value, (str, Mapping)) or not hasattr(value, "__iter__") - - -if IDOM_DEBUG_MODE.current: - - _debug_is_single_child = _is_single_child - - def _is_single_child(value: Any) -> bool: - if _debug_is_single_child(value): - return True - - from .types import ComponentType - - if hasattr(value, "__iter__") and not hasattr(value, "__len__"): - logger.error( - f"Did not verify key-path integrity of children in generator {value} " - "- pass a sequence (i.e. list of finite length) in order to verify" - ) - else: - for child in value: - if isinstance(child, ComponentType) and child.key is None: - logger.error(f"Key not specified for child in list {child}") - elif isinstance(child, Mapping) and "key" not in child: - # remove 'children' to reduce log spam - child_copy = {**child, "children": _EllipsisRepr()} - logger.error(f"Key not specified for child in list {child_copy}") - - return False - - class _EllipsisRepr: - def __repr__(self) -> str: - return "..." + if isinstance(value, (str, Mapping)) or not hasattr(value, "__iter__"): + return True + if IDOM_DEBUG_MODE.current: + _validate_child_key_integrity(value) + return False + + +def _validate_child_key_integrity(value: Any) -> None: + if hasattr(value, "__iter__") and not hasattr(value, "__len__"): + logger.error( + f"Did not verify key-path integrity of children in generator {value} " + "- pass a sequence (i.e. list of finite length) in order to verify" + ) + else: + for child in value: + if isinstance(child, ComponentType) and child.key is None: + logger.error(f"Key not specified for child in list {child}") + elif isinstance(child, Mapping) and "key" not in child: + # remove 'children' to reduce log spam + child_copy = {**child, "children": _EllipsisRepr()} + logger.error(f"Key not specified for child in list {child_copy}") + + +class _EllipsisRepr: + def __repr__(self) -> str: + return "..." diff --git a/src/idom/logging.py b/src/idom/logging.py index 4f77e72c2..d90887497 100644 --- a/src/idom/logging.py +++ b/src/idom/logging.py @@ -10,10 +10,7 @@ "version": 1, "disable_existing_loggers": False, "loggers": { - "idom": { - "level": "DEBUG" if IDOM_DEBUG_MODE.current else "INFO", - "handlers": ["console"], - }, + "idom": {"handlers": ["console"]}, }, "handlers": { "console": { @@ -37,5 +34,10 @@ """IDOM's root logger instance""" -if IDOM_DEBUG_MODE.current: - ROOT_LOGGER.debug("IDOM is in debug mode") +@IDOM_DEBUG_MODE.subscribe +def _set_debug_level(debug: bool) -> None: + if debug: + ROOT_LOGGER.setLevel("DEBUG") + ROOT_LOGGER.debug("IDOM is in debug mode") + else: + ROOT_LOGGER.setLevel("INFO") diff --git a/src/idom/web/module.py b/src/idom/web/module.py index 9e13d6900..9985900f3 100644 --- a/src/idom/web/module.py +++ b/src/idom/web/module.py @@ -36,7 +36,7 @@ def module_from_url( url: str, fallback: Optional[Any] = None, - resolve_exports: bool = IDOM_DEBUG_MODE.current, + resolve_exports: bool | None = None, resolve_exports_depth: int = 5, unmount_before_update: bool = False, ) -> WebModule: @@ -65,7 +65,11 @@ def module_from_url( file=None, export_names=( resolve_module_exports_from_url(url, resolve_exports_depth) - if resolve_exports + if ( + resolve_exports + if resolve_exports is not None + else IDOM_DEBUG_MODE.current + ) else None ), unmount_before_update=unmount_before_update, @@ -80,7 +84,7 @@ def module_from_template( package: str, cdn: str = "https://esm.sh", fallback: Optional[Any] = None, - resolve_exports: bool = IDOM_DEBUG_MODE.current, + resolve_exports: bool | None = None, resolve_exports_depth: int = 5, unmount_before_update: bool = False, ) -> WebModule: @@ -159,7 +163,7 @@ def module_from_file( name: str, file: Union[str, Path], fallback: Optional[Any] = None, - resolve_exports: bool = IDOM_DEBUG_MODE.current, + resolve_exports: bool | None = None, resolve_exports_depth: int = 5, unmount_before_update: bool = False, symlink: bool = False, diff --git a/tests/test__option.py b/tests/test__option.py index 45e5e896a..02d79a6d2 100644 --- a/tests/test__option.py +++ b/tests/test__option.py @@ -74,3 +74,25 @@ def test_option_set_default(): assert not opt.is_set() assert opt.set_default("new-value") == "new-value" assert opt.is_set() + + +def test_cannot_subscribe_immutable_option(): + opt = Option("A_FAKE_OPTION", "default", mutable=False) + with pytest.raises(TypeError, match="Immutable options cannot be subscribed to"): + opt.subscribe(lambda value: None) + + +def test_option_subscribe(): + opt = Option("A_FAKE_OPTION", "default") + + calls = [] + opt.subscribe(calls.append) + assert calls == ["default"] + + opt.current = "default" + # value did not change, so no trigger + assert calls == ["default"] + + opt.current = "new-1" + opt.current = "new-2" + assert calls == ["default", "new-1", "new-2"] diff --git a/tests/test_config.py b/tests/test_config.py new file mode 100644 index 000000000..2eae6e373 --- /dev/null +++ b/tests/test_config.py @@ -0,0 +1,7 @@ +from idom.config import IDOM_DEBUG_MODE + + +def test_idom_debug_mode_toggle(): + # just check that nothing breaks + IDOM_DEBUG_MODE.current = True + IDOM_DEBUG_MODE.current = False From d3eb025182b7660e8908eb3b3a0d46ff71bae7a3 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 17:37:08 -0800 Subject: [PATCH 2/6] update changelog --- docs/source/about/changelog.rst | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index 4b4cf55be..7587273ec 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -32,6 +32,11 @@ Unreleased - :pull:`835` - ability to customize the ```` element of IDOM's built-in client. - :pull:`835` - ``vdom_to_html`` utility function. +- :pull:`843` - Ability to subscribe to changes that are made to mutable options. + +**Fixed** + +- :issue:`582` - ``IDOM_DEBUG_MODE`` is now mutable and can be changed at runtime v0.41.0 From 07ddc068c77c1d4d982a2df476cc98381ec65f7c Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 19:08:16 -0800 Subject: [PATCH 3/6] remove check for children key in attrs --- tests/test_core/test_vdom.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/tests/test_core/test_vdom.py b/tests/test_core/test_vdom.py index 008a4f724..aeca2b4ea 100644 --- a/tests/test_core/test_vdom.py +++ b/tests/test_core/test_vdom.py @@ -317,16 +317,6 @@ def test_invalid_vdom(value, error_message_pattern): validate_vdom_json(value) -@pytest.mark.skipif(not IDOM_DEBUG_MODE.current, reason="Only logs in debug mode") -def test_debug_log_if_children_in_attributes(caplog): - idom.vdom("div", {"children": ["hello"]}) - assert len(caplog.records) == 1 - assert caplog.records[0].message.startswith( - "Reserved key 'children' found in attributes" - ) - caplog.records.clear() - - @pytest.mark.skipif(not IDOM_DEBUG_MODE.current, reason="Only logs in debug mode") def test_debug_log_cannot_verify_keypath_for_genereators(caplog): idom.vdom("div", (1 for i in range(10))) From 3aee8cf01cd3a43c6e456b6de5cf73723c6365cc Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 19:24:51 -0800 Subject: [PATCH 4/6] fix tests --- src/idom/_option.py | 4 ++++ tests/test__option.py | 3 +++ tests/test_config.py | 28 +++++++++++++++++++++++++--- 3 files changed, 32 insertions(+), 3 deletions(-) diff --git a/src/idom/_option.py b/src/idom/_option.py index 85fbaa142..3d2295f16 100644 --- a/src/idom/_option.py +++ b/src/idom/_option.py @@ -107,7 +107,11 @@ def unset(self) -> None: """Remove the current value, the default will be used until it is set again.""" if not self._mutable: raise TypeError(f"{self} cannot be modified after initial load") + old = self.current delattr(self, "_current") + if self.current != old: + for sub_func in self._subscribers: + sub_func(self.current) def __repr__(self) -> str: return f"Option({self._name}={self.current!r})" diff --git a/tests/test__option.py b/tests/test__option.py index 02d79a6d2..144860117 100644 --- a/tests/test__option.py +++ b/tests/test__option.py @@ -96,3 +96,6 @@ def test_option_subscribe(): opt.current = "new-1" opt.current = "new-2" assert calls == ["default", "new-1", "new-2"] + + opt.unset() + assert calls == ["default", "new-1", "new-2", "default"] diff --git a/tests/test_config.py b/tests/test_config.py index 2eae6e373..20f21fe4d 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,7 +1,29 @@ -from idom.config import IDOM_DEBUG_MODE +import pytest + +from idom import config +from idom._option import Option + + +@pytest.fixture(autouse=True) +def reset_options(): + options = [value for value in config.__dict__.values() if isinstance(value, Option)] + + should_unset = object() + original_values = [] + for opt in options: + original_values.append(opt.current if opt.is_set() else should_unset) + + yield + + for opt, val in zip(options, original_values): + if val is should_unset: + if opt.is_set(): + opt.unset() + else: + opt.current = val def test_idom_debug_mode_toggle(): # just check that nothing breaks - IDOM_DEBUG_MODE.current = True - IDOM_DEBUG_MODE.current = False + config.IDOM_DEBUG_MODE.current = True + config.IDOM_DEBUG_MODE.current = False From e4b9357ad4d210eed1ea967bdde35694c66d38e2 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 19:39:04 -0800 Subject: [PATCH 5/6] fix resolve_exports default --- src/idom/web/module.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/idom/web/module.py b/src/idom/web/module.py index 9985900f3..1c6eba98c 100644 --- a/src/idom/web/module.py +++ b/src/idom/web/module.py @@ -213,7 +213,11 @@ def module_from_file( file=target_file, export_names=( resolve_module_exports_from_file(source_file, resolve_exports_depth) - if resolve_exports + if ( + resolve_exports + if resolve_exports is not None + else IDOM_DEBUG_MODE.current + ) else None ), unmount_before_update=unmount_before_update, @@ -240,7 +244,7 @@ def module_from_string( name: str, content: str, fallback: Optional[Any] = None, - resolve_exports: bool = IDOM_DEBUG_MODE.current, + resolve_exports: bool | None = None, resolve_exports_depth: int = 5, unmount_before_update: bool = False, ) -> WebModule: @@ -284,7 +288,11 @@ def module_from_string( file=target_file, export_names=( resolve_module_exports_from_file(target_file, resolve_exports_depth) - if resolve_exports + if ( + resolve_exports + if resolve_exports is not None + else IDOM_DEBUG_MODE.current + ) else None ), unmount_before_update=unmount_before_update, From 9e4adc9a91f271d11e1da0c844a921f81862244f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 29 Nov 2022 19:46:33 -0800 Subject: [PATCH 6/6] remove unused import --- src/idom/core/layout.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 16f51ba25..bbc1848a5 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -4,7 +4,6 @@ import asyncio from collections import Counter from contextlib import ExitStack -from functools import wraps from logging import getLogger from typing import ( Any,