Dom: Disconnect single target instead of all

[zbraniecki]

Current specification of MutationObserver allows users to connect an observer to multiple target root nodes, but disconnect always acts on all of them.

It makes it hard to write systems that modify targets in reaction to mutations because they need to disconnect the target on which the mutation happened, modify, and reconnect.

A simple solution would be to add a single parameter to disconnect with targetNode.

[travisleithead]

This seems very reasonable to me. It would not be a burden implementation-wise.

[zbraniecki]

@smaug---- @annevk - thoughts?

[smaug----]

So disconnect() would do then what? Currently disconnect() clears the currently collected MutationRecords and it also removes transient observers.
I don't quite understand what disconnect(someNode) would do to the record queue?

[ArkadiuszMichalski]

Why limited only to one target? This would be more useful disconnect(Node... nodes), so without passing argument act like what we have now (remove all registered observers), for one or more nodes check if they are exist in context object’s list of nodes and remove registered observer for that nodes. Record queue for both case probably should be cleared.

[smaug----]

So even if MutationObserver keeps observing nodeA after calling disconnect(nodeB), the records which were created because of observing nodeA, are just lost. Feels a bit odd to me, and error prone.
One needs to remember to call takeRecords to not have stuff lost.

For the original use case would something like suppress(node)/unsuppress(node) work? It wouldn't affect to the queued records, but would just prevent creating new ones for the particular registered observer or relevant transient observers.

[ArkadiuszMichalski]

Right, clearing all queued records may be a problem (everything depends on the situation). Remembering to use MutationObserver.takeRecords() before disconnecting single node, if necessary, is inconvenient. But suppress(node)/unsuppress(node) looks interesting.

[justinfagnani]

To add a different use case, for which I don't think suppress would work, in the Custom Elements v1 polyfill, I want to be able to observe the main document and the roots of several disconnected DOM trees. If any of the observed roots are added to another, then I want to unobserve the root of the child subtree because the subtree is now being observed by the parent subtree's observer.

I would propose adding an unobserve() method to parallel observe(), rather than change disconnect().

As for the record queue, I'd still need records from the unobserved subtree, so would prefer they remain, but could do a takeRecords() if necessary.

[trusktr]

Record queue for both case probably should be cleared.

I think removing only the records for the unobserved nodes makes sense.

I would propose adding an unobserve() method to parallel observe(), rather than change disconnect().

That could work. As end user, I don't care if it is unobserve(node) or disconnect(node), as long as they work the same (stop observing the node, and any following observer callback will not contain records for that node). unobserve sounds better as a reciprocal to observe.

As for the record queue, I'd still need records from the unobserved subtree, so would prefer they remain

I think that is confusing. If we unobserve(target), no future records should be callbacked for target, to keep things simple.

If you still need records from the unobserved subtree, you can call unobserve(node) _after_ you've gotten those records (f.e. in the next observer callback).

[annevk]

Is this still something that's needed? It would help to have a clear proposal (ideally in OP) to point implementers to.

[justinfagnani]

From the Web Components world, I wouldn't be surprised if there are a few use cases that come up from needing to observe multiple shadow roots or ancestor trees as they're being distributed to slots, or with multiple custom element instances observing their light DOM and unobserving on disconnection.

[trusktr]

Is this still something that's needed?

Yes, makes code cleaner, and leads to less resource usage.

[trusktr]

Right now I use a new MO on each element, with the same callback function, so that I can remove each observer as needed. Seems like I could save memory usage if it were only a single MO.

[annevk]

Reading through the thread again it seems the main problem is that it's still unclear what the semantics should be. E.g., @justinfagnani argues the records need to be kept and @trusktr argues just those for the node in question need to be removed (it's unclear to me how that'd work, we don't store sufficient information for that today).

If someone can figure out a proposal here in terms of concrete changes to the specification it might be easier to obtain implementer interest.

[nielsvanmidden]

This would be very useful. Let me give you an example I run into:
Currently I'm building a web application with React and Redux. This application is able to run from both the server and the client. But since the server isn't supporting any browser APIs we use middleware. This middleware connects React components to JavaScript services (singletons) that enhance those components with the use of browser API's. This middleware is applied to the JavaScript client bundle only.

In one specific scenario we have components that dispatch a register action when componentDidMount() (React lifecycle API) is invoked. And whenever this action is dispatched it starts observing the particular node for mutations (subscribeNode()).

class ServiceCards {
  constructor() {
    this.subscribeNode = this.subscribeNode.bind(this);
    this.observer = new MutationObserver(mutationsToObserve);
  }

  subscribeNode(node) {
    node.addEventListener('transitionend', this.onTransitionEnd);
    this.observer.observe(node, mutationOptions);
  }

  onTransitionEnd(ev) {
    if (this.classList.contains('is-selected') && ev.propertyName === 'width') {
      this.classList.remove('is-selected');
    }
  }
}

Because React is in control when a component is either mounted or unmounted, disconnect doesn't mean so much to me. Also if a single component gets unmounted, I don't want the observer to disconnect. I'd rather dispatch an action that is able to unsubscribe a single node here.

[annevk]

@nielsvanmidden thank you, but this doesn't really address the questions from @smaug---- upthread (and the resulting discussion) as far as I can tell.

[trusktr]

Hope this becomes reality. unobserve(target) as reciprocal to observe(target) would be great, leaving disconnect as is. Calling unobserve(target) would cancel any future callbacks (and records) for that target, and leave future callbacks and records in place for other targets, as @justinfagnani proposed. 👍

[ikeyan]

@trusktr The problem is,

callbacks (and records) for that target

and

future callbacks and records in place for other targets

can have nodes in common. For example, if a node A is an ancestor of a node B like in this HTML,

<body>
<div> = A
  <ul>
    <li></li> = B
  </div>
</div>
</body>

and if you observe both A and B, how unobserve() should behave?

observer.observe(A, {subtree: true, childList: true});
observer.observe(B, {attributes: true});
// Now if someone add a class attribute to B,
// observer has a queued item {target: B, attributeName: "class"}
observer.unobserve(B);

I think three behaviors are mentioned in this issue.

[ikeyan]

1. disconnect() semantics: the method should discard all mutations in the queue (@ArkadiuszMichalski
   mentioned here)
2. I think this is error prone and surprising behavior

[ikeyan]

1. unobserve() with queue filtering semantics: the method should behave as if there was no observe() call(s) on the node. (@trusktr mentioned here)
2. mutations in the queue should be filtered
   
   
   
   • if no other observe() calls on other targets cover the mutation, remove it.
   
   
   • otherwise, leave as it is.
   
   
   • the problem is, mutations can change nodes' hierarchy, and thus the correct cancellation is actually impossible. (@annevk)
   
   

For example, consider this scenario.

• observer.observe(A, {subtree: true, attributes: true})
• observer.observe(B, {attributes: true})
• mutation queued: add a class attribute to node B
• (not observed) move node B from <ul> to <body>
• observer.unobserve(B)

The addition of class attribute to node B was covered by node A _at that time_, but not when unobserve() is called, and since node additions/removals were not observed, you cannot reconstruct the past hierarchy.
So, if you implement the queue filtering, you can only use the current hierarchy (wrong result), or you have to remember the node hierarchy at every mutation just for this purpose (memory consuming).

[ikeyan]

1. unobserve() semantics without queue filtering: same as 3. but no queue filtering ( @justinfagnani mentioned here, @smaug---- mentioned here)
2. mutations already in the queue are left as they are and are eventually processed by the callback function.
3. since MutationObserver is asynchronous, I think this is acceptable and efficient solution.

[ikeyan]

So I think unobserve() semantics without queue filtering is the most promising behavior to implement.

[trusktr]

Wouldn't it be weird to if someone unobserves something, and then for some reason the callback still fires later? Wouldn't this cause confusion for end users of the API?

As @justinfagnani mentioned in https://github.com/whatwg/dom/issues/126#issuecomment-228455689, there could be a separate takeRecords() API for users that really need this.

For example,

observer.observe(node)

// ... then later ...

observer.takeRecords() // take the records that may already be queued
// ... do anything with records belonging to `node` ...
observer.unobserve(node) // will not fire a future callback with records that would've had target = node

observer.observe(A, {subtree: true, childList: true});
observer.observe(B, {attributes: true});
// Now if someone add a class attribute to B,
// observer has a queued item {target: B, attributeName: "class"}
observer.unobserve(B);

and if you observe both A and B, how unobserve() should behave?

@ikeyan The records of the attribute changes for B will not be included in the future callback. However, the records for the target A will still be included, even if those include B. At least, that's a simple way for it to work.

I think the key is that unobserve(B) will unobserve the specific behavior defined when we called observe(B). If B happens to be a node in the childList+subtree observation behavior of some another node A, then unobserving B should not have any impact on the observation defined on target node A.

[ikeyan]

@trusktr
So, the records should know which nodes they are for?
Then, I also think it's a sound and simpler behavior.

// node A is a descendant of node B
observer.observe(A, {subtree: true, attributes: true});
observer.observe(B, {attributes: true});
// mutation queued: add a class attribute to node B
observer._queue = [{for: new Set([A, B]), message: "a class attribute added to B"}]
observer.unobserve(B);
// queue filtering. remove B from `for` attribute, and if `for` gets empty, the mutation record is thrown away
observer._queue = [{for: new Set([A]), message: "a class attribute added to B"}]

[trusktr]

That seems simple! :+1:

I think the _queue would be a Set too right?

API consistency between MutationObserver, ResizeObserver, IntersectionObserver, and any other Observer would be nice.

[pengzhengyi]

I encountered an infinite-loop scenario from improperly using disconnect (assuming it works like unobserve for all targets in observer's node list)

1. receiving a mutation record at an observed target (inside MutationObserver's callback)
2. stop observing this target by calling disconnect (unexpected: the target still remains in the mutation observer's node list)
3. handle the mutation record (action here will manipulate the DOM and will trigger another mutation record if this target is still observed)
4. re-observing the previously observed target -- call observe on this target (the target now have two occurrences in the mutation observer's node list)
5. this already-handled-once mutation record is dispatched again to newly added target due to live editing of node list
6. infinite loop

My current fix is recreate a MutaionObserver and restore it to desired state whenever I need to re-observe a target.

I think I mistakenly used disconnect as unobserve for all, which might be another reason why a separate unobserve method is beneficial for inexperienced programmer like me.