infernojs / inferno

:fire: An extremely fast, React-like JavaScript library for building modern user interfaces
https://infernojs.org
MIT License
16.07k stars 634 forks source link

Feature request: support promises in lifecycle components. #1204

Open danfma opened 7 years ago

danfma commented 7 years ago

Hey, guys!

I like a lot of this library and its performance improvements over other js libraries. However, one thing that I always have missed in React and other VirtualDom like libraries are support for promises at the component lifecycle, just like we have with Aurelia. Now, I found this feature on Dio.JS too.

Please, give a look at its documentation to understand what I'm talking (https://dio.js.org/) and give me a feedback with your thoughts. This helps a lot with animations.

Havunen commented 7 years ago

It sounds interesting feature. IMO we could support this if it does not come with a lot of overhead. Are you referring to update state with promises? Or which feature exactly, can you add direct link please.

danfma commented 7 years ago

Actually, I think that it will be more interesting to support promises as the return of the lifecycle methods. So, I could, for example, implement a componentWillUmount that will return a promise, and only after that, the component will be removed from the screen.

This opens space for async rendering too.

thepian commented 7 years ago

I think Observables is going to start showing up as another option for this sort of thing, but that would probably be a fundamentally different Component API.

thysultan commented 6 years ago

Thenables would cover more future ground as far as future interop between observables/promises are concerned. @Havunen DIO allows you to return a Promise/thenable from componentWillUnmount that defers unmounting of the DOM node to when the Promise is resolved.

trueadm commented 6 years ago

@thysultan Older versions of Inferno had something similar to this in regards to Promise support. The issue is that Promises can't resolve sync, and it lead to UIs breaking when used this way so I removed it. It also prevented the rest of the component tree from unmounting till the promise resolved as the events have to occur in the same order.

thysultan commented 6 years ago

@trueadm Promises resolving async is what you would want in this case. Paired with Node.animate and the onfinnish event would allow you to design declarative unmount animations around this.

class A {
    componentWillUnmount(node) {
        return new Promise((resolve) => {
            node.animate({...keyframes}, {...options}).onfinnish = resolve
        })
    }
    render() {
        ...
    }
}

The implementation details of this in DIO halt only the DOM nodes removal and not the execution of componentWillUnmount lifecycle events down the tree given that componentWillUnmount is not reliant on the DOM node being removed when executed.

trueadm commented 6 years ago

@thysultan That's the point though. Those heuristics are not right, componentWillUnmount is a sync event and making it async in React causes the world to blow up in many apps. If a child component does a setState of a parent further up the tree in its componentWillUnmount, then componentWillUnmount might get fired twice on child components. You have to tackle this properly at the root – provide a form of asyncComponentWillUnmount that doesn't suffer from these issues. There are thousands of third-party UI libraries that might break with this change on a single component in React land.

thysultan commented 6 years ago

@trueadm The lifecycle flow is sync in the DIO's implementation, the only aspect that is async is the resulting native DOM nodes removal and that too is only when componentWillUnmount returns a Promise/thenable.

From the scope of the virtual representation the element in question has been removed, only the related call to the native Node.removeChild(...) has been deferred.

That is – returning a Promise does not change the sync nature of the componentWillUnmount lifecycle, but only the "when" regarding the final call to the Node.removeChild API is executed.

Havunen commented 6 years ago

@thysultan How do you handle use case when domNode1 is going to be replaced by domNode2, do you then defer the replaceNode call? Or do you append the next one and then call remove for the node1 later?

Another use case is that when those calls are nested? And higher order component has removed node1's container

thysultan commented 6 years ago

Or do you append the next one and then call remove for the node1 later?

@Havunen Something like this, Instead of replaceNode i create domNode2 and insertBefore domNode1, then remove domNode1 when appropriate(sync/async).