Skip to content

Commit

Permalink
client2: rendering: fix crash when moving a node
Browse files Browse the repository at this point in the history
Previously, client2 crashed when an already rendered node was moved by
appending or inserting it twice within the same HTML tree.

    moving_node = Div('node 3')

    html = HTML(
        Div(
            'node 1',
            moving_node,
        ),
        Div('node 2'),
    )

    self.show(html)

    html[1].append(moving_node)

    self.show(html)  # resulted in: ERROR  lona.view_runtime  client error raised: node with id 952 is already cached

When a node is moved by adding it twice to the same node tree, internally, it
gets destroyed at the old location, and a new node, with the old id, gets
rendered in the new location.

Previously, the rendering engines node cache got not cleaned after the first
step, so all child nodes of `moving_node`, which is a text node with the text
"node 3" in this case, remained cached.
When the rendering engine attempted to render a new div, identical to the
former removed one, the cache function would raise an error because the old
text node still was cached.

This patch adds a cache cleaning call to the rendering engines `_remove_node`
method and adds a rendering test for moving nodes.

Signed-off-by: Florian Scherf <mail@florianscherf.de>
  • Loading branch information
fscherf committed Jul 13, 2023
1 parent 919a618 commit 3cd59e6
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 165 deletions.
1 change: 1 addition & 0 deletions lona/client2/_lona/client2/rendering-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ export class LonaRenderingEngine {
node.remove();

this._remove_widget_if_present(node_id);
this._clean_node_cache();
};

_remove_widget_if_present(node_id) {
Expand Down
Loading

0 comments on commit 3cd59e6

Please sign in to comment.