Bi-directional binding re-renders everything

[kvanbere]

You can see here:
http://jsfiddle.net/wwEBd/

This makes my application extremely slow and choppy. How should I proceed?

[kvanbere]

Both onkeydown and the bi-directional binding cause a render.

[lhorie]

The workaround for now is to use onkeydown for both enter key detection and data binding, but yeah, this is a design bug.

Gonna need to think of a proper way to deal with this scenario

[kvanbere]

Is the re-render on bidirectional binding also a bug, or intended?

When bindings are updated, they should not make the DOM dirty.

[lhorie]

by design, re-renders happen on every event handler. You can make a binding render less aggressively by using something like onblur instead of oninput (but in your case onkeydown is rendering regardless).

So there are two parts to this: one is that event handlers that get triggered simultaneously shouldn't cause more than one render, and there needs to be some way of conditionally preventing redraws (i.e. in this case, we only want to redraw if onkeydown has a keyCode == 13)

[darsain]

I thought that all rendering is deferred to next animation frame, which would mean that multiple events firing at the same time (or from now until the next animation frame) would still cause only one re-render.

So, why doesn't it work like that? :)

[lhorie]

I've modified the code to always use rAF, so you should no longer see double redrawing when both onkeydown and oninput are defined. This change is in origin/next. Note that this change makes redrawing asynchronous all the time now. In theory this shouldn't affect anyone unless they are trying to poke the DOM synchronously after a m.module or m.route call (which is what configs are supposed to be for).

There's still the design issue of how to conditionally prevent a redraw.

[kvanbere]

I'm thinking events like onkeydown should only redraw if the user uses m.startComputation/m.endComputation or explicit m.redraw() maybe?

In my use case this is actually easier than attempting to cancel it every time.

[kvanbere]

Also, can we have a macro like .computation( .. ) to make start/end safer? with automatic try/finally, something like

function computation(f) {
  try { return m.startComputation(), f(); }
  finally { m.endComputation(); }
}

.. although I'm not sure return works quite like that in a try/finally block.

This actually means we can do stuff like:

onkeydown: m.computation(originalHandler),

[ilsenem]

Maybe some kind of AngularJS $apply analog will do the trick? It is really much easier to use single entry point for computation detection.

[kvanbere]

I'm starting to think maybe even just removing startComputation and endComputation and giving redraw an optional function parameter like m.redraw(fn) would be cleaner and leave less api/code. computation is also kind of a stupid thing to call it, too.

[lhorie]

start/endComputation are meant to be used to defer redrawing in asynchronous cases, so wrapping them in a $apply-like function doesn't really work unless this function passes endComputation as a argument via callback passing style. The implementation is basically a counter that redraws when count reaches zero after a startComputation call:

//pseudo code for handling an event w/ ajax stuff
count = 0
internalOnclickHandler = function() {
  count++ //startComputation() - yields 1

  //app-space onkeydown callback----

  //m.request -----
  count++ //startComputation() - yields 2
  ajaxSomething().then(function() {

    //more app-space code here-----------------------------
    foo()
    bar()
    //end more app-space code------------------------------

    count-- // endComputation() //yields 0 => redraw
  })
  //end m.request() ----

  //end app-space onkeydown callback----------

  count-- //endComputation() - yields 1
}

For redrawing in the synchronous case, you can just use m.redraw()

So there are two ways to handle the spammy scenario:

1 - have events that don't redraw. This can be accomplished by removing the start/end calls in onkeydown and whatever else is spammy (which can cause surprises since some events now behave differently), or by explicitly adding support for some alternate event handler like onrawkeydown or whatever

2 - add an API call that disables the redraw, i.e. a m.preventRedraw()

[Raynos]

So there are two ways to handle the spammy scenario:

If your delaying redrawing until raf then why does the spammy scenario matter ?

[kvanbere]

Double event firing has been fixed, he's talking about redraws triggered by things like onkeydown which are "spammy".

[Raynos]

By spammy you mean you want to throttle the onkeydown redrawing by MORE then one draw per frame ?

[lhorie]

@Raynos no, it's so that we can do things like

element.onkeydown = function(e) {
  if (e.keyCode == 13) {
    redraw()
  }
  //otherwise don't even bother diffing
}

This is important if you listen to key combos like ctrl+space or whatever. You don't want the app running diffs on every frame while you've got your finger down on the ctrl key.

[Raynos]

@lhorie oh, I understand what you mean.

Can't you just hook into m.prop ? i.e. only redraw when an m.prop is mutated instead of redrawing after every DOM event

[Raynos]

@lhorie btw sync vs async rendering is a BIG deal.

React and Ractive have sync rendering by default for good reasons, there systems involve reading the DOM immediately after rendering and users of the frameworks assume that the DOM is in the "new state" immediately after rendering.

There was a recent thread on the react mailing list about why async rendering by default causes subtle bugs and race conditions.

om and mercury have async rendering by default for performance reasons. However both of these frameworks encourage people to not read from the DOM. Mercury has a documented pattern of how to deal with "I want this code to run once this view has asynchronously rendered" to deal with the small edge case of wanting to read from the DOM after a render.

I highly recommend you clearly document and broadcast the change from sync to async rendering. Maybe even by bumping the major version number (it's a breaking change).

[lhorie]

@Raynos re: render on m.prop change - no. Mithril supports POJOs a la Angular, so that would be a major breaking change.

Anyways, I'd like to do a bit of refactoring on this before publishing it. Some scenarios can still be handled synchronously and there's no good reason for them not to be.

[lhorie]

FYI, I'm going to release v0.1.19 without the preventRedraw feature, because #157 needs to land on stable.

[lhorie]

The keydown spamming issue can now be addressed with the new API described in #159

This is in origin/next and scheduled to be released in v0.1.20

[kvanbere]

Thanks!

[lhorie]

FYI, the next release is reverting the change made for this issue because this causes #214 and #288. If you have issues with double redrawing from onkeypress+oninput, you should use m.redraw.strategy("none") on one of the events from now on