Incorrect Node Index Calculation and Duplicate Insertions in `RemoteMutationObserver`
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 1.3k
- Forks
- 91
- Avg merge
- 9d 14h
- Merged PRs (30d)
- 38
Description
Description
We have observed persistent issues with RemoteMutationObserver when used in combination with the iframe sandbox. Specifically, problems arise when mutation records contain interleaving removals and additions of nodes, leading to incorrect behavior where nodes are not properly added or removed. Additionally, under certain conditions, nodes may be inserted multiple times on the host side.
Steps to Reproduce
A reproducible scenario for the index issue can be found in the kitchen-sink demo app of this fork, where a mutation replaces two child nodes with two new child nodes.
Initial DOM State
<ui-stack>
<ui-text>Loading 1</ui-text>
<ui-text>Loading 2</ui-text>
</ui-stack>
After State Update
<ui-stack>
<ui-button>Button 1</ui-button>
<ui-button>Button 2</ui-button>
</ui-stack>
Detected Mutations by MutationObserver
| Action | Previous Sibling | Node | Next Sibling |
|---|---|---|---|
| Remove | null | <ui-text>Loading 1</ui-text> |
<ui-text>Loading 2</ui-text> |
| Insert | <ui-text>Loading 2</ui-text> |
<ui-button>Button 1</ui-button> |
null |
| Remove | null | <ui-text>Loading 2</ui-text> |
<ui-button>Button 1</ui-button> |
| Insert | <ui-button>Button 1</ui-button> |
<ui-button>Button 2</ui-button> |
null |
Mutation Records Generated by RemoteMutationObserver
| Action | Node | Index |
|---|---|---|
| Remove | null | 0 |
| Insert | <ui-button>Button 1</ui-button> |
0 |
| Remove | null | 0 |
| Insert | <ui-button>Button 2</ui-button> |
1 |
Incorrect Final DOM State
<ui-stack>
<ui-button>Loading 2</ui-button>
<ui-button>Button 2</ui-button>
</ui-stack>
In this state, <ui-text>Loading 2</ui-text> was incorrectly retained instead of being removed, and <ui-button>Button 1</ui-button> is missing.
A small side node
Prior to this bugfix, the second example here even caused a crash, as described in this issue. However, this fix merely acts as a temporary patch rather than addressing the root problem. I even would suggest to be more strict (throwing errors) when invariants are not met, like a node that could not be found in the receivers lookup registries. This is how errors in the algorithm or logic are first noticed and subsequent errors are avoided.
Root Cause Analysis
1. Unreliable Index Calculation
- The issue stems from how
RemoteMutationObserverdetermines the index of inserted and removed nodes. - When processing multiple mutation records within the same mutation event, the index calculation is unreliable.
- This is because the mutation event occurs after the mutation has already been applied.
- The removed node
<ui-text>Loading 2</ui-text>no longer exists in the parent’s child list at the time of index determination. - As a result, the algorithm incorrectly assumes an index of
0, leading to incorrect mutations.
2. Duplicate Insertions
- In rare cases, nodes are inserted multiple times on the host side.
- This typically occurs when replacing a node with a subtree of deeply nested nodes.
- Although
RemoteMutationObservertries to avoid duplicate insertions usingaddedNode.contains(node), this check is insufficient because it only detects direct children, not deep descendants.
Expected Behavior
RemoteMutationObserver should ensure correct node insertions and removals, even when multiple mutation records are involved or deeply nested structures are inserted. It should also prevent duplicate insertions of the same node.
Proposed Solution
1. Use Reference Nodes Instead of Indices
A more reliable approach is to replace index-based positioning with remote IDs of reference nodes:
- For removals: The removed node itself serves as the reference.
- For insertions: The reference should be the next or previous sibling node, with the next sibling recommended, as this simplifies processing on the receiving end:
- If the successor is
null, the node should be appended. - Otherwise, the node should be inserted before the reference.
- If the successor is
2. Prevent Duplicate Insertions on the Host Side
- Rather than implementing recursive descendant checks in
RemoteMutationObserver(which would be costly), the host/receiver now checks before inserting a node whether it already exists under the expected parent. - Since the parent is known, this check is efficient and avoids unnecessary overhead.
This combination ensures both correctness and performance across a range of complex mutation scenarios.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with RemoteMutationObserver and the reproduction in examples/kitchen-sink/app/remote/examples/react-mutations.tsx. Trace how records are converted into remote mutations and how the host/receiver inserts nodes, paying particular attention to interleaved removals, additions, and nested subtrees. Done means replacement sequences preserve the expected DOM and no node is inserted more than once.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- typescript
- Domain
- frontend, web-dev
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100