Re: [whatwg/dom] Disconnect single target instead of all (#126)

@annevk

> Reading through the thread again it seems the main problem is that it's still unclear what the semantics should be

1. @ikeyan described the semantics regarding what happens to the records queue [here](https://github.com/whatwg/dom/issues/126#issuecomment-631768440), which seems very reasonable: namely using a `Set` to track which elements a record applies to, and removing that record only if the `Set` is empty.
2. Based on @ArkadiuszMichalski's [comment](https://github.com/whatwg/dom/issues/126#issuecomment-167374814), having `unobserve(...nodes: Node[])` (using TypeScript syntax here) would be useful. If this strays from patterns already used in the other `*Observer` APIs (f.e. [`ResizeObserver.unobserve`](https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserver/unobserve)), would it be more worth accepting only a single arg for consistency?
3. @justinfagnani [mentioned](https://github.com/whatwg/dom/issues/126#issuecomment-228455689) that the suggested sync `takeRecords()` method would be useful when we want to `unobserve` a particular node but still need access to the pending records before removing the node, otherwise those lost records can result in effects unexpectedly not happening and leading to bugs.
  - Based on the possibility of bugs due to effects that didn't run, the option that `unobserve()` would stop queuing new records for the unobserved element but still fire the existing records in the upcoming task seems reasonable. I.e. "stop observingthe node, but still tell me what already happened before I stopped observing". **I'm starting to think that this may be the option that leads to less bugs.**
  - The `takeRecords` is one way to give people the choice on whether they want to observe the changes that _already happened_ after they call `unobserve`. However, this means that the default behavior would be the potentially buggy removal of not-yet-handled observations after calling `unobserve`.
  - If we'd still like to give people a choice (some people might decide "Hey, I really don't care what will happen or what already happened to this node anymore.", which is totally valid in certain cases) then I think it would be better to default to the less-bug-prone behavior (which is that `unobserve(node)` stops observing for changes at the point the method is called, but still notifies of already-happened changes in the upcoming tick), and either
    - the user can call `takeRecords()` and do nothing with the result to opt into ignoring the already-happened observations,
    - or there can be an option that can be passed to `unobserve`, for example `unobserve(node, {skipExistingRecords: true})`, but this would preclude the var args idea `unobserve(...nodes)` from being done in an ideal way.
  - What do other `*Observer` APIs currently do when `unobserve`ing a single node?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/126#issuecomment-916540096

Received on Friday, 10 September 2021 00:40:21 UTC