diff --git a/docs/source/about/changelog.rst b/docs/source/about/changelog.rst index 9e4ce9b9c..a83b0cc29 100644 --- a/docs/source/about/changelog.rst +++ b/docs/source/about/changelog.rst @@ -27,6 +27,7 @@ Unreleased - :issue:`789` - Conditionally rendered components cannot use contexts - :issue:`773` - Use strict equality check for text, numeric, and binary types in hooks +- :issue:`801` - Accidental mutation of old model causes invalid JSON Patch **Changed** diff --git a/noxfile.py b/noxfile.py index a3588e164..ac9fcf08f 100644 --- a/noxfile.py +++ b/noxfile.py @@ -98,7 +98,7 @@ def example(session: Session) -> None: def docs(session: Session) -> None: """Build and display documentation in the browser (automatically reloads on change)""" install_requirements_file(session, "build-docs") - install_idom_dev(session, extras="all") + install_idom_dev(session) session.run( "python", "scripts/live_docs.py", @@ -188,7 +188,7 @@ def test_python_suite(session: Session) -> None: session.install(".[all]") else: posargs += ["--cov=src/idom", "--cov-report", "term"] - install_idom_dev(session, extras="all") + install_idom_dev(session) session.run("pytest", *posargs) @@ -234,7 +234,7 @@ def test_python_build(session: Session) -> None: def test_docs(session: Session) -> None: """Verify that the docs build and that doctests pass""" install_requirements_file(session, "build-docs") - install_idom_dev(session, extras="all") + install_idom_dev(session) session.run( "sphinx-build", "-a", # re-write all output files @@ -370,7 +370,8 @@ def install_requirements_file(session: Session, name: str) -> None: session.install("-r", str(file_path)) -def install_idom_dev(session: Session, extras: str = "stable") -> None: +def install_idom_dev(session: Session, extras: str = "all") -> None: + session.run("pip", "--version") if "--no-install" not in session.posargs: session.install("-e", f".[{extras}]") else: diff --git a/requirements/pkg-extras.txt b/requirements/pkg-extras.txt index e7257ccd8..2b2079706 100644 --- a/requirements/pkg-extras.txt +++ b/requirements/pkg-extras.txt @@ -12,7 +12,7 @@ uvicorn[standard] >=0.13.4 # extra=flask flask -markupsafe<2.1 +markupsafe>=1.1.1,<2.1 flask-cors flask-sock diff --git a/setup.py b/setup.py index f89c04e07..ba3b0e453 100644 --- a/setup.py +++ b/setup.py @@ -5,13 +5,12 @@ import subprocess import sys import traceback -from distutils import log -from distutils.command.build import build # type: ignore -from distutils.command.sdist import sdist # type: ignore +from logging import StreamHandler, getLogger from pathlib import Path from setuptools import find_packages, setup from setuptools.command.develop import develop +from setuptools.command.sdist import sdist if sys.platform == "win32": @@ -22,6 +21,15 @@ def list2cmdline(cmd_list): return " ".join(map(pipes.quote, cmd_list)) +log = getLogger() +log.addHandler(StreamHandler(sys.stdout)) + + +# ----------------------------------------------------------------------------- +# Basic Constants +# ----------------------------------------------------------------------------- + + # the name of the project NAME = "idom" @@ -167,10 +175,18 @@ def run(self): package["cmdclass"] = { "sdist": build_javascript_first(sdist), - "build": build_javascript_first(build), "develop": build_javascript_first(develop), } +if sys.version_info < (3, 10, 6): + from distutils.command.build import build + + package["cmdclass"]["build"] = build_javascript_first(build) +else: + from setuptools.command.build_py import build_py + + package["cmdclass"]["build_py"] = build_javascript_first(build_py) + # ----------------------------------------------------------------------------- # Install It diff --git a/src/idom/core/layout.py b/src/idom/core/layout.py index a02be353e..d257b3995 100644 --- a/src/idom/core/layout.py +++ b/src/idom/core/layout.py @@ -230,10 +230,17 @@ def _render_component( else: key, index = new_state.key, new_state.index parent.children_by_key[key] = new_state - # need to do insertion in case where old_state is None and we're appending - parent.model.current["children"][index : index + 1] = [ - new_state.model.current - ] + # need to add this model to parent's children without mutating parent model + old_parent_model = parent.model.current + old_parent_children = old_parent_model["children"] + parent.model.current = { + **old_parent_model, + "children": [ + *old_parent_children[:index], + new_state.model.current, + *old_parent_children[index + 1 :], + ], + } finally: # avoid double render self._rendering_queue.remove_if_pending(life_cycle_state.id) diff --git a/tests/test_client.py b/tests/test_client.py index 86b5a61a7..ff7d826e2 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -5,6 +5,7 @@ from playwright.async_api import Browser import idom +from idom.backend.utils import find_available_port from idom.testing import BackendFixture, DisplayFixture @@ -12,6 +13,7 @@ async def test_automatic_reconnect(browser: Browser): + port = find_available_port("localhost") page = await browser.new_page() # we need to wait longer here because the automatic reconnect is not instant @@ -22,7 +24,7 @@ def OldComponent(): return idom.html.p({"id": "old-component"}, "old") async with AsyncExitStack() as exit_stack: - server = await exit_stack.enter_async_context(BackendFixture(port=8000)) + server = await exit_stack.enter_async_context(BackendFixture(port=port)) display = await exit_stack.enter_async_context( DisplayFixture(server, driver=page) ) @@ -43,7 +45,7 @@ def NewComponent(): return idom.html.p({"id": f"new-component-{state}"}, f"new-{state}") async with AsyncExitStack() as exit_stack: - server = await exit_stack.enter_async_context(BackendFixture(port=8000)) + server = await exit_stack.enter_async_context(BackendFixture(port=port)) display = await exit_stack.enter_async_context( DisplayFixture(server, driver=page) ) diff --git a/tests/test_core/test_hooks.py b/tests/test_core/test_hooks.py index 9c3d8c2f3..e0bcba2e7 100644 --- a/tests/test_core/test_hooks.py +++ b/tests/test_core/test_hooks.py @@ -11,12 +11,10 @@ current_hook, strictly_equal, ) -from idom.core.layout import Layout -from idom.core.serve import render_json_patch +from idom.core.layout import Layout, LayoutUpdate from idom.testing import DisplayFixture, HookCatcher, assert_idom_did_log, poll from idom.testing.logs import assert_idom_did_not_log from idom.utils import Ref -from tests.tooling.asserts import assert_same_items async def test_must_be_rendering_in_layout_to_use_hooks(): @@ -42,31 +40,35 @@ def SimpleStatefulComponent(): sse = SimpleStatefulComponent() async with idom.Layout(sse) as layout: - patch_1 = await render_json_patch(layout) - assert patch_1.path == "" - assert_same_items( - patch_1.changes, - [ - {"op": "add", "path": "/tagName", "value": ""}, - { - "op": "add", - "path": "/children", - "value": [{"children": ["0"], "tagName": "div"}], - }, - ], + update_1 = await layout.render() + assert update_1 == LayoutUpdate( + path="", + old=None, + new={ + "tagName": "", + "children": [{"tagName": "div", "children": ["0"]}], + }, ) - patch_2 = await render_json_patch(layout) - assert patch_2.path == "" - assert patch_2.changes == [ - {"op": "replace", "path": "/children/0/children/0", "value": "1"} - ] - - patch_3 = await render_json_patch(layout) - assert patch_3.path == "" - assert patch_3.changes == [ - {"op": "replace", "path": "/children/0/children/0", "value": "2"} - ] + update_2 = await layout.render() + assert update_2 == LayoutUpdate( + path="", + old=update_1.new, + new={ + "tagName": "", + "children": [{"tagName": "div", "children": ["1"]}], + }, + ) + + update_3 = await layout.render() + assert update_3 == LayoutUpdate( + path="", + old=update_2.new, + new={ + "tagName": "", + "children": [{"tagName": "div", "children": ["2"]}], + }, + ) async def test_set_state_callback_identity_is_preserved(): diff --git a/tests/test_core/test_layout.py b/tests/test_core/test_layout.py index ce419c0e4..4349faabc 100644 --- a/tests/test_core/test_layout.py +++ b/tests/test_core/test_layout.py @@ -12,8 +12,7 @@ from idom.config import IDOM_DEBUG_MODE from idom.core.component import component from idom.core.hooks import use_effect, use_state -from idom.core.layout import Layout, LayoutEvent -from idom.core.serve import render_json_patch +from idom.core.layout import Layout, LayoutEvent, LayoutUpdate from idom.testing import ( HookCatcher, StaticEventHandler, @@ -69,7 +68,7 @@ def Component(): async def test_simple_layout(): - set_state_hook = idom.Ref(None) + set_state_hook = idom.Ref() @idom.component def SimpleComponent(): @@ -77,24 +76,21 @@ def SimpleComponent(): return idom.vdom(tag) async with idom.Layout(SimpleComponent()) as layout: - path, changes = await render_json_patch(layout) - - assert path == "" - assert_same_items( - changes, - [ - {"op": "add", "path": "/children", "value": [{"tagName": "div"}]}, - {"op": "add", "path": "/tagName", "value": ""}, - ], + update_1 = await layout.render() + assert update_1 == LayoutUpdate( + path="", + old=None, + new={"tagName": "", "children": [{"tagName": "div"}]}, ) set_state_hook.current("table") - path, changes = await render_json_patch(layout) - assert path == "" - assert changes == [ - {"op": "replace", "path": "/children/0/tagName", "value": "table"} - ] + update_2 = await layout.render() + assert update_2 == LayoutUpdate( + path="", + old=update_1.new, + new={"tagName": "", "children": [{"tagName": "table"}]}, + ) async def test_component_can_return_none(): @@ -113,55 +109,55 @@ async def test_nested_component_layout(): @idom.component def Parent(): state, parent_set_state.current = idom.hooks.use_state(0) - return idom.html.div(state, Child(key="c")) + return idom.html.div(state, Child()) @idom.component def Child(): state, child_set_state.current = idom.hooks.use_state(0) return idom.html.div(state) - async with idom.Layout(Parent(key="p")) as layout: - path, changes = await render_json_patch(layout) - - assert path == "" - assert_same_items( - changes, - [ + def make_parent_model(state, model): + return { + "tagName": "", + "children": [ { - "op": "add", - "path": "/children", - "value": [ - { - "children": [ - "0", - { - "children": [{"children": ["0"], "tagName": "div"}], - "tagName": "", - }, - ], - "tagName": "div", - } - ], - }, - {"op": "add", "path": "/tagName", "value": ""}, + "tagName": "div", + "children": [str(state), model], + } ], + } + + def make_child_model(state): + return { + "tagName": "", + "children": [{"tagName": "div", "children": [str(state)]}], + } + + async with idom.Layout(Parent()) as layout: + update_1 = await layout.render() + assert update_1 == LayoutUpdate( + path="", + old=None, + new=make_parent_model(0, make_child_model(0)), ) parent_set_state.current(1) - path, changes = await render_json_patch(layout) - assert path == "" - assert changes == [ - {"op": "replace", "path": "/children/0/children/0", "value": "1"} - ] + update_2 = await layout.render() + assert update_2 == LayoutUpdate( + path="", + old=update_1.new, + new=make_parent_model(1, make_child_model(0)), + ) child_set_state.current(1) - path, changes = await render_json_patch(layout) - assert path == "/children/0/children/1" - assert changes == [ - {"op": "replace", "path": "/children/0/children/0", "value": "1"} - ] + update_3 = await layout.render() + assert update_3 == LayoutUpdate( + path="/children/0/children/1", + old=update_2.new["children"][0]["children"][1], + new=make_child_model(1), + ) @pytest.mark.skipif( @@ -184,39 +180,35 @@ def BadChild(): with assert_idom_did_log(match_error="error from bad child"): async with idom.Layout(Main()) as layout: - patch = await render_json_patch(layout) - assert_same_items( - patch.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - { - "tagName": "div", - "children": [ - { - "tagName": "", - "children": [ - {"tagName": "div", "children": ["hello"]} - ], - }, - { - "tagName": "", - "error": "ValueError: error from bad child", - }, - { - "tagName": "", - "children": [ - {"tagName": "div", "children": ["hello"]} - ], - }, - ], - } - ], - }, - {"op": "add", "path": "/tagName", "value": ""}, - ], + assert (await layout.render()) == LayoutUpdate( + path="", + old=None, + new={ + "tagName": "", + "children": [ + { + "tagName": "div", + "children": [ + { + "tagName": "", + "children": [ + {"tagName": "div", "children": ["hello"]} + ], + }, + { + "tagName": "", + "error": "ValueError: error from bad child", + }, + { + "tagName": "", + "children": [ + {"tagName": "div", "children": ["hello"]} + ], + }, + ], + } + ], + }, ) @@ -240,36 +232,32 @@ def BadChild(): with assert_idom_did_log(match_error="error from bad child"): async with idom.Layout(Main()) as layout: - patch = await render_json_patch(layout) - assert_same_items( - patch.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - { - "children": [ - { - "children": [ - {"children": ["hello"], "tagName": "div"} - ], - "tagName": "", - }, - {"error": "", "tagName": ""}, - { - "children": [ - {"children": ["hello"], "tagName": "div"} - ], - "tagName": "", - }, - ], - "tagName": "div", - } - ], - }, - {"op": "add", "path": "/tagName", "value": ""}, - ], + assert (await layout.render()) == LayoutUpdate( + path="", + old=None, + new={ + "tagName": "", + "children": [ + { + "children": [ + { + "children": [ + {"children": ["hello"], "tagName": "div"} + ], + "tagName": "", + }, + {"error": "", "tagName": ""}, + { + "children": [ + {"children": ["hello"], "tagName": "div"} + ], + "tagName": "", + }, + ], + "tagName": "div", + } + ], + }, ) @@ -283,32 +271,28 @@ def Child(): return {"tagName": "div", "children": {"tagName": "h1"}} async with idom.Layout(Main()) as layout: - patch = await render_json_patch(layout) - assert_same_items( - patch.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - { - "children": [ - { - "children": [ - { - "children": [{"tagName": "h1"}], - "tagName": "div", - } - ], - "tagName": "", - } - ], - "tagName": "div", - } - ], - }, - {"op": "add", "path": "/tagName", "value": ""}, - ], + assert (await layout.render()) == LayoutUpdate( + path="", + old=None, + new={ + "tagName": "", + "children": [ + { + "children": [ + { + "children": [ + { + "children": [{"tagName": "h1"}], + "tagName": "div", + } + ], + "tagName": "", + } + ], + "tagName": "div", + } + ], + }, ) @@ -493,7 +477,7 @@ def Child(): hook.latest.schedule_render() - update = await render_json_patch(layout) + update = await layout.render() assert update.path == "/children/0/children/0/children/0" @@ -620,22 +604,18 @@ def Inner(): return idom.html.div("hello") async with idom.Layout(Outer()) as layout: - update = await render_json_patch(layout) - assert_same_items( - update.changes, - [ - { - "op": "add", - "path": "/children", - "value": [ - { - "children": [{"children": ["hello"], "tagName": "div"}], - "tagName": "", - } - ], - }, - {"op": "add", "path": "/tagName", "value": ""}, - ], + assert (await layout.render()) == LayoutUpdate( + path="", + old=None, + new={ + "tagName": "", + "children": [ + { + "children": [{"children": ["hello"], "tagName": "div"}], + "tagName": "", + } + ], + }, ) diff --git a/tests/test_core/test_dispatcher.py b/tests/test_core/test_serve.py similarity index 100% rename from tests/test_core/test_dispatcher.py rename to tests/test_core/test_serve.py diff --git a/tests/tooling/loop.py b/tests/tooling/loop.py index 043908694..6169f5176 100644 --- a/tests/tooling/loop.py +++ b/tests/tooling/loop.py @@ -69,9 +69,7 @@ def one_task_finished(future): if is_current: loop.run_until_complete( - wait_for( - asyncio.gather(*to_cancel, loop=loop, return_exceptions=True), TIMEOUT - ) + wait_for(asyncio.gather(*to_cancel, return_exceptions=True), TIMEOUT) ) else: # user was responsible for cancelling all tasks