Redraw proposal

[dead-claudia]

/cc @pygy @tivac @lhorie

Building on #1765, #1776, #1165, #1837, among others, I'm making a more universal proposal that:

• Unifies m.redraw(), m.redraw.sync(), and granular redraws
• Provides better defined behavior for edge cases for both redrawing and mounting
• Provides the ability to await async rendering as appropriate

This should hopefully catch all three in a way that makes sense and has much better defined behavior.

1. m.redraw(vnode?) schedules an async redraw, with all roots by default or optionally with just a single vnode, and returns a Promise resolved when done.
2. m.redraw.sync(vnode?) performs a sync redraw, with all roots by default or optionally with a single vnode, and blocks until done. If we are currently rendering any root, calls to this will throw an error, even if vnode or its parents were not scheduled to be updated this iteration.
3. m.mount(elem, component?) always performs a sync render/redraw. It's uncommon that this is called more than once in the same frame, and it allows us to batch the usually first render at the same time as the browser's first or (if delayed with DOMContentLoaded) second paint. In addition, it'll make it easier for people to test the state of their DOM in unit and integration tests.
   • This is done using the same rules as m.redraw.sync(), called without the vnode.
4. m.redraw.safe() returns whether it's safe for a sync redraw. This returns false if and only if we are currently rendering any root, and allows people to know if m.redraw.sync is safe to run without resorting to an unsafe try-catch that may catch other legitimate errors.

In the event m.redraw is called with a vnode:

1. If the vnode isn't actually mounted to any root, throw an error as a safety check.
2. If no global redraw is scheduled, nor the vnode itself already enqueued, enqueue the vnode.
3. Return a promise resolved after the next redraw completes, rejected if rendering the vnode itself errors.

In the event m.redraw is called without a vnode:

1. If a global redraw is already scheduled, return a promise resolved after the next redraw completes.
2. Set a flag marking a global redraw is scheduled.
3. If any vnode was scheduled, ignore the vnodes in the queue when redrawing (they'll get addressed anyways), but still call the resolving/rejecting functions.
4. Return a promise resolved after the next redraw completes.

---

Does this help in making the redraw story more consistent, more powerful, and easier to understand? Also, did I miss anything glaring with this?

[JAForbes]

I much prefer m.redraw.sync to m.redraw(true).

I'm envisioning a lot of code like m.redraw.safe() && m.redraw.sync(). Could we collapse that into

m.redraw.sync(() => {
  // This code only executes if its safe.
})

Could we have a separate function for vnode specific redraws? E.g. m.redraw.local( vnode ). Because if not we could be right back to .then( m.redraw ) having unexpected behaviour.

The naming could be anything, I'm just looking for a specific function.

[pygy]

@isiahmeadows 👎 on A.2 and A.4, you said in #1765 that you had changed your mind about having m.redraw.sync schedule a redraw in the same tick when called from hooks "for obvious reasons", but those reasons are not at all obvious to me. It leaves the door open for infinite redraw loops, but those could be truncated.

For A.3, m.mount should only render the tree that is mounted (that's how it works in #1592), independently of what m.redraw() does.

I'm also ambivalent about returning Promises (having once championed such hooks). You can achieve similar results from the views an hooks, in a way that is often more declarative.

[dead-claudia]

@pygy

First, I'll start out by saying that this proposal is where I stand right now. My views have changed over time and this best represents where I stand now. I have taken previous points and suggestions, and I have backed off some of my previous opinions because of concerns that you and others brought up. I'd rather have someone tell me where I went wrong over building something that's broken by design.

👎 on A.2 and A.4, you said in #1765 that you had changed your mind about having m.redraw.sync schedule a redraw in the same tick when called from hooks "for obvious reasons", but those reasons are not at all obvious to me. It leaves the door open for infinite redraw loops, but those could be truncated.

Let me clarify why I wanted to disallow it: It could lead to race conditions and leave Mithril in an invalid state, just like in nested m.render() calls. Mithril does employ occasional global state for its own bookkeeping, and allowing nested renders can mess up with that global state. That's why I proposed throwing.

I think that if throwing is a problem, we should just be more robust against thrown errors in the first place. They might be exceptional for the end-user, but internally, they're a very normal result for us, something we should account for.

For A.3, m.mount should only render the tree that is mounted (that's how it works in #1592), independently of what m.redraw() does.

Note that I said render/redraw, without preference on what method is used. I agree that we should only render; I only lumped the two together to emphasize the timing.

I'm also ambivalent about returning Promises (having once championed such hooks). You can achieve similar results from the views an hooks, in a way that is often more declarative.

I know this. We could also optionally accept a callback, but there is the small legitimate use case of needing to know when it finishes. It primarily shows up in the case of interactive components, where you need to redraw on user input, and you have further work to do afterwards on the updated DOM. It's not common at all, but we should still enable it.

Although, now that I think of it, most of those uses could just use m.redraw.sync(), since it's almost never redrawing in those cases. You may have a point.

[dead-claudia]

@JAForbes

m.redraw.sync() would usually be called in things like non-element DOM event handlers, where you would need to react to an external event that isn't managed by Mithril. (Progress notifications come to mind here.) And usually, the callee can easily determine that Mithril should never be redrawing.

m.redraw.safe() is for the 0.1% of the time that you don't already know if Mithril is redrawing or not. IMHO it may not be necessary, but it's trivial to add regardless, since we have to perform the same exact check in m.redraw.sync().

Could we have a separate function for vnode specific redraws? E.g. m.redraw.local( vnode ). Because if not we could be right back to .then( m.redraw ) having unexpected behaviour.

I'm assuming that would look like this:

• m.redraw.local(vnode)
• m.redraw.sync.local(vnode)

I'm okay with it. Just more of a +0, since we can just assert that vnode is really a vnode before scheduling the local redraw.

[ludbek]

Where are we with this proposal? Have we decided the new api for m.redraw?

[dead-claudia]

@ludbek I'm awaiting @pygy's response to this comment, and hoping for some feedback from @tivac and/or @lhorie if possible.

[pygy]

@isiahmeadows if you don't mind, I'm going to implement #1592 and m.redraw.sync() as strictly synchronous (as per your last suggestion in #1765) plus m.redraw.safe().

The intermediate m.redraw.asap() (guaranteed to succeed in the current tick, but Zalgo-ish in nature) can wait (maybe indefinitely). Hooks and granular redraws can be implemented later.

[dead-claudia]

I'm good with you going ahead and implementing it.

Hooks and granular redraws can be implemented later.

Yeah, I was just suggesting that as more of a big-picture thing to ensure that if/when it gets implemented, we have an API in the end for that feature that makes sense.

[tivac]

#1592 implements the big API bits of of this proposal that we agreed we wanted.

It doesn't handle the m.redraw(vnode?) case but I'm not sure how much we care. @isiahmeadows @pygy is this closable now?

[dead-claudia]

@tivac I'd be okay with closing this once we make a decision on m.redraw.safe() (which was missing from #1592, possibly by mistake @pygy?). I filed #1907 for subtree rendering.

[dead-claudia]

Although m.redraw.safe() could probably be better done as m.render.locked(elem) (i.e. it contains the one we're rendering), to make it more obviously applicable to other situations (like the majority need for #1868).

[tivac]

Given the lack of progress/discussion on this I'm closing it out for now.