Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Avoid the need of the MutationObserver callback #1019

Closed
Lcfvs opened this issue Sep 26, 2021 · 8 comments
Closed

[Proposal] Avoid the need of the MutationObserver callback #1019

Lcfvs opened this issue Sep 26, 2021 · 8 comments
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest

Comments

@Lcfvs
Copy link

Lcfvs commented Sep 26, 2021

Hi all,

I would like to suggest an improvement on the MutationObserver.

Motivations

Imho, the current implemetation has a problem increasing the code complexity: the need of to treat the mutations in a callback.

Depending on the strategies...

  • it can enforce to wrap all the behaviors in a same function
  • it can enforce to mutate something external written by the developer to manage the mutations externally (disallowing the immutable stategies on that management)
  • it can complicate the treatment for a specific target

The suggested improvement

An idea I implemented to simplify the behavior is to turn the observer as an iterable.

My approach, to see the suggested behavior:

export class MutationObserver extends globalThis.MutationObserver {
  #wait = resolve => this.#resolve = resolve
  #register = () => this.#promises.push(new Promise(this.#wait))
  #records = []
  #resolve = null
  #promises = []

  constructor (callback = null) {
    super(records => {
      this.#records.push(...records)
      this.#resolve?.()
      this.#register()
      callback?.(records, this)
    })

    this.#register()
  }

  disconnect () {
    this.#resolve()
    super.disconnect()
  }

  async* [Symbol.asyncIterator] () {
    let index = 0

    for (const promise of this.#promises) {
      await promise
      
      for (; index < this.#records.length; index += 1) {
        yield this.#records[index]
      }
    }
  }
}

Usage

const frag = document.createDocumentFragment()

const append = () => frag.append(document.createElement('div'))

const observer = new MutationObserver()

observer.observe(frag, {
  childList: true
})

append()
append()
setTimeout(append, 1000)
setTimeout(() => observer.disconnect(), 3000)
setTimeout(append, 4000) // not observed

for await (const record of observer) {
  console.log(record)
}

console.log('first loop end')

for await (const record of observer) {
  console.log(record)
}

console.log('second loop end')

Any thoughts about it, please?

@annevk
Copy link
Member

annevk commented Sep 27, 2021

Hey, thanks for contributing your idea. I find the arguments against the current approach a little hard to follow. It also seems to me that using async iterators would incur some overhead as well as result in some delay due to promises.

Having said that, if this were an extremely popular way to consume node tree mutations I could be persuaded to add an API for it. Have you developed this into a library for people to use?

@annevk annevk added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Sep 27, 2021
@WebReflection
Copy link

FWIWI ...

if this were an extremely popular ...

I've implemented MO in LinkeDOM and used to polyfill for years and never needed this pattern which I am struggling to understand what is useful for ... it just keeps accumulating mutations, growing the heap, running these late, plus if the proposed code works, seems like an ad-hoc user-land use case that can be addressed, definitively not what I'd like to have instead of the current MO?

@Lcfvs
Copy link
Author

Lcfvs commented Sep 28, 2021

Thanks for your feedback.

I will try to detail a bit my arguments but, to be honest, the explanations aren't my best skill. Don't hesitate to ask if something remains opaque.

In a few words, the most problem is the unidirectional side of the callback.

* it can enforce to wrap all the behaviors in a same function

An enclosing function implies 2 major problems:

  • No way to simply return the mutations, sequentially or not, to treat them externally
  • In a project lifecycle, we can often need to add some new behaviors, but if we can't modify the function (i.e. provided by a library/framework), the 2 lone solutions are
    • to create a new MO but it can be really difficult to manage the behaviors order, particularly for the async ones
    • to create a new function adding the new behavior and await a nested call of the original one... if we have several nesting levels like that, it risks to provide an horrible thing to maintain
* it can enforce to mutate something external written by the developer to manage the records externally (disallowing the immutable stategies on that management)

Alternatively to the previous point, a solution can be to have something like a Pub/Sub to push the records but:

  • we need to know that object into the function, which can require to create a specific component to run our tasks, maybe for each MO and it can overcomplicate the dependency injection
  • working myself on some projects, oriented fully immutable by design, the idea to mutate something is really a problem we can only solve by, again, by something like a Pub/Sub... then also unidirectional
* it can complicate the treatment for a specific target

Since an observer can observe more than one target, if we want to some tasks on a specific target, we only have 2 choices:

  • to create a distinct MO, also increasing the complexity of an order management
  • to know that target into the callback, then to have a new function, to specifically filter the records for that target
    Using a external loop, it's pretty simple to filter it, like this
observer.observe(target, options)

for await (const record of observer) {
  if (record.target !== target) {
    continue
  }

  // do something with the record
}

Using an async iterator, we can simplify all of these points, preserve the immutability if needed, we can retrieve the records externally and do the tasks in any order, without the need to create a manager, keeping the things easily maintainable .

Having said that, I agree with @WebReflection about the growing heap, it's probably a bad idea, I didn't think about it... a better idea is maybe to make an unique iterator, using a .getIterator(), like how I made for my eventual next proposal, an EventHandler: https://gist.github.com/Lcfvs/bc70fbfc309167b1cfecb9ad924dd7b3

@Lcfvs
Copy link
Author

Lcfvs commented Sep 28, 2021

Oh, and, no, I haven't done a library yet using these features but maybe a day, it will probably depend on the pros/cons raised in this discussion....

I'm still a bit busy on my immutable & JS-typed things, actually... I just made it, for the PoCs, due to a usecase needed to help on TCD... it was a funny thing to make. :P

@annevk
Copy link
Member

annevk commented Sep 28, 2021

Thanks. I think in that case this kind of thing is better left to userland for now. There's two main reasons to add something to a web platform standard in my view:

  1. A capability that cannot be achieved in userland.
  2. There's a popular pattern in userland that could benefit in some way from being available by default. (E.g., better performance or better developer ergonomics.)

@annevk annevk closed this as completed Sep 28, 2021
@Lcfvs
Copy link
Author

Lcfvs commented Sep 28, 2021

Imho, it simplify the developer ergonomics but, ok, no worries. :)

@annevk
Copy link
Member

annevk commented Sep 28, 2021

Perhaps, but adoption as a popular library is important to demonstrate that as well.

@Lcfvs
Copy link
Author

Lcfvs commented Sep 28, 2021

I see, I see, I'm working a lot to make some innovative solutions but I can't afford to make them popular. ^^'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest
Development

No branches or pull requests

3 participants