Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: support promises in lifecycle components. #1204

Open
danfma opened this issue Sep 1, 2017 · 10 comments
Open

Feature request: support promises in lifecycle components. #1204

danfma opened this issue Sep 1, 2017 · 10 comments

Comments

@danfma
Copy link

danfma commented Sep 1, 2017

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
Copy link
Member

Havunen commented Sep 2, 2017

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
Copy link
Author

danfma commented Sep 6, 2017

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
Copy link

thepian commented Sep 23, 2017

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
Copy link
Contributor

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
Copy link
Member

trueadm commented Nov 14, 2017

@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
Copy link
Contributor

@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
Copy link
Member

trueadm commented Nov 14, 2017

@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
Copy link
Contributor

thysultan commented Nov 14, 2017

@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
Copy link
Member

Havunen commented Dec 7, 2017

@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
Copy link
Contributor

thysultan commented Dec 7, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants