WebReflection / hyperHTML

A Fast & Light Virtual DOM Alternative
ISC License
3.07k stars 112 forks source link

Disappearing lists #39

Closed joshgillies closed 7 years ago

joshgillies commented 7 years ago

Trying to track down the cause of this issue, and identify if it's a bug in hyperHTML or an issue with my implementation.

Given the below example I expect a list of buttons to render; initially as:

<button>Hello World!</button>

Then update as:

<button>Hello World!</button><button>Hello Friend!</button>

Instead of the expected result, the elements are removed entirely from the DOM after the initial update.

const hyperHTML = require('hyperhtml')

function Button () {
  const html = hyperHTML.wire()
  return function update (data) {
    return html`
      <button>
        ${data.text}
      </button>
    `
  }
}

function List (component) {
  const html = hyperHTML.wire()
  const items = []
  return function update (list) {
    return html`${
      list.map((item, i) =>
        (items[i] || (items[i] = component()))({ text: item }))
    }`
  }
}

const buttonList = List(Button)

const app = hyperHTML.bind(document.body.appendChild(document.createElement('div')))

app`${buttonList(['Hello World!'])}`

setTimeout(() => buttonList(['Hello World!', 'Hello Friend!']), 1000)
joshgillies commented 7 years ago

Note in the above example, having the List component return a template wrapped in a node produces the expected output. eg:

hyperHTML.wire()`${list.map(/*...*/)}` // works for first render
// vs.
hyperHTML.wire()`<div>${list.map(/*...*/)}</div>` // works for first and all subsequent renders

It might be worth noting somewhere in the docs that for a hyperHTML.wire() to work correctly it's template requires at least one top level node?

WebReflection commented 7 years ago

It might be worth noting somewhere in the docs that for a hyperHTML.wire() to work correctly it's template requires at least one top level node.

you mean ... this is not explicit enough? https://github.com/WebReflection/hyperHTML/blob/master/GETTING_STARTED.md#remember-to-see-content-you-need-a-live-node

image

About your issue, the problem is that you can never update anything without using a tag

hyperHTML is all about tagging templates, if you just invoke functions you passed along, you'll just invoke those functions and no magic whatsoever will happen.

// this is wrong
setTimeout(() => buttonList(['Hello World!', 'Hello Friend!']), 1000)

// this is correct
setTimeout(() => app`${buttonList(['Hello World!', 'Hello Friend!'])}`, 1000)

Last, but not least, the problem with your code is that you return a wire instead of just returning a list.

// use this instead
function List (component) {
  const items = []
  return function update (list) {
    return list.map((item, i) =>
        (items[i] || (items[i] = component()))({ text: item }))
  }
}

A wire with a list of items will return just an array but it's template is as static as any other template so it will never change when the length of the list changes too.

In hyperHTML you can have string, nodes, or an array of strings and nodes. In this case you want a List to be handled like ... a List so all you have to do is to return it as such, and not as a wire.

TL;DR you cannot appendChild(hyperHTML.wire()) because wires are a mechanism to create hyperHTML content within main renders like the document body or any other node.

joshgillies commented 7 years ago

Ugh, thanks for the reminder... I knew I was doing something wrong there! 👍