Skip to content

Fix get_widget for a child element of a memoized component - #57

Draft
maartenbreddels wants to merge 1 commit into
masterfrom
fix/get-widget-child-elements
Draft

Fix get_widget for a child element of a memoized component#57
maartenbreddels wants to merge 1 commit into
masterfrom
fix/get-widget-child-elements

Conversation

@maartenbreddels

Copy link
Copy Markdown
Contributor

Supersedes #48, fixes widgetti/solara#927.

Why

A component creates an element, passes it as a child into a function component, and calls get_widget(el) from an effect. On a rerender where the creating component re-executes (new element object) but the receiving component is skipped because its arguments compare equal by value, the child context still maps the old element object, and get_widget fails with "not found in all known widgets".

Why #48 did not work

Two concrete gaps, not a design problem:

  1. fix: get_widget could fail with 'x not found in all known widgets' #48 predates the fast renderer. At its own head its suite is green. Rebased onto master, its _render(..., old_to_new) signature change is not mirrored in _RenderContextFast._render, so REACTON_FAST=1 breaks wholesale.
  2. Its substitution stopped at the first element boundary: _visit_children_values does not descend into an element's own args, so HBox(children=[VBox(children=[arg])]) left arg unmapped. New test test_get_widget_fail_on_rerender_nested_widgets covers it.

What

#48's approach, an old-to-new element map threaded through _render, applied to both renderers, with the substitution descending through nested elements, the map dropped once a component actually re-executes, and the fast renderer's el is el_prev fast path disabled while a map is active. The helpers skip when nothing changed and when a component takes no element props.

Tests: #48's three tests, plus the nested-widgets one. 190 passed on both renderers.

Please review with care

  • The reused root element is mutated in place (root_element.args/kwargs), as in fix: get_widget could fail with 'x not found in all known widgets' #48. A component that holds an element across renders in use_memo or state and re-emits it could get substituted. No test covers that.
  • old_to_new is identity-keyed; equal-but-distinct siblings map by traversal position, which relies on utils.equals implying structural alignment.
  • Benchmarks, REACTON_FAST=1, interleaved A/B: force_update_wide, memo_subtree_skip, list_reorder, teardown at parity; root_update about 5% slower, the element-prop walk on each skipped component.

🤖 Generated with Claude Code

A component that passes an element to a function component keeps a
reference to that element and looks the widget up with get_widget().
When only the outer component re-executes, the inner component is
skipped because its arguments compare equal by value. Its root element
is then reused, so the widget stays registered under the element object
of the previous render pass, while the caller holds a new one.

We now map the old child elements to the new ones and substitute them in
the reused root element, so both the element tree and get_widget() see
the current objects. The map is passed down, because a component deeper
in the tree can reuse its root element too, and it also reaches elements
nested inside widget elements.

Reworked from #48, which only covered the classic renderer and stopped
at the first element boundary.

Fixes widgetti/solara#927
@maartenbreddels
maartenbreddels marked this pull request as draft September 3, 2026 12:12
@maartenbreddels

Copy link
Copy Markdown
Contributor Author

Parking this as a draft for now. Context for picking it up later:

  • The bug and the three original tests come from fix: get_widget could fail with 'x not found in all known widgets' #48; the fourth test, test_get_widget_fail_on_rerender_nested_widgets, covers the case fix: get_widget could fail with 'x not found in all known widgets' #48 missed (an element nested one container deeper than the memoized component's direct children).
  • What fix: get_widget could fail with 'x not found in all known widgets' #48 was missing, concretely: it predates _RenderContextFast, so its _render signature change is not mirrored there, and its substitution stopped at the first element boundary. Both are addressed here, and the tests pass on both renderers (190 passed, 4 skipped).
  • Alternatives considered and rejected: a value-equality fallback inside get_widget (leaves elements / element_to_widget keyed on stale objects, so rc.find() and reconciliation disagree, and it masks real stale-element bugs), and re-executing a memoized component whenever an element argument changes identity (defeats the memoization).
  • Open questions before merging: the reused root element is mutated in place (root_element.args / kwargs), as in fix: get_widget could fail with 'x not found in all known widgets' #48; a component that keeps an element across renders in use_memo or state and re-emits it could get substituted, and no test covers that. root_update is about 5% slower on the fast renderer (interleaved A/B; the other benchmarks at parity), the cost of walking element props on each skipped component.

@maartenbreddels

Copy link
Copy Markdown
Contributor Author

GitHub did not start the pull_request run for this branch; the full unit-test matrix ran via workflow_dispatch instead and passed, 23 jobs: https://github.com/widgetti/reacton/actions/runs/33753168419

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

get_widget can fail

1 participant