From c7ac7b324e12b420014ed8fa7c102b2f5dc22be4 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Tue, 6 Jun 2023 00:00:02 -0600 Subject: [PATCH 01/13] report better error for attrs that cannot be serialized also misc changes to config options to make this eaiser in particular ability to specify options as parents --- src/py/reactpy/reactpy/_option.py | 46 ++++++++++++-------- src/py/reactpy/reactpy/config.py | 63 ++++++++++++++++++---------- src/py/reactpy/reactpy/core/serve.py | 14 ++++++- src/py/reactpy/reactpy/core/vdom.py | 5 ++- src/py/reactpy/tests/test__option.py | 25 +++++++++-- src/py/reactpy/tests/test_utils.py | 4 -- 6 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/py/reactpy/reactpy/_option.py b/src/py/reactpy/reactpy/_option.py index 1421f33a3..5475fbb8e 100644 --- a/src/py/reactpy/reactpy/_option.py +++ b/src/py/reactpy/reactpy/_option.py @@ -8,6 +8,13 @@ _O = TypeVar("_O") logger = getLogger(__name__) +UNDEFINED = cast(Any, object()) + + +def deprecate(option: Option[_O], message: str) -> Option[_O]: + option.__class__ = _DeprecatedOption + option._deprecation_message = message + return option class Option(Generic[_O]): @@ -16,8 +23,9 @@ class Option(Generic[_O]): def __init__( self, name: str, - default: _O | Option[_O], + default: _O = UNDEFINED, mutable: bool = True, + parent: Option[_O] | None = None, validator: Callable[[Any], _O] = lambda x: cast(_O, x), ) -> None: self._name = name @@ -28,12 +36,15 @@ def __init__( 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: + if parent is not None: + if not (parent.mutable and self.mutable): + raise TypeError("Parent and child options must be mutable") + self._default = parent.default + parent.subscribe(self.set_current) + elif default is not UNDEFINED: self._default = default + else: + raise TypeError("Must specify either a default or a parent option") logger.debug(f"{self._name}={self.current}") @@ -81,11 +92,19 @@ def set_current(self, new: Any) -> None: Raises a ``TypeError`` if this option is not :attr:`Option.mutable`. """ + old = self.current + if new is old: + return None + if not self._mutable: msg = f"{self} cannot be modified after initial load" raise TypeError(msg) - old = self.current - new = self._current = self._validator(new) + + try: + new = self._current = self._validator(new) + except ValueError as error: + raise ValueError(f"Invalid value for {self._name}: {new!r}") from error + logger.debug(f"{self._name}={self._current}") if new != old: for sub_func in self._subscribers: @@ -119,15 +138,8 @@ def __repr__(self) -> str: return f"Option({self._name}={self.current!r})" -class DeprecatedOption(Option[_O]): # nocov - def __init__(self, message: str, *args: Any, **kwargs: Any) -> None: - self._deprecation_message = message - super().__init__(*args, **kwargs) - +class _DeprecatedOption(Option[_O]): # nocov @Option.current.getter # type: ignore def current(self) -> _O: - warn( - self._deprecation_message, - DeprecationWarning, - ) + warn(self._deprecation_message, DeprecationWarning) return super().current diff --git a/src/py/reactpy/reactpy/config.py b/src/py/reactpy/reactpy/config.py index 6dc29096c..a80031ef4 100644 --- a/src/py/reactpy/reactpy/config.py +++ b/src/py/reactpy/reactpy/config.py @@ -6,41 +6,58 @@ from pathlib import Path from tempfile import TemporaryDirectory -from reactpy._option import Option as _Option +from reactpy._option import Option -REACTPY_DEBUG_MODE = _Option( - "REACTPY_DEBUG_MODE", - default=False, - validator=lambda x: bool(int(x)), -) -"""This immutable option turns on/off debug mode +TRUE_VALUES = {"true", "1"} +FALSE_VALUES = {"false", "0"} -The string values ``1`` and ``0`` are mapped to ``True`` and ``False`` respectively. -When debug is on, extra validation measures are applied that negatively impact -performance but can be used to catch bugs during development. Additionally, the default -log level for ReactPy is set to ``DEBUG``. -""" +def boolean(value: str | bool) -> bool: + if isinstance(value, bool): + return value + elif not isinstance(value, str): + raise TypeError(f"Expected str or bool, got {type(value).__name__}") + + if value.lower() in TRUE_VALUES: + return True + elif value.lower() in FALSE_VALUES: + return False + else: + raise ValueError( + f"Invalid boolean value {value!r} - expected " + f"one of {list(TRUE_VALUES | FALSE_VALUES)}" + ) -REACTPY_CHECK_VDOM_SPEC = _Option( - "REACTPY_CHECK_VDOM_SPEC", - default=REACTPY_DEBUG_MODE, - validator=lambda x: bool(int(x)), + +REACTPY_DEBUG_MODE = Option( + "REACTPY_DEBUG_MODE", default=False, validator=boolean, mutable=True ) -"""This immutable option turns on/off checks which ensure VDOM is rendered to spec +"""Get extra logs and validation checks at the cost of performance. -The string values ``1`` and ``0`` are mapped to ``True`` and ``False`` respectively. +This will enable the following: -By default this check is off. When ``REACTPY_DEBUG_MODE=1`` this will be turned on but can -be manually disablled by setting ``REACTPY_CHECK_VDOM_SPEC=0`` in addition. +- :data:`REACTPY_CHECK_VDOM_SPEC` +- :data:`REACTPY_CHECK_JSON_ATTRS` +""" + +REACTPY_CHECK_VDOM_SPEC = Option("REACTPY_CHECK_VDOM_SPEC", parent=REACTPY_DEBUG_MODE) +"""Checks which ensure VDOM is rendered to spec For more info on the VDOM spec, see here: :ref:`VDOM JSON Schema` """ -# Because these web modules will be linked dynamically at runtime this can be temporary +REACTPY_CHECK_JSON_ATTRS = Option("REACTPY_CHECK_JSON_ATTRS", parent=REACTPY_DEBUG_MODE) +"""Checks that all VDOM attributes are JSON serializable + +The VDOM spec is not able to enforce this on its own since attributes could anything. +""" + +# Because these web modules will be linked dynamically at runtime this can be temporary. +# Assigning to a variable here ensures that the directory is not deleted until the end +# of the program. _DEFAULT_WEB_MODULES_DIR = TemporaryDirectory() -REACTPY_WEB_MODULES_DIR = _Option( +REACTPY_WEB_MODULES_DIR = Option( "REACTPY_WEB_MODULES_DIR", default=Path(_DEFAULT_WEB_MODULES_DIR.name), validator=Path, @@ -52,7 +69,7 @@ set of publicly available APIs for working with the client. """ -REACTPY_TESTING_DEFAULT_TIMEOUT = _Option( +REACTPY_TESTING_DEFAULT_TIMEOUT = Option( "REACTPY_TESTING_DEFAULT_TIMEOUT", 5.0, mutable=False, diff --git a/src/py/reactpy/reactpy/core/serve.py b/src/py/reactpy/reactpy/core/serve.py index 61a7e4ce6..bee7283d9 100644 --- a/src/py/reactpy/reactpy/core/serve.py +++ b/src/py/reactpy/reactpy/core/serve.py @@ -7,6 +7,7 @@ from anyio import create_task_group from anyio.abc import TaskGroup +from reactpy.config import REACTPY_DEBUG_MODE from reactpy.core.types import LayoutEventMessage, LayoutType, LayoutUpdateMessage logger = getLogger(__name__) @@ -49,7 +50,18 @@ async def _single_outgoing_loop( layout: LayoutType[LayoutUpdateMessage, LayoutEventMessage], send: SendCoroutine ) -> None: while True: - await send(await layout.render()) + update = await layout.render() + try: + await send(update) + except Exception: + if not REACTPY_DEBUG_MODE.current: + msg = ( + "Failed to send update. More info may be available " + "if you enabling debug mode by settings " + "`reactpy.config.REACTPY_DEBUG_MODE.current = True`." + ) + logger.error(msg) + raise async def _single_incoming_loop( diff --git a/src/py/reactpy/reactpy/core/vdom.py b/src/py/reactpy/reactpy/core/vdom.py index 0548c6afc..32eeca485 100644 --- a/src/py/reactpy/reactpy/core/vdom.py +++ b/src/py/reactpy/reactpy/core/vdom.py @@ -1,5 +1,6 @@ from __future__ import annotations +import json import logging from collections.abc import Mapping, Sequence from functools import wraps @@ -8,7 +9,7 @@ from fastjsonschema import compile as compile_json_schema from reactpy._warnings import warn -from reactpy.config import REACTPY_DEBUG_MODE +from reactpy.config import REACTPY_CHECK_JSON_ATTRS, REACTPY_DEBUG_MODE from reactpy.core._f_back import f_module_name from reactpy.core.events import EventHandler, to_event_handler_function from reactpy.core.types import ( @@ -199,6 +200,8 @@ def vdom( attributes, event_handlers = separate_attributes_and_event_handlers(attributes) if attributes: + if REACTPY_CHECK_JSON_ATTRS.current: + json.dumps(attributes) model["attributes"] = attributes if children: diff --git a/src/py/reactpy/tests/test__option.py b/src/py/reactpy/tests/test__option.py index 63f2fada8..d6de3ed3f 100644 --- a/src/py/reactpy/tests/test__option.py +++ b/src/py/reactpy/tests/test__option.py @@ -3,7 +3,7 @@ import pytest -from reactpy._option import DeprecatedOption, Option +from reactpy._option import Option, deprecate def test_option_repr(): @@ -33,7 +33,7 @@ def test_option_validator(): opt.current = "0" assert opt.current is False - with pytest.raises(ValueError, match="invalid literal for int"): + with pytest.raises(ValueError, match="Invalid value"): opt.current = "not-an-int" @@ -102,10 +102,29 @@ def test_option_subscribe(): def test_deprecated_option(): - opt = DeprecatedOption("is deprecated!", "A_FAKE_OPTION", None) + opt = deprecate(Option("A_FAKE_OPTION", None), "is deprecated!") with pytest.warns(DeprecationWarning, match="is deprecated!"): assert opt.current is None with pytest.warns(DeprecationWarning, match="is deprecated!"): opt.current = "something" + + +def test_option_parent(): + parent_opt = Option("A_FAKE_OPTION", "default-value", mutable=True) + child_opt = Option("A_FAKE_OPTION", parent=parent_opt) + assert child_opt.mutable + assert child_opt.current == "default-value" + + parent_opt.current = "new-value" + assert child_opt.current == "new-value" + + +def test_option_parent_child_must_be_mutable(): + mut_parent_opt = Option("A_FAKE_OPTION", "default-value", mutable=True) + immu_parent_opt = Option("A_FAKE_OPTION", "default-value", mutable=False) + with pytest.raises(TypeError, match="must be mutable"): + Option("A_FAKE_OPTION", parent=mut_parent_opt, mutable=False) + with pytest.raises(TypeError, match="must be mutable"): + Option("A_FAKE_OPTION", parent=immu_parent_opt, mutable=None) diff --git a/src/py/reactpy/tests/test_utils.py b/src/py/reactpy/tests/test_utils.py index c71057f15..ca3080358 100644 --- a/src/py/reactpy/tests/test_utils.py +++ b/src/py/reactpy/tests/test_utils.py @@ -204,10 +204,6 @@ def test_del_html_body_transform(): html.div(SOME_OBJECT), f"
{html_escape(str(SOME_OBJECT))}
", ), - ( - html.div({"someAttribute": SOME_OBJECT}), - f'
', - ), ( html.div( "hello", html.a({"href": "https://example.com"}, "example"), "world" From 8e7fde2e24c3a17130817a67ac9c0ed9677436aa Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 7 Jun 2023 13:51:01 -0600 Subject: [PATCH 02/13] add more tests --- src/py/reactpy/reactpy/config.py | 6 +++++- src/py/reactpy/tests/test__option.py | 7 +++++++ src/py/reactpy/tests/test_config.py | 24 ++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/py/reactpy/reactpy/config.py b/src/py/reactpy/reactpy/config.py index a80031ef4..8371e6d08 100644 --- a/src/py/reactpy/reactpy/config.py +++ b/src/py/reactpy/reactpy/config.py @@ -3,6 +3,8 @@ variables or, for those which allow it, a programmatic interface. """ +from __future__ import annotations + from pathlib import Path from tempfile import TemporaryDirectory @@ -12,9 +14,11 @@ FALSE_VALUES = {"false", "0"} -def boolean(value: str | bool) -> bool: +def boolean(value: str | bool | int) -> bool: if isinstance(value, bool): return value + elif isinstance(value, int): + return bool(value) elif not isinstance(value, str): raise TypeError(f"Expected str or bool, got {type(value).__name__}") diff --git a/src/py/reactpy/tests/test__option.py b/src/py/reactpy/tests/test__option.py index d6de3ed3f..9e7eefbc4 100644 --- a/src/py/reactpy/tests/test__option.py +++ b/src/py/reactpy/tests/test__option.py @@ -128,3 +128,10 @@ def test_option_parent_child_must_be_mutable(): Option("A_FAKE_OPTION", parent=mut_parent_opt, mutable=False) with pytest.raises(TypeError, match="must be mutable"): Option("A_FAKE_OPTION", parent=immu_parent_opt, mutable=None) + + +def test_no_default_or_parent(): + with pytest.raises( + TypeError, match="must specify either a default value or a parent" + ): + Option("A_FAKE_OPTION") diff --git a/src/py/reactpy/tests/test_config.py b/src/py/reactpy/tests/test_config.py index ecbbb998c..3428c3e28 100644 --- a/src/py/reactpy/tests/test_config.py +++ b/src/py/reactpy/tests/test_config.py @@ -27,3 +27,27 @@ def test_reactpy_debug_mode_toggle(): # just check that nothing breaks config.REACTPY_DEBUG_MODE.current = True config.REACTPY_DEBUG_MODE.current = False + + +def test_boolean(): + assert config.boolean(True) is True + assert config.boolean(False) is False + assert config.boolean(1) is True + assert config.boolean(0) is False + assert config.boolean("true") is True + assert config.boolean("false") is False + assert config.boolean("True") is True + assert config.boolean("False") is False + assert config.boolean("TRUE") is True + assert config.boolean("FALSE") is False + assert config.boolean("1") is True + assert config.boolean("0") is False + + with pytest.raises(ValueError): + config.boolean("2") + + with pytest.raises(ValueError): + config.boolean("") + + with pytest.raises(TypeError): + config.boolean(None) From 4e772f4fac386ca20cded4facc376d90627bcd4a Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sat, 10 Jun 2023 12:18:40 -0600 Subject: [PATCH 03/13] no need to be clever - just inherit --- src/py/reactpy/reactpy/_option.py | 14 +++++++------- src/py/reactpy/tests/test__option.py | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/py/reactpy/reactpy/_option.py b/src/py/reactpy/reactpy/_option.py index 5475fbb8e..9f5ccacfc 100644 --- a/src/py/reactpy/reactpy/_option.py +++ b/src/py/reactpy/reactpy/_option.py @@ -11,12 +11,6 @@ UNDEFINED = cast(Any, object()) -def deprecate(option: Option[_O], message: str) -> Option[_O]: - option.__class__ = _DeprecatedOption - option._deprecation_message = message - return option - - class Option(Generic[_O]): """An option that can be set using an environment variable of the same name""" @@ -138,7 +132,13 @@ def __repr__(self) -> str: return f"Option({self._name}={self.current!r})" -class _DeprecatedOption(Option[_O]): # nocov +class DeprecatedOption(Option[_O]): + """An option that will warn when it is accessed""" + + def __init__(self, *args: Any, message: str, **kwargs: Any) -> None: + super().__init__(*args, **kwargs) + self._deprecation_message = message + @Option.current.getter # type: ignore def current(self) -> _O: warn(self._deprecation_message, DeprecationWarning) diff --git a/src/py/reactpy/tests/test__option.py b/src/py/reactpy/tests/test__option.py index 9e7eefbc4..89b65f9d3 100644 --- a/src/py/reactpy/tests/test__option.py +++ b/src/py/reactpy/tests/test__option.py @@ -3,7 +3,7 @@ import pytest -from reactpy._option import Option, deprecate +from reactpy._option import DeprecatedOption, Option def test_option_repr(): @@ -102,7 +102,7 @@ def test_option_subscribe(): def test_deprecated_option(): - opt = deprecate(Option("A_FAKE_OPTION", None), "is deprecated!") + opt = DeprecatedOption("A_FAKE_OPTION", None, message="is deprecated!") with pytest.warns(DeprecationWarning, match="is deprecated!"): assert opt.current is None From 469fa68b285e5633db39694bccde05ee3c6f7be7 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 11 Jun 2023 16:22:24 -0600 Subject: [PATCH 04/13] fix tests --- src/py/reactpy/reactpy/_option.py | 11 ++++++++++- src/py/reactpy/tests/test__option.py | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/py/reactpy/reactpy/_option.py b/src/py/reactpy/reactpy/_option.py index 9f5ccacfc..09d0304a9 100644 --- a/src/py/reactpy/reactpy/_option.py +++ b/src/py/reactpy/reactpy/_option.py @@ -141,5 +141,14 @@ def __init__(self, *args: Any, message: str, **kwargs: Any) -> None: @Option.current.getter # type: ignore def current(self) -> _O: - warn(self._deprecation_message, DeprecationWarning) + try: + # we access the current value during init to debug log it + # no need to warn unless it's actually used. since this attr + # is only set after super().__init__ is called, we can check + # for it to determine if it's being accessed by a user. + msg = self._deprecation_message + except AttributeError: + pass + else: + warn(msg, DeprecationWarning) return super().current diff --git a/src/py/reactpy/tests/test__option.py b/src/py/reactpy/tests/test__option.py index 89b65f9d3..929e17488 100644 --- a/src/py/reactpy/tests/test__option.py +++ b/src/py/reactpy/tests/test__option.py @@ -132,6 +132,6 @@ def test_option_parent_child_must_be_mutable(): def test_no_default_or_parent(): with pytest.raises( - TypeError, match="must specify either a default value or a parent" + TypeError, match="Must specify either a default or a parent option" ): Option("A_FAKE_OPTION") From 6b00eb159ddeacec592c7c2bc95160864bc6dd66 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Sun, 11 Jun 2023 18:00:18 -0600 Subject: [PATCH 05/13] ignore missing cov --- src/py/reactpy/reactpy/core/serve.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/py/reactpy/reactpy/core/serve.py b/src/py/reactpy/reactpy/core/serve.py index bee7283d9..7b218c558 100644 --- a/src/py/reactpy/reactpy/core/serve.py +++ b/src/py/reactpy/reactpy/core/serve.py @@ -53,7 +53,7 @@ async def _single_outgoing_loop( update = await layout.render() try: await send(update) - except Exception: + except Exception: # nocov if not REACTPY_DEBUG_MODE.current: msg = ( "Failed to send update. More info may be available " From b46b2872c2348585b8deeb4643fb5c3f5f7c1d32 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 20:15:24 -0600 Subject: [PATCH 06/13] add test for non-json serializable attrs --- src/py/reactpy/reactpy/backend/_common.py | 2 +- src/py/reactpy/reactpy/core/serve.py | 2 +- src/py/reactpy/reactpy/core/vdom.py | 10 +++------- src/py/reactpy/tests/test_core/test_hooks.py | 9 +++++++++ 4 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/py/reactpy/reactpy/backend/_common.py b/src/py/reactpy/reactpy/backend/_common.py index 80b4eeee1..17983a033 100644 --- a/src/py/reactpy/reactpy/backend/_common.py +++ b/src/py/reactpy/reactpy/backend/_common.py @@ -114,7 +114,7 @@ def vdom_head_elements_to_html(head: Sequence[VdomDict] | VdomDict | str) -> str head = cast(VdomDict, {**head, "tagName": ""}) return vdom_to_html(head) else: - return vdom_to_html(html._(head)) + return vdom_to_html(html._(*head)) @dataclass diff --git a/src/py/reactpy/reactpy/core/serve.py b/src/py/reactpy/reactpy/core/serve.py index 7b218c558..3a530e854 100644 --- a/src/py/reactpy/reactpy/core/serve.py +++ b/src/py/reactpy/reactpy/core/serve.py @@ -57,7 +57,7 @@ async def _single_outgoing_loop( if not REACTPY_DEBUG_MODE.current: msg = ( "Failed to send update. More info may be available " - "if you enabling debug mode by settings " + "if you enabling debug mode by setting " "`reactpy.config.REACTPY_DEBUG_MODE.current = True`." ) logger.error(msg) diff --git a/src/py/reactpy/reactpy/core/vdom.py b/src/py/reactpy/reactpy/core/vdom.py index 32eeca485..a845d51b5 100644 --- a/src/py/reactpy/reactpy/core/vdom.py +++ b/src/py/reactpy/reactpy/core/vdom.py @@ -1,7 +1,6 @@ from __future__ import annotations import json -import logging from collections.abc import Mapping, Sequence from functools import wraps from typing import Any, Protocol, cast, overload @@ -26,9 +25,6 @@ VdomJson, ) -logger = logging.getLogger() - - VDOM_JSON_SCHEMA = { "$schema": "http://json-schema.org/draft-07/schema", "$ref": "#/definitions/element", @@ -328,18 +324,18 @@ def _is_single_child(value: Any) -> bool: def _validate_child_key_integrity(value: Any) -> None: if hasattr(value, "__iter__") and not hasattr(value, "__len__"): - logger.error( + warn( 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}") + warn(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}") + warn(f"Key not specified for child in list {child_copy}") class _CustomVdomDictConstructor(Protocol): diff --git a/src/py/reactpy/tests/test_core/test_hooks.py b/src/py/reactpy/tests/test_core/test_hooks.py index 453d07c99..7231794ad 100644 --- a/src/py/reactpy/tests/test_core/test_hooks.py +++ b/src/py/reactpy/tests/test_core/test_hooks.py @@ -1257,3 +1257,12 @@ def bad_effect(): await layout.render() component_hook.latest.schedule_render() await layout.render() # no error + + +@pytest.mark.skipif( + not REACTPY_DEBUG_MODE.current, + reason="only check json serialization in debug mode", +) +def test_raise_for_non_json_attrs(): + with pytest.raises(TypeError, match="JSON serializable"): + reactpy.html.div({"non_json_serializable_object": object()}) From de3a73319526fca5a68317f0640ffdb55ab83729 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 20:18:04 -0600 Subject: [PATCH 07/13] add changelog entry --- docs/source/about/changelog.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index f739ce980..62e7d47eb 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -23,7 +23,10 @@ more info, see the :ref:`Contributor Guide `. Unreleased ---------- -No changes. +**Fixed** + +- :issue:`930` - better traceback for JSON serialization errors (via :pull:`1008`) +- :issue:`437` - explain that JS component attributes must be JSON (via :pull:`1008`) v1.0.0 From c18c4b8e55957fc169f52c7d7137d16ac5143f8b Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 21:02:52 -0600 Subject: [PATCH 08/13] fix tests --- src/py/reactpy/reactpy/core/vdom.py | 4 +- src/py/reactpy/tests/test_core/test_hooks.py | 9 ---- src/py/reactpy/tests/test_core/test_vdom.py | 44 ++++++++++++-------- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/py/reactpy/reactpy/core/vdom.py b/src/py/reactpy/reactpy/core/vdom.py index a845d51b5..840a09c7c 100644 --- a/src/py/reactpy/reactpy/core/vdom.py +++ b/src/py/reactpy/reactpy/core/vdom.py @@ -331,11 +331,11 @@ def _validate_child_key_integrity(value: Any) -> None: else: for child in value: if isinstance(child, ComponentType) and child.key is None: - warn(f"Key not specified for child in list {child}") + warn(f"Key not specified for child in list {child}", UserWarning) elif isinstance(child, Mapping) and "key" not in child: # remove 'children' to reduce log spam child_copy = {**child, "children": _EllipsisRepr()} - warn(f"Key not specified for child in list {child_copy}") + warn(f"Key not specified for child in list {child_copy}", UserWarning) class _CustomVdomDictConstructor(Protocol): diff --git a/src/py/reactpy/tests/test_core/test_hooks.py b/src/py/reactpy/tests/test_core/test_hooks.py index 7231794ad..453d07c99 100644 --- a/src/py/reactpy/tests/test_core/test_hooks.py +++ b/src/py/reactpy/tests/test_core/test_hooks.py @@ -1257,12 +1257,3 @@ def bad_effect(): await layout.render() component_hook.latest.schedule_render() await layout.render() # no error - - -@pytest.mark.skipif( - not REACTPY_DEBUG_MODE.current, - reason="only check json serialization in debug mode", -) -def test_raise_for_non_json_attrs(): - with pytest.raises(TypeError, match="JSON serializable"): - reactpy.html.div({"non_json_serializable_object": object()}) diff --git a/src/py/reactpy/tests/test_core/test_vdom.py b/src/py/reactpy/tests/test_core/test_vdom.py index 76e26e46f..37abad1d2 100644 --- a/src/py/reactpy/tests/test_core/test_vdom.py +++ b/src/py/reactpy/tests/test_core/test_vdom.py @@ -280,28 +280,36 @@ def test_invalid_vdom(value, error_message_pattern): validate_vdom_json(value) -@pytest.mark.skipif(not REACTPY_DEBUG_MODE.current, reason="Only logs in debug mode") -def test_debug_log_cannot_verify_keypath_for_genereators(caplog): - reactpy.vdom("div", (1 for i in range(10))) - assert len(caplog.records) == 1 - assert caplog.records[0].message.startswith( - "Did not verify key-path integrity of children in generator" - ) - caplog.records.clear() - +@pytest.mark.skipif(not REACTPY_DEBUG_MODE.current, reason="Only warns in debug mode") +def test_warn_cannot_verify_keypath_for_genereators(): + with pytest.warns(UserWarning) as record: + reactpy.vdom("div", (1 for i in range(10))) + assert len(record) == 1 + assert ( + record[0] + .message.args[0] + .startswith("Did not verify key-path integrity of children in generator") + ) -@pytest.mark.skipif(not REACTPY_DEBUG_MODE.current, reason="Only logs in debug mode") -def test_debug_log_dynamic_children_must_have_keys(caplog): - reactpy.vdom("div", [reactpy.vdom("div")]) - assert len(caplog.records) == 1 - assert caplog.records[0].message.startswith("Key not specified for child") - caplog.records.clear() +@pytest.mark.skipif(not REACTPY_DEBUG_MODE.current, reason="Only warns in debug mode") +def test_warn_dynamic_children_must_have_keys(): + with pytest.warns(UserWarning) as record: + reactpy.vdom("div", [reactpy.vdom("div")]) + assert len(record) == 1 + assert record[0].message.args[0].startswith("Key not specified for child") @reactpy.component def MyComponent(): return reactpy.vdom("div") - reactpy.vdom("div", [MyComponent()]) - assert len(caplog.records) == 1 - assert caplog.records[0].message.startswith("Key not specified for child") + with pytest.warns(UserWarning) as record: + reactpy.vdom("div", [MyComponent()]) + assert len(record) == 1 + assert record[0].message.args[0].startswith("Key not specified for child") + + +@pytest.mark.skipif(not REACTPY_DEBUG_MODE.current, reason="only checked in debug mode") +def test_raise_for_non_json_attrs(): + with pytest.raises(TypeError, match="JSON serializable"): + reactpy.html.div({"non_json_serializable_object": object()}) From f4aa5075c6a1b23476aceb991491ddc98e534893 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 21:24:03 -0600 Subject: [PATCH 09/13] try node 16? https://github.com/nodesource/distributions#using-ubuntu-3 --- docs/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Dockerfile b/docs/Dockerfile index 76a8ad7ee..cc43d8f7b 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -4,7 +4,7 @@ WORKDIR /app/ # Install NodeJS # -------------- -RUN curl -sL https://deb.nodesource.com/setup_14.x | bash - +RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - RUN apt-get install -yq nodejs build-essential RUN npm install -g npm@8.5.0 From c3545097e35f8145ef95adc46a97e91cdf45ad2e Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 21:51:09 -0600 Subject: [PATCH 10/13] install npm explicitey --- docs/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/Dockerfile b/docs/Dockerfile index cc43d8f7b..452327e2d 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -5,7 +5,7 @@ WORKDIR /app/ # Install NodeJS # -------------- RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - -RUN apt-get install -yq nodejs build-essential +RUN apt-get install -yq nodejs npm build-essential RUN npm install -g npm@8.5.0 # Install Poetry From aebbf875c5d171ed98f7fa851870a46091ca1d31 Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 21:53:32 -0600 Subject: [PATCH 11/13] try install in different order --- docs/Dockerfile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/Dockerfile b/docs/Dockerfile index 452327e2d..176ac6c8f 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -4,8 +4,8 @@ WORKDIR /app/ # Install NodeJS # -------------- -RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - -RUN apt-get install -yq nodejs npm build-essential +RUN apt-get install -y build-essential +RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - && apt-get install -y nodejs RUN npm install -g npm@8.5.0 # Install Poetry From d6c9718b57d82e24cacd2a165d4595ea123a185a Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 21:58:53 -0600 Subject: [PATCH 12/13] update first --- docs/Dockerfile | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/Dockerfile b/docs/Dockerfile index 176ac6c8f..95e91beb5 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -2,9 +2,13 @@ FROM python:3.9 WORKDIR /app/ +# Misc Dependencies +# ----------------- +RUN apt-get update +RUN apt-get install -y build-essential + # Install NodeJS # -------------- -RUN apt-get install -y build-essential RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - && apt-get install -y nodejs RUN npm install -g npm@8.5.0 From e83eb472ef17b7f91b808c6f9712abcf341000dd Mon Sep 17 00:00:00 2001 From: rmorshea Date: Wed, 14 Jun 2023 22:04:45 -0600 Subject: [PATCH 13/13] explicitely install npm i guess... --- docs/Dockerfile | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/Dockerfile b/docs/Dockerfile index 95e91beb5..39b9c51be 100644 --- a/docs/Dockerfile +++ b/docs/Dockerfile @@ -2,14 +2,10 @@ FROM python:3.9 WORKDIR /app/ -# Misc Dependencies -# ----------------- -RUN apt-get update -RUN apt-get install -y build-essential - # Install NodeJS # -------------- -RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - && apt-get install -y nodejs +RUN curl -fsSL https://deb.nodesource.com/setup_16.x | bash - +RUN apt-get install -y build-essential nodejs npm RUN npm install -g npm@8.5.0 # Install Poetry