From 29ff24da042c8fdd9c4dc575d78b89b46aaa621b Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Mon, 28 Sep 2026 14:21:57 -0700 Subject: [PATCH 1/2] web: tolerate nodes created and dropped inside one patch batch --- platform/web/melange/shell/lui_web_apply.ml | 51 +++++++++++++++------ 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/platform/web/melange/shell/lui_web_apply.ml b/platform/web/melange/shell/lui_web_apply.ml index 812b570..ff5b214 100644 --- a/platform/web/melange/shell/lui_web_apply.ml +++ b/platform/web/melange/shell/lui_web_apply.ml @@ -184,11 +184,14 @@ let mount_inserted_child renderer child current = Lui_web_overlay.mount_toast renderer child current.platform_node | _ -> () -let insert_child_dom renderer parent child index = - let parent_dom = Nodes.dom_node renderer parent in - let child_dom = Nodes.dom_node renderer child in +let insert_child_dom renderer previous_nodes parent child index = + let parent_dom = Nodes.dom_node_before renderer previous_nodes parent in + let child_dom = Nodes.dom_node_before renderer previous_nodes child in match Store.node renderer.web_store child with - | None -> invalid_arg "unknown DOM child" + | None -> + (* dropped again inside this batch — insert anyway so the later + remove op finds the element where the op stream put it *) + Util.insert_dom_child parent_dom child_dom index | Some current -> ( match Store.standard_kind current with | Some BottomTab -> ( @@ -226,8 +229,8 @@ let insert_child_dom renderer parent child index = (dom_child_container renderer parent parent_dom) child_dom (visible_child_index renderer parent index)) -let apply_insert_child renderer parent child index = - insert_child_dom renderer parent child index; +let apply_insert_child renderer previous_nodes parent child index = + insert_child_dom renderer previous_nodes parent child index; Lui_web_split.update_split renderer parent; Lui_web_focus.refresh_button_context renderer child; Lui_web_split.update_split renderer parent; @@ -396,12 +399,25 @@ let apply_remove_prop renderer node property = | None -> invalid_arg "standard property targets extension node") | None -> ()) +(* A node created and dropped inside the same batch never reaches the DOM: + the store already reflects the batch's final state, so ops that mention it + have no element to act on and are skipped. *) +let known_node renderer previous_nodes node = + match Store.node renderer.web_store node with + | Some _ -> true + | None -> prev_node previous_nodes node <> None + let apply_dom_op renderer previous_nodes operation = match operation with - | CreateNode (node, kind) -> apply_create renderer node kind - | CreateExtension (node, _identifier, _fingerprint) -> - W.Element.setAttribute "id" (Util.node_dom_id node) - (Nodes.dom_node renderer node) + | CreateNode (node, kind) -> + if Store.node renderer.web_store node <> None then + apply_create renderer node kind + | CreateExtension (node, _identifier, _fingerprint) -> ( + match Store.node renderer.web_store node with + | Some current -> + W.Element.setAttribute "id" (Util.node_dom_id node) + current.platform_node + | None -> ()) | DropNode node -> Ext.cleanup_extension_node renderer previous_nodes node; cleanup_node renderer node @@ -413,11 +429,20 @@ let apply_dom_op renderer previous_nodes operation = | RemoveExtensionProp (node, property) -> Ext.remove_extension_property renderer node property | InsertChild (parent, child, index) -> - apply_insert_child renderer parent child index + if + known_node renderer previous_nodes parent + && known_node renderer previous_nodes child + then apply_insert_child renderer previous_nodes parent child index | RemoveChild (parent, child) -> - apply_remove_child renderer previous_nodes parent child + if + known_node renderer previous_nodes parent + && known_node renderer previous_nodes child + then apply_remove_child renderer previous_nodes parent child | MoveChild (parent, child, index) -> - apply_move_child renderer previous_nodes parent child index + if + known_node renderer previous_nodes parent + && known_node renderer previous_nodes child + then apply_move_child renderer previous_nodes parent child index let apply_dom_batch renderer previous_nodes batch = List.iter From 9d252c3bc18c8f776ddb7f4634d74ede9956d7ef Mon Sep 17 00:00:00 2001 From: Tienson Qin Date: Mon, 28 Sep 2026 04:28:59 -0700 Subject: [PATCH 2/2] runtime: cancel pending ops for same-batch create+drop; absorb events for dropped nodes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - drop_node/emit_dropped_subtree: when a node's create op is still in pending_ops, prune every queued op mentioning it and skip the drop op instead of emitting an unreplayable create+drop pair (the backend commits the batch before DOM replay, so post-commit lookups for the node always miss and the Invalid_argument wedges the generation). - dispatch: return early when the event's node is no longer live — listeners can fire after their node was dropped. - web apply: include the failing op JSON and label the roving-focus passes in Invalid_argument messages for diagnosability. --- platform/web/melange/shell/lui_web_apply.ml | 16 ++++-- src/lui_runtime.ml | 58 ++++++++++++++++++--- 2 files changed, 64 insertions(+), 10 deletions(-) diff --git a/platform/web/melange/shell/lui_web_apply.ml b/platform/web/melange/shell/lui_web_apply.ml index ff5b214..fefe825 100644 --- a/platform/web/melange/shell/lui_web_apply.ml +++ b/platform/web/melange/shell/lui_web_apply.ml @@ -446,8 +446,16 @@ let apply_dom_op renderer previous_nodes operation = let apply_dom_batch renderer previous_nodes batch = List.iter - (fun operation -> apply_dom_op renderer previous_nodes operation) + (fun operation -> + try apply_dom_op renderer previous_nodes operation + with Invalid_argument msg -> + invalid_arg + (Printf.sprintf "op %s: %s" (Lui_wire.encode_op operation) msg)) batch.ops; - ignore (Lui_web_focus.update_all_horizontal_group_roving renderer); - ignore (Lui_web_focus.update_all_tree_roving renderer); - ignore (Lui_web_focus.update_all_toolbar_roving renderer) + let label_roving label f = + try ignore (f renderer) + with Invalid_argument msg -> invalid_arg (label ^ ": " ^ msg) + in + label_roving "hgroup-roving" Lui_web_focus.update_all_horizontal_group_roving; + label_roving "tree-roving" Lui_web_focus.update_all_tree_roving; + label_roving "toolbar-roving" Lui_web_focus.update_all_toolbar_roving diff --git a/src/lui_runtime.ml b/src/lui_runtime.ml index 6a1f42f..07a5b13 100644 --- a/src/lui_runtime.ml +++ b/src/lui_runtime.ml @@ -456,6 +456,55 @@ let emit_extension_property_diff application node old_values desired_values = enqueue application (set_extension_prop_op node property value)) desired_values +(* Ops are replayed post-commit: by then a node dropped in the same + batch no longer resolves through the store or the pre-batch mirror, + so any queued op mentioning it can never apply. A node created and + dropped within one batch therefore cancels out entirely — its whole + op group (including structural detach ops) is pruned and no drop op + is emitted. Nodes dropped across batches keep their ops: pre-existing + records resolve through the pre-batch mirror, and the store still + needs the remove-child op that precedes a drop. *) +let operation_mentions_node node operation = + let subject = + match operation with + | CreateNode (subject, _) + | CreateExtension (subject, _, _) + | DropNode subject + | SetProp (subject, _, _) + | RemoveProp (subject, _) + | SetExtensionProp (subject, _, _) + | RemoveExtensionProp (subject, _) + | InsertChild (subject, _, _) + | RemoveChild (subject, _) + | MoveChild (subject, _, _) -> + Some subject + in + let child = + match operation with + | InsertChild (_, child, _) + | RemoveChild (_, child) + | MoveChild (_, child, _) -> + Some child + | _ -> None + in + subject = Some node || child = Some node + +let pending_create_exists application node = + List.exists + (function + | CreateNode (candidate, _) | CreateExtension (candidate, _, _) -> + candidate = node + | _ -> false) + !(application.pending_ops) + +let enqueue_drop application node = + if pending_create_exists application node then + application.pending_ops := + List.filter + (fun operation -> not (operation_mentions_node node operation)) + !(application.pending_ops) + else enqueue application (drop_node_op node) + let rec emit_dropped_subtree application saved removed_set node = let children = match Hashtbl.find_opt saved.checkpoint_children node with @@ -469,7 +518,7 @@ let rec emit_dropped_subtree application saved removed_set node = emit_dropped_subtree application saved removed_set child end) children; - enqueue application (drop_node_op node) + enqueue_drop application node let reconcile_subtree application saved parent old_root candidate_root = let parent = canonical_node application parent in @@ -851,7 +900,7 @@ and drop_node application node = Hashtbl.remove application.runtime_parents node; Hashtbl.remove application.runtime_reload_keys node; application.runtime_extension_dirty := true; - enqueue application (drop_node_op node) + enqueue_drop application node and remove_child application parent child = let parent = canonical_node application parent in @@ -1153,10 +1202,7 @@ let event_is_value_echo properties event = let dispatch application event = let node = canonical_node application (event_node event) in - if - (not (Hashtbl.mem application.mounted_nodes node)) - && not (Hashtbl.mem application.runtime_extension_nodes node) - then + if not (node_live application node) then (* a DOM element can receive an in-flight event after being unmounted mid-dispatch (e.g. a capture-phase handler re-renders); ignore it *) false