Add Tag.find_by_id and use it to avoid interal cache to save memory

Author: frmdstryr

setup.py

@@ -15,7 +15,7 @@
 
 setup(
     name='enaml-web',
-    version='0.12.2',
+    version='0.13.0',
     author='CodeLV',
     author_email='[email protected]',
     url='https://github.com/codelv/enaml-web',

tests/test_html.py

@@ -47,7 +47,7 @@ def test_hello_world(app):
     assert view.render()
     assert len(list(view.proxy.child_widgets())) == 2
     head = next(view.proxy.xpath("//head"))
-    html = head.parent_widget()
+    html = head.parent
     assert html.tag == "html"
 
 
@@ -465,7 +465,7 @@ def on_modified(change):
     assert v["id"] == parent_ref  # value is also the dom inserted
 
 
-def test_note_insert_before(app):
+def test_node_insert_before(app):
     Page = compile_source(
         dedent(
             """
@@ -513,8 +513,13 @@ def on_modified(change):
     # enaml-web should block extraneous moved events
     assert len(evts) == 1
 
-    e = evts[-1]["value"]
-    assert e["index"] == 4
+    # Verify the modified event
+    e = evts[-1]
+    assert e["type"] == "event" and e["name"] == "modified"
+
+    v = e["value"]
+    assert v["type"] == "added" and v["name"] == "children"
+    assert v["index"] == 4
 
 
 def test_node_removed(app):
@@ -620,16 +625,49 @@ def on_modified(change):
     r = [li.text for li in view.proxy.widget.xpath("/html/body/ul/li")]
     assert r == ["1", "2", "3", "4", "6"]
 
+    # Save the nodes we're moving
+    ul_id = view.proxy.widget.xpath("/html/body/ul")[0].attrib["id"]
+    list_items = view.proxy.widget.xpath("/html/body/ul/li")
+    li2 = list_items[1]
+    assert li2.text == "2"
+    li2_id = li2.attrib["id"]
+
+    li3 = list_items[2]
+    assert li3.text == "3"
+    li3_id = li3.attrib["id"]
+
+    li4 = list_items[3]
+    assert li4.text == "4"
+    li4_id = li4.attrib["id"]
+
     # Swap adjacent items
     view.render(menu=["3", "2", "4"])
     r = [li.text for li in view.proxy.widget.xpath("/html/body/ul/li")]
     assert r == ["1", "3", "2", "4", "6"]
 
+    # Moved 3 to before 2
+    print(evts)
+    assert len(evts) == 1
+    v = evts[-1]["value"]
+    assert v["type"] == "moved" and v["name"] == "children"
+    assert v["value"] == li3_id and v["id"] == ul_id and v["index"] == 1
+
     # Swap two items that are not adjacent (3 and 4)
     view.render(menu=["4", "2", "3"])
     r = [li.text for li in view.proxy.widget.xpath("/html/body/ul/li")]
     assert r == ["1", "4", "2", "3", "6"]
 
+    # Moved 4 to before 3
+    print(evts)
+    assert len(evts) == 3
+    v = evts[-2]["value"]
+    assert v["type"] == "moved" and v["name"] == "children"
+    assert v["value"] == li4_id and v["id"] == ul_id and v["index"] == 1
+
+    v = evts[-1]["value"]
+    assert v["type"] == "moved" and v["name"] == "children"
+    assert v["value"] == li2_id and v["id"] == ul_id and v["index"] == 2
+
     # view.render(menu=["4", "2", "3"])
     # r = [li.text for li in view.proxy.widget.xpath("/html/body/ul/li")]
     # assert r == ["1", "4", "2", "3", "6"]

web/components/html.py

@@ -59,11 +59,7 @@ class ProxyTag(ProxyToolkitObject):
     #: Reference to the declaration
     declaration = ForwardTyped(lambda: Tag)
 
-    #: A cached reference to the root element.
-    #: WARNING: If the root is changed this becomes invalid
-    root = ForwardTyped(lambda: ProxyTag)
-
-    def xpath(self, query: str, **kwargs) -> Generator[ProxyToolkitObject, None, None]:
+    def xpath(self, query: str, **kwargs) -> Generator["ProxyTag", None, None]:
         """Perform an xpath lookup on the node"""
         raise NotImplementedError
 
@@ -189,18 +185,15 @@ def _update_proxy(self, change: ChangeDict):
                 handler(value)
             else:
                 proxy.set_attribute(name, value)
-            root = proxy.root
-            if root is not None and root.rendered:
-                self._notify_modified(
-                    root.declaration,
-                    {
-                        "id": self.id,
-                        "type": change["type"],
-                        "name": name,
-                        "value": value,
-                        "oldvalue": change["oldvalue"],
-                    },
-                )
+            self._notify_modified(
+                {
+                    "id": self.id,
+                    "type": change["type"],
+                    "name": name,
+                    "value": value,
+                    "oldvalue": change["oldvalue"],
+                },
+            )
 
     # =========================================================================
     # Object API
@@ -209,32 +202,23 @@ def _update_proxy(self, change: ChangeDict):
     def child_added(self, child: Declarative):
         super().child_added(child)
         if self.proxy_is_active and isinstance(child, Tag):
-            proxy = self.proxy
-            assert proxy is not None
-            root = proxy.root
-            assert root is not None
-            if root.rendered:
-                self._notify_modified(
-                    root.declaration,
-                    {
-                        "id": self.id,
-                        "type": "added",
-                        "name": "children",
-                        "value": child.render(),
-                        "index": self._child_index(child),
-                    },
-                )
+            self._notify_modified(
+                {
+                    "id": self.id,
+                    "type": "added",
+                    "name": "children",
+                    "value": child.render(),
+                    "index": self._child_index(child),
+                },
+            )
 
     def child_moved(self, child: Declarative):
         super().child_moved(child)
         if self.proxy_is_active and isinstance(child, Tag):
             proxy = self.proxy
             assert proxy is not None
-            root = proxy.root
-            assert root is not None
-            if root.rendered and proxy.child_moved(child.proxy):
+            if proxy.child_moved(child.proxy):
                 self._notify_modified(
-                    root.declaration,
                     {
                         "id": self.id,
                         "type": "moved",
@@ -252,28 +236,26 @@ def child_removed(self, child: Declarative):
         """
         super().child_removed(child)
         if self.proxy_is_active and isinstance(child, Tag):
-            proxy = self.proxy
-            assert proxy is not None
-            root = proxy.root
-            assert root is not None
-            if root.rendered:
-                self._notify_modified(
-                    root.declaration,
-                    {
-                        "id": self.id,
-                        "type": "removed",
-                        "name": "children",
-                        "value": child.id,
-                    },
-                )
-
-    def _notify_modified(self, root: Optional[Tag], change: dict[str, Any]):
+            self._notify_modified(
+                {
+                    "id": self.id,
+                    "type": "removed",
+                    "name": "children",
+                    "value": child.id,
+                },
+            )
+
+    def _notify_modified(self, change: dict[str, Any]):
         """Trigger a modified event on the root node. Subclasses may override
         this to update change parameters if needed.
 
         """
-        if root is not None:
-            root.modified(change)
+        root = self.root_object()
+        if isinstance(root, Html):
+            proxy = root.proxy
+            assert proxy is not None
+            if proxy.rendered:
+                root.modified(change)
 
     def _child_index(self, child: Tag) -> int:
         """Find the index of the child ignoring any pattern nodes"""
@@ -282,6 +264,25 @@ def _child_index(self, child: Tag) -> int:
     # =========================================================================
     # Tag API
     # =========================================================================
+    def find_by_id(self, id: str) -> Optional[Tag]:
+        """Find a child node with the given id.
+
+        Parameters
+        ----------
+        id: str
+            The id to look for.
+
+        Returns
+        -------
+        results: Optional[Tag]
+            The first node with the given id or None.
+
+        """
+        for child in self.traverse():
+            if isinstance(child, Tag) and child.id == id:
+                return child
+        return None
+
     def xpath(self, query: str, **kwargs) -> list[Tag]:
         """Find nodes matching the given xpath query
 
@@ -300,7 +301,7 @@ def xpath(self, query: str, **kwargs) -> list[Tag]:
         """
         proxy = self.proxy
         assert proxy is not None
-        return [n.declaration for n in proxy.xpath(query, **kwargs)]
+        return [n for n in proxy.xpath(query, **kwargs)]
 
     def prepare(self, **kwargs: dict[str, Any]):
         """Prepare this node for rendering.

web/components/md.py

@@ -10,7 +10,7 @@
 @author: jrm
 """
 
-from typing import Any, Optional
+from typing import Any
 from atom.api import (
     Typed,
     ForwardTyped,
@@ -23,7 +23,6 @@
     observe,
 )
 from enaml.core.declarative import d_
-from .html import Tag
 from .raw import Raw, ProxyRawNode
 
 
@@ -89,8 +88,8 @@ def _update_proxy(self, change: ChangeDict):
         """The superclass implementation is sufficient."""
         super()._update_proxy(change)
 
-    def _notify_modified(self, root: Optional[Tag], change: dict[str, Any]):
+    def _notify_modified(self, change: dict[str, Any]):
         """Update the notification"""
         if change["type"] == "update" and change["name"] == "source":
             change["value"] = self.render()
-        super()._notify_modified(root, change)
+        super()._notify_modified(change)

web/impl/lxml_toolkit_object.py

@@ -14,7 +14,7 @@
 
 from functools import lru_cache
 from typing import Any, Type, Union, Optional, Generator
-from atom.api import Atom, Bool, Member, Typed, Event, Dict
+from atom.api import Atom, Bool, Member, Typed, Event
 from lxml.etree import _Element, Element, SubElement, tostring
 from web.components.html import ProxyTag
 
@@ -66,8 +66,6 @@ def create_widget(self):
         """
         d = self.declaration
         parent = d.parent.proxy
-        self.root = parent.root
-        self.root.cache[d.id] = self
         self.widget = SubElement(parent.widget, d.tag)
 
     def init_widget(self):
@@ -117,7 +115,7 @@ def init_widget(self):
 
         for m in get_fields(d.__class__):
             name = m.name
-            value = m.do_getattr(d)
+            value = getattr(d, name)
             if value is True:
                 set_attr(name, name)
             elif value:
@@ -145,15 +143,6 @@ def destroy(self):
         and set its parent to None.
 
         """
-        if self.root is not None:
-            if (root := self.root) and (cache := root.cache):
-                try:
-                    del cache[self.declaration.id]
-                except KeyError:
-                    pass
-
-            del self.root
-
         if self.widget is not None:
             parent = self.widget.getparent()
             if parent is not None:
@@ -224,19 +213,21 @@ def render(
         """Render the widget tree into a string"""
         return tostring(self.widget, method=method, encoding=encoding, **kwargs)
 
-    def xpath(self, query: str, **kwargs) -> Generator[WebComponent, None, None]:
+    def xpath(self, query: str, **kwargs) -> Generator[ProxyTag, None, None]:
         """Get the node(s) matching the query"""
         w = self.widget
         if w is None:
             return None
         nodes = w.xpath(query, **kwargs)
         if not nodes:
             return None
-        if root := self.root:
-            lookup = root.cache.get
-            for node in nodes:
-                if obj := lookup(node.get("id")):
-                    yield obj
+        find_by_id = self.declaration.find_by_id
+        for node in nodes:
+            node_id = node.get("id")
+            if not node_id:
+                continue
+            if obj := find_by_id(node_id):
+                yield obj
 
     def parent_widget(self) -> Optional[_Element]:
         """Get the parent toolkit widget for this object.
@@ -332,24 +323,14 @@ def set_draggable(self, draggable: bool):
 class RootWebComponent(WebComponent):
     """A root component"""
 
-    #: Components are cached for lookup by id so xpath queries from lxml
-    #: can retrieve their declaration component
-    cache = Dict()
-
     #: Flag to indicate whether this node was rendered. This is used by the
     #: declaration to avoid creating unnecessary modified events.
     rendered = Bool()
 
     def create_widget(self):
         d = self.declaration
-        self.root = self.cache[d.id] = self
         self.widget = Element(d.tag)
 
     def render(self, *args, **kwargs):
         self.rendered = True
         return super().render(*args, **kwargs)
-
-    def destroy(self):
-        del self.root
-        del self.cache
-        super().destroy()