choojs / nanocomponent

🚃 - create performant HTML components
https://leaflet.choo.io/
MIT License
366 stars 30 forks source link

this.rerender() undefined #85

Closed sonn-gamm closed 6 years ago

sonn-gamm commented 6 years ago

Hi!

Noob here so I might just be missing something obvious or complicating things for no reason.

I am in a situation where I have to create a choo component that sets its width and height values by first loading an image as a child node, and then using that image to defines width and height of the root node. That works.

The problem comes when I add a resize EventListener and try to rerender() the component. If I log the function this.rerender() I always get undefined.

Some relevant code (file here):

load (el) {
  const resize = () => {
    if (el.firstChild.firstChild.nodeName.toLowerCase() === 'img') {
      this.width = el.firstElementChild.clientWidth
      this.height = (el.firstElementChild.firstChild.dataset.height / el.firstElementChild.firstChild.dataset.width) * el.firstElementChild.firstChild.clientWidth
    } else {
      this.width = el.firstElementChild.firstChild.clientWidth
      this.height = (el.firstElementChild.firstChild.firstChild.dataset.height / el.firstElementChild.firstChild.firstChild.dataset.width) * el.firstElementChild.firstChild.firstChild.clientWidth
    }

    this.rerender.bind(this)()
    console.log( this.rerender.bind(this)() )
}

resize()

window.addEventListener('resize', function () {
  resize()
})

}

update (el) {
  return true
}

and how I create the component (file here):

function entry (state, emit) {
  return ov(page[0].children).map(function (item, i) {

  var card = state.cache(Card, i)
    return card.render(state, emit, item, i, state.cards[i])
  })
}

Could it be that state.cache does not see that I am changing the width and height values when resizing the page, so... I should be explicit about that?

Thanks, af

tornqvist commented 6 years ago

Firstly, you've got the constructor arguments wrong in your Card component. They should be id, state and emit, and id should preferably be forwarded to super. But that shouldn't cause any problem in your use case, since you overwrite them in createElement.

Secondly, this.rerender() isn't supposed to return anything (hence undefined), it just rerenders the element in place. Also, there's no need for the bind in your use case.

Is the issue that you component doesn't rerender on resize, or was it just that rerender returns undefined?

sonn-gamm commented 6 years ago

Thanks for the tips!

My issue is that the component does not rerender on resize, but from what you are saying now, rerender is not supposed to do that?

bcomnes commented 6 years ago

You would have to hook an event listener to detect resize, and you could trigger a re-render at that point. You could do various ways. Since this event fires a lot, you may need to throttle or de-bounce it. You can use re-render to rerender and update the element, including resize or reinit canvases or whatever you want.

The above example looks like a scoping issue that I'm not spotting where the error might be.

Also, since this is image resizing and not something more complex, responsive CSS + Layout solutions likely exist that don't require event listeners and JS. That solution would be my inclination if possible.

sonn-gamm commented 6 years ago

Blessed the conversation I had with you guys!

In an attempt to try another solution, I replaced

this.width = el.firstElementChild.clientWidth
this.height = (el.firstElementChild.firstChild.dataset.height / el.firstElementChild.firstChild.dataset.width) * el.firstElementChild.firstChild.clientWidth

with

this.width = el.firstElementChild.clientWidth
this.height = (el.firstElementChild.firstChild.dataset.height / el.firstElementChild.firstChild.dataset.width) * this.width

so using the cached this.width value from the line above, and the resize began working correctly!

In order to better understand, what's the logic behind this? That I'm using a cached value?

bcomnes commented 6 years ago

Hard to parse exactly whats going on without more context, but using a cached value is sometimes necessary depending on the order of things.

sonn-gamm commented 6 years ago

My hunch without knowing how choo and nanocomponent work, is that maybe since I am using state.cache() to create the component, choo was not finding any reason to trigger a diff and therefore re-render the component?

function entry (state, emit) {
      return ov(page[0].children).map(function (item, i) {

        var card = state.cache(Card, i)
        return card.render(state, emit, item, i, state.cards[i])
      })
}

(ref)

Anyway, glad this worked out after 5 weeks, and thanks for the chat, very helpful!

goto-bus-stop commented 6 years ago

In your first line you're using:

el.firstElementChild.clientWidth

as the this.width value. But in the second line you're multiplying by

el.firstElementChild.firstChild.clientWidth

that's probably what's causing the difference

sonn-gamm commented 6 years ago

Ahah, yeah! You're actually right — just tested 😅

Thanks @goto-bus-stop for the extra eyes on the code 👍