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

Add link to redux_persist #19

Closed
Cretezy opened this issue Mar 9, 2018 · 9 comments
Closed

Add link to redux_persist #19

Cretezy opened this issue Mar 9, 2018 · 9 comments

Comments

@Cretezy
Copy link

Cretezy commented Mar 9, 2018

Hey, this is my first day of Dart and I found there was no way to persist to a redux store without writing the code yourself all the time. I made a small library called redux_persist to help with that.

I'd like to add a link to the library in the README, as this is probably a common pattern with flutter_redux!

Also, if you have a few minutes to space, check out the library and let me know of any issues you see on it! Thanks for flutter_redux!

@brianegan
Copy link
Owner

brianegan commented Mar 10, 2018

Hey, way cool! Thanks for adding to the community :)

I like the direction of the library... glad you've made the base classes abstract, making it easy to plug in different storage options. Happy to link to the lib.

There are a couple things I noticed when I checked it out:

  1. You could have stronger type safety. I think you even have a few type errors, actually... I'd recommend adding an analysis_options.yaml to your projects like the following and fix the errors: https://github.com/brianegan/flutter_redux/blob/5f3c6ac7153b385c03d5e0a4cb0295daf9075516/analysis_options.yaml
  2. How would you feel about adding tests? I'd really like to ensure the packages we write for the community to are well tested so folks can feel comfortable relying on it! Examples of testing file storage: https://github.com/brianegan/flutter_architecture_samples/blob/master/example/todos_repository/test/file_storage_test.dart
  3. I feel like the Middleware should load the state from storage in response to an action (InitAppAction or something), rather than through an extra Widget. That would more closely follow the normal redux conventions, and you could trigger the load again if need be.
  4. Is there some error case? What if the data fails to load? Could you dispatch a LoadFailed action?

@Cretezy
Copy link
Author

Cretezy commented Mar 11, 2018

  1. I agree that I need to add some more type safety, however I'm fairly certain there's no errors, and I do have analysis_options.yaml.

  2. Tests are up next for v1.0.0! It's really important, and it's coming for sure!

  3. I see what you mean here. It's probably a good idea to change the current LoadAction to RehydrateAction, and make the LoadAction be what requests the load. I'll look in implementing it in the next version.

  4. Good point, I'll also add error handling for saves/loads, and an action to come with it.

Thank you very much for the feedback, as a new Dart/Flutter dev it helps to get guided a bit.

@brianegan
Copy link
Owner

brianegan commented Mar 11, 2018

@Cretezy Ah, I must have pulled down an older copy before ya added the analysis options!

One thing I'd recommend: Putting on not only strong mode, but the additional options you see linked in the sample above. It shows a few type errors, which will cause ya problems for Dart 2 support (learning that the hard way haha, thought strong mode was strong enough!).

Great work overall :) Thanks again!

@Cretezy
Copy link
Author

Cretezy commented Mar 12, 2018

Alright I got all the type errors fixed up! I understand the errors better now, thank you!

I did almost all the changes you've recommended in v0.5.0 (Cretezy/redux_persist@6b305b0).

Next thing to tackle is tests, and migrations!

@brianegan
Copy link
Owner

Heck yah, nice work! Thanks for taking in the feedback and responding in such top form :)

Once you get those tests in place, hit me up and I'll pop a link in the readme!

@Cretezy
Copy link
Author

Cretezy commented Mar 15, 2018

Added some tests!

@brianegan
Copy link
Owner

@Cretezy Cool, thanks for taking all the feedback and making such good improvements! I'll pop a link into the README :)

One more piece of feedback: It looks like the Flutter side could still use some tests as well!

We've recently added some recipes to the Flutter Cookbook that demonstrates how to test shared_preferences and file io if you want to try your hand at it:

https://flutter.io/cookbook/persistence/key-value/
https://flutter.io/cookbook/persistence/reading-writing-files/

@Cretezy
Copy link
Author

Cretezy commented Mar 17, 2018

Yep, Flutter and Web tests are coming for v1.0! I'm also going to create a more generic FileStorageEngine!

@brianegan
Copy link
Owner

Pushed up an addition to the README. Thanks again for your contributions :D :D :D

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

No branches or pull requests

2 participants