[rewrite] suggestions for lifecycle methods

[pygy]

Appologies in advance for the stream-of-thoughts style, but my current, highly multitasking occupation (young kids, yeah!) leaves me little time for synthesis:

This was initially raised by @trueadm (back and forth with @barneycarroll):

is it me, or are lowercase event names hard to read? onbeforeremove is just asking for typos [...] even better, IMO created, attached, detached, updated, removed for elements [...] as DOM nodes can be detached but not actually removed (very useful when dealing with third party libraries)

I'm not sure the detached vs removed is relevant here since mithril has a recycling pool and third-party

Upon that I suggested the purely cosmetic

• creating => view => created
• updating => view => updated
• removing => done => removed

Which neatly highlights the symmetry between the handlers of the various phases.

While Barney seemed to like the idea, others were skeptical as the README was updated a few hours ago to announce stability.

---

Err, reading the code, I realize now that onbeforeremove and onremove are both called before removing the actual node. Wouldn't it be better to detach the DOM nodes then call onremove?

We could detect onbeforeremove.length == 2 => pass done and wait, otherwise go ahead and remove ASAP.

On the implementation/perf side, putting the done callback creation in a helper function that's only called when necessary would probably help.

[leeoniya]

FWIW, in domvm rather than passing a done callback into the hook, i check if a Promise is returned and use that to chain the actual node removal. This also doubles as an "intent" signal that can be used internally to properly coalese or .all() a bunch of stuff...though not all that complexity is fleshed out ATM.

[lhorie]

The rationale behind using onfoo convention is to mimic the naming convention of DOM level 1 event attributes (e.g. onerror, onbeforeunload). IMHO, using event names (e.g. error, load, transitionend) is appropriate for procedural APIs such addEventListener (where it's obvious from the method name that this is an on ... kind of thing), but not as self-descriptive for a declarative DSL like HTML or hyperscript.

Aso, as you said, I just changed the readme to indicate that the rendering API is stable now, and I'm using these hooks in a day job project, so I'm less inclined to change their names now (especially since onbeforeupdate has already been changed once)

I'm not super convinced that onbeforeremove is really an issue wrt typos

re: onremove being called before detaching, my thinking was that there's value in allowing reading DOM values prior to removal, but I couldn't think of any practical value in calling the hook after detachment

[pygy]

@leeoniya the trouble here is that Leo aims for IE9 compat, and, currently, the renderer only depends on renderer/node, not even on streams.

@lhorie Yes, I had read the conversations that lead to the current naming scheme, which does make some sense.

Regardless of the typo potential, what I'm now suggesting is to make the names and behaviors as regular and predictable as possible.

beforehook => DOM happens => afterhook

A potential application of post DOM removal hook would be benchmarking, but I understand it is a very peripheral goal.

Another thing was wondering (at least, for benchmarks it makes sense) is whether we should run the post-dom manipulation hooks from leaf to stem (the changes are AFAICT trivial: reverse the hooks iteration direction and call onremove after traversing the children rather than before).

I don't have a stronger case at this point.

Edit: also, apologies for not coming with this earlier, I only had the insight after Dominic suggested created and friends...

[lhorie]

For benchmarking you should be using m.render, imho. Do you mean you would want to benchmark subsections of the diff pass?

[thysultan]

can't onbeforeunload, on... all be normalised .toLowerCase() so that one could use onBeforeUnload as well?

[barneycarroll]

Written a lot of opinionated drafts and ditched them.

@pygy your point about keeping Mithril modular is kinda legit — it's nice not to have the one big ball of mud Mithril v0 eventually became — but it's a shame that streams module acknowledges the value of interoperable APIs and yet elects to make onbeforeremove's async API so difficult to compose, and inconsistent with other API surfaces with Greater Mithril.

On the other hand, I can now see the self-justifying beauty of Mithril v1 as a dependency-free and extremely lightweight collection of modules. When you consider that the whole self-sufficient package is development-dependency free, reliance on the inflexible and outmoded callback argument API makes total sense — you can use it standalone in a world that hasn't even heard of Promises compliance and do what it claims to do. It's just a shame that that trumps user intentions.

As @leeoniya says elsewhere, returning thenables makes excellent sense. It's a convention that's standard in JS front end land that aids composability and "don't make me think". I did this when I implemented the Mithril Exitable plugin, which is AFAIK the best example of bringing the functionality under discussion to Mithril v0, and provided a minimal guide on how you might achieve this depending on your environment and credible dependency conditions.

The thing that makes this conversation so difficult is that the plaintiffs are talking about use case scenarios but the current APIs are just the way they are and make sense on isolated academic levels. On a whimsical level, it'd be great to have user stories as counter-arguments to justify the current state of affairs.

---

Aside: if onremove occurs before DOM removal, why would you even need it? To all practical intents and purposes, why are there distinct onbeforeremove and onremove methods?

[barneycarroll]

Forgot to mention, WRT the supposed value of totally independent Mithril modules: I think it would be a shame if the "make it smaller" drive-by comments were more influential to this disparity of API conventions than the credible use case stories of people who actually used the library. Mithril has always been tiny. If these modules are being kept inter-independent for their benefit, it may well be a pyrrhic academic victory.

[pygy]

@barneycarroll I don't remember ever arguing for modularity, but it's quite obviously where Leo wants to go, and I go along with the flow because it seems pointless to argue against what must have been a huge time investment for him.

How would you like to compose the promises beyond the Promise.all-like behavior implemented by the done callback? Callbacks are problematic when IMO the user is expected to provide the continuation, since it leads to pyramids of doom. Here, done is just a trigger that can be passed around. No doom, no hell, it's all nice and dandy.

If you deal with promises: return aPromise vs aPromise.then(done, done).... The second is slightly more error-prone in that you must remember to pass done twice since finally isn't part of the spec, but it's not atrocious either.

[lhorie]

re: returning a promise from onbeforeremove - I think it would be more cumbersome to return a promise than to call a callback in this case

I imagined that the typical use case would've been something along these lines:

m("div", {onbeforeremove: function() {
  setTimeout(done, 1000) //something that takes a callback
}})

With a promise, it'd look like this:

m("div", {onbeforeremove: function() {
  return new Promise(function(resolve) {
    setTimeout(resolve, 1000)
  })
}})

Here's an example with velocity.js (also callback based)

m("div", {onbeforeremove: function(vnode, done) {
  Velocity(vnode.dom, {opacity: 1}, {complete: done})
}})

//vs

m("div", {onbeforeremove: function(vnode) {
  return new Promise(function(resolve) {
    Velocity(vnode.dom, {opacity: 1}, {complete: resolve})
  })
}})

And one w/ Element.animate (promise-based)

m("div", {onbeforeremove: function(vnode, done) {
  vnode.dom.animate([{opacity: 1}]).finished.then(done)
}})

//vs

m("div", {onbeforeremove: function(vnode) {
  return vnode.dom.animate([{opacity: 1}]).finished
}})

So you only get a benefit if your animation API uses promises, and even then, you only save about a dozen characters at best (and at worst case, you'd be including a Promise polyfill that you wouldn't otherwise need).

[pygy]

One benefit of promises over the current implementation of callbacks is the "resolved at most once" semantics. With the current callback implementation, even if you wait to know how many onbeforeremove handlers will be called before calling the first, you can call done() repeatedly to trigger a premature removal.

[lhorie]

@pygy I'm not sure I'm following. Whether it's premature or not is something the person writing the onbeforeremove callback would determine. One could resolve a promise prematurely/repeatedly as well, but I think those cases fall into the "you are doing it wrong" bucket.

[pygy]

The done() expected counter is shared by all onbeforeremove handlers of a sub-tree. By erroneously calling done() multiple times form one handler, you can terminate the onbeforeremove phase prematurely.

It would be a bug in app space, but one that would not occur with a domvm-like, Promise-based interface.

[leeoniya]

@lhorie i guess the main benefit of returning a promise is to allow the lib to infer whether the onbeforeremove invocation should be treated as synchronous or not. Depending on the lib, it may have an effect on the cleanup algos, etc. This hook is not always used for animating and can be used for detaching handlers from an element, clearing timers.

I'm not making an argument for a thennable return; done() is in fact cleaner for most async cases, but loses the sync vs async signaling ability, requiring that all onbeforeremove hooks are treated asynchronously. It may not matter in Mithril's case.

@pygy there are a bunch of cases where doing stupid crap inside lifecycle hooks will screw up the vtree state, like manually invoking redraw() (assuming there are no complex locking mechanisms in place). using Promises only kind-of eliminates just one scenario.

[pygy]

You can infer sync vs async intent by looking at the length of the handler: 1 => sync; 2 => async, pass done.

I know there are other ways to shoot oneself in the feet, I just wanted to highlight a weakness of the current implementation (it could be fixed by creating per hook done callbacks that remember they've bee called, for example).

[tivac]

I think they're fine as-is.

[pygy]

For the names, I would prefer the ones I suggested, but I won't argue fiercely for them either, that's a detail.