shama / on-load

On load/unload events for DOM elements using a MutationObserver
113 stars 19 forks source link

allow for multiple onloads #19

Closed ahdinosaur closed 7 years ago

ahdinosaur commented 7 years ago

hey! here's a pull request to fix https://github.com/shama/bel/issues/60.

this is a breaking change:

now exports function that receives root node (default: document.body) and returns what was previously exported (the onload function).

this allows mutation observations to happen when the export is explicitly called, rather than when the module is required. because of this, there is now also a .end() function attached to the onload function returned which disconnects the mutation observer.

i'm not sure if this is the best way, but it works and was easy to implement. open to alternative implementation ideas. :smile:

ahdinosaur commented 7 years ago

ah, you might be able to do this with a WeakMap!

yoshuawuyts commented 7 years ago

I kinda managed to make this not a problem; the constraint of a single on-load is fine I think, just requires more tight design. Using weakMap or anything similar feels icky, given it complicates browser support further

ahdinosaur commented 7 years ago

wow wow this is a tough cookie. :cookie:

after a few other attempts to solve this, i kinda feel like this pull request (or something that involves a similar breaking change) is the only way to do this.

  1. next i tried this with weakmaps. then i realized the edge cases when working with a morphdom-esque strategy and how @shama is way more clever that i thought possible. :smile_cat:
  2. then i tried to refactor on-load to support multiple onload / onunload handlers by having an array of handlers instead of a single handler. i'm not saying this is impossible, but my brain turned to mush. :watermelon:
  3. last i tried to do this by integrating with the morphdom lifecycle hooks, but i'm not sure how to avoid the problem below.

so why is this so difficult? basically, as i learned from running the super helpful tests, you can't assume that "conceptual elements" (components) map 1-to-1 with real dom elements. for example, imagine you have two element creators, one for a home page and one for an about page, each with separate onload / onunload handlers. when navigating between pages, no new dom element is created, but we expect our handlers to be called.

currently this problem is solved with comparing a special attribute key added to the dom elements and comparing the function's caller. the latter feels like a hack, considering Function.caller is non-standard.

i think this pull request is on the right track because i think calling onLoad = OnLoad() and bel = Bel() to create a unique reference to that onload /onunload or element creator is the least hacky way to disambiguate similar elements using the creator's identity. this would allow us to get rid of the current usage of caller.

thoughts?

I kinda managed to make this not a problem; the constraint of a single on-load is fine I think, just requires more tight design.

why constrain ourselves to a single load handler? i'm not sure i understand the arbitrary limit, when competing libraries like react, virtual-dom, etc have no such limit.

ahdinosaur commented 7 years ago

this is too hard, maybe not a good idea.

yoshuawuyts commented 7 years ago

haha, actually coming around on this - being able to nest on-load is kinda needed :P