Too many calls to forceUpdate

[duhseekoh]

I've recently used a similar mixin for React. What I've seen so far with using it is that N number additions to a collection triggers forceUpdate N number times.

I changed:

model.on('add remove reset sort', function () { this.forceUpdate(); }, this);

to this (assuming inclusion of underscore or lo-dash):

this._throttledForceUpdate = _.throttle(this.forceUpdate.bind(this, null),  1000);
model.on('add remove reset sort', this._throttledForceUpdate, this);

At most, force update will be called once a second here. Interested in your thoughts on this.

[clayallsopp]

Seems pretty reasonable to me - is throttle preferable to debounce in this case?

[duhseekoh]

With throttle you get the immediacy. So when that first add event or set of add events that synchronously come in, the forceUpdate is called right away. It's also called once at the end too, which is probably the only potential downside, but one extra call to react's render is most likely negligible.

With debounce, before anything is seen, there is at least that waiting period of 1000ms (or whatever its chosen to be). If debounce were used, it'd probably be best to bring that time down to 10ms (to give the sense of immediacy) or some amount that is just enough for the collection to trigger all of the add events before debounce is open to another call.

[clayallsopp]

Ahhh yeah, good call. I've added this in 865e32267e58bf4f96cdf9617fe7183d8b7907d3 - I turned down the timeout to 500ms, and made the throttle wrapper a local variable. Take a look, but should be good to go

[rdworth]

This causes a rendering issue with a paged collection. See http://jsfiddle.net/54pfU/ . Click next page a couple times and notice the first remove is rendered and not until 500ms later does the rest of the list update. From where I'm sitting, I don't see an issue with calling forceUpdate N times because React already has infrastructure in place to ensure rendering is batched. And if it ever needs to be worked around on a case-by-case basis, that can be done in React rather than here. Or am I missing something?

[duhseekoh]

From what I understand, React batches DOM updates, but doesn't batch calls to render. Also, it's not batching calls to update its 'virtual dom'. It's doing that N number of times. Any logic outside of actually manipulating the DOM in the render method will be called N number of times. Throw a console.log statement in there.

I see your issue here, which may mean debounce with a very small delay (~10ms?) would work around this.

[clayallsopp]

Curious if @petehunt / @zpao / @spicyj have an opinion on this

[petehunt]

@dicicco2 we do batch calls to render(): http://jsfiddle.net/SfybB/

By default this is only enabled within event handlers, but you can use my react-raf-batching package on npm to enable this for things outside of event handlers too.

[rdworth]

@dicicco2 thanks. With my test case, debounce 10ms did the trick

[sophiebits]

I would assume that debounce 0 would work as well.

@duhseekoh How are you adding N elements to a collection? Maybe it's possible to receive only one event from Backbone.

[gaearon]

@duhseekoh @clayallsopp

I may have the wrong understanding, but _.debounce accepts a third parameter called immediate. If you set it to true, the debounced function will be invoked without the delay the first time:

Pass true for the immediate parameter to cause debounce to trigger the function on the leading instead of the trailing edge of the wait interval.

You don't need throttle for this.