From 9696e67910516fdbd32ce36ad94317122e8ed153 Mon Sep 17 00:00:00 2001 From: Kodjo Sossouvi Date: Sun, 2 Nov 2025 18:46:44 +0100 Subject: [PATCH] Refactored Binding for better concern consideration --- src/myfasthtml/controls/helpers.py | 35 ++-- src/myfasthtml/core/bindings.py | 186 ++++++++++++++----- src/myfasthtml/core/utils.py | 12 ++ src/myfasthtml/test/MyFT.py | 9 + src/myfasthtml/test/testclient.py | 11 +- tests/core/test_bindings.py | 283 +++++++++++++++++++++++++++-- tests/test_integration.py | 7 +- 7 files changed, 459 insertions(+), 84 deletions(-) create mode 100644 src/myfasthtml/test/MyFT.py diff --git a/src/myfasthtml/controls/helpers.py b/src/myfasthtml/controls/helpers.py index 954e9f2..cd17d26 100644 --- a/src/myfasthtml/controls/helpers.py +++ b/src/myfasthtml/controls/helpers.py @@ -1,8 +1,8 @@ from fasthtml.components import * -from myfasthtml.core.bindings import Binding +from myfasthtml.core.bindings import Binding, BooleanConverter, DetectionMode, UpdateMode from myfasthtml.core.commands import Command -from myfasthtml.core.utils import merge_classes, get_default_ft_attr +from myfasthtml.core.utils import merge_classes, get_default_ft_attr, is_checkbox class mk: @@ -37,17 +37,28 @@ class mk: @staticmethod def manage_binding(ft, binding: Binding): - if binding: - if ft.tag in ["input"]: - # update the component to post on the correct route input and forms only - htmx = binding.get_htmx_params() - ft.attrs |= htmx - - # update the binding with the ft - ft_attr = binding.ft_attr or get_default_ft_attr(ft) - ft_name = ft.attrs.get("name") - binding.bind_ft(ft, ft_name, ft_attr) # force the ft + if not binding: + return ft + if ft.tag in ["input"]: + # update the component to post on the correct route input and forms only + htmx = binding.get_htmx_params() + ft.attrs |= htmx + + # update the binding with the ft + ft_attr = binding.ft_attr or get_default_ft_attr(ft) + ft_name = ft.attrs.get("name") + + if is_checkbox(ft): + data_converter = BooleanConverter() + detection_mode = DetectionMode.AttributePresence + update_mode = UpdateMode.AttributePresence + else: + data_converter = None + detection_mode = None + update_mode = None + + binding.bind_ft(ft, ft_name, ft_attr, data_converter, detection_mode, update_mode) # force the ft return ft @staticmethod diff --git a/src/myfasthtml/core/bindings.py b/src/myfasthtml/core/bindings.py index 0c3cf78..1866d6b 100644 --- a/src/myfasthtml/core/bindings.py +++ b/src/myfasthtml/core/bindings.py @@ -1,10 +1,10 @@ import logging import uuid from enum import Enum -from typing import Optional +from typing import Optional, Any from fasthtml.fastapp import fast_app -from myutils.observable import make_observable, bind, collect_return_values +from myutils.observable import make_observable, bind, collect_return_values, unbind from myfasthtml.core.constants import Routes, ROUTE_ROOT from myfasthtml.core.utils import get_default_attr @@ -105,44 +105,34 @@ class BooleanConverter(DataConverter): class Binding: - def __init__(self, data, - attr=None, - data_converter: DataConverter = None, - ft=None, - ft_name=None, - ft_attr=None, - detection_mode: DetectionMode = DetectionMode.ValueChange, - update_mode: UpdateMode = UpdateMode.ValueChange): + def __init__(self, data: Any, attr: str = None): """ - Creates a new binding object between a data object used as a pivot and an HTML element. - The same pivot object must be used for different bindings. - This will allow the binding between the HTML elements - - :param data: object used as a pivot - :param attr: attribute of the data object - :param ft: HTML element to bind to - :param ft_name: name of the HTML element to bind to (send by the form) - :param ft_attr: value of the attribute to bind to (send by the form) + Creates a new binding object between a data object and an HTML element. + The binding is not active until bind_ft() is called. + + Args: + data: Object used as a pivot + attr: Attribute of the data object to bind """ self.id = uuid.uuid4() self.htmx_extra = {} self.data = data self.data_attr = attr or get_default_attr(data) - self.data_converter = data_converter - self.ft = self._safe_ft(ft) - self.ft_name = ft_name - self.ft_attr = ft_attr - self.detection_mode = detection_mode - self.update_mode = update_mode - self._detection = self._factory(detection_mode) - self._update = self._factory(update_mode) + # UI-related attributes (configured later via bind_ft) + self.ft = None + self.ft_name = None + self.ft_attr = None + self.data_converter = None + self.detection_mode = DetectionMode.ValueChange + self.update_mode = UpdateMode.ValueChange - make_observable(self.data) - bind(self.data, self.data_attr, self.notify) + # Strategy objects (configured later) + self._detection = None + self._update = None - # register the command - BindingsManager.register(self) + # Activation state + self._is_active = False def bind_ft(self, ft, @@ -152,25 +142,43 @@ class Binding: detection_mode: DetectionMode = None, update_mode: UpdateMode = None): """ - Update the elements to bind to - :param ft: - :param name: - :param attr: - :param data_converter: - :param detection_mode: - :param update_mode: - :return: + Configure the UI element and activate the binding. + + Args: + ft: HTML element to bind to + name: Name of the HTML element (sent by the form) + attr: Attribute of the HTML element to bind to + data_converter: Optional converter for data transformation + detection_mode: How to detect changes from UI + update_mode: How to update the UI element + + Returns: + self for method chaining """ + # Deactivate if already active + if self._is_active: + self.deactivate() + + # Configure UI elements self.ft = self._safe_ft(ft) self.ft_name = name - self.ft_attr = attr or self.ft_attr - self.data_converter = data_converter or self.data_converter - self.detection_mode = detection_mode or self.detection_mode - self.update_mode = update_mode or self.update_mode + self.ft_attr = attr + # Update optional parameters if provided + if data_converter is not None: + self.data_converter = data_converter + if detection_mode is not None: + self.detection_mode = detection_mode + if update_mode is not None: + self.update_mode = update_mode + + # Create strategy objects self._detection = self._factory(self.detection_mode) self._update = self._factory(self.update_mode) + # Activate the binding + self.activate() + return self def get_htmx_params(self): @@ -180,6 +188,21 @@ class Binding: } def notify(self, old, new): + """ + Callback when the data attribute changes. + Updates the UI element accordingly. + + Args: + old: Previous value + new: New value + + Returns: + Updated ft element + """ + if not self._is_active: + logger.warning(f"Binding '{self.id}' received notification but is not active") + return None + logger.debug(f"Binding '{self.id}': Changing from '{old}' to '{new}'") self.ft = self._update.update(self.ft, self.ft_name, self.ft_attr, old, new) @@ -198,6 +221,53 @@ class Binding: logger.debug(f"Nothing to trigger in {values}.") return None + def activate(self): + """ + Activate the binding by setting up observers and registering it. + Should only be called after the binding is fully configured. + + Raises: + ValueError: If the binding is not fully configured + """ + if self._is_active: + logger.warning(f"Binding '{self.id}' is already active") + return + + # Validate configuration + self._validate_configuration() + + # Setup observable + make_observable(self.data) + bind(self.data, self.data_attr, self.notify) + + # Register in manager + BindingsManager.register(self) + + # Mark as active + self._is_active = True + + logger.debug(f"Binding '{self.id}' activated for {self.data_attr}") + + def deactivate(self): + """ + Deactivate the binding by removing observers and unregistering it. + Can be called multiple times safely. + """ + if not self._is_active: + logger.debug(f"Binding '{self.id}' is not active, nothing to deactivate") + return + + # Remove observer + unbind(self.data, self.data_attr, self.notify) + + # Unregister from manager + BindingsManager.unregister(self.id) + + # Mark as inactive + self._is_active = False + + logger.debug(f"Binding '{self.id}' deactivated") + @staticmethod def _safe_ft(ft): """ @@ -228,6 +298,25 @@ class Binding: else: raise ValueError(f"Invalid detection mode: {mode}") + def _validate_configuration(self): + """ + Validate that the binding is fully configured before activation. + + Raises: + ValueError: If required configuration is missing + """ + if self.ft is None: + raise ValueError(f"Binding '{self.id}': ft element is required") + + # if self.ft_name is None: + # raise ValueError(f"Binding '{self.id}': ft_name is required") + + if self._detection is None: + raise ValueError(f"Binding '{self.id}': detection strategy not initialized") + + if self._update is None: + raise ValueError(f"Binding '{self.id}': update strategy not initialized") + def htmx(self, trigger=None): if trigger: self.htmx_extra["hx-trigger"] = trigger @@ -241,6 +330,17 @@ class BindingsManager: def register(binding: Binding): BindingsManager.bindings[str(binding.id)] = binding + @staticmethod + def unregister(binding_id: str): + """ + Unregister a binding from the manager. + + Args: + binding_id: ID of the binding to unregister + """ + if str(binding_id) in BindingsManager.bindings: + del BindingsManager.bindings[str(binding_id)] + @staticmethod def get_binding(binding_id: str) -> Optional[Binding]: return BindingsManager.bindings.get(str(binding_id)) diff --git a/src/myfasthtml/core/utils.py b/src/myfasthtml/core/utils.py index dc841d0..e976a82 100644 --- a/src/myfasthtml/core/utils.py +++ b/src/myfasthtml/core/utils.py @@ -1,9 +1,12 @@ import logging +from bs4 import Tag +from fastcore.xml import FT from fasthtml.fastapp import fast_app from starlette.routing import Mount, Route from myfasthtml.core.constants import Routes, ROUTE_ROOT +from myfasthtml.test.MyFT import MyFT utils_app, utils_rt = fast_app() logger = logging.getLogger("Commands") @@ -101,6 +104,15 @@ def get_default_attr(data): return next(iter(all_attrs)) +def is_checkbox(elt): + if isinstance(elt, (FT, MyFT)): + return elt.tag == "input" and elt.attrs.get("type", None) == "checkbox" + elif isinstance(elt, Tag): + return elt.name == "input" and elt.attrs.get("type", None) == "checkbox" + else: + return False + + @utils_rt(Routes.Commands) def post(session: str, c_id: str): """ diff --git a/src/myfasthtml/test/MyFT.py b/src/myfasthtml/test/MyFT.py new file mode 100644 index 0000000..036858a --- /dev/null +++ b/src/myfasthtml/test/MyFT.py @@ -0,0 +1,9 @@ +from dataclasses import dataclass, field + + +@dataclass +class MyFT: + tag: str + attrs: dict + children: list['MyFT'] = field(default_factory=list) + text: str | None = None diff --git a/src/myfasthtml/test/testclient.py b/src/myfasthtml/test/testclient.py index 78cf9b2..5893f2f 100644 --- a/src/myfasthtml/test/testclient.py +++ b/src/myfasthtml/test/testclient.py @@ -1,7 +1,5 @@ -import dataclasses import json import uuid -from dataclasses import dataclass from typing import Self from bs4 import BeautifulSoup, Tag @@ -11,6 +9,7 @@ from starlette.responses import Response from starlette.testclient import TestClient from myfasthtml.core.utils import mount_utils +from myfasthtml.test.MyFT import MyFT verbs = { 'hx_get': 'GET', @@ -21,14 +20,6 @@ verbs = { } -@dataclass -class MyFT: - tag: str - attrs: dict - children: list['MyFT'] = dataclasses.field(default_factory=list) - text: str | None = None - - class TestableElement: """ Represents an HTML element that can be interacted with in tests. diff --git a/tests/core/test_bindings.py b/tests/core/test_bindings.py index 5aa2d53..fa93903 100644 --- a/tests/core/test_bindings.py +++ b/tests/core/test_bindings.py @@ -4,7 +4,13 @@ import pytest from fasthtml.components import Label, Input from myutils.observable import collect_return_values -from myfasthtml.core.bindings import BindingsManager, Binding, DetectionMode +from myfasthtml.core.bindings import ( + BindingsManager, + Binding, + DetectionMode, + UpdateMode, + BooleanConverter +) @dataclass @@ -39,12 +45,15 @@ def test_i_can_register_a_binding_with_default_attr(data): def test_i_can_retrieve_a_registered_binding(data): - binding = Binding(data) + elt = Label("hello", id="label_id") + binding = Binding(data).bind_ft(elt, name="label_name") + assert BindingsManager.get_binding(binding.id) is binding def test_i_can_reset_bindings(data): - Binding(data) + elt = Label("hello", id="label_id") + Binding(data).bind_ft(elt, name="label_name") assert len(BindingsManager.bindings) != 0 BindingsManager.reset() @@ -53,7 +62,7 @@ def test_i_can_reset_bindings(data): def test_i_can_bind_an_element_to_a_binding(data): elt = Label("hello", id="label_id") - Binding(data, ft=elt) + Binding(data).bind_ft(elt, name="label_name") data.value = "new value" @@ -63,9 +72,9 @@ def test_i_can_bind_an_element_to_a_binding(data): def test_i_can_bind_an_element_attr_to_a_binding(data): - elt = Input(value="somme value", id="input_id") + elt = Input(value="some value", id="input_id") - Binding(data, ft=elt, ft_attr="value") + Binding(data).bind_ft(elt, name="input_name", attr="value") data.value = "new value" @@ -78,13 +87,13 @@ def test_bound_element_has_an_id(): elt = Label("hello") assert elt.attrs.get("id", None) is None - Binding(Data(), ft=elt) + Binding(Data()).bind_ft(elt, name="label_name") assert elt.attrs.get("id", None) is not None def test_i_can_collect_updates_values(data): elt = Label("hello") - Binding(data, ft=elt) + Binding(data).bind_ft(elt, name="label_name") data.value = "new value" collected = collect_return_values(data) @@ -98,7 +107,7 @@ def test_i_can_collect_updates_values(data): def test_i_can_react_to_value_change(data): elt = Input(name="input_elt", value="hello") - binding = Binding(data, ft=elt, ft_name="input_elt", ft_attr="value") + binding = Binding(data).bind_ft(elt, name="input_elt", attr="value") res = binding.update({"input_elt": "new value"}) @@ -107,7 +116,7 @@ def test_i_can_react_to_value_change(data): def test_i_do_not_react_to_other_value_change(data): elt = Input(name="input_elt", value="hello") - binding = Binding(data, ft=elt, ft_name="input_elt", ft_attr="value") + binding = Binding(data).bind_ft(elt, name="input_elt", attr="value") res = binding.update({"other_input_elt": "new value"}) @@ -116,8 +125,12 @@ def test_i_do_not_react_to_other_value_change(data): def test_i_can_react_to_attr_presence(data): elt = Input(name="input_elt", type="checkbox") - binding = Binding(data, ft=elt, ft_name="input_elt", ft_attr="checked", - detection_mode=DetectionMode.AttributePresence) + binding = Binding(data).bind_ft( + elt, + name="input_elt", + attr="checked", + detection_mode=DetectionMode.AttributePresence + ) res = binding.update({"checked": "true"}) @@ -126,9 +139,251 @@ def test_i_can_react_to_attr_presence(data): def test_i_can_react_to_attr_non_presence(data): elt = Input(name="input_elt", type="checkbox") - binding = Binding(data, ft=elt, ft_name="input_elt", ft_attr="checked", - detection_mode=DetectionMode.AttributePresence) + binding = Binding(data).bind_ft( + elt, + name="input_elt", + attr="checked", + detection_mode=DetectionMode.AttributePresence + ) res = binding.update({}) assert len(res) == 1 + + +def test_i_can_create_a_binding_without_activation(data): + """ + A binding created without calling bind_ft should not be active. + """ + binding = Binding(data, "value") + + assert binding._is_active is False + assert binding.ft is None + assert binding.ft_name is None + assert binding.ft_attr is None + assert BindingsManager.get_binding(binding.id) is None + + +def test_i_can_activate_binding_via_bind_ft(data): + """ + Calling bind_ft should automatically activate the binding. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value") + + binding.bind_ft(elt, name="label_name") + + assert binding._is_active is True + assert binding.ft is elt + assert binding.ft_name == "label_name" + assert BindingsManager.get_binding(binding.id) is binding + + +def test_i_cannot_notify_when_not_active(data): + """ + A non-active binding should not update the UI when data changes. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value") + binding.ft = elt + binding.ft_name = "label_name" + + # Change data without activating the binding + result = binding.notify("old", "new") + + assert result is None + assert elt.children[0] == "hello" # Should not have changed + + +def test_i_can_deactivate_a_binding(data): + """ + Deactivating a binding should clean up observers and unregister it. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value").bind_ft(elt, name="label_name") + + assert binding._is_active is True + assert BindingsManager.get_binding(binding.id) is binding + + binding.deactivate() + + assert binding._is_active is False + assert BindingsManager.get_binding(binding.id) is None + + +def test_i_can_reactivate_a_binding(data): + """ + After deactivation, a binding can be reactivated by calling bind_ft again. + """ + elt1 = Label("hello", id="label_id_1") + binding = Binding(data, "value").bind_ft(elt1, name="label_name_1") + + binding.deactivate() + assert binding._is_active is False + + elt2 = Label("world", id="label_id_2") + binding.bind_ft(elt2, name="label_name_2") + + assert binding._is_active is True + assert binding.ft is elt2 + assert binding.ft_name == "label_name_2" + + +def test_bind_ft_deactivates_before_reconfiguring(data): + """ + Calling bind_ft on an active binding should deactivate it first, + then reconfigure and reactivate. + """ + elt1 = Label("hello", id="label_id_1") + elt2 = Label("world", id="label_id_2") + + binding = Binding(data, "value").bind_ft(elt1, name="label_name_1") + + # Change data to verify old binding works + data.value = "updated" + assert elt1.children[0] == "updated" + + # Reconfigure with new element + binding.bind_ft(elt2, name="label_name_2") + + # Change data again + data.value = "final" + + # Old element should not update + assert elt1.children[0] == "updated" + + # New element should update + assert elt2.children[0] == "final" + + +def test_deactivate_can_be_called_multiple_times(data): + """ + Calling deactivate multiple times should be safe (idempotent). + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value").bind_ft(elt, name="label_name") + + binding.deactivate() + binding.deactivate() # Should not raise an error + binding.deactivate() # Should not raise an error + + assert binding._is_active is False + + +def test_i_cannot_activate_without_configuration(data): + """ + Calling activate directly without proper configuration should raise ValueError. + """ + binding = Binding(data, "value") + + with pytest.raises(ValueError, match="ft element is required"): + binding.activate() + + +def test_activation_validates_ft_name(data): + """ + Activation should fail if ft_name is not configured. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value") + binding.ft = elt + binding._detection = binding._factory(DetectionMode.ValueChange) + binding._update = binding._factory(UpdateMode.ValueChange) + + with pytest.raises(ValueError, match="ft_name is required"): + binding.activate() + + +def test_activation_validates_strategies(data): + """ + Activation should fail if detection/update strategies are not initialized. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value") + binding.ft = elt + binding.ft_name = "label_name" + + with pytest.raises(ValueError, match="detection strategy not initialized"): + binding.activate() + + +def test_i_can_chain_bind_ft_calls(data): + """ + bind_ft should return self for method chaining. + """ + elt = Label("hello", id="label_id") + + binding = Binding(data, "value").bind_ft(elt, name="label_name") + + assert isinstance(binding, Binding) + assert binding._is_active is True + + +def test_bind_ft_updates_optional_parameters(data): + """ + bind_ft should update optional parameters if provided. + """ + elt = Input(name="input_elt", type="checkbox") + + binding = Binding(data, "value") + + binding.bind_ft( + elt, + name="input_elt", + attr="checked", + data_converter=BooleanConverter(), + detection_mode=DetectionMode.AttributePresence, + update_mode=UpdateMode.AttributePresence + ) + + assert binding.detection_mode == DetectionMode.AttributePresence + assert binding.update_mode == UpdateMode.AttributePresence + assert isinstance(binding.data_converter, BooleanConverter) + + +def test_deactivated_binding_does_not_update_on_data_change(data): + """ + After deactivation, changes to data should not update the UI element. + """ + elt = Label("hello", id="label_id") + binding = Binding(data, "value").bind_ft(elt, name="label_name") + + # Verify it works when active + data.value = "first update" + assert elt.children[0] == "first update" + + # Deactivate + binding.deactivate() + + # Change data - element should NOT update + data.value = "second update" + assert elt.children[0] == "first update" + + +def test_multiple_bindings_can_coexist(data): + """ + Multiple bindings can be created and managed independently. + """ + elt1 = Label("hello", id="label_id_1") + elt2 = Input(value="world", id="input_id_2") + + binding1 = Binding(data, "value").bind_ft(elt1, name="label_name") + binding2 = Binding(data, "value").bind_ft(elt2, name="input_name", attr="value") + + assert len(BindingsManager.bindings) == 2 + assert binding1._is_active is True + assert binding2._is_active is True + + # Change data - both should update + data.value = "updated" + assert elt1.children[0] == "updated" + assert elt2.attrs["value"] == "updated" + + # Deactivate one + binding1.deactivate() + assert len(BindingsManager.bindings) == 1 + + # Change data - only binding2 should update + data.value = "final" + assert elt1.children[0] == "updated" # Not changed + assert elt2.attrs["value"] == "final" # Changed diff --git a/tests/test_integration.py b/tests/test_integration.py index 8eda717..683ee6f 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -69,7 +69,7 @@ class TestingBindings: data = Data("hello world") input_elt = Input(name="input_name") label_elt = Label() - mk.manage_binding(input_elt, Binding(data, ft_attr="value")) + mk.manage_binding(input_elt, Binding(data)) mk.manage_binding(label_elt, Binding(data)) return input_elt, label_elt @@ -85,10 +85,7 @@ class TestingBindings: data = Data(True) input_elt = Input(name="input_name", type="checkbox") label_elt = Label() - mk.manage_binding(input_elt, Binding(data, ft_attr="checked", - detection_mode=DetectionMode.AttributePresence, - update_mode=UpdateMode.AttributePresence, - data_converter=BooleanConverter())) + mk.manage_binding(input_elt, Binding(data)) mk.manage_binding(label_elt, Binding(data)) return input_elt, label_elt