fix: re-render mutable Html children in vstack/hstack on flush#8626
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
There was a problem hiding this comment.
Pull request overview
Fixes a rendering bug where mutable Html children (notably mo.status.spinner) embedded inside mo.vstack / mo.hstack would not reflect updates after output.flush(), because parent layouts previously captured (“froze”) child HTML at construction time.
Changes:
- Update the flex layout implementation to retain live
Htmlchild references and rebuild container HTML on each.textaccess. - Add a regression test ensuring that mutating a child
Htmlupdates the parent vstack/hstack output.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
marimo/_plugins/stateless/flex.py |
Reworks _FlexContainerHtml to store live children and dynamically rebuild .text so flushes reflect child mutations. |
tests/_plugins/stateless/test_flex.py |
Adds a regression test covering live updates for mutable Html children in vstack/hstack. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @property | ||
| def text(self) -> str: # type: ignore[override] | ||
| """Re-render children live on every access.""" | ||
| return self._build_text() |
There was a problem hiding this comment.
_FlexContainerHtml.text rebuilds and returns fresh HTML but never updates the inherited Html state (self._text, serialized_mime_bundle, virtual file scanning state). This can make the object’s internal representation/serialization inconsistent with what .text returns (e.g., any code path that serializes __dict__ will still see the initial HTML). Consider updating _text (and any derived fields that are expected to reflect the current HTML) when rebuilding, or overriding the relevant serialization/mime methods to keep a single source of truth.
| def __init__( | ||
| self, | ||
| style: str, | ||
| live_children: list[Html], | ||
| child_flexes: Optional[Sequence[Optional[float]]], | ||
| ) -> None: | ||
| self._style = style | ||
| self._live_children = live_children | ||
| self._child_flexes = child_flexes | ||
| super().__init__(self._build_text()) | ||
|
|
There was a problem hiding this comment.
_FlexContainerHtml.__init__ calls super().__init__(self._build_text()), and Html.__init__ immediately calls self._mime_(), which in turn reads self.text. Since text is overridden to call _build_text(), the container HTML is built twice during construction. Consider avoiding the extra render (e.g., ensure _mime_() during init doesn’t trigger a rebuild) to keep construction cost proportional to the number/size of children.
| # Only make the wrapper a flex container for nested stacks so their | ||
| # flex: 1 and justify work. Leaf content (e.g. mo.stat) fills the | ||
| # wrapper when it is a block. |
There was a problem hiding this comment.
we should preserve the original comments
There was a problem hiding this comment.
Good catch, restored the original comments. Thanks!
Light2Dark
left a comment
Light2Dark
left a comment
There was a problem hiding this comment.
i think it looks good, pending lint fixes
|
recheck |
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.20.5-dev69 |
Fixes #8618
Problem
mo.status.spinnerupdates were silently lost when the spinner was placed inside a layout container likemo.vstackormo.hstack.Root cause:
_flex()eagerly evaluatedas_html(item).textfor each child at construction time, baking a frozen HTML string into the layout. Whenspinner.update()later mutatedspinner._textand calledoutput.flush(), the parent vstack re-sent its own frozen HTML, the spinner's new state was never visible.Fix
_FlexContainerHtmlnow stores liveHtmlreferences instead of frozen strings, and overrides thetextproperty to call_build_text()on every access. This means eachflush()re-reads the current.textof every child, propagating updates through the layout hierarchy.Testing
Added
test_mutable_html_children_update_livetotests/_plugins/stateless/test_flex.py— verifies that mutating a child's_textafter embedding it invstack/hstackis reflected in the parent's.text.