From 8ec39c5f5ace4dc798b80910f067482f94cf453f Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 10 Jan 2022 20:55:43 -0800 Subject: [PATCH 1/8] make index the default key --- src/idom/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/idom/config.py b/src/idom/config.py index 855b1d295..bf07f9891 100644 --- a/src/idom/config.py +++ b/src/idom/config.py @@ -58,7 +58,7 @@ IDOM_FEATURE_INDEX_AS_DEFAULT_KEY = _Option( "IDOM_FEATURE_INDEX_AS_DEFAULT_KEY", - default=False, + default=True, mutable=False, validator=lambda x: bool(int(x)), ) From 41518b5b6faca4d15981363bde14aa4379492259 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 10 Jan 2022 21:48:59 -0800 Subject: [PATCH 2/8] fix tests for index is default key --- src/idom/testing.py | 100 ++++++++++++++++++++------- tests/test_core/test_hooks.py | 120 ++++++++++++++------------------- tests/test_core/test_layout.py | 35 +++++----- 3 files changed, 140 insertions(+), 115 deletions(-) diff --git a/src/idom/testing.py b/src/idom/testing.py index 24592208e..87e853cc1 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -14,6 +14,7 @@ Generic, Iterator, List, + NoReturn, Optional, Tuple, Type, @@ -173,8 +174,12 @@ def __exit__( return None +class LogAssertionError(AssertionError): + """An assertion error raised in relation to log messages.""" + + @contextmanager -def assert_idom_logged( +def assert_idom_did_log( match_message: str = "", error_type: type[Exception] | None = None, match_error: str = "", @@ -192,11 +197,13 @@ def assert_idom_logged( error_pattern = re.compile(match_error) try: - with capture_idom_logs() as handler: + with capture_idom_logs(use_existing=clear_matched_records) as log_records: yield None - finally: + except Exception: + raise + else: found = False - for record in list(handler.records): + for record in list(log_records): if ( # record message matches message_pattern.findall(record.getMessage()) @@ -222,36 +229,79 @@ def assert_idom_logged( ): found = True if clear_matched_records: - handler.records.remove(record) + log_records.remove(record) if not found: # pragma: no cover - conditions = [] - if match_message: - conditions.append(f"log message pattern {match_message!r}") - if error_type: - conditions.append(f"exception type {error_type}") - if match_error: - conditions.append(f"error message pattern {match_error!r}") - raise AssertionError( - "Could not find a log record matching the given " - + " and ".join(conditions) + _raise_log_message_error( + "Could not find a log record matching the given", + match_message, + error_type, + match_error, ) @contextmanager -def capture_idom_logs() -> Iterator[_LogRecordCaptor]: - """Capture logs from IDOM""" - if _LOG_RECORD_CAPTOR_SINGLTON in ROOT_LOGGER.handlers: - # this is being handled by an outer capture context - yield _LOG_RECORD_CAPTOR_SINGLTON - return None +def assert_idom_did_not_log( + match_message: str = "", + error_type: type[Exception] | None = None, + match_error: str = "", + clear_matched_records: bool = False, +) -> Iterator[None]: + """Assert the inverse of :func:`assert_idom_did_log`""" + try: + with assert_idom_did_log( + match_message, error_type, match_error, clear_matched_records + ): + yield None + except LogAssertionError: + pass + else: + _raise_log_message_error( + "Did find a log record matching the given", + match_message, + error_type, + match_error, + ) + + +def _raise_log_message_error( + prefix: str, + match_message: str = "", + error_type: type[Exception] | None = None, + match_error: str = "", +) -> NoReturn: + conditions = [] + if match_message: + conditions.append(f"log message pattern {match_message!r}") + if error_type: + conditions.append(f"exception type {error_type}") + if match_error: + conditions.append(f"error message pattern {match_error!r}") + raise LogAssertionError(prefix + " " + " and ".join(conditions)) + - ROOT_LOGGER.addHandler(_LOG_RECORD_CAPTOR_SINGLTON) +@contextmanager +def capture_idom_logs(use_existing: bool = False) -> Iterator[list[logging.LogRecord]]: + """Capture logs from IDOM + + Parameters: + use_existing: + If already inside an existing capture context yield the same list of logs. + This is useful if you need to mutate the list of logs to affect behavior in + the outer context. + """ + if use_existing: + for handler in reversed(ROOT_LOGGER.handlers): + if isinstance(handler, _LogRecordCaptor): + yield handler.records + return None + + handler = _LogRecordCaptor() + ROOT_LOGGER.addHandler(handler) try: - yield _LOG_RECORD_CAPTOR_SINGLTON + yield handler.records finally: - ROOT_LOGGER.removeHandler(_LOG_RECORD_CAPTOR_SINGLTON) - _LOG_RECORD_CAPTOR_SINGLTON.records = [] + ROOT_LOGGER.removeHandler(handler) class _LogRecordCaptor(logging.NullHandler): diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index 8ecb705ee..ed7b1b0e7 100644 --- a/tests/test_core/test_hooks.py +++ b/tests/test_core/test_hooks.py @@ -6,7 +6,7 @@ import idom from idom.core.dispatcher import render_json_patch from idom.core.hooks import LifeCycleHook -from idom.testing import HookCatcher +from idom.testing import HookCatcher, assert_idom_did_log from tests.general_utils import assert_same_items @@ -79,89 +79,68 @@ def SimpleStatefulComponent(): assert first_hook is h -def test_use_state_with_constructor(driver, display, driver_wait): +async def test_use_state_with_constructor(): constructor_call_count = idom.Ref(0) + set_outer_state = idom.Ref() + set_inner_key = idom.Ref() + set_inner_state = idom.Ref() + def make_default(): constructor_call_count.current += 1 return 0 @idom.component def Outer(): - hook = idom.hooks.current_hook() - - async def on_click(event): - hook.schedule_render() - - return idom.html.div( - idom.html.button( - {"onClick": on_click, "id": "outer"}, "update outer (rerun constructor)" - ), - Inner(), - ) + state, set_outer_state.current = idom.use_state(0) + inner_key, set_inner_key.current = idom.use_state("first") + return idom.html.div(state, Inner(key=inner_key)) @idom.component def Inner(): - count, set_count = idom.hooks.use_state(make_default) + state, set_inner_state.current = idom.use_state(make_default) + return idom.html.div(state) - async def on_click(event): - set_count(count + 1) - - return idom.html.div( - idom.html.button( - {"onClick": on_click, "id": "inner"}, - "update inner with state constructor", - ), - idom.html.p({"id": "count-view"}, count), - ) - - display(Outer) + with idom.Layout(Outer()) as layout: + await layout.render() - outer = driver.find_element("id", "outer") - inner = driver.find_element("id", "inner") - count = driver.find_element("id", "count-view") + assert constructor_call_count.current == 1 - driver_wait.until(lambda d: constructor_call_count.current == 1) - driver_wait.until(lambda d: count.get_attribute("innerHTML") == "0") + set_outer_state.current(1) + await layout.render() - inner.click() + assert constructor_call_count.current == 1 - driver_wait.until(lambda d: constructor_call_count.current == 1) - driver_wait.until(lambda d: count.get_attribute("innerHTML") == "1") + set_inner_state.current(1) + await layout.render() - outer.click() + assert constructor_call_count.current == 1 - driver_wait.until(lambda d: constructor_call_count.current == 2) - driver_wait.until(lambda d: count.get_attribute("innerHTML") == "0") + set_inner_key.current("second") + await layout.render() - inner.click() + assert constructor_call_count.current == 2 - driver_wait.until(lambda d: constructor_call_count.current == 2) - driver_wait.until(lambda d: count.get_attribute("innerHTML") == "1") +async def test_set_state_with_reducer_instead_of_value(): + count = idom.Ref() + set_count = idom.Ref() -def test_set_state_with_reducer_instead_of_value(driver, display): def increment(count): return count + 1 @idom.component def Counter(): - count, set_count = idom.hooks.use_state(0) - return idom.html.button( - { - "id": "counter", - "onClick": lambda event: set_count(increment), - }, - f"Count: {count}", - ) - - display(Counter) + count.current, set_count.current = idom.hooks.use_state(0) + return idom.html.div(count.current) - client_counter = driver.find_element("id", "counter") + with idom.Layout(Counter()) as layout: + await layout.render() - for i in range(3): - assert client_counter.get_attribute("innerHTML") == f"Count: {i}" - client_counter.click() + for i in range(4): + assert count.current == i + set_count.current(increment) + await layout.render() def test_set_state_checks_identity_not_equality(driver, display, driver_wait): @@ -356,15 +335,15 @@ def cleanup(): async def test_use_effect_cleanup_occurs_on_will_unmount(): - outer_component_hook = HookCatcher() + set_key = idom.Ref() 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(): - return ComponentWithEffect() + key, set_key.current = idom.use_state("first") + return ComponentWithEffect(key=key) @idom.component def ComponentWithEffect(): @@ -387,7 +366,7 @@ def cleanup(): assert not cleanup_triggered.current - outer_component_hook.latest.schedule_render() + set_key.current("second") await layout.render() assert cleanup_triggered.current @@ -592,13 +571,13 @@ def bad_cleanup(): assert re.match("Post-render effect .*?", first_log_line) -async def test_error_in_effect_pre_unmount_cleanup_is_gracefully_handled(caplog): - outer_component_hook = HookCatcher() +async def test_error_in_effect_pre_unmount_cleanup_is_gracefully_handled(): + set_key = idom.Ref() @idom.component - @outer_component_hook.capture def OuterComponent(): - return ComponentWithEffect() + key, set_key.current = idom.use_state("first") + return ComponentWithEffect(key=key) @idom.component def ComponentWithEffect(): @@ -611,13 +590,14 @@ def bad_cleanup(): return idom.html.div() - with idom.Layout(OuterComponent()) as layout: - await layout.render() - outer_component_hook.latest.schedule_render() - await layout.render() # no error - - first_log_line = next(iter(caplog.records)).msg.split("\n", 1)[0] - assert re.match("Pre-unmount effect .*? failed", first_log_line) + with assert_idom_did_log( + match_message=r"Pre-unmount effect .*? failed", + error_type=ValueError, + ): + with idom.Layout(OuterComponent()) as layout: + await layout.render() + set_key.current("second") + await layout.render() # no error async def test_use_reducer(): diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index 08df5e7b3..e7d2a39ad 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -13,7 +13,7 @@ from idom.testing import ( HookCatcher, StaticEventHandler, - assert_idom_logged, + assert_idom_did_log, capture_idom_logs, ) from tests.general_utils import assert_same_items @@ -21,9 +21,9 @@ @pytest.fixture(autouse=True) def no_logged_errors(): - with capture_idom_logs() as handler: + with capture_idom_logs() as logs: yield - for record in handler.records: + for record in logs: if record.exc_info: raise record.exc_info[1] @@ -144,7 +144,7 @@ def OkChild(): def BadChild(): raise ValueError("error from bad child") - with assert_idom_logged( + with assert_idom_did_log( match_error="error from bad child", clear_matched_records=True, ): @@ -188,7 +188,7 @@ def OkChild(): def BadChild(): raise ValueError("error from bad child") - with assert_idom_logged( + with assert_idom_did_log( match_error="error from bad child", clear_matched_records=True, ): @@ -335,17 +335,12 @@ def wrapper(*args, **kwargs): @add_to_live_hooks def Outer(): nonlocal set_inner_component - inner_component, set_inner_component = idom.hooks.use_state(InnerOne()) + inner_component, set_inner_component = idom.hooks.use_state(Inner(key="first")) return inner_component @idom.component @add_to_live_hooks - def InnerOne(): - return idom.html.div() - - @idom.component - @add_to_live_hooks - def InnerTwo(): + def Inner(): return idom.html.div() with idom.Layout(Outer()) as layout: @@ -354,9 +349,9 @@ def InnerTwo(): assert len(live_hooks) == 2 last_live_hooks = live_hooks.copy() - # We expect the hook for `InnerOne` to be garbage collected since it the - # component will get replaced. - set_inner_component(InnerTwo()) + # We expect the hook for `InnerOne` to be garbage collected since the component + # will get replaced. + set_inner_component(Inner(key="second")) await layout.render() assert len(live_hooks - last_live_hooks) == 1 @@ -649,7 +644,7 @@ def ComponentReturnsDuplicateKeys(): idom.html.div(key="duplicate"), idom.html.div(key="duplicate") ) - with assert_idom_logged( + with assert_idom_did_log( error_type=ValueError, match_error=r"Duplicate keys \['duplicate'\] at '/'", clear_matched_records=True, @@ -692,7 +687,7 @@ def raise_error(): return idom.html.button({"onClick": raise_error}) - with assert_idom_logged( + with assert_idom_did_log( match_error="bad event handler", clear_matched_records=True, ): @@ -719,7 +714,7 @@ def Child(state): idom.hooks.use_effect(lambda: lambda: print("unmount", state)) return idom.html.div(state) - with assert_idom_logged( + with assert_idom_did_log( r"Did not render component with model state ID .*? - component already unmounted", ): with idom.Layout(Parent()) as layout: @@ -758,7 +753,7 @@ def Child(): "component": Child(key="the-same-key"), } - with assert_idom_logged( + with assert_idom_did_log( error_type=ValueError, match_error="prior element with this key wasn't a component", clear_matched_records=True, @@ -789,7 +784,7 @@ def Child(): "component": Child(key="the-same-key"), } - with assert_idom_logged( + with assert_idom_did_log( error_type=ValueError, match_error="prior element with this key was a component", clear_matched_records=True, From 3f75219ee5a8c3003e81864d20cb918b31358ed9 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 10 Jan 2022 23:45:59 -0800 Subject: [PATCH 3/8] add tests to get cov of testing.py --- src/idom/testing.py | 13 ++-- tests/test_core/test_hooks.py | 4 +- tests/test_core/test_layout.py | 16 ++--- tests/test_testing.py | 125 +++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 16 deletions(-) create mode 100644 tests/test_testing.py diff --git a/src/idom/testing.py b/src/idom/testing.py index 87e853cc1..4e304b58f 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -179,7 +179,7 @@ class LogAssertionError(AssertionError): @contextmanager -def assert_idom_did_log( +def assert_idom_logged( match_message: str = "", error_type: type[Exception] | None = None, match_error: str = "", @@ -247,9 +247,9 @@ def assert_idom_did_not_log( match_error: str = "", clear_matched_records: bool = False, ) -> Iterator[None]: - """Assert the inverse of :func:`assert_idom_did_log`""" + """Assert the inverse of :func:`assert_idom_logged`""" try: - with assert_idom_did_log( + with assert_idom_logged( match_message, error_type, match_error, clear_matched_records ): yield None @@ -297,11 +297,15 @@ def capture_idom_logs(use_existing: bool = False) -> Iterator[list[logging.LogRe return None handler = _LogRecordCaptor() + original_level = ROOT_LOGGER.level + + ROOT_LOGGER.setLevel(logging.DEBUG) ROOT_LOGGER.addHandler(handler) try: yield handler.records finally: ROOT_LOGGER.removeHandler(handler) + ROOT_LOGGER.setLevel(original_level) class _LogRecordCaptor(logging.NullHandler): @@ -314,9 +318,6 @@ def handle(self, record: logging.LogRecord) -> bool: return True -_LOG_RECORD_CAPTOR_SINGLTON = _LogRecordCaptor() - - class HookCatcher: """Utility for capturing a LifeCycleHook from a component diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index ed7b1b0e7..cdc87d12f 100644 --- a/tests/test_core/test_hooks.py +++ b/tests/test_core/test_hooks.py @@ -6,7 +6,7 @@ import idom from idom.core.dispatcher import render_json_patch from idom.core.hooks import LifeCycleHook -from idom.testing import HookCatcher, assert_idom_did_log +from idom.testing import HookCatcher, assert_idom_logged from tests.general_utils import assert_same_items @@ -590,7 +590,7 @@ def bad_cleanup(): return idom.html.div() - with assert_idom_did_log( + with assert_idom_logged( match_message=r"Pre-unmount effect .*? failed", error_type=ValueError, ): diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index e7d2a39ad..366d9c3bd 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -13,7 +13,7 @@ from idom.testing import ( HookCatcher, StaticEventHandler, - assert_idom_did_log, + assert_idom_logged, capture_idom_logs, ) from tests.general_utils import assert_same_items @@ -144,7 +144,7 @@ def OkChild(): def BadChild(): raise ValueError("error from bad child") - with assert_idom_did_log( + with assert_idom_logged( match_error="error from bad child", clear_matched_records=True, ): @@ -188,7 +188,7 @@ def OkChild(): def BadChild(): raise ValueError("error from bad child") - with assert_idom_did_log( + with assert_idom_logged( match_error="error from bad child", clear_matched_records=True, ): @@ -644,7 +644,7 @@ def ComponentReturnsDuplicateKeys(): idom.html.div(key="duplicate"), idom.html.div(key="duplicate") ) - with assert_idom_did_log( + with assert_idom_logged( error_type=ValueError, match_error=r"Duplicate keys \['duplicate'\] at '/'", clear_matched_records=True, @@ -687,7 +687,7 @@ def raise_error(): return idom.html.button({"onClick": raise_error}) - with assert_idom_did_log( + with assert_idom_logged( match_error="bad event handler", clear_matched_records=True, ): @@ -714,7 +714,7 @@ def Child(state): idom.hooks.use_effect(lambda: lambda: print("unmount", state)) return idom.html.div(state) - with assert_idom_did_log( + with assert_idom_logged( r"Did not render component with model state ID .*? - component already unmounted", ): with idom.Layout(Parent()) as layout: @@ -753,7 +753,7 @@ def Child(): "component": Child(key="the-same-key"), } - with assert_idom_did_log( + with assert_idom_logged( error_type=ValueError, match_error="prior element with this key wasn't a component", clear_matched_records=True, @@ -784,7 +784,7 @@ def Child(): "component": Child(key="the-same-key"), } - with assert_idom_did_log( + with assert_idom_logged( error_type=ValueError, match_error="prior element with this key was a component", clear_matched_records=True, diff --git a/tests/test_testing.py b/tests/test_testing.py new file mode 100644 index 000000000..432feb874 --- /dev/null +++ b/tests/test_testing.py @@ -0,0 +1,125 @@ +import logging + +import pytest + +from idom import testing +from idom.log import ROOT_LOGGER as logger + + +def test_assert_idom_logged_does_not_supress_errors(): + with pytest.raises(RuntimeError, match="expected error"): + with testing.assert_idom_logged(): + raise RuntimeError("expected error") + + +def test_assert_idom_logged_message(): + with testing.assert_idom_logged(match_message="my message"): + logger.info("my message") + + with testing.assert_idom_logged(match_message=r".*"): + logger.info("my message") + + +def test_assert_idom_logged_error(): + with testing.assert_idom_logged( + match_message="log message", + error_type=ValueError, + match_error="my value error", + ): + try: + raise ValueError("my value error") + except ValueError: + logger.exception("log message") + + with pytest.raises( + AssertionError, + match=r"Could not find a log record matching the given", + ): + with testing.assert_idom_logged( + match_message="log message", + error_type=ValueError, + match_error="my value error", + ): + try: + # change error type + raise RuntimeError("my value error") + except RuntimeError: + logger.exception("log message") + + with pytest.raises( + AssertionError, + match=r"Could not find a log record matching the given", + ): + with testing.assert_idom_logged( + match_message="log message", + error_type=ValueError, + match_error="my value error", + ): + try: + # change error message + raise ValueError("something else") + except ValueError: + logger.exception("log message") + + with pytest.raises( + AssertionError, + match=r"Could not find a log record matching the given", + ): + with testing.assert_idom_logged( + match_message="log message", + error_type=ValueError, + match_error="my value error", + ): + try: + # change error message + raise ValueError("my error message") + except ValueError: + logger.exception("something else") + + +def test_assert_idom_logged_assertion_error_message(): + with pytest.raises( + AssertionError, + match=r"Could not find a log record matching the given", + ): + with testing.assert_idom_logged( + # put in all possible params full assertion error message + match_message=r".*", + error_type=Exception, + match_error=r".*", + ): + pass + + +def test_assert_idom_logged_ignores_level(): + original_level = logger.level + logger.setLevel(logging.INFO) + try: + with testing.assert_idom_logged(match_message=r".*"): + # this log would normally be ignored + logger.debug("my message") + finally: + logger.setLevel(original_level) + + +def test_assert_idom_did_not_log(): + with testing.assert_idom_did_not_log(match_message="my message"): + pass + + with testing.assert_idom_did_not_log(match_message=r"something else"): + logger.info("my message") + + with pytest.raises( + AssertionError, + match=r"Did find a log record matching the given", + ): + with testing.assert_idom_did_not_log( + # put in all possible params full assertion error message + match_message=r".*", + error_type=Exception, + match_error=r".*", + ): + try: + raise Exception("something") + except Exception: + logger.exception("something") From 2c26b518ba39951b60346b998febc0dc5e2409b6 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 10 Jan 2022 23:52:02 -0800 Subject: [PATCH 4/8] remove deprecation warning --- src/idom/config.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/src/idom/config.py b/src/idom/config.py index bf07f9891..456868be9 100644 --- a/src/idom/config.py +++ b/src/idom/config.py @@ -5,7 +5,6 @@ from pathlib import Path from tempfile import TemporaryDirectory -from warnings import warn from ._option import Option as _Option @@ -69,14 +68,3 @@ 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.current: - warn( - "In the next release, the feature flag IDOM_FEATURE_INDEX_AS_DEFAULT_KEY will " - "be activated by default. To try this out before the next release simply set " - "IDOM_FEATURE_INDEX_AS_DEFAULT_KEY=1 as an environment variable. After this " - "change, you can revert to the old behavior by setting it to 0 instead. If you " - "have questions or issues with this change report them here: " - "https://github.com/idom-team/idom/issues/351", - DeprecationWarning, - ) From 760c4794531005a172612f9fadbcf51f7116c3f1 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Mon, 10 Jan 2022 23:59:49 -0800 Subject: [PATCH 5/8] move no cover to dynamic key default also update IDOM_FEATURE_INDEX_AS_DEFAULT_KEY docstring --- src/idom/config.py | 8 +++++--- src/idom/core/layout.py | 4 ++-- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/idom/config.py b/src/idom/config.py index 456868be9..c4d73ff96 100644 --- a/src/idom/config.py +++ b/src/idom/config.py @@ -63,8 +63,10 @@ ) """Use the index of elements/components amongst their siblings as the default key. -In a future release this flag's default value will be set to true, and after that, this -flag will be removed entirely and the indices will always be the default key. +The flag's default value is set to true. To return to legacy behavior set +``IDOM_FEATURE_INDEX_AS_DEFAULT_KEY=0``. In a future release, this flag will be 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 +For more information on changes to this feature flag see: +https://github.com/idom-team/idom/issues/351 """ diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index 0e228b83f..511e2ee3f 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -698,10 +698,10 @@ def _process_child_type_and_key( if IDOM_FEATURE_INDEX_AS_DEFAULT_KEY.current: - def _default_key(index: int) -> Any: # pragma: no cover + def _default_key(index: int) -> Any: return index else: - def _default_key(index: int) -> Any: + def _default_key(index: int) -> Any: # pragma: no cover return object() From 7dd70b60c3ff0a0e104b454f8b2edd7e11759edc Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 11 Jan 2022 00:19:27 -0800 Subject: [PATCH 6/8] rename to avoid hook rook rule complaint --- src/idom/testing.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/idom/testing.py b/src/idom/testing.py index 4e304b58f..e310e4fe8 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -197,7 +197,7 @@ def assert_idom_logged( error_pattern = re.compile(match_error) try: - with capture_idom_logs(use_existing=clear_matched_records) as log_records: + with capture_idom_logs(yield_existing=clear_matched_records) as log_records: yield None except Exception: raise @@ -281,16 +281,18 @@ def _raise_log_message_error( @contextmanager -def capture_idom_logs(use_existing: bool = False) -> Iterator[list[logging.LogRecord]]: +def capture_idom_logs( + yield_existing: bool = False, +) -> Iterator[list[logging.LogRecord]]: """Capture logs from IDOM Parameters: - use_existing: + yield_existing: If already inside an existing capture context yield the same list of logs. This is useful if you need to mutate the list of logs to affect behavior in the outer context. """ - if use_existing: + if yield_existing: for handler in reversed(ROOT_LOGGER.handlers): if isinstance(handler, _LogRecordCaptor): yield handler.records From 69108b6efe3e7161cebf2f2cbdb9833675087909 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 11 Jan 2022 00:27:40 -0800 Subject: [PATCH 7/8] fix last style complaint --- tests/test_testing.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/test_testing.py b/tests/test_testing.py index 432feb874..8c7529bcd 100644 --- a/tests/test_testing.py +++ b/tests/test_testing.py @@ -3,7 +3,7 @@ import pytest from idom import testing -from idom.log import ROOT_LOGGER as logger +from idom.log import ROOT_LOGGER def test_assert_idom_logged_does_not_supress_errors(): @@ -14,10 +14,10 @@ def test_assert_idom_logged_does_not_supress_errors(): def test_assert_idom_logged_message(): with testing.assert_idom_logged(match_message="my message"): - logger.info("my message") + ROOT_LOGGER.info("my message") with testing.assert_idom_logged(match_message=r".*"): - logger.info("my message") + ROOT_LOGGER.info("my message") def test_assert_idom_logged_error(): @@ -29,7 +29,7 @@ def test_assert_idom_logged_error(): try: raise ValueError("my value error") except ValueError: - logger.exception("log message") + ROOT_LOGGER.exception("log message") with pytest.raises( AssertionError, @@ -44,7 +44,7 @@ def test_assert_idom_logged_error(): # change error type raise RuntimeError("my value error") except RuntimeError: - logger.exception("log message") + ROOT_LOGGER.exception("log message") with pytest.raises( AssertionError, @@ -59,7 +59,7 @@ def test_assert_idom_logged_error(): # change error message raise ValueError("something else") except ValueError: - logger.exception("log message") + ROOT_LOGGER.exception("log message") with pytest.raises( AssertionError, @@ -74,7 +74,7 @@ def test_assert_idom_logged_error(): # change error message raise ValueError("my error message") except ValueError: - logger.exception("something else") + ROOT_LOGGER.exception("something else") def test_assert_idom_logged_assertion_error_message(): @@ -92,14 +92,14 @@ def test_assert_idom_logged_assertion_error_message(): def test_assert_idom_logged_ignores_level(): - original_level = logger.level - logger.setLevel(logging.INFO) + original_level = ROOT_LOGGER.level + ROOT_LOGGER.setLevel(logging.INFO) try: with testing.assert_idom_logged(match_message=r".*"): # this log would normally be ignored - logger.debug("my message") + ROOT_LOGGER.debug("my message") finally: - logger.setLevel(original_level) + ROOT_LOGGER.setLevel(original_level) def test_assert_idom_did_not_log(): @@ -107,7 +107,7 @@ def test_assert_idom_did_not_log(): pass with testing.assert_idom_did_not_log(match_message=r"something else"): - logger.info("my message") + ROOT_LOGGER.info("my message") with pytest.raises( AssertionError, @@ -122,4 +122,4 @@ def test_assert_idom_did_not_log(): try: raise Exception("something") except Exception: - logger.exception("something") + ROOT_LOGGER.exception("something") From aafd6eebd036a68b3bd088ea371d4fb3d2948d08 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 11 Jan 2022 09:37:32 -0800 Subject: [PATCH 8/8] misc fixes --- .../_examples/super_simple_chart/app.py | 8 +++++- .../_examples/matplotlib_plot.py | 1 + .../_examples/snake_game.py | 25 +++++++++++++++---- src/idom/core/proto.py | 9 ++++--- src/idom/core/vdom.py | 14 +++++++---- src/idom/web/module.py | 4 +-- 6 files changed, 45 insertions(+), 16 deletions(-) diff --git a/docs/source/escape-hatches/_examples/super_simple_chart/app.py b/docs/source/escape-hatches/_examples/super_simple_chart/app.py index 2f2e17556..9f0563a97 100644 --- a/docs/source/escape-hatches/_examples/super_simple_chart/app.py +++ b/docs/source/escape-hatches/_examples/super_simple_chart/app.py @@ -4,7 +4,13 @@ file = Path(__file__).parent / "super-simple-chart.js" -ssc = web.module_from_file("super-simple-chart", file, fallback="⌛") +ssc = web.module_from_file( + "super-simple-chart", + file, + fallback="⌛", + # normally this option is not required + replace_existing=True, +) SuperSimpleChart = web.export(ssc, "SuperSimpleChart") diff --git a/docs/source/reference-material/_examples/matplotlib_plot.py b/docs/source/reference-material/_examples/matplotlib_plot.py index 94ab3af4a..6dffb79db 100644 --- a/docs/source/reference-material/_examples/matplotlib_plot.py +++ b/docs/source/reference-material/_examples/matplotlib_plot.py @@ -71,6 +71,7 @@ def poly_coef_input(index, callback): "onChange": callback, }, ), + key=index, ) diff --git a/docs/source/reference-material/_examples/snake_game.py b/docs/source/reference-material/_examples/snake_game.py index 4a5736fe4..8f1478499 100644 --- a/docs/source/reference-material/_examples/snake_game.py +++ b/docs/source/reference-material/_examples/snake_game.py @@ -18,7 +18,12 @@ def GameView(): game_state, set_game_state = idom.hooks.use_state(GameState.init) if game_state == GameState.play: - return GameLoop(grid_size=6, block_scale=50, set_game_state=set_game_state) + return GameLoop( + grid_size=6, + block_scale=50, + set_game_state=set_game_state, + key="game loop", + ) start_button = idom.html.button( {"onClick": lambda event: set_game_state(GameState.play)}, @@ -40,7 +45,12 @@ def GameView(): """ ) - return idom.html.div({"className": "snake-game-menu"}, menu_style, menu) + return idom.html.div( + {"className": "snake-game-menu"}, + menu_style, + menu, + key="menu", + ) class Direction(enum.Enum): @@ -154,14 +164,18 @@ def create_grid(grid_size, block_scale): [ idom.html.div( {"style": {"height": f"{block_scale}px"}}, - [create_grid_block("black", block_scale) for i in range(grid_size)], + [ + create_grid_block("black", block_scale, key=i) + for i in range(grid_size) + ], + key=i, ) for i in range(grid_size) ], ) -def create_grid_block(color, block_scale): +def create_grid_block(color, block_scale, key): return idom.html.div( { "style": { @@ -170,7 +184,8 @@ def create_grid_block(color, block_scale): "backgroundColor": color, "outline": "1px solid grey", } - } + }, + key=key, ) diff --git a/src/idom/core/proto.py b/src/idom/core/proto.py index ed5d01369..e12120533 100644 --- a/src/idom/core/proto.py +++ b/src/idom/core/proto.py @@ -22,11 +22,14 @@ """Simple function returning a new component""" +Key = Union[str, int] + + @runtime_checkable class ComponentType(Protocol): """The expected interface for all component-like objects""" - key: Optional[Any] + key: Key | None """An identifier which is unique amongst a component's immediate siblings""" def render(self) -> VdomDict: @@ -74,7 +77,7 @@ def __exit__( class _VdomDictOptional(TypedDict, total=False): - key: str + key: Key | None children: Sequence[ # recursive types are not allowed yet: # https://github.com/python/mypy/issues/731 @@ -101,7 +104,7 @@ class ImportSourceDict(TypedDict): class _OptionalVdomJson(TypedDict, total=False): - key: str + key: Key error: str children: List[Any] attributes: Dict[str, Any] diff --git a/src/idom/core/vdom.py b/src/idom/core/vdom.py index 8859b21c2..853c412a8 100644 --- a/src/idom/core/vdom.py +++ b/src/idom/core/vdom.py @@ -129,7 +129,7 @@ def is_vdom(value: Any) -> bool: def vdom( tag: str, *attributes_and_children: VdomAttributesAndChildren, - key: str = "", + key: str | int | None = None, event_handlers: Optional[EventHandlerMapping] = None, import_source: Optional[ImportSourceDict] = None, ) -> VdomDict: @@ -169,7 +169,7 @@ def vdom( if event_handlers: model["eventHandlers"] = event_handlers - if key != "": + if key is not None: model["key"] = key if import_source is not None: @@ -182,7 +182,7 @@ class _VdomDictConstructor(Protocol): def __call__( self, *attributes_and_children: VdomAttributesAndChildren, - key: str = ..., + key: str | int | None = ..., event_handlers: Optional[EventHandlerMapping] = ..., import_source: Optional[ImportSourceDict] = ..., ) -> VdomDict: @@ -200,7 +200,7 @@ def make_vdom_constructor( def constructor( *attributes_and_children: VdomAttributesAndChildren, - key: str = "", + key: str | int | None = None, event_handlers: Optional[EventHandlerMapping] = None, import_source: Optional[ImportSourceDict] = None, ) -> VdomDict: @@ -333,7 +333,11 @@ def _is_single_child(value: Any) -> bool: 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": ...} + 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 "..." diff --git a/src/idom/web/module.py b/src/idom/web/module.py index bc6b8cbb6..74e58d63c 100644 --- a/src/idom/web/module.py +++ b/src/idom/web/module.py @@ -195,8 +195,8 @@ def module_from_file( Using this option has negative performance consequences since all DOM elements must be changed on each render. See :issue:`461` for more info. replace_existing: - Whether to replace the source for a module with the same name if - if already exists. Otherwise raise an error. + Whether to replace the source for a module with the same name if it already + exists. Otherwise raise an error. """ source_file = Path(file) target_file = _web_module_path(name)