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

refactor/ try flutter hooks #225

Merged
merged 5 commits into from
Sep 23, 2024
Merged

refactor/ try flutter hooks #225

merged 5 commits into from
Sep 23, 2024

Conversation

simon-the-shark
Copy link
Member

So this refactor is optional - if you don't like it, we don't need to merge it. Everything should work exactly the same without it.

So hooks are basically thing borrowed from React and I was previously skeptical as they don't feel 100% Fluttery in a way. But once you get used to them, they have much less boilerplate, they are very consice (and statefull widget is not really). Also you can very easily write reusable logic (by creating your own hooks - by just writing a function). You can read more about hooks e.g. here https://riverpod.dev/docs/concepts/about_hooks or here https://pub.dev/packages/flutter_hooks

Also they are reccomended by riverpod as solution for "local state" and riverpod should be more about global/bussiness state.

But again, this PR is purely refactor and should change any behaviour. We can skip it or merge it, tell me what you think

Copy link
Member

@mikolaj-jalocha mikolaj-jalocha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got in love with hooks from the first line of this review ❤️

Code now got a lot more readable and easier to understand (especially for someone not deep or experienced in flutter). Tbh code now resembles Android native in some ways of managing state.

I like the idea and opt for it with all my heart in it. I leave some questions of mine

@@ -0,0 +1,8 @@
import "package:flutter_hooks/flutter_hooks.dart";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, instead of that function, we alternatively could use default useEffect function always passing empty list? And since we often need to run some state related function once, on init, this wrapper is handy. Am I right?
If yes, it is nice improvement actually. Hooks got my heart.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exactly - we can use useEffect with empty dependency list everytime, but we can easily create custom hooks like this in very simple way to introduce reusability. This is very simple example of a custom hook, but custom hooks can also have a big chunks of logic too

});
widget.onQueryChanged(query);
}
final onTapOutside = useCallback(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow up to previous question (about useEffectOnInit), the callback here'd be executed the first time the hook is invoked, and then whenever focus node changes (meaning the whole list changes)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there is hook called useMemoized which caches some expensive operation/creation of sometiming and recalculates only when someting in the dependecy list changes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and useCallback is a simple wrapper on useMemoized but for functions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this basically means it creates this function inside only first time and then on rebuild, it doesn't create this function again but uses previous instance - unless a key in dependecy list has changed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this basically means it creates this function inside only first time and then on rebuild, it doesn't create this function

I thin I got it now! Thank you

@simon-the-shark simon-the-shark merged commit ac414e8 into main Sep 23, 2024
2 checks passed
@simon-the-shark simon-the-shark deleted the refactor/try-hooks branch September 23, 2024 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants