createElement called multiple times even when update always return false

mafintosh commented 5 years ago

There is a decent chance I misunderstood the README, but I was expecting createElement only be called once if update always returned false, to make embedding of other DOM elements easier.

This doesn't always seem to be the case when using multiple components. Here is a test-case:

// save as app.js

var html = require('nanohtml')
var morph = require('nanomorph')
var Button = require('./component.js')

var c1 = new Component('a')
var c2 = new Component('b')

var tick = 0

// render both 10 times
for (var i = 0; i < 10; i++) {
  render(c1)
  render(c2)
}

// some render function that just updates a counter
function render (c) {
  const b = html`<body>
      ${c.render('hi:' + tick++)}
    </body>`

  morph(document.body, b)
}

// save as component.js

// This component is the one from the README in the section about mutable components.
// Modified to write out every time createElement is called.

var Nanocomponent = require('nanocomponent')
var html = require('nanohtml')

class Component extends Nanocomponent {
  constructor () {
    super()
    this.text = ''
  }

  createElement (text) {
    console.log('Calling createElement')
    this.text = text
    return html`<h1>${text}</h1>`
  }

  update (text) {
    if (text !== this.text) {
      this.text = text
      this.element.innerText = this.text   // Directly update the element
    }
    return false                           // Don't call createElement again
  }

  unload (text) {
    console.log('No longer mounted on the DOM!')
  }
}

module.exports = Component

Running the above app will result in 20 createElements being written out. If only one component is used then it's only written out once. Is that working as expected? If so how would you embed a third party dom element.

For now we've published a fork of nanomorph that supports short circuiting "guarded" elements for this using https://github.com/hyperdivision/nanomorph/blob/master/index.js#L63 and https://github.com/hyperdivision/nanomorph-guard

Thanks for all the great stuff btw! :)

ungoldman commented 5 years ago

I think this is working as expected! BECAUSE!

In your example:

render(c1) replaces the content of body with the resultant html from const b
render(c2) replaces the content of body with the resultant html from const b
c1 is no longer in the DOM. unload is called
render(c1) replaces the content of body with the resultant html from const b
since c1 is not in the DOM, the getter for this.element returns undefined
createElement is called for c1
render(c2) replaces the content of body with the resultant html from const b
c2 is no longer in the DOM. unload is called
....

So removing the component from the DOM, then rendering it in the DOM, causes createElement to fire again.

Nanocomponent works great with third party DOM elements as long as you don't remove the enclosing Nanocomponent from the DOM. Hopefully that makes sense!

Here's a working example incorporating your example from nanomorph-guard:

var morph = require('nanomorph')
var html = require('nanohtml')
var Component = require('nanocomponent')

class Example extends Component {
  constructor () {
    super()
    this.tick = 0
    this.internal = 0
  }
  createElement () {
    this.tick++

    var interval = setInterval(() => {
      if (!this.element) clearInterval(interval)
      this.internal++
      var p = this.element.querySelector('#internal')
      p.innerText = `Example internally updated ${this.internal} times on ${Date.now()}`
    }, 500)

    return html`
      <div>
        <p>Example createElement called ${this.tick} times</p>
        <p id="internal"></p>
      </div>
    `
  }
  update () {
    return false
  }
}

var widget = new Example()
var p = document.createElement('p')
var morphs = 0
var manuals = 0

function updateMorph () {
  console.log('updateMorph', morphs++)
  morph(document.body, html`<body>
    <p>Updated ${morphs} times by nanomorph on ${Date.now()}</p>
    ${widget.render()}
    ${p}
  </body>`)
}

function updateManual () {
  console.log('updateManual', manuals++)
  p.innerText = `Updated ${manuals} manually on ${Date.now()}`
}

updateMorph()

setInterval(updateManual, 1000)
setInterval(updateMorph, 5000)

Hope that helps!

bcomnes commented 5 years ago

Thanks @ungoldman !

ungoldman commented 5 years ago

Was wondering if you could use npx to execute a bin from an arbitrary github repo... you can!

npx https://github.com/ungoldman/nanocomponent-issue-88

bcomnes commented 5 years ago

I wonder if it works on gists.

ungoldman commented 5 years ago

Hopefully this answered your question @mafintosh.

Closing this issue for now.

mafintosh commented 5 years ago

Thanks @ungoldman, that explains the behaivor. I'd suggest we update the example in the README to be more clearer about this, https://github.com/choojs/nanocomponent#mutating-the-components-instead-of-re-rendering, to not confuse someone else :)

choojs / nanocomponent

createElement called multiple times even when update always return false #88