From 1eed4d7bda289af7c731f61a85a714bb98296bbf Mon Sep 17 00:00:00 2001 From: Ed Krohne Date: Wed, 17 Jun 2026 20:01:01 -0700 Subject: [PATCH 1/3] fix: retain UI elements in layout containers so they aren't garbage collected --- marimo/_output/hypertext.py | 45 +++++++-- marimo/_plugins/stateless/accordion.py | 58 +++++++---- marimo/_plugins/stateless/callout.py | 27 +++-- marimo/_plugins/stateless/carousel.py | 36 ++++--- marimo/_plugins/stateless/sidebar.py | 28 ++++-- marimo/_plugins/stateless/style.py | 15 ++- marimo/_plugins/ui/_impl/tabs.py | 21 ++-- tests/_output/test_hypertext.py | 44 ++++++++ tests/_plugins/stateless/test_accordion.py | 60 +++++++++++ tests/_plugins/stateless/test_callout.py | 50 ++++++++++ tests/_plugins/stateless/test_carousel.py | 51 ++++++++++ tests/_plugins/stateless/test_sidebar.py | 111 ++++++++++++++++++++- tests/_plugins/stateless/test_style.py | 30 ++++++ tests/_plugins/ui/_impl/test_tabs.py | 30 ++++++ 14 files changed, 531 insertions(+), 75 deletions(-) create mode 100644 tests/_plugins/stateless/test_accordion.py create mode 100644 tests/_plugins/stateless/test_callout.py create mode 100644 tests/_plugins/stateless/test_carousel.py diff --git a/marimo/_output/hypertext.py b/marimo/_output/hypertext.py index c2df29b9453..8700d62c457 100644 --- a/marimo/_output/hypertext.py +++ b/marimo/_output/hypertext.py @@ -14,7 +14,7 @@ from marimo._utils.methods import getcallable if TYPE_CHECKING: - from collections.abc import Iterator + from collections.abc import Iterator, Sequence from marimo._plugins.ui._core.ui_element import UIElement from marimo._plugins.ui._impl.batch import batch as batch_plugin @@ -289,7 +289,39 @@ def _repr_html_(self) -> str: return self.text -class _BlockWrapped(Html): +class ContainerHtml(Html): + """Base class for Html wrappers that contain other renderables. + + A `ContainerHtml` holds *strong* references to its children and re-renders + from their live `.text` on every access. This matters for two reasons: + + - UI elements are only weakly referenced by the UI element registry, so a + container that merely froze `child.text` at construction time would let + its children be garbage collected -- breaking interactivity for elements + returned from helper functions. Holding strong references keeps the + children alive, mirroring `mo.hstack`/`mo.vstack`. + - Mutable children (e.g. `mo.status.spinner`) re-render live rather than + being frozen at construction time. + + Subclasses store any extra rendering state on `self` *before* calling + `super().__init__()` and implement `_build_text` in terms of + `self._children`. + """ + + def __init__(self, children: Sequence[Html]) -> None: + self._children: list[Html] = list(children) + super().__init__(self._build_text()) + + def _build_text(self) -> str: + raise NotImplementedError + + @property + def text(self) -> str: # type: ignore[override] + """Re-render children live on every access.""" + return self._build_text() + + +class _BlockWrapped(ContainerHtml): """Wraps another Html in a plain block `
`. Used by :meth:`Html.center`, :meth:`Html.left`, and :meth:`Html.right` @@ -303,17 +335,12 @@ class _BlockWrapped(Html): """ def __init__(self, inner: Html) -> None: - self._inner = inner - super().__init__(self._build_text()) + super().__init__([inner]) def _build_text(self) -> str: from marimo._output.builder import h - return h.div(self._inner.text) - - @property - def text(self) -> str: - return self._build_text() + return h.div(self._children[0].text) MARIMO_NO_JS_KEY = "MARIMO_NO_JS" diff --git a/marimo/_plugins/stateless/accordion.py b/marimo/_plugins/stateless/accordion.py index 6509ae94c82..34c70ef1325 100644 --- a/marimo/_plugins/stateless/accordion.py +++ b/marimo/_plugins/stateless/accordion.py @@ -2,13 +2,50 @@ from __future__ import annotations from marimo._output.formatting import as_html -from marimo._output.hypertext import Html +from marimo._output.hypertext import ContainerHtml, Html from marimo._output.md import md from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin from marimo._plugins.stateless.lazy import lazy as lazy_ui +class _AccordionHtml(ContainerHtml): + """Html produced by `mo.accordion()`; keeps live references to its children.""" + + def __init__( + self, + items: dict[str, object], + multiple: bool = False, + lazy: bool = False, + ) -> None: + self._multiple = multiple + self._lazy = lazy + + if self._lazy: + self._tabs = [lazy_ui(tab) for tab in items.values()] + else: + self._tabs = [ + md(tab) if isinstance(tab, str) else as_html(tab) + for tab in items.values() + ] + + self._labels = [md(label) for label in items] + + super().__init__([*self._tabs, *self._labels]) + + def _build_text(self) -> str: + return build_stateless_plugin( + component_name="marimo-accordion", + args={ + "labels": [label.text for label in self._labels], + "multiple": self._multiple, + }, + slotted_html="".join( + [f"
{tab.text}
" for tab in self._tabs] + ), + ) + + @mddoc def accordion( items: dict[str, object], multiple: bool = False, lazy: bool = False @@ -43,21 +80,4 @@ def accordion( returns the item to render. """ - def render_content(tab: object) -> str: - if lazy: - return lazy_ui(tab).text - if isinstance(tab, str): - return md(tab).text - return as_html(tab).text - - item_labels = [md(label).text for label in items] - item_content = "".join( - ["
" + render_content(item) + "
" for item in items.values()] - ) - return Html( - build_stateless_plugin( - component_name="marimo-accordion", - args={"labels": item_labels, "multiple": multiple}, - slotted_html=item_content, - ) - ) + return _AccordionHtml(items=items, multiple=multiple, lazy=lazy) diff --git a/marimo/_plugins/stateless/callout.py b/marimo/_plugins/stateless/callout.py index d121e7a55cd..aa72da2bdef 100644 --- a/marimo/_plugins/stateless/callout.py +++ b/marimo/_plugins/stateless/callout.py @@ -4,15 +4,31 @@ from typing import Literal from marimo._output.formatting import as_html -from marimo._output.hypertext import Html +from marimo._output.hypertext import ContainerHtml, Html from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin +CalloutKind = Literal["neutral", "warn", "success", "info", "danger"] + + +class _CalloutHtml(ContainerHtml): + """Html produced by `mo.callout()`; keeps a live reference to its child.""" + + def __init__(self, child: Html, kind: CalloutKind) -> None: + self._kind = kind + super().__init__([child]) + + def _build_text(self) -> str: + return build_stateless_plugin( + component_name="marimo-callout-output", + args={"html": self._children[0].text, "kind": self._kind}, + ) + @mddoc def callout( value: object, - kind: Literal["neutral", "warn", "success", "info", "danger"] = "neutral", + kind: CalloutKind = "neutral", ) -> Html: """Build a callout output. @@ -23,9 +39,4 @@ def callout( Returns: Html (marimo.Html): An HTML object. """ - return Html( - build_stateless_plugin( - component_name="marimo-callout-output", - args={"html": as_html(value).text, "kind": kind}, - ) - ) + return _CalloutHtml(as_html(value), kind) diff --git a/marimo/_plugins/stateless/carousel.py b/marimo/_plugins/stateless/carousel.py index e250fcb40f4..d5c4a6fd11e 100644 --- a/marimo/_plugins/stateless/carousel.py +++ b/marimo/_plugins/stateless/carousel.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING from marimo._output.formatting import as_html -from marimo._output.hypertext import Html +from marimo._output.hypertext import ContainerHtml, Html from marimo._output.md import md from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin @@ -13,6 +13,25 @@ from collections.abc import Sequence +class _CarouselHtml(ContainerHtml): + """Html produced by `mo.carousel()`; keeps live references to its items.""" + + def __init__(self, items: Sequence[object]) -> None: + super().__init__( + [ + md(item) if isinstance(item, str) else as_html(item) + for item in items + ] + ) + + def _build_text(self) -> str: + return build_stateless_plugin( + component_name="marimo-carousel", + args={}, + slotted_html="".join(c.text for c in self._children), + ) + + @mddoc def carousel( items: Sequence[object], @@ -30,17 +49,4 @@ def carousel( mo.carousel([mo.md("..."), mo.ui.text_area()]) ``` """ - item_content = "".join( - [ - (md(item).text if isinstance(item, str) else as_html(item).text) - for item in items - ] - ) - - return Html( - build_stateless_plugin( - component_name="marimo-carousel", - args={}, - slotted_html=item_content, - ) - ) + return _CarouselHtml(items) diff --git a/marimo/_plugins/stateless/sidebar.py b/marimo/_plugins/stateless/sidebar.py index 93e6c2876d2..0aefb9145f0 100644 --- a/marimo/_plugins/stateless/sidebar.py +++ b/marimo/_plugins/stateless/sidebar.py @@ -5,14 +5,14 @@ from marimo._output import md from marimo._output.formatting import as_html -from marimo._output.hypertext import Html +from marimo._output.hypertext import ContainerHtml, Html from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin from marimo._plugins.stateless.flex import vstack @mddoc -class sidebar(Html): +class sidebar(ContainerHtml): """Displays content in a sidebar. This is a special layout component that will display the content in a sidebar @@ -78,17 +78,23 @@ def __init__( item = vstack([item, footer], justify="space-between") # Build props - props: dict[str, Any] = {} + self._props: dict[str, Any] = {} if width is not None: # Width must be a string for JSON serialization - props["width"] = str(width) - - super().__init__( - build_stateless_plugin( - "marimo-sidebar", - props, - as_html(item).text, - ) + self._props["width"] = str(width) + + # Retain a strong reference to the (processed) item so a wrapped UI + # element is not garbage collected. The UI element registry holds + # elements weakly and the slotted HTML only freezes their text, so + # without this the item -- and, for the list form, the vstack wrapping + # it -- would be collected and lose interactivity. + super().__init__([as_html(item)]) + + def _build_text(self) -> str: + return build_stateless_plugin( + "marimo-sidebar", + self._props, + self._children[0].text, ) # Not supported diff --git a/marimo/_plugins/stateless/style.py b/marimo/_plugins/stateless/style.py index 0ed07bd6c9c..6231ec771e3 100644 --- a/marimo/_plugins/stateless/style.py +++ b/marimo/_plugins/stateless/style.py @@ -5,10 +5,21 @@ from marimo._output.builder import h from marimo._output.formatting import as_dom_node -from marimo._output.hypertext import Html +from marimo._output.hypertext import ContainerHtml, Html from marimo._output.rich_help import mddoc +class _StyledHtml(ContainerHtml): + """Html produced by `mo.style()`; keeps a live reference to its child.""" + + def __init__(self, child: Html, style_str: str) -> None: + self._style_str = style_str + super().__init__([child]) + + def _build_text(self) -> str: + return h.div(children=self._children[0].text, style=self._style_str) + + @mddoc def style( item: object, style: dict[str, Any] | None = None, **kwargs: Any @@ -45,4 +56,4 @@ def style( style_str = ";".join( [f"{key}:{value}" for key, value in combined_style.items()] ) - return Html(h.div(children=as_dom_node(item).text, style=style_str)) + return _StyledHtml(as_dom_node(item), style_str) diff --git a/marimo/_plugins/ui/_impl/tabs.py b/marimo/_plugins/ui/_impl/tabs.py index cb749b3447a..1f2c993b603 100644 --- a/marimo/_plugins/ui/_impl/tabs.py +++ b/marimo/_plugins/ui/_impl/tabs.py @@ -4,6 +4,7 @@ from typing import TYPE_CHECKING, Final, Literal from marimo._output.formatting import as_html +from marimo._output.hypertext import Html from marimo._output.md import md from marimo._output.rich_help import mddoc from marimo._plugins.stateless.lazy import lazy as lazy_ui @@ -78,17 +79,25 @@ def __init__( label: str = "", on_change: Callable[[str], None] | None = None, ) -> None: - def render_content(tab: object) -> str: + def render_content(tab: object) -> Html: if lazy: - return lazy_ui(tab).text + return lazy_ui(tab) if isinstance(tab, str): - return md(tab).text - return as_html(tab).text + return md(tab) + return as_html(tab) + + # Retain strong references to the rendered tab contents. The slotted + # HTML only freezes their text, and the UI element registry holds + # elements weakly, so without this a UI element placed in a tab would + # be garbage collected and lose its interactivity. + self._children: list[Html] = [ + render_content(tab) for tab in tabs.values() + ] tab_items = "".join( [ - "
" + render_content(tab) + "
" - for tab in tabs.values() + "
" + child.text + "
" + for child in self._children ] ) diff --git a/tests/_output/test_hypertext.py b/tests/_output/test_hypertext.py index ed5a99eaf9e..815eab64d4f 100644 --- a/tests/_output/test_hypertext.py +++ b/tests/_output/test_hypertext.py @@ -1,9 +1,13 @@ from __future__ import annotations +import gc +import weakref + import pytest from marimo._dependencies.dependencies import DependencyManager from marimo._output.hypertext import ( + ContainerHtml, Html, patch_html_for_non_interactive_output, ) @@ -12,6 +16,13 @@ from marimo._plugins.ui._impl.table import table +class _SingleChildContainer(ContainerHtml): + """Minimal ContainerHtml used to exercise the base-class contract.""" + + def _build_text(self) -> str: + return f"
{self._children[0].text}
" + + def test_html_initialization(): html = Html("

Hello, World!

") assert html.text == "

Hello, World!

" @@ -151,6 +162,39 @@ def test_html_center_live_updates_propagate(): assert "initial" not in centered.text +def test_container_html_retains_children() -> None: + # ContainerHtml must hold strong references to its children. UI elements + # are held only weakly by the UI registry, so a container that dropped its + # children (e.g. by freezing child.text) would let them be garbage + # collected -- the root cause of the .style()/.callout() interactivity bug. + child = Html("child") + child_ref = weakref.ref(child) + + container = _SingleChildContainer([child]) + assert "child" in container.text + + # Drop our only strong handle; the container must keep the child alive. + del child + gc.collect() + + assert child_ref() is not None, ( + "ContainerHtml did not retain a strong reference to its child" + ) + + +def test_container_html_renders_children_live() -> None: + # ContainerHtml re-renders from each child's live text on every access, so + # mutable children (e.g. mo.status.spinner) keep updating rather than being + # frozen at construction time. + child = Html("
initial
") + container = _SingleChildContainer([child]) + assert "initial" in container.text + + child._text = "
updated
" + assert "updated" in container.text + assert "initial" not in container.text + + def test_html_callout(): html = Html("

Important

") callout = html.callout(kind="warn") diff --git a/tests/_plugins/stateless/test_accordion.py b/tests/_plugins/stateless/test_accordion.py new file mode 100644 index 00000000000..717e189aec9 --- /dev/null +++ b/tests/_plugins/stateless/test_accordion.py @@ -0,0 +1,60 @@ +# Copyright 2026 Marimo. All rights reserved. +from __future__ import annotations + +import gc +import weakref + +import marimo as mo +from marimo._output.hypertext import Html +from marimo._plugins.stateless.accordion import accordion + + +def test_accordion_retains_strong_references() -> None: + # Regression test: accordion() must keep a strong reference to its children + # so that wrapped UI elements are not garbage collected (the UI registry + # holds children only weakly). + tabs = {f"label {n}": Html(f"tab {n}") for n in range(3)} + tab_refs = {label: weakref.ref(tab) for label, tab in tabs.items()} + + result = accordion(tabs) + assert isinstance(result, Html) + assert all(ref() is not None for ref in tab_refs.values()) + + del tabs + gc.collect() + + assert all(ref() is not None for ref in tab_refs.values()), ( + "accordion() did not retain a reference to all its children; " + "at least one child was garbage collected" + ) + + +def test_lazy_accordion_retains_strong_references() -> None: + tabs = {f"label {n}": Html(f"tab {n}") for n in range(3)} + factories = {label: (lambda tab=tab: tab) for label, tab in tabs.items()} + tab_refs = {label: weakref.ref(tab) for label, tab in tabs.items()} + + result = accordion(factories, lazy=True) + assert isinstance(result, Html) + assert all(ref() is not None for ref in tab_refs.values()) + + del tabs + del factories + gc.collect() + + assert all(ref() is not None for ref in tab_refs.values()), ( + "accordion() did not retain a reference to all its (lazily created) children; " + "at least one child was garbage collected" + ) + + +def test_accordion_child_updates_live() -> None: + # A mutable child (e.g. mo.status.spinner) re-renders on each access rather + # than being frozen at construction time. + with mo.status.spinner(title="Loading") as spinner: + result = accordion({"label": spinner}) + assert "Loading" in result.text + + spinner.update(title="Done") + assert "Done" in result.text + assert "Loading" not in result.text diff --git a/tests/_plugins/stateless/test_callout.py b/tests/_plugins/stateless/test_callout.py new file mode 100644 index 00000000000..b17dec91bd3 --- /dev/null +++ b/tests/_plugins/stateless/test_callout.py @@ -0,0 +1,50 @@ +# Copyright 2026 Marimo. All rights reserved. +from __future__ import annotations + +import gc +import weakref + +import marimo as mo +from marimo._output.hypertext import Html +from marimo._plugins.stateless.callout import callout + + +def test_callout_renders_child() -> None: + # callout embeds its child (JSON-escaped into the data-html attribute) in a + # marimo-callout-output component with the given kind. + result = callout(Html("

Important

"), kind="warn") + assert isinstance(result, Html) + assert "marimo-callout-output" in result.text + assert "warn" in result.text + assert "Important" in result.text + + +def test_callout_retains_strong_reference_to_child() -> None: + # Regression test: callout() must keep a strong reference to its child so + # that wrapped UI elements are not garbage collected (the UI registry holds + # children only weakly). + child = Html("child") + child_ref = weakref.ref(child) + + result = callout(child, kind="info") + assert child_ref() is not None + + del child + gc.collect() + + assert child_ref() is not None, ( + "callout() did not retain a reference to its child; " + "the wrapped element can be garbage collected" + ) + + +def test_callout_child_updates_live() -> None: + # A mutable child (e.g. mo.status.spinner) re-renders on each access rather + # than being frozen at construction time. + with mo.status.spinner(title="Loading") as spinner: + result = callout(spinner, kind="neutral") + assert "Loading" in result.text + + spinner.update(title="Done") + assert "Done" in result.text + assert "Loading" not in result.text diff --git a/tests/_plugins/stateless/test_carousel.py b/tests/_plugins/stateless/test_carousel.py new file mode 100644 index 00000000000..9df533de0d4 --- /dev/null +++ b/tests/_plugins/stateless/test_carousel.py @@ -0,0 +1,51 @@ +# Copyright 2026 Marimo. All rights reserved. +from __future__ import annotations + +import gc +import weakref + +import marimo as mo +from marimo._output.hypertext import Html +from marimo._plugins.stateless.carousel import carousel + + +def test_carousel_renders_items() -> None: + # carousel embeds each item's HTML, unescaped, inside a marimo-carousel + # component. + result = carousel([Html("

First

"), Html("

Second

")]) + assert isinstance(result, Html) + assert "marimo-carousel" in result.text + assert "

First

" in result.text + assert "

Second

" in result.text + + +def test_carousel_retains_strong_references() -> None: + # Regression test: carousel() must keep a strong reference to its children + # so that wrapped UI elements are not garbage collected (the UI registry + # holds children only weakly). + items = [Html(f"item {n}") for n in range(3)] + item_refs = [weakref.ref(item) for item in items] + + result = carousel(items) + assert isinstance(result, Html) + assert all(ref() is not None for ref in item_refs) + + del items + gc.collect() + + assert all(ref() is not None for ref in item_refs), ( + "carousel() did not retain a reference to all its children; " + "at least one child was garbage collected" + ) + + +def test_carousel_child_updates_live() -> None: + # A mutable child (e.g. mo.status.spinner) re-renders on each access rather + # than being frozen at construction time. + with mo.status.spinner(title="Loading") as spinner: + result = carousel([spinner]) + assert "Loading" in result.text + + spinner.update(title="Done") + assert "Done" in result.text + assert "Loading" not in result.text diff --git a/tests/_plugins/stateless/test_sidebar.py b/tests/_plugins/stateless/test_sidebar.py index 055e57fd262..3cecc58885f 100644 --- a/tests/_plugins/stateless/test_sidebar.py +++ b/tests/_plugins/stateless/test_sidebar.py @@ -1,7 +1,11 @@ from __future__ import annotations +import gc +import weakref + import pytest +import marimo as mo from marimo._output.hypertext import Html from marimo._plugins.stateless.sidebar import sidebar @@ -10,35 +14,35 @@ def test_sidebar_string(): """Test sidebar with string input.""" result = sidebar("test content") assert isinstance(result, Html) - assert "width" not in result._text + assert "width" not in result.text def test_sidebar_list(): """Test sidebar with list input.""" result = sidebar(["item1", "item2"]) assert isinstance(result, Html) - assert "width" not in result._text + assert "width" not in result.text def test_sidebar_with_footer(): """Test sidebar with footer.""" result = sidebar("content", footer="footer text") assert isinstance(result, Html) - assert "width" not in result._text + assert "width" not in result.text def test_sidebar_list_both_width_and_footer(): """Test sidebar with both width and footer.""" result = sidebar(["item1", "item2"], footer=["footer1", "footer2"]) assert isinstance(result, Html) - assert "width" not in result._text + assert "width" not in result.text def test_sidebar_with_width(): """Test sidebar with custom width.""" result = sidebar("content", width="300px") assert isinstance(result, Html) - assert "data-width='"300px"'" in result._text + assert "data-width='"300px"'" in result.text def test_sidebar_unsupported_methods(): @@ -56,3 +60,100 @@ def test_sidebar_unsupported_methods(): s.callout() with pytest.raises(TypeError): s.style() + + +def test_sidebar_retains_strong_reference_to_item() -> None: + # Regression test: sidebar() must keep a strong reference to its item so + # that a wrapped UI element is not garbage collected (the UI registry holds + # children only weakly). + child = Html("child") + child_ref = weakref.ref(child) + + result = sidebar(child) + assert isinstance(result, Html) + # the item is rendered, unescaped, inside the sidebar component + assert "marimo-sidebar" in result.text + assert "child" in result.text + assert child_ref() is not None + + del child + gc.collect() + + assert child_ref() is not None, ( + "sidebar() did not retain a reference to its item; " + "the wrapped element can be garbage collected" + ) + + +def test_sidebar_retains_list_items() -> None: + # The list form wraps items in a vstack; sidebar must retain that wrapper + # (and thus the items), not freeze its text and drop it. + a = Html("a") + b = Html("b") + refs = [weakref.ref(a), weakref.ref(b)] + + result = sidebar([a, b]) + assert isinstance(result, Html) + assert "a" in result.text + assert "b" in result.text + + del a, b + gc.collect() + + assert all(ref() is not None for ref in refs), ( + "sidebar() did not retain its list items; " + "at least one was garbage collected" + ) + + +def test_sidebar_retains_item_and_footer() -> None: + # With a footer, item and footer are combined in a vstack; sidebar must + # retain it so neither the item nor the footer is collected. + item = Html("item") + footer = Html("footer") + item_ref = weakref.ref(item) + footer_ref = weakref.ref(footer) + + result = sidebar(item, footer=footer) + assert isinstance(result, Html) + assert "item" in result.text + assert "footer" in result.text + + del item, footer + gc.collect() + + assert item_ref() is not None, "sidebar() dropped its item (with footer)" + assert footer_ref() is not None, "sidebar() dropped its footer" + + +def test_sidebar_retains_list_footer_items() -> None: + # A list footer is itself wrapped in a vstack before being combined with + # the item, so the footer's items must survive too. + f1 = Html("f1") + f2 = Html("f2") + refs = [weakref.ref(f1), weakref.ref(f2)] + + result = sidebar("body", footer=[f1, f2]) + assert isinstance(result, Html) + assert "f1" in result.text + assert "f2" in result.text + + del f1, f2 + gc.collect() + + assert all(ref() is not None for ref in refs), ( + "sidebar() did not retain its list footer items; " + "at least one was garbage collected" + ) + + +def test_sidebar_item_updates_live() -> None: + # A mutable item (e.g. mo.status.spinner) re-renders on each access rather + # than being frozen at construction time. + with mo.status.spinner(title="Loading") as spinner: + result = sidebar(spinner) + assert "Loading" in result.text + + spinner.update(title="Done") + assert "Done" in result.text + assert "Loading" not in result.text diff --git a/tests/_plugins/stateless/test_style.py b/tests/_plugins/stateless/test_style.py index a0b77d4394a..42080481b30 100644 --- a/tests/_plugins/stateless/test_style.py +++ b/tests/_plugins/stateless/test_style.py @@ -1,8 +1,13 @@ # Copyright 2026 Marimo. All rights reserved. from __future__ import annotations +import gc + +from marimo import ui from marimo._output.hypertext import Html from marimo._plugins.stateless.style import style +from marimo._runtime.context import get_context +from marimo._runtime.runtime import Kernel def test_style_with_dict(): @@ -57,3 +62,28 @@ def test_style_overwrite_dict_with_kwargs(): result = style("Test content", style={"color": "red"}, color="blue") assert isinstance(result, Html) assert result.text == "
Test content
" + + +def test_style_keeps_ui_element_registered(executing_kernel: Kernel) -> None: + # Regression test: a UI element wrapped with .style() (e.g. returned + # from a helper) must stay alive and registered, otherwise its weak + # registry entry is reaped and the element loses interactivity. + del executing_kernel + + registry = get_context().ui_element_registry + slider = ui.slider(1, 10) + object_id = slider._id + assert registry.get_object(object_id) is slider + + styled = slider.style(border="1px solid red") + assert styled.text # touch output + + # Drop the only direct handle; the styled wrapper must keep it alive. + del slider + gc.collect() + + assert object_id in registry._objects, ( + ".style() let the UI element be garbage collected; " + "it was removed from the registry and is no longer interactive" + ) + assert registry._objects[object_id]() is not None diff --git a/tests/_plugins/ui/_impl/test_tabs.py b/tests/_plugins/ui/_impl/test_tabs.py index c3107fcf3da..042cd38a970 100644 --- a/tests/_plugins/ui/_impl/test_tabs.py +++ b/tests/_plugins/ui/_impl/test_tabs.py @@ -1,7 +1,11 @@ # Copyright 2026 Marimo. All rights reserved. from __future__ import annotations +import gc + from marimo._plugins import ui +from marimo._runtime.context import get_context +from marimo._runtime.runtime import Kernel def test_tabs_basic() -> None: @@ -58,3 +62,29 @@ def test_tabs_vertical_orientation() -> None: assert tab._component_args["orientation"] == "vertical" # pyright: ignore[reportPrivateUsage] # Orientation should not affect selection behavior assert tab.value == "Tab 1" + + +def test_tabs_keeps_ui_element_registered(executing_kernel: Kernel) -> None: + # Regression test: a UIElement placed in a tab (e.g. returned from a + # helper) must stay alive and registered. mo.ui.tabs freezes each tab's + # rendered HTML and keeps only the tab keys, so it must retain the tab + # contents; otherwise the weak registry entry is reaped on garbage + # collection and the element loses interactivity. + del executing_kernel + + registry = get_context().ui_element_registry + slider = ui.slider(1, 10) + object_id = slider._id + assert registry.get_object(object_id) is slider + + result = ui.tabs({"Tab": slider}) + assert object_id in result.text # the slider is rendered into the tab + + del slider + gc.collect() + + assert object_id in registry._objects, ( + "mo.ui.tabs let a tab's UI element be garbage collected; " + "it was removed from the registry and is no longer interactive" + ) + assert registry._objects[object_id]() is not None From 27ddb63c3297e57f0ed34ff80c5ffa638faf6e98 Mon Sep 17 00:00:00 2001 From: Ed Krohne Date: Wed, 17 Jun 2026 20:11:22 -0700 Subject: [PATCH 2/3] annotation fix --- marimo/_plugins/stateless/accordion.py | 1 + 1 file changed, 1 insertion(+) diff --git a/marimo/_plugins/stateless/accordion.py b/marimo/_plugins/stateless/accordion.py index 34c70ef1325..dce34fc86a1 100644 --- a/marimo/_plugins/stateless/accordion.py +++ b/marimo/_plugins/stateless/accordion.py @@ -21,6 +21,7 @@ def __init__( self._multiple = multiple self._lazy = lazy + self._tabs: list[Html] if self._lazy: self._tabs = [lazy_ui(tab) for tab in items.values()] else: From c34002e3afaa7b49312142f8ec5828763eecd738 Mon Sep 17 00:00:00 2001 From: Ed Krohne Date: Wed, 17 Jun 2026 20:51:12 -0700 Subject: [PATCH 3/3] Eliminated factory methods, streamlining the new container classes. --- marimo/_plugins/stateless/accordion.py | 70 ++++++++++++-------------- marimo/_plugins/stateless/callout.py | 36 ++++++------- marimo/_plugins/stateless/carousel.py | 39 ++++++-------- marimo/_plugins/stateless/style.py | 53 +++++++++---------- 4 files changed, 88 insertions(+), 110 deletions(-) diff --git a/marimo/_plugins/stateless/accordion.py b/marimo/_plugins/stateless/accordion.py index dce34fc86a1..d31a8883d4b 100644 --- a/marimo/_plugins/stateless/accordion.py +++ b/marimo/_plugins/stateless/accordion.py @@ -9,8 +9,37 @@ from marimo._plugins.stateless.lazy import lazy as lazy_ui -class _AccordionHtml(ContainerHtml): - """Html produced by `mo.accordion()`; keeps live references to its children.""" +@mddoc +class accordion(ContainerHtml): + """Accordion of one or more items. + + Args: + items: a dictionary of item names to item content; strings are + interpreted as markdown + multiple: whether to allow multiple items to be open simultaneously + lazy: a boolean, whether to lazily load the accordion content. + This is a convenience that wraps each accordion in a `mo.lazy` + component. + + Returns: + An `Html` object. + + Example: + ```python3 + mo.accordion( + {"Tip": "Use accordions to let users reveal and hide content."} + ) + ``` + + Accordion content can be lazily loaded: + + ```python3 + mo.accordion({"View total": expensive_item}, lazy=True) + ``` + + where `expensive_item` is the item to render, or a callable that + returns the item to render. + """ def __init__( self, @@ -45,40 +74,3 @@ def _build_text(self) -> str: [f"
{tab.text}
" for tab in self._tabs] ), ) - - -@mddoc -def accordion( - items: dict[str, object], multiple: bool = False, lazy: bool = False -) -> Html: - """Accordion of one or more items. - - Args: - items: a dictionary of item names to item content; strings are - interpreted as markdown - multiple: whether to allow multiple items to be open simultaneously - lazy: a boolean, whether to lazily load the accordion content. - This is a convenience that wraps each accordion in a `mo.lazy` - component. - - Returns: - An `Html` object. - - Example: - ```python3 - mo.accordion( - {"Tip": "Use accordions to let users reveal and hide content."} - ) - ``` - - Accordion content can be lazily loaded: - - ```python3 - mo.accordion({"View total": expensive_item}, lazy=True) - ``` - - where `expensive_item` is the item to render, or a callable that - returns the item to render. - """ - - return _AccordionHtml(items=items, multiple=multiple, lazy=lazy) diff --git a/marimo/_plugins/stateless/callout.py b/marimo/_plugins/stateless/callout.py index aa72da2bdef..db7641bbeb8 100644 --- a/marimo/_plugins/stateless/callout.py +++ b/marimo/_plugins/stateless/callout.py @@ -4,32 +4,15 @@ from typing import Literal from marimo._output.formatting import as_html -from marimo._output.hypertext import ContainerHtml, Html +from marimo._output.hypertext import ContainerHtml from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin CalloutKind = Literal["neutral", "warn", "success", "info", "danger"] -class _CalloutHtml(ContainerHtml): - """Html produced by `mo.callout()`; keeps a live reference to its child.""" - - def __init__(self, child: Html, kind: CalloutKind) -> None: - self._kind = kind - super().__init__([child]) - - def _build_text(self) -> str: - return build_stateless_plugin( - component_name="marimo-callout-output", - args={"html": self._children[0].text, "kind": self._kind}, - ) - - @mddoc -def callout( - value: object, - kind: CalloutKind = "neutral", -) -> Html: +class callout(ContainerHtml): """Build a callout output. Args: @@ -39,4 +22,17 @@ def callout( Returns: Html (marimo.Html): An HTML object. """ - return _CalloutHtml(as_html(value), kind) + + def __init__( + self, + value: object, + kind: CalloutKind = "neutral", + ) -> None: + self._kind = kind + super().__init__([as_html(value)]) + + def _build_text(self) -> str: + return build_stateless_plugin( + component_name="marimo-callout-output", + args={"html": self._children[0].text, "kind": self._kind}, + ) diff --git a/marimo/_plugins/stateless/carousel.py b/marimo/_plugins/stateless/carousel.py index d5c4a6fd11e..589e964d61e 100644 --- a/marimo/_plugins/stateless/carousel.py +++ b/marimo/_plugins/stateless/carousel.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING from marimo._output.formatting import as_html -from marimo._output.hypertext import ContainerHtml, Html +from marimo._output.hypertext import ContainerHtml from marimo._output.md import md from marimo._output.rich_help import mddoc from marimo._plugins.core.web_component import build_stateless_plugin @@ -13,8 +13,21 @@ from collections.abc import Sequence -class _CarouselHtml(ContainerHtml): - """Html produced by `mo.carousel()`; keeps live references to its items.""" +@mddoc +class carousel(ContainerHtml): + """Create a carousel of items. + + Args: + items: A list of items. + + Returns: + An `Html` object. + + Example: + ```python3 + mo.carousel([mo.md("..."), mo.ui.text_area()]) + ``` + """ def __init__(self, items: Sequence[object]) -> None: super().__init__( @@ -30,23 +43,3 @@ def _build_text(self) -> str: args={}, slotted_html="".join(c.text for c in self._children), ) - - -@mddoc -def carousel( - items: Sequence[object], -) -> Html: - """Create a carousel of items. - - Args: - items: A list of items. - - Returns: - An `Html` object. - - Example: - ```python3 - mo.carousel([mo.md("..."), mo.ui.text_area()]) - ``` - """ - return _CarouselHtml(items) diff --git a/marimo/_plugins/stateless/style.py b/marimo/_plugins/stateless/style.py index 6231ec771e3..e951005a0c8 100644 --- a/marimo/_plugins/stateless/style.py +++ b/marimo/_plugins/stateless/style.py @@ -5,25 +5,12 @@ from marimo._output.builder import h from marimo._output.formatting import as_dom_node -from marimo._output.hypertext import ContainerHtml, Html +from marimo._output.hypertext import ContainerHtml from marimo._output.rich_help import mddoc -class _StyledHtml(ContainerHtml): - """Html produced by `mo.style()`; keeps a live reference to its child.""" - - def __init__(self, child: Html, style_str: str) -> None: - self._style_str = style_str - super().__init__([child]) - - def _build_text(self) -> str: - return h.div(children=self._children[0].text, style=self._style_str) - - @mddoc -def style( - item: object, style: dict[str, Any] | None = None, **kwargs: Any -) -> Html: +class style(ContainerHtml): """Wrap an object in a styled container. Example: @@ -44,16 +31,26 @@ def style( Html: An HTML object representing the item wrapped in a div with the specified styles. """ - # Initialize combined_style with style dict if provided, - # otherwise empty dict - combined_style = style or {} - - # Add kwargs to combined_style, converting snake_case to kebab-case - for key, value in kwargs.items(): - kebab_key = key.replace("_", "-") - combined_style[kebab_key] = value - - style_str = ";".join( - [f"{key}:{value}" for key, value in combined_style.items()] - ) - return _StyledHtml(as_dom_node(item), style_str) + + def __init__( + self, + item: object, + style: dict[str, Any] | None = None, + **kwargs: Any, + ) -> None: + # Initialize combined_style with style dict if provided, + # otherwise empty dict + combined_style = style or {} + + # Add kwargs to combined_style, converting snake_case to kebab-case + for key, value in kwargs.items(): + kebab_key = key.replace("_", "-") + combined_style[kebab_key] = value + + self._style_str = ";".join( + [f"{key}:{value}" for key, value in combined_style.items()] + ) + super().__init__([as_dom_node(item)]) + + def _build_text(self) -> str: + return h.div(children=self._children[0].text, style=self._style_str)