WebReflection / hyperHTML

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

Uncaught TypeError: item.render is not a function #255

Closed justintaddei closed 6 years ago

justintaddei commented 6 years ago

I am getting this error upon rendering my custom element: Uncaught TypeError: item.render is not a function at ...\hyperhtml\cjs\objects\Updates.js:44

The code throwing the error in hyperHTML:

// returns nodes from wires and components
const asNode = (item, i) => {
  return 'ELEMENT_NODE' in item ?
    item :
    (item.constructor === Wire ?
      // in the Wire case, the content can be
      // removed, post-pended, inserted, or pre-pended and
      // all these cases are handled by domdiff already
      /* istanbul ignore next */
      ((1 / i) < 0 ?
        (i ? item.remove() : item.last) :
        (i ? item.insert() : item.first)) :
      asNode(item.render(), i));
}

I wish I could provide more information but I honestly have no idea why this is happening. If you know the general reason such an error would be thrown I can look more into it and report my findings. I would post my code but it is for a closed source project.

pinguxx commented 6 years ago

can you reproduce this in a codepen?, hard to tell without any code to review

justintaddei commented 6 years ago

I would expect I don't know which part of the code is the problem. I can try to explain the code and logic leading to the error.

I have an array of MyData objects which I Array.map over to create a list.

interface MyData {
  type: "single" | "group";
  name?: string;
  ids: string[];
}

So let's say I have

this.myDataList = <MyData>[ . . . ]

class MyDataElement extands HyperHTMLElement {

private myDataList: MyData[] = [];

constructor() {
   super();
}

created() {
    this.myDataList = getDataList();
    this.render();
}

addID(id) {
    this.myDataList.push({
      type: "single",
      ids: [id]
    });

    this.render();
}

render() {
    // Other unrelated logic here

    return this.renderList()
}

renderList() {
   return this.myDataList.map(data => this.renderData(data));
}

renderData(data) {
    if (data.type === 'single')
        return this.renderSingle(data.ids[0]);

    return wire()`
        <li>
            <ul>
                ${data.ids.map(id=>
                  this.renderSingle(id)
                )}
            </ul>
        </li>
    `
}

async renderSingle(id) {
    const remoteData = await getData(id)

    return wire()`
        <li id=${id}>
            Name: ${remoteData.name}
        </li>
    `
}
}

The issue only occers when myDataList is as follows:

MyDataElement.myDataList = [
  {
     type: "group",
     ids: ["a","b","c", ....]
  }
]

And I call MyDataList.addData("some_id") (not as a static method but on the instance of course).

That is to say that if myDataList contains a "group", and I push on a "single" it fails to render. And only when a "group" is the first item in the array.

pinguxx commented 6 years ago

maybe its because this

...
if (data.type === 'single')
        return this.renderSingle(data.ids[0]);
...

returns a promise not a wire right?

WebReflection commented 6 years ago

I wish I could provide more information

pasting a core function that works everywhere isn't really helping much, but I can tell you you might have an array of mixed items, which IIRC is not allowed.

if you have [dom, promise, dom, wire] as single interpolation that's going to break, so that as soon as you have the possibility to return a promise, within a list of items, you better return such list as Promise.all(...)

TL;DR please check if this change would solve your issue:

renderList() {
   return Promise.all(this.myDataList.map(data => this.renderData(data)));
}
justintaddei commented 6 years ago

I was unaware of that limitation. I will try Promise.all as soon as I'm home and report back. Thank you very much for your help.

justintaddei commented 6 years ago

Promise.all() did the trick! Thank you so much and I apologize for the vagueness of the codebase.

WebReflection commented 6 years ago

I was unaware of that limitation

it's a compromise for performance. If I have to check thousand of items and react to every possible different case, hyperHTML would loose every benchmark battle.

99% of the time, your list is always either already known, or fully asynchronous.

Glad I've solved though. Happy coding!

justintaddei commented 6 years ago

That makes a lot of sense. Maybe it's worth mentioning in the docs?

WebReflection commented 6 years ago

it surely is, will do that soon