inferno icon indicating copy to clipboard operation
inferno copied to clipboard

Feature request: support promises in lifecycle components.

Open danfma opened this issue 6 years ago • 10 comments

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.

danfma avatar Sep 01 '17 16:09 danfma

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.

Havunen avatar Sep 02 '17 15:09 Havunen

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.

danfma avatar Sep 06 '17 21:09 danfma

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.

thepian avatar Sep 23 '17 17:09 thepian

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.

thysultan avatar Nov 14 '17 17:11 thysultan

@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.

trueadm avatar Nov 14 '17 18:11 trueadm

@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.

thysultan avatar Nov 14 '17 18:11 thysultan

@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.

trueadm avatar Nov 14 '17 19:11 trueadm

@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.

thysultan avatar Nov 14 '17 19:11 thysultan

@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

Havunen avatar Dec 07 '17 15:12 Havunen

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).

thysultan avatar Dec 07 '17 15:12 thysultan