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/config.py b/src/idom/config.py index 855b1d295..c4d73ff96 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 @@ -58,25 +57,16 @@ 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)), ) """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 """ - -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, - ) 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() 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/testing.py b/src/idom/testing.py index 24592208e..e310e4fe8 100644 --- a/src/idom/testing.py +++ b/src/idom/testing.py @@ -14,6 +14,7 @@ Generic, Iterator, List, + NoReturn, Optional, Tuple, Type, @@ -173,6 +174,10 @@ def __exit__( return None +class LogAssertionError(AssertionError): + """An assertion error raised in relation to log messages.""" + + @contextmanager def assert_idom_logged( match_message: 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(yield_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,85 @@ 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_logged`""" + try: + with assert_idom_logged( + 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, + ) - ROOT_LOGGER.addHandler(_LOG_RECORD_CAPTOR_SINGLTON) + +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)) + + +@contextmanager +def capture_idom_logs( + yield_existing: bool = False, +) -> Iterator[list[logging.LogRecord]]: + """Capture logs from IDOM + + Parameters: + 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 yield_existing: + for handler in reversed(ROOT_LOGGER.handlers): + if isinstance(handler, _LogRecordCaptor): + yield handler.records + return None + + handler = _LogRecordCaptor() + original_level = ROOT_LOGGER.level + + ROOT_LOGGER.setLevel(logging.DEBUG) + 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) + ROOT_LOGGER.setLevel(original_level) class _LogRecordCaptor(logging.NullHandler): @@ -264,9 +320,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/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) diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index 8ecb705ee..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 +from idom.testing import HookCatcher, assert_idom_logged 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_logged( + 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..366d9c3bd 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -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] @@ -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 diff --git a/tests/test_testing.py b/tests/test_testing.py new file mode 100644 index 000000000..8c7529bcd --- /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 + + +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"): + ROOT_LOGGER.info("my message") + + with testing.assert_idom_logged(match_message=r".*"): + ROOT_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: + ROOT_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: + ROOT_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: + ROOT_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: + ROOT_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 = ROOT_LOGGER.level + ROOT_LOGGER.setLevel(logging.INFO) + try: + with testing.assert_idom_logged(match_message=r".*"): + # this log would normally be ignored + ROOT_LOGGER.debug("my message") + finally: + ROOT_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"): + ROOT_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: + ROOT_LOGGER.exception("something")