bdd_widget_test icon indicating copy to clipboard operation
bdd_widget_test copied to clipboard

Made the hooks async by default

Open daniel-deboeverie-lemon opened this issue 1 year ago • 3 comments

daniel-deboeverie-lemon avatar Feb 06 '24 14:02 daniel-deboeverie-lemon

Thanks for the proposed changes. Would you please describe in a bit more details why you believe Future would be a better choice?

olexale avatar Feb 10 '24 16:02 olexale

99 percent of use cases for hooks I could think of require you to await something. The main point of this pr was to make the methods async by default, since you don't want to always have to change that, and awaiting an async method that is actually sync in implementation has no drawback. We could change it back to using FutureOr instead, but I would keep the default addition of the async modifier to the methods

daniel-deboeverie-lemon avatar Feb 11 '24 11:02 daniel-deboeverie-lemon

99 percent of use cases for hooks I could think of require you to await something.

That's an interesting observation. I'd say all initializations and/or preparations in my widget tests are synchronous. However, I don't use Hooks myself as steps composition works well for all my cases, so maybe I'm missing something.

I believe it is better for the users to have synchronous behavior as the default. At least that what I would generally suggest to any developer without knowing their context — keep it simpler, as it is always easy to change it when needed.

Do you see any other drawbacks of keeping the existing solution, except of annoying need of adding the async modifier?

olexale avatar Feb 11 '24 22:02 olexale