Mithril.js: Calling m.redraw in a lifecycle method

[andraaspar]

The documentation contradicts itself. In describing the auto-redraw system, it gives an example of calling m.redraw inside oncreate, and says:

If you need to explicitly trigger a redraw within a lifecycle method, you should call m.redraw(), which will trigger an asynchronous redraw.

In the API description for m.redraw, it goes:

You should not call m.redraw from a lifecycle method. Doing so will result in undefined behavior.

Which one is correct?

[isiahmeadows]

It's been discussed before, with the current semantics discussed here, and the docs are partially wrong in the first case, and completely wrong in the second:

• If outside a lifecycle method, it executes synchronously.
• If inside a lifecycle method, it executes asynchronously.
• In both cases, it's throttled, with excess calls dropped.

Note that there is an outstanding proposal to not drop the excess calls: #1728

[pygy]

Actually, that's not completely true.

function throttle(callback) {
    //60fps translates to 16.6ms, round it down since setTimeout requires int
    var time = 16
    var last = 0, pending = null
    var timeout = typeof requestAnimationFrame === "function" ? requestAnimationFrame : setTimeout
    return function() {
        var now = Date.now()
        if (last === 0 || now - last >= time) {
            last = now
            callback()
        }
        else if (pending === null) {
            pending = timeout(function() {
                pending = null
                callback()
                last = Date.now()
            }, time - (now - last))
        }
    }
}

Here's the throttling logic in full. Each mount point (routed or not) has a throttled, private redraw() function. The are all called in sequence on m.redraw().

• If there were no redraws in the last 16 milliseconds, the private redraw() is synchronous.
• If a redraw occurred in the last 16 milliseconds and no redraws are scheduled, schedule a redraw for the next frame.
• Otherwise don't do anything.

When m.redraw() is called from synchronous hooks (i.e. all but deferred onremove), it means that a redraw() just occurred for the current mount point and none are scheduled yet, hence the async behavior.

Out of hooks, if there were no recent m.redraw(), calling it twice in a row causes two redraws, one synchronous, and one on the next frame (this is also true for a delayed onremove).

This is further complicated by the fact that m.route.set() and history.back() / .forward() (plus browser nav) trigger the redraw throttle logic, but only for the routed mount point. The rest of the UI is not refreshed.

Also, if there's a m.redraw() call from the hooks of a routed mount point, after a route change, it will trigger a synchronous redraw of all other mount points, and schedule a redraw for the routed mount point on the next frame.

In other words, it's a mess :-)

[isiahmeadows]

@pygy Well...that's confusing, and a beast to document.

I feel that the implementation could be rewritten, though, in a way that actually makes more sense.

[pygy]

@isiahmeadows the route-specific weirdness can be addressed separately

throttling m.redraw() rather than the individual roots is definitely appealing. I wonder why @lhorie went the granular way... In pre-v1 times, each root had its own redraw() method tacked on, but that went away with the #1404 fix. I don't know/remember why it was kept that way.

We'd also have to re-think how to do the first draw for mount() and route() (#1592 addresses that IIRC). I think that each mount/route call should only draw the corresponding root, synchronously if possible.

Other remarks about your proposed changes:

• having the throttle logic abstracted away allows to turn async tests into sync ones (as done in #1592) which is nice because we get rid of the flaky tests that still plague the test suite.

For your "from scratch" version:

• I'm not sure getting rid of the closure is worth it here. This code is not perf sensitive... and you'll have to bind that state before exposing m.redraw() which would create a closure anyway.
• you're passing state to render() what's that for?

[isiahmeadows]

@pygy

throttling m.redraw() rather than the individual roots is definitely appealing. I wonder why @lhorie went the granular way...

We'd also have to re-think how to do the first draw for mount() and route() (#1592 addresses that IIRC). I think that each mount/route call should only draw the corresponding root, synchronously if possible.

I started toying around with my idea, and began to realize this was an issue. You'll have to restructure the redraw API to do it, and subscriptions have to be something manipulable first-class. In particular, you'd have to make mount points first-class and redraw a method of them (rather than a global m.redraw()). Also, you'd have to keep the root trees separate.

Other remarks about your proposed changes:

• having the throttle logic abstracted away allows to turn async tests into sync ones (as done in #1592) which is nice because we get rid of the flaky tests that still plague the test suite.

I agree, although you'd need to get those from the $window instead of globally. I updated my gist to reflect this.

For your "from scratch" version:

• I'm not sure getting rid of the closure is worth it here. This code is not perf sensitive... and you'll have to bind that state before exposing m.redraw() which would create a closure anyway.

I usually idiomatically avoid closures in part due to personal preference (I think in a more data-driven, procedural style rather than OO/functional/etc.), but also because they are occasionally prone to memory leaks. I added "with my idioms" to the description, to further clarify that I meant it as just my style rather than what I'd put in core. (I'd put the closure-based one into core normally.)

It usually results in larger source code, but I also prefer finer grained control over procedural steps, so I have more room for optimization. I prefer state to be explicit over implicit, because it's easier to track what you actually need vs what's really there.

• you're passing state to render() what's that for?

That's the same state that lives in the closure-based version. The only difference is how that state is created, in an object or in a closure. It represents the same data either way.

[akihikodaki]

Hi, I found a strange behavior when calling m.redraw in oncreate.
Here is a PoC. https://jsfiddle.net/kgv3r8t7/5/
In short, m.redraw in oncreate works for m.mount, but it doesn't for m.route. Isn't it expected to be redraw also for m.route?

[isiahmeadows]

@akihikodaki Read from here. TL;DR: it's a mess and the reason this bug exists.

[isiahmeadows]

Closing since this is indirectly resolved via m.redraw() being always async and m.redraw.sync() being always sync with semantics to address this specific issue.