onremove() should return a promise for actually removing the element

[infinnie]

I wish the function onremove() were to return a promise to tell the library to actually remove the element, instead of our having to manually do so in our own implementation.

[jorgebucaran]

@infinnie You only need to remove the element when you use onremove, if you don't then we remove it for you. It's like schrodinger's cat 😄

If you mean we should return a promise regardless, then IMO that would be too opinionated and not as flexible as the current approach, but it would certainly make onremove prettier.

I assume you wanted to do this?

<div
  onremove={element =>
    new Promise(resolve => {
      // ... maybe a cool fade-out animation ...
      fadeout(element, () => {
        resolve(element)
      })
    })}
/>

[jorgebucaran]

Have you considered wrapping your promise-returning-handler-function in this:

function promiseToVDOM(p) {
  p.then(element => {
    if (element.parentNode) {
      element.parentNode.removeChild(element)
    }
  })
}

<div onremove={promiseToVDOM( /* your promise returning code */ )}>

[okwolf]

@jbucaran how about supporting onremove returning a thunk? This seems like another case of an asynchronous update, except to the DOM instead of state. For example:

// If fadeout takes a callback function
<div onremove={element => remove => fadeout(element, remove)} />

// Or if fadeout returns a Promise
<div onremove={element => remove => fadeout(element).then(remove)} />

So instead of ignoring the return of data.onremove it would look something more like:

function removeElement(parent, element, data) {
  if (data && data.onremove) {
    var remover = data.onremove(element)
    if (typeof remover === "function") {
      remover(function() {
        parent.removeChild(element)
      });
    }
  } else {
    parent.removeChild(element)
  }
}

[jorgebucaran]

Ping @zaceno.

[okwolf]

@jbucaran An alternative would be to emit an event so users could keep DRY and write a handler with all your fancy removal logic in one place. You would definitely need to pass the element, and possibly the data in case there was anything in the VDOM that would help decide how to remove. I can't really think of the use case for that though.

[Swizz]

We don't want to support Promise in the core right ? So I approve the idea to make thunk a common practice for async operations.

[zaceno]

@okwolf , IIRC we used to have the remover passed as a second argument (with the elem as the first) . I think we removed it because elem.parentNode.removeChild(elem) is almost as easy, and to keep the api symmetrical.

IMO the thunk approach looks fine to me, at least compared to supporting promises in core -- but I don't really see enough value in that added complication.

[okwolf]

@JorgeBucaran it looks like #385 will implement my thunk suggestion and close this: https://github.com/hyperapp/hyperapp/blob/2d0f87effb8f75d6a4c8b854054eb4b2c6714028/src/app.js#L212

[jorgebucaran]

@okwolf Correct.