dom
dom copied to clipboard
`childList` records across multiple `MutationObserver`s are observed in fundamentally incorrect order
Note Re-opening of #1105 because that issue was closed for not being succinct, although the problem was described fine. This time I will not include additional comments showing attempts to work around the problem.
I'm not asking for usage help.
I'm pointing out the difficulties involved with writing proper
MutationObserver
code for child tracking (across a tree, not just on a single element), and the intent of this issue is to spark ideas for a better API (again, not usage help).
Trying to port from DOM Mutation Events, or from dis/connectedCallbacks on children (which aren't available on builtin elements) to a MutationObserver
pattern that initializes or cleans up code based on when children are connected then disconnected (which works with built-in and custom elements), is nearly impossible without an immense and cumbersome effort.
I've been using MutationObserver
for years now, and am now discovering bugs in certain cases that I didn't notice before (I should have written better tests, but also MutationObserver
is complicated and cumbersome, the latter being my point and wishing to ideate a better API).
The Problem
Note I will only describe the problem here. #1105 has examples of complicated user code that attempt to work around the problem, but I'm not asking for help with those examples. I'm only trying to show the complexity of the problem, and wishing for a better API.
Note Also note
MutationObserver
is not bad for all its use cases, the problem I will describe here only relates to tracking children. In the ideal future world, a new API would replace thechildList
feature only, and that is the only scope of this issue.
In the following example, the connected and disconnected events fire in the wrong order, which leave connected children erroneously cleaned up:
https://codepen.io/trusktr/pen/oNqrNXE/3aad3bb7315877d00c7c42e3d77fed5a?editors=1010
In this one, the behavior is a bit different, but still not as desired:
https://codepen.io/trusktr/pen/MWVMWKe/091e6f303bd773f2754304fb1c9bff30?editors=1000
With standard connectedCallback
and disconnectedCallback
, they always fire in this order per each custom element when an element disconnected and re-connected:
disconnectedCallback()
connectedCallback()
However, with this naive MutationObserver
usage, the order (in the above two examples) when a child is removed then re-connected is
child connected
child removed
which will cause unexpected behavior, and will require people to start trying to work around the issue as I've described in further comments in #1105.
DOM Mutation Events are much much easier to use. MutationObserver
is incredibly difficult to work with. (wrt observing children)
Possible Solutions:
-
MutationObserver
event callbacks could be called in such a way thatchildList
records are perfectly interleaved in the same order as actions that happened in the DOM. This would complete solve the problem, and code would remain simple. - All the issues of Mutation Events could be fixed and exposed in a new event API.
- There is not any aspect of event patterns in general that make them inherently bad. It just happens to be that the way Mutation Events worked was not ideal and browsers implemented them differently with bugs. That's not to say we can't come up with an event pattern that works well.
- A new synchronous alternative to Mutation Events could be created that would be no worse than today's
connectedCallback
anddisconnectedCallback
methods (those are synchronous, and synchronicity is not the issue, but code ordering is), being an opt-in feature with no such thing as event bubbling in the way or the heaviness ofEventTarget
(Event
being designed specifically for a tree of nodes, when not all event patterns need to be associated with a tree at all).
Okay, so the issue here, I believe, is that when multiple MutationObserver are used to observe tree mutations of different elements, addedNodes
can be queued to one MutationObserver
before removedNodes
is queued to another MutationObserver
even if the removal happened before the insertion.
The solution in this case, based on the original design intent of MutationObserver
is that you should be using a single MutationObserver
to monitor both insertion & removal. Using multiple MutationObservers
and relying on the order of records across them isn't really a use case supported by the design of this API.
The solution in this case, based on the original design intent of
MutationObserver
is that you should be using a singleMutationObserver
to monitor both insertion & removal.
There are some issues with that:
- Once a parent is removed from DOM, it can not have its children independently observed. Having per-element MOs allows that option. https://jsfiddle.net/epo8rhf2
- This issue, https://github.com/whatwg/dom/issues/126, prevents independent cleanup of
MutationObservers
, making it not yet viable for scenarios where removed parents need to no longer track their children.- Further, point 1 would allow to also observe disconnected parents while still observing a document, although this re-introduces the issue of this thread.
- One MO could be used for all elements if point 2 is solved, but I believe that also introduces the same issue as this thread.
Using multiple
MutationObservers
and relying on the order of records across them isn't really a use case supported by the design of this API.
True, that's not a current design, but I believe the spec could be updated to behave as described in point 1 of the Possible Solutions
: the callbacks would need to be called multiple times per MO, interleaved with other MOs, such that we can guarantee that the final result is that all mutations have been executed in the same order they happened in the app.
People shouldn't be depending on the order of MO callbacks currently, because the order is essentially undefined (or incorrect) wrt actual DOM mutation order, so this should be a fairly non-breaking change, I think? If not, then option 3 of the Possible Solutions
will be the way.
MutationObserver is a 10 year old API at this point. I don't think we can do (1) without causing some breakage. We definitely don't want to do (3) because mutation events still are a source of many security vulnerabilities in browser engines, and it's extremely expensive (i.e. hurt performance) to fire those events.
- Once a parent is removed from DOM, it can not have its children independently observed. Having per-element MOs allows that option. https://jsfiddle.net/epo8rhf2
What do you mean by this? Mutation observer can observe a node as long as you call observe
function on it regardless of whether the node was a part of some other subtree or not.
Mutation observer can observe a node as long as you call
observe
function on it regardless of whether the node was a part of some other subtree or not.
That's the same thing I said, but it may introduce the same problem as the OP: having more than one observe()
introduces unexpected ordering (maybe?), but I didn't test this because there is no unobserve()
API as per bullet 2 and I wanted to unobserve
specific elements when finished with them (in their disconnectedCallbacks). And therefore I cannot have a single MutationObserver!
Quick question: if I let go of a node, will this GC its connection to the MO? If so, maybe it could work. But I know that if I simply could simply unobserve a node, then there would no issue.
Further, wanting to unobserve nodes that are still in the tree is another valid use case (for example, hide a piece of UI with an outro animation, and keep it invisible for a while, toggling classes and observing/unobserving, but without unloading all its things (f.e. WebGL content)).
MutationObserver is a 10 year old API at this point. I don't think we can do (1) without causing some breakage. We definitely don't want to do (3) because mutation events still are a source of many security vulnerabilities in browser engines, and it's extremely expensive (i.e. hurt performance) to fire those events.
What would be the alternative?
Possible Solution (option 3), idea 1
What could it look like? Maybe something like this, with similar patterns, but without the complication, using a new name:
const observer = new ChildObserver(changes => {
for (const change of changes) {
if (change.addedNodes) console.log('added nodes', change.addedNodes)
else console.log('removed node', change.removedNodes)
}
})
observer.observe(parent)
// ... later ...
observer.unobserve(parent)
This is very similar to MutationObserver
, except that callbacks would run in a way that guarantees changes are presented in the same order as they happened. This means that a callback can fire more than once, interleaved with other observer callbacks, in order to guarantee mutations are presented in the order they happened.
Note here that if a single element is added, removed, added, removed, then addedNodes
will contain only a single item for each iteration. However, similarly to MO, added/removedNodes
can contain multiple nodes if they happened at the same time so long as the ordering guarantee is upheld.
Possible Solution (option 3), idea 2 (simpler)
The following would be simpler for end usage, but not as aligned with the other *Observer
API shapes:
const observer = new ChildObserver({
addedNodes(addedNodes, parentAddedTo) {
console.log('added nodes to parent', parentAddedTo, addedNodes)
},
removedNodes(removedNodes, parentRemovedFrom) {
console.log('removed nodes from parent', parentRemovedFrom, removedNodes)
},
})
observer.observe(parent)
// ... later ...
observer.unobserve(parent)
Note This is super simple: this drastically reduces user code complexity by not requiring the end user to track previous parents. In some cases, the user cannot reasonably know a parent if they didn't have a reference to it first, hence cannot signal to the parent (in cases where this need exists).
Likewise, the callbacks are called in an order that guarantees all mutations are observed in the same order they actually happened.
The parentAddedTo
and parentRemovedFrom
parameters come second, because those are more likely to be unused, so as to avoid having unused initial parameters (f.e. when we already have those refs outside of the callback scopes, etc).
Using multiple
MutationObservers
and relying on the order of records across them isn't really a use case supported by the design of this API.
The ordering of adding the records to the observers is already guaranteed by the spec given using the sync .takeRecords()
means the records must already be added to the observers in the correct order:
// Sync version of first example:
// https://codepen.io/trusktr/pen/oNqrNXE/3aad3bb7315877d00c7c42e3d77fed5a
setTimeout(() => {
const t = three
t.remove()
console.log(two.o.takeRecords());
console.log(one.o.takeRecords());
one.append(t)
console.log(one.o.takeRecords());
}, 1000)
Why would the mutation microtasks be out of order even if the records are inserted into the observers in order?
I can sympathize with the fact that observers can be tricky to use, but we cannot change the ordering now. This API has shipped years ago. So unless there is some kind of interoperability or compatibility issue or an actual bug, I'm not sure what to make of this issue.
I'm not sure what to make of this issue.
That we need a new and better API, like in the hypothetical ChildObserver
examples above that were proposed as possible solutions.
OP doesn't even mention that? If this is a feature proposal I'd recommend presenting it in a clearer fashion so it can be evaluated in a more straightforward manner.
The OP mentions in title text "Possible Solutions", then in a further comment (not very far below) there are two "Possible Solution" sections also in title text. Easy to miss.
Possible Solution Revisited
This problem can easily be solved by adding a new APi like ChildObserver
which
- is concerned only with the equivalent of
MutationObserver
'schildList
option (in other words,childObserver.observe(el)
is similar tomutationObserver.observe(el, {childList: true})
) - would have a single option,
subtree
, the same asMutationObserver
's (in other words,childObserver.observe(el, {subtree: true})
is similar tomutationObserver.observe(el, {childList: true, subtree: true})
) - callbacks will be called in a way such that observations are always in the correct order within the mutation observer microtask (this means a
ChildObserver
's callback may be called multiple times in within the microtask, such that when there are multipleChildObserver
s, all of their callbacks run such that the order in which mutation records are presented is guaranteed to be the same order in which they actually happened)
This would not be a breaking change to MutationObserver
, but a new API that would become the default way to observe childList
(subtree
or not) without records being presented in the incorrect order.
We could then soft-deprecate MutationObserver childList
(f.e. the articles in the community would write about the best practice of using ChildObserver
instead, and websites like MDN would mark this clearly in the docs.
It seems to me like this addition would be fairly trivial to add. The only difference is
- instead of a list of queued observers with records per observer,
- we'd have a list of mutation record tuples with each tuple associated with an observer
- when a mutation happens, if the last tuple was for the current observer, append a new record to the tuple
- otherwise append a new tuple for the current observer and add the record to the new tuple
- in the mutation microtask, iterate all record tuples
- for the current tuple, call the associated observer's callback with the tuple
Done!
This will prevent end users from ever having to spend time with these difficult-to-handle situations.
The example from the above codepen would now look like very similar, with MutationObserver
replaced with ChildObserver
, and would simply work:
<x-el id="one">
one
<x-el id="two">
two
<x-el id="three">
three
</x-el>
</x-el>
</x-el>
class XEl extends HTMLElement {
connectedCallback() {
this.o = new ChildObserver(changes => {
for (const change of changes) {
if (change.addedNodes.length) console.log('added child, initialize:', change.addedNodes)
if (change.removedNodes.length) console.log('removed child, cleanup:', change.removedNodes)
}
})
this.o.observe(this)
}
disconnectedCallback() {
this.o.disconnect()
}
}
customElements.define('x-el', XEl)
setTimeout(() => {
const t = three
t.remove()
one.append(t)
}, 1000)
The broken output from MutationObserver is:
added child, initialize: NodeList [x-el#three]
removed child, cleanup: NodeList [x-el#three]
The new correct output using ChildObserver will be:
removed child, cleanup: NodeList [x-el#three]
added child, initialize: NodeList [x-el#three]
Polyfill Idea
I believe we can polyfill this like so:
- each instance of the polyfilled
ChildObserver
class have a sharedMutationObserver
that observes the common root node on behalf of allChildObserver
s in the root node, withchildList: true, subtree: true
- the
MutationObserver
callbacks will schedule (if not already scheduled) a microtask that will run after the mutation observer microtask - in the next microtask, we have all the records for the whole root node in correct order, and from this we can map to the tuples that are associated with
ChildObserver
s- we can find the associated
ChildObserver
for a mutation by traversing up frommutationRecord.target
, matching elements that are observation targets of aChildObserver
- we can find the associated
- finally we can run callbacks for
ChildObserver
s based on the order of those tuples - Each Child observer will be opted out if
disconnect
is called. This is what is not possible with separate MutationObservers due to ordering issues.
Because MutationObserver
ordering is not correct, it is impossible to run ChildObserver
callbacks interleaved with MutationObserver
callbacks in the native implementation.
As with this polyfill, the native implementation will simply run a child observer microtask after the mutation observer microtask. The assumption is that all MutationObserver
callbacks work as-is, nothing breaks there, and ChildObserver
behavior is entirely new, operates separately in its new microtask.
Having myself battled with bugs introduced by MO's unpredictable/undocumented behaviour, I can't help but wonder how y'all make success of it. IMO nothing beats the event model we once had, but yeah I get the concerns often raised.
I'm not sure if https://github.com/webqit/realdom resonates with the case here, but I often just want a realtime DOM mutations API so I built that.
The one thing that was excellent about DOM Mutation Events is that reactions were always in order (in their case, because they were synchronous).
Going async is great, but maintaining order is also highly important, or else people get shot in the foot.
Custom Element connected
/attributeChanged
/disconnectedCallback
s are synchronous. People write large apps with custom elements today, and they work fine, which proves that a synchronous event pattern can be as performant as custom element callbacks, differing for the most part only in API shape.
I'm not sure if https://github.com/webqit/realdom resonates with the case here, but I often just want a realtime DOM mutations API so I built that.
Oxford, your work is excellent!
I was thinking of patching DOM APIs like that as well, but its a bit of work that I haven't gotten to yet.
However! Now that you've built a library around this, I think I might just migrate to this.
This may help me simplify Lume's composed tree tracker implementation. I had previously written it in a simpler way, but then reaction ordering problems caused difficult-to-track bugs.
I have some plans to implement CSS for Lume's 3D WebGL elements, and I think this will help a lot because it will guarantee that my code just works without worrying about ordering issues.