snabbdom / snabbdom

A virtual DOM library with focus on simplicity, modularity, powerful features and performance.
MIT License
11.86k stars 1.11k forks source link

Skip the function equality check in thunks #93

Closed zhangchiqing closed 8 years ago

zhangchiqing commented 8 years ago

Hi,

I'm using thunks to optimize the rerendering of a big list. In my use case, I found the function equality check here makes it hard to tell snabbdom that the view is unchanged.

So in my use case, I'm rendering a big list, and I need to handle the click event on each single list item. I have a function to create the list item. This function takes two parameters, the first one is the click event handler, the other one is the view state.

var createListItem = R.curry(function(clickhandler, itemState) {
  return h('li', [
    h('a', {
      on: { click: clickhandler.bind(undefined, itemState) },
    }, itemState.name)
  ]);
});

var clickhandler = function(item) {
  console.log(item.url);
};

var view = function(model, action$) {
  return R.map(createListItem(clickhandler), model.items);
};

So I made some changes to use the thunk as below in order to optimize the rerendering when there is only one item gets updated

var view = function(model, action$) {
  return R.map(function(itemState) {
    return thunk(itemState.id, createListItem(clickhandler), itemState);
  }, model.items);
};

But since createListItem is curried, createListItem(clickhandler) will always return a new function even though it will return the same value as before. With the function equality check, it will be considered as state changed, which will trigger the rerender for every single item.

In my fork, I removed the function equality check, and then it works. But I wonder if there are better way to do it in the case where the createListItem function takes a curried function as parameter.

Thanks!

paldepind commented 8 years ago

Would it be possible for you to define createListItem(clickhandler) outside of your rendering? Then you would also avoid creating new function in every iteration of R.map.

Otherwise I think we should consider removing the function equality checking.

zhangchiqing commented 8 years ago

I need to bind each item's data to the clickhandler, so that when clicking on the list item, the clickhandler function can get the item's data.

clickhandler.bind(undefined, itemState)

To move the clickhandler function outside of the rendering, I have to put the data to the dom element's data attributes (not sure if there is better way). Then in the clickhandler I could read the data from the dom element's attribute. In this way, I can share this clickhandler function across all list items, but it's kind of hacky :p

I think there are still cases where the item's state could be a more complex object, which I can't easily put the data to dom's data attributes.

So yes, I think it makes sense to remove the function equality checking.

paldepind commented 8 years ago

I think you will be happy to discover that the event listeners module has a feature that makes it very easy to dynamically pass data to event listeners:

var createListItem = R.curry(function(clickhandler, itemState) {
  return h('li', [
    h('a', {
      on: {click: [clickhandler, itemState]},
    }, itemState.name)
  ]);
});

This avoids both the ugly bind and the unnecessary creation of event handler functions.

For more information see the documentation.

Let me know if this solves your problem.

Oh, and btw, Ramda has R.bind that is a bit nicer than passing undefined to bind.

paldepind commented 8 years ago

I am closing this. Feel free to reopen if the above does not solve your issue.