search

odoo / owl

OWL: A web framework for structured, dynamic and maintainable applications
https://odoo.github.io/owl/
Other
1.1k stars 332 forks source link

added useEffectAsync #1590

Closed mshahbazi closed 4 months ago

mshahbazi commented 4 months ago

Using async function in useEffect raises cleanup is not a function error, since the return value is a promise rather than a callable function. For async cases, useEffectAsync can handle this issue

sdegueldre commented 4 months ago

Hello @mshahbazi, thank you for submitting a patch to the Owl repo, it's always nice to see people are interested in the project and willing to contribute.

As it stands, your implementation is prone to race conditions: what happens if a component is mounted then unmounted before the promise of the effect resolves? The cleanup will not be run because it hasn't been set yet, and now you have some side-effect which was performed but not reversed, which is likely to lead to bugs and memory leaks.

While it's possible to work around this limitation with a more thorough implementation, I don't think the added value for this kind of function is high enough to warrant adding it to the project, especially considering this function doesn't require any access to owl internals for its implementation and can be implemented entirely in user code without issue.

mshahbazi commented 4 months ago

Thanks. You are right. It makes sense.