View updater fails to reuse nodes when 3+ widgets are removed (syncToMarks lookahead)

1

Hi,

I’ve run into a weird edge-case where NodeViews are being unnecessarily destroyed and recreated during updates. I’ve traced this to the lookahead logic in ViewTreeUpdater.syncToMarks from prosemirror-view.

When a transaction removes 3 or more widget decorations (possibly nodes too but I’ve only repro’d this successfully with widgets) that were immediately preceding one or more nodes with marks, the view updater fails to find the existing mark

This happens because:

  1. In updateChildren, the “dead” widget views from the previous state are still present in updater.top.children.

  2. syncToMarks attempts to find a matching MarkView by scanning ahead, but it has a hard-coded limit of 3 items (this.index + 3).

  3. If there are 3+ dead widgets “blocking” the view, the loop terminates before finding the valid MarkView behind them.

  4. ProseMirror assumes the MarkView is gone, destroys the existing one (and all its children), and creates a new empty one, and then recreates all nodes/nodeviews that would sit inside of that in the DOM.

I have created a minimal example that reproduces the bug:

Clicking “Trigger Bug” removes 3 widgets. You will see in the console/UI that the subsequent target nodes are then all destroyed and recreated.

In this example there’s only 10 simple nodes, but we’ve observed cases where hundreds of nodes can be recreated all at once, which adds up quickly if those nodes are expensive to re-initialise. We’ve also observed cases where this can cause weird effects like nodes flickering from mouse-hover interactions causing rapid destruction/creation.

Would you be open to me raising a PR that improves this lookahead behaviour?

The simplest fix that I can think of seems to be modifying the loop in syncToMarks to skip over nodes that have no mark applied when scanning for a mark match, rather than counting them against the loop limit. This would allow the updater to “see through” the dead widgets (or any other dead content that could similarly cause this issue) to the content behind them, but I’d need to spend more time investigating and testing that to verify it works well.

If you have any thoughts or suggestions here on what could be a good path forwards other than what I’ve suggested though, I’d love to hear them :folded_hands:

Thanks!

2

The DOM reconciling in ProseMirror is fundamentally heuristic-based, so there’s no guarantee being made of updates always being the most minimal. That being said, this behavior was really rather wasteful. Attached patch should improve it.

1 Like
3

That seems to do the trick, thank you!