v2.2.0 may have made on-load too sensitive

[timwis]

I hope you'll pardon another choo-specific example, but I was trying to put together a demo for this fellow and noticed that on-load was being triggered every time the state changes, even if nothing actually changes in the DOM. It only started happening in v2.2.0. Here's a demo. Is that intended behavior?

[shama]

No problem! It will be cool to get this fine tuned to be able to lifecycle native dom elements (and not just do the easy and less performant way of adding an observer per element). So don't hesitate to report anything!

Firing an event on each update is the current intended behavior but I could experiment with detecting whether the node is the same as before and only data-onloadid attr has change, thus avoiding firing that event. Let me play around with some things and get back to you.

[timwis]

Interesting that that's intended behaviour -- and I can see how the behaviour I'm expecting may conflict with morphdom in general. I think it's really the equivalent of React's componentDidMount event. A hook you'd use to, say, initialize Leaflet, or fire off an ajax request when the view is mounted. Wouldn't that involve setting an unique data-onloadid on each element that onload is listening to, and keeping track of which IDs are currently mounted? (Rather than changing the element's id each time it's mounted) Anyway, thanks for working on this.

[knownasilya]

My issue might be related to this as well, see https://github.com/yoshuawuyts/choo/issues/1#issuecomment-229398499

[shama]

The hard part with native is uniquely identifying the element creator. As the element can be created from anywhere. Here is an earlier hacky attempt which uses the caller and error stack to identify the element.

I think we can get to the equivalent behavior to componentDidMount by being a bit more strict on attribute change mutations. I have some ideas I'll play around with...

if those ideas don't work out, the simple way to build the equivalent behavior is attach a mutation observer to each element instead of the global one here. It wont be as performant but could be improved later.

[shama]

@timwis @knownasilya Can you confirm this is the behavior you're looking for?

function page1 () {
  return onload(yo`<div>page1</div>`, function () {
    console.log('page1 on')
  }, function () {
    console.log('page1 off')
  })
}
function page2 () {
  return onload(yo`<div>page2</div>`, function () {
    console.log('page2 on')
  }, function () {
    console.log('page2 off')
  })
}

var root = page1()
document.body.appendChild(root)
// logs page1 on

root = yo.update(root, page1())
// nothing logged

root = yo.update(root, page2())
// page1 off, page2 on

root = yo.update(root, page2())
// nothing logged

document.body.removeChild(root)
// page2 off

[shama]

Pushed to https://github.com/shama/on-load/tree/3.0.0, give a try and let me know if that has the correct behavior now, thanks!

[timwis]

@shama it looks like it's working great! The above code you posted is working. Here's a slightly revised version that demonstrates it a little better:

var onload = require('on-load')
var yo = require('yo-yo')

function page1 () {
  var tree = yo`<div>page1</div>`
  onload(tree, function () {
    console.log('[page1 on]')
  }, function () {
    console.log('[page1 off]')
  })
  return tree
}
function page2 () {
  var tree = yo`<div>page2</div>`
  onload(tree, function () {
    console.log('[page2 on]')
  }, function () {
    console.log('[page2 off]')
  })
  return tree
}

var root = page1()
document.body.appendChild(root)
// logs page1 on

window.setTimeout(function () {
  console.log('updating with no changes')
  root = yo.update(root, page1())
}, 1000)
// nothing logged

window.setTimeout(function () {
  console.log('switching pages')
  root = yo.update(root, page2())
}, 2000)
// page1 off, page2 on

window.setTimeout(function () {
  console.log('updating with no changes')
  root = yo.update(root, page2())
}, 3000)
// nothing logged

window.setTimeout(function () {
  console.log('removing view')
  document.body.removeChild(root)
}, 4000)
// page2 off

And here's a test I made that is closer to my choo example above without being a choo example:

const onload = require('on-load')
const yo = require('yo-yo')

const page1 = (state) => {
  const tree = yo`<div>page1 ${state}</div>`
  onload(tree, () => console.log('page1 on'), () => console.log('page1 off'))
  return tree
}

const page2 = () => {
  const tree = yo`<div>page2</div>`
  onload(tree, () => console.log('page2 on'), () => console.log('page2 off'))
  return tree
}

let root = page1('foo')
document.body.appendChild(root) // logs page1 on

const controls = yo`
  <div>
    <button onclick=${() => root = yo.update(root, page1('foo'))}>Update page1, no changes</button>
    <button onclick=${() => yo.update(root, page1('bar'))}>Update page1, change state</button>
    <button onclick=${() => yo.update(root, page2())}>Switch to page2</button>
  </div>`

document.body.appendChild(controls)

For anyone on the thread, to try these out, use npm install shama/on-load#3.0.0 to install from github instead of npmjs.com.

@shama thanks again for your quick work on this!

[shama]

Published as 3.0.0. Thanks for your help @timwis!

[yoshuawuyts]

@shama wooooh! - you merging this upstream to yo-yo? :sparkles:

edit: was too fast in all excitement, just now saw https://github.com/yoshuawuyts/choo/issues/1#issuecomment-229623077; still needs a few more hoops to be jumped I guess hah :D

[shama]

Just published 4.4.0 of bel (no breaking changes, only adding a feature). So new installs of yo-yo now have the ability to onload.