groue / GRDBCombine

GRDB ❤️ Combine
MIT License
223 stars 16 forks source link

Add DatabaseRequest and DatabaseEditable types + SwiftUI Demo #35

Closed apptekstudios closed 4 years ago

apptekstudios commented 4 years ago

I've been loving getting to know GRDB over the last few days and thought I would experiment with how to best integrate it into SwiftUI projects. This PR adds two types that both conform to ObservableObject and can be used directly in SwiftUI views.

DatabaseRequest allows one to define a fetchRequest and have the view automatically update when the result changes.

DatabaseEditable allows for SwiftUI views to store and mutate a value conforming to MutablePersistableRecord. It provides options for automatically updating the database row on mutation, as well as manually committing changes to the database by calling commitChanges().

These are best explained by checking out the SwiftUI demo project I have included in the PR 😄

groue commented 4 years ago

Hello @apptekstudios,

Thanks for your interest and suggestions! I will not accept this pull request, though, and I suggest you keep on using DatabaseRequest and DatabaseEditable in your own apps, or in their dedicated repository.

Four reasons why:

  1. They can be written using public apis: they do not need to ship in the main repository.
  2. I'm not sure they have been used in enough different apps in order to be sure that their value is general (vs. dedicated to the specific use cases you met in the last few days).
  3. They are not documented. I'm thankful for the demo app, but someone would eventually have to write the documentation anyway. Writing documentation helps a lot figuring out if an API makes sense for other people. I recommend practicing this exercise.
  4. DatabaseEditable fosters the splitting of database changes into distinct database transactions, with the bad consequence that it makes it uneasy to avoid some undesired inconsistent database states once you started using it. It goes against the Rule 2 of the Concurrency Guide. You'll find an alternative spelling of the same topic in Good Practices for Designing Record Types (although focused on reads only). I wonder if DatabaseRequest falls in the same trap.

All in all, I'm very thankful, and I'm very willing to learn from users who use GRDBCombine in their applications. I'll also keep on directing the library so that it remains on a path that fosters a sane usage of the database, especially regarding concurrency and consistency traps.

On the same vein as DatabaseRequest, I'm looking at @FetchRequest. Unfortunately, it can't be implemented today with the current version of Swift (at least, it wasn't last time I tried). One of the reasons why it is interesting is that it grabs the database connection from the environment. The other reason why it is interesting is, obviously, that it is the Apple-vetted technique in order to feed SwiftUI from Core Data. We should get inspiration from it for GRDBCombine.

apptekstudios commented 4 years ago

Thank you for your detailed response @groue! These are good reasons and I appreciate the time and thought you put into the design/documentation of GRDB 😃I agree that database injection is currently an issue in SwiftUI and hope that we will have the ability to create property wrappers that access the environment (such as FetchRequest) in future versions 🤞

FYI for anyone who reads this in search of a SwiftUI-oriented GRDB package: https://github.com/apptekstudios/AS_GRDBSwiftUI As I refine my generalisable code I will be updating this repository 😄

groue commented 4 years ago

Thanks for the link, @apptekstudios :-) Happy GRDB!

apptekstudios commented 4 years ago

FYI @groue I've finally figured out how to achieve the equivalent of @FetchRequest. This is now implemented in my repo with a fully-functional demo project 😄

Here's the link again here in case anyone searches for it: https://github.com/apptekstudios/AS_GRDBSwiftUI I will be maintaining this package as it is now in heavy use in one of our apps. 👍 🎉 It makes sense to keep this separate to GRDBCombine due to the dependence on SwiftUI.

groue commented 4 years ago

Hello @apptekstudios! That's super cool, thank you. I'll have a look, and I'm quite looking for learning new techniques :-)