Memoized elements trigger onload on every render

[timwis]

I've noticed that if you memoize your el, the onload callback gets called at every re-render. I've written up a demo that compares a non-memoized sub view to a memoized sub view.

const choo = require('choo')
const html = require('choo/html')
const app = choo()

app.model({
  state: {
    title: 'Hello'
  },
  reducers: {
    update: (data, state) => ({ title: data })
  }
})

const view = (state, prev, send) => {
  return html`
    <div>
      <button onclick=${() => send('update', 'world')}>Update state</button>
      ${subView(state, prev, send)}
      ${memoizedSubView(state, prev, send)}
    </div>`
}

const subView = (state, prev, send) => html`<div onload=${(el) => console.log('subView loaded')}>subView</div>`

let memoizedEl
const memoizedSubView = (state, prev, send) => {
  memoizedEl = memoizedEl || html`<div onload=${(el) => console.log('memoizedSubView loaded')}>memoizedSubView</div>`
  return memoizedEl
}

app.router((route) => [
  route('/', view)
])

const tree = app.start()
document.body.appendChild(tree)

Any idea why?

[shama]

Hmm weird, I can repro this but not sure why. I'll keep looking into a fix.

[shama]

It's interesting, given this example:

var saved = null
function parent () {
  saved = saved || onload(yo`<div>parent</div>`, function () {
    results.push('parent on')
  }, function () {
    results.push('parent off')
  })
  return saved
}
function app () {
  return yo`<div>${parent()}</div>`
}
var root = app()
document.body.appendChild(root)
setInterval(function () {
  root = yo.update(root, app())
}, 1000)

For some reason each call to yo.update() is quickly removing and adding the same node. We could fix this here by queueing events, so if an load is called immediately after an unload, it ignores it. But morphdom shouldn't be updating the node in the DOM to begin with.

We should look into a fix on morphdom or see how @yoshuawuyts's https://github.com/yoshuawuyts/nanodiff pans out. Or try diffhtml again. As a better way to fix this.

[yoshuawuyts]

Or try diffhtml again

Quick reply (I'm about to head out again): I tried minifying diffhtml and it's like 10kb; would def be a step back 😢 - despite the relatively large investment, creating our own fork of morphdom might be the best way to go - means we could merge in the event-copy code from yo-yo turning it into truly just a grab bag of packages.

[timwis]

@yoshuawuyts @shama I've started drafting tests for expected functionality. Is this format helpful? Or would pseudo code or requirebins be better? Happy to add more if you can verbally describe them.

[yoshuawuyts]

@timwis looking neat!

[shama]

@timwis Cool! Yep those are helpful. We could even add choo directly related tests here if you want. I can give you commit access.

The test.js file is at the point where it should be split into it's own folder. We can start adding choo related test files there and just test.skip any desired behavior not yet implemented.

[timwis]

Hey guys,

We should look into a fix on morphdom

Does morphdom v2 help this issue? (As far as I know, we can't really use things like maps in choo without this because L.init() alters the <div id="map"></div> by adding child nodes, styles, etc. so morphdom will always try to patch it at each re-render)

[timwis]

Update: just tested your test case above with morphdom 2 and I'm seeing the same result :-/

[yoshuawuyts]

Updated test case to work with morphdom#77:

const html = require('choo/html')
const choo = require('choo')
const app = choo()

app.model({
  state: {
    count: 0,
    toggled: true
  },
  reducers: {
    update: (data, state) => ({ count: state.count + 1 }),
    toggle: (data, state) => ({ toggled: !state.toggled })
  }
})

const view = (state, prev, send) => {
  return (state.toggled)
  ? html`
    <div>
      <h1>count: ${state.count}</h1>
      <button onclick=${() => send('update')}>Update state</button>
      <button onclick=${() => send('toggle')}>Toggle memo</button>
      ${subView(state, prev, send)}
      ${memoizedSubView(state, prev, send)}
    </div>`
  : html`
    <div>
      <h1>count: ${state.count}</h1>
      <button onclick=${() => send('update', 'world')}>Update state</button>
      <button onclick=${() => send('toggle')}>Toggle memo</button>
      ${subView(state, prev, send)}
    </div>`
}

function subView (state, prev, send) {
  return html`
    <div onload=${(el) => console.log('subView loaded')}>
      subView
    </div>
  `
}

let memoizedEl = null
let mounted = false
function memoizedSubView (state, prev, send) {
  if (!memoizedEl) {
    memoizedEl = html`
      <div
        onunload=${(el) => {
          console.log('memoizedSubView unloaded')
          mounted = false
        }}
        onload=${(el) => console.log('memoizedSubView loaded')}>
        memoizedSubView
      </div>
    `
    mounted = true
    return memoizedEl
  } else if (!mounted) {
    mounted = true
    return memoizedEl
  } else {
    const placeholder = html`<template></template>`
    placeholder.isSameNode = (el) => el.isSameNode(memoizedEl)
    return placeholder
  }
}

app.router(['/', view])

const tree = app.start()
document.body.appendChild(tree)

Tested and works 🎉🎉🎉🎉🎉🎉

edit: published a playground with the patched dependencies checked in ✨