From 688875406133bcf45ca462ba5f7c599a7a8e0618 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 1 Aug 2024 10:31:50 +0100 Subject: [PATCH 1/5] mutate via data bind --- src/textual/dom.py | 4 ++-- src/textual/reactive.py | 11 +++++++++++ tests/test_reactive.py | 30 ++++++++++++++++++++++++++++++ 3 files changed, 43 insertions(+), 2 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index 55ce52afea..74d2e07060 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -41,7 +41,7 @@ from .css.styles import RenderStyles, Styles from .css.tokenize import IDENTIFIER from .message_pump import MessagePump -from .reactive import Reactive, ReactiveError, _watch +from .reactive import Reactive, ReactiveError, _Mutated, _watch from .timer import Timer from .walk import walk_breadth_first, walk_depth_first from .worker_manager import WorkerManager @@ -334,7 +334,7 @@ def setter(value: object) -> None: """Set bound data.""" _rich_traceback_omit = True Reactive._initialize_object(self) - setattr(self, variable_name, value) + setattr(self, variable_name, _Mutated(value)) return setter diff --git a/src/textual/reactive.py b/src/textual/reactive.py index 17b109d77e..64b6e007ce 100644 --- a/src/textual/reactive.py +++ b/src/textual/reactive.py @@ -42,6 +42,13 @@ ReactableType = TypeVar("ReactableType", bound="DOMNode") +class _Mutated: + """A wrapper to indicate a value was mutated.""" + + def __init__(self, value: Any) -> None: + self.value = value + + class ReactiveError(Exception): """Base class for reactive errors.""" @@ -273,6 +280,10 @@ def _set(self, obj: Reactable, value: ReactiveType, always: bool = False) -> Non f"Node is missing data; Check you are calling super().__init__(...) in the {obj.__class__.__name__}() constructor, before setting reactives." ) + if isinstance(value, _Mutated): + value = value.value + always = True + self._initialize_reactive(obj, self.name) if hasattr(obj, self.compute_name): diff --git a/tests/test_reactive.py b/tests/test_reactive.py index ad2da5724d..2684dcae5f 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -783,3 +783,33 @@ def compose(self) -> ComposeResult: widget.mutate_reactive(TestWidget.names) # Watcher should be invoked assert watched_names == [[], ["Paul"], ["Jessica"]] + + +async def test_mutate_reactive_data_bind() -> None: + """https://github.com/Textualize/textual/issues/4825""" + + # Record mutations to TestWidget.messages + widget_messages: list[list[str]] = [] + + class TestWidget(Widget): + messages: reactive[list[str]] = reactive(list, init=False) + + def watch_messages(self, names: list[str]) -> None: + widget_messages.append(names.copy()) + + class TestApp(App): + messages: reactive[list[str]] = reactive(list, init=False) + + def compose(self) -> ComposeResult: + yield TestWidget().data_bind(TestApp.messages) + + app = TestApp() + async with app.run_test() as pilot: + assert widget_messages == [[]] + app.messages.append("foo") + # Mutations aren't detected + assert widget_messages == [[]] + # Explicitly mutate app reactive + app.mutate_reactive(TestApp.messages) + # Mutating app, will also invoke watchers on any data binds + assert widget_messages == [[], ["foo"]] From 05dad38fcd8941fb8e7006e3a802394763a47c3f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 1 Aug 2024 10:43:10 +0100 Subject: [PATCH 2/5] better test --- src/textual/dom.py | 6 +++++- tests/test_reactive.py | 15 ++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/textual/dom.py b/src/textual/dom.py index 74d2e07060..d3b4b83e9e 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -8,6 +8,7 @@ import re import threading +from copy import copy from functools import lru_cache, partial from inspect import getfile from typing import ( @@ -334,7 +335,10 @@ def setter(value: object) -> None: """Set bound data.""" _rich_traceback_omit = True Reactive._initialize_object(self) - setattr(self, variable_name, _Mutated(value)) + # Copy the bound value + # Sharing the same instance with mutable objects can lead to confusing behavior + # Wrap the value in `_Mutated` so the setter knows to invoke watchers etc + setattr(self, variable_name, _Mutated(copy(value))) return setter diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 2684dcae5f..49ab1fd0f6 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -804,12 +804,25 @@ def compose(self) -> ComposeResult: yield TestWidget().data_bind(TestApp.messages) app = TestApp() - async with app.run_test() as pilot: + async with app.run_test(): + test_widget = app.query_one(TestWidget) assert widget_messages == [[]] + assert test_widget.messages == [] + + # Previously setting a mutable object would lead to shared references + assert app.messages is not test_widget.messages + + # Mutate app app.messages.append("foo") # Mutations aren't detected assert widget_messages == [[]] + assert app.messages == ["foo"] + assert test_widget.messages == [] # Explicitly mutate app reactive app.mutate_reactive(TestApp.messages) # Mutating app, will also invoke watchers on any data binds assert widget_messages == [[], ["foo"]] + assert app.messages == ["foo"] + assert test_widget.messages == ["foo"] + + assert app.messages is not test_widget.messages From e0e3f8d1ffa14c71ce9f4d68bd39181a40cf2205 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 1 Aug 2024 12:06:33 +0100 Subject: [PATCH 3/5] no copy --- docs/guide/reactivity.md | 3 ++- src/textual/dom.py | 8 +++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/guide/reactivity.md b/docs/guide/reactivity.md index ff7a1cebdf..eb058a130b 100644 --- a/docs/guide/reactivity.md +++ b/docs/guide/reactivity.md @@ -410,7 +410,8 @@ Note the call to `mutate_reactive`. Without it, the display would not update whe ## Data binding -Reactive attributes from one widget may be *bound* (connected) to another widget, so that changes to a single reactive will automatically update another widget (potentially more than one). +Reactive attributes may be *bound* (connected) to attributes on child widgets, so that changes to the parent are automatically reflected in the children. +This can simplify working with compound widgets where the value of an attribute might be used in multiple places. To bind reactive attributes, call [data_bind][textual.dom.DOMNode.data_bind] on a widget. This method accepts reactives (as class attributes) in positional arguments or keyword arguments. diff --git a/src/textual/dom.py b/src/textual/dom.py index d3b4b83e9e..3dd5bc757a 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -8,7 +8,6 @@ import re import threading -from copy import copy from functools import lru_cache, partial from inspect import getfile from typing import ( @@ -280,6 +279,7 @@ def compose(self) -> ComposeResult: yield WorldClock("Asia/Tokyo").data_bind(WorldClockApp.time) ``` + Raises: ReactiveError: If the data wasn't bound. @@ -335,10 +335,8 @@ def setter(value: object) -> None: """Set bound data.""" _rich_traceback_omit = True Reactive._initialize_object(self) - # Copy the bound value - # Sharing the same instance with mutable objects can lead to confusing behavior - # Wrap the value in `_Mutated` so the setter knows to invoke watchers etc - setattr(self, variable_name, _Mutated(copy(value))) + # Wrap the value in `_Mutated` so the setter knows to invoke watchers etc. + setattr(self, variable_name, _Mutated(value)) return setter From 301f008e6ffc66fdb0d7b992855be3f958a08005 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 1 Aug 2024 12:12:48 +0100 Subject: [PATCH 4/5] test fix --- tests/test_reactive.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/test_reactive.py b/tests/test_reactive.py index 49ab1fd0f6..54b57e337e 100644 --- a/tests/test_reactive.py +++ b/tests/test_reactive.py @@ -809,20 +809,18 @@ def compose(self) -> ComposeResult: assert widget_messages == [[]] assert test_widget.messages == [] - # Previously setting a mutable object would lead to shared references - assert app.messages is not test_widget.messages + # Should be the same instance + assert app.messages is test_widget.messages # Mutate app app.messages.append("foo") # Mutations aren't detected assert widget_messages == [[]] assert app.messages == ["foo"] - assert test_widget.messages == [] + assert test_widget.messages == ["foo"] # Explicitly mutate app reactive app.mutate_reactive(TestApp.messages) # Mutating app, will also invoke watchers on any data binds assert widget_messages == [[], ["foo"]] assert app.messages == ["foo"] assert test_widget.messages == ["foo"] - - assert app.messages is not test_widget.messages From 3cdc6537a410ca200ab89ae457f7f5273ce6e8a9 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Thu, 1 Aug 2024 15:09:43 +0100 Subject: [PATCH 5/5] note --- src/textual/dom.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/textual/dom.py b/src/textual/dom.py index 3dd5bc757a..6e84f53ee3 100644 --- a/src/textual/dom.py +++ b/src/textual/dom.py @@ -253,6 +253,10 @@ def mutate_reactive(self, reactive: Reactive[ReactiveType]) -> None: this method after your reactive is updated. This will ensure that all the reactive _superpowers_ work. + !!! note + + This method will cause watchers to be called, even if the value hasn't changed. + Args: reactive: A reactive property (use the class scope syntax, i.e. `MyClass.my_reactive`). """