felangel / bloc

A predictable state management library that helps implement the BLoC design pattern
https://bloclibrary.dev
MIT License
11.79k stars 3.39k forks source link

feat: create a Repository class which all the repositories can inherit from #3758

Closed O-Hannonen closed 1 year ago

O-Hannonen commented 1 year ago

Description As of now, a repository provided with RepositoryProvider can be any class. Creating a class that all the repositories inherit from would unify the behaviour of all the repositories. This superclass could introduce methods like close and initialize, which are anyways needed in most of the repositories. When the methods are inherited from a superclass, repositories would be easier to initialize and close in bulk, or maybe even automatically.

Taking it a step further, the Repository superclass could also introduce a way to communicate back to the application (other than function return values). This could work much like a Cubit; inside the repository emit() function could be called to update a stream of states (or whatever the repository output shall be called). Additional (and optional, all the previous methods would still work) ways of reacting to repository events sounds like a no-brainer to me.

Desired Solution

Here is an example implementation of the desired solution.

Alternatives Considered

I've experimented with similar features here. It would be nicer to have this functionality in the core bloc library, instead of having to use extra dependencies. The linked package is just a quick demonstration/test of the functionalities. I somewhat dislike the RepositoryChannel and its callback functions in that package, as they are not that nicely mockable. A much nicer way would be if the Repository had similar capabilities (streams and states) as a Cubit, as well as pre-written tools (in bloc_test) to test them.

One thing that I do like about the callback functions is, that they can also return values. This means the repository can receive some additional data from other repositories or from anywhere in the app, without having to take any repositories or extra classes as parameters.

For instance, with the package linked above, in AuthenticationRepository (using firebase_auth), we can have a channel for populating user model with data from database before updating the stream of user models. All this could be implemented so, that the AuthenticationRepository does not have to worry about interacting with other repositories or populating the user data. Here is an example how that would work:

First we create a channel, that is passed as parameter to the repository:

class AuthenticationChannel extends RepositoryChannel {
  AuthenticationChannel({
    required this.onUserUpdate,
  });

  final FutureOr<User> Function(User) onUserUpdate;
}

Inside the repository we use the channel:

class AuthenticationRepository extends Repository<AuthenticationChannel> {
  AuthenticationRepository({
    required AuthenticationChannel channel,
  })  : super(channel);

  final FirebaseAuth _firebaseAuth = FirebaseAuth.instanceFor(app: Firebase.app());

  User currentUser = User.empty;

  /// Returns a stream of auth changes (Users). This stream is triggered when:
  /// 1. The listener is created
  /// 2. A user signs in
  /// 3. The current user signs out
  Stream<User> authChanges() => _firebaseAuth.authStateChanges().asyncMap(_mapNewAuthUser);

  /// Runs some code on the user before returning the newly triggered user to authState stream.
  Future<User> _mapNewAuthUser(auth.User? user) async {
    /// Stream is triggered by unauthenticated user.
    if (user == null) return currentUser = User.empty;

    /// Populating the user with data from firebase auth.
    final userModel = User.empty.copyWith(
      id: user.uid,
      email: user.email,
      displayName: user.displayName,
      photoUrl: user.photoURL,
    );

    /// Calling a method on the channel to populate the user object
    /// with data from a database. The method returns a new User object,
    /// which is then returned to the authState stream and updated to
    /// the currentUser variable.
    return currentUser = await channel.onUserUpdate.call(userModel);
  }
}

And finally wherever the repository is used:

return RepositoryProvider(
    create: (c) => AuthenticationRepository(
        channel: AuthenticationChannel(
            onUserUpdate:  c.read<DatabaseRepository>().populateUser,
        ),
    ),
    child: SomeWidget(),
);

This is just an example use case to keep in mind when thinking about different solutions to this feature.

tenhobi commented 1 year ago

Hi @O-Hannonen, I think it is nice that repository/service/whatever can be literally any class. From my experience on big projects, the interfaces of those classes, together with the overall architecture, are so unique that enforcing some repository class is not possible. Probably even an interface would not work. Not to mention that in bloc, you can use platform channels, API services, other internal calls, and streams, ...

So if you find a benefit, creating an extension package (as you did) or creating it in the scope of your project would fit the ecosystem the most.

O-Hannonen commented 1 year ago

I see that it would benefit the bloc ecosystem if a Repository would be as opinionated as Bloc and Cubit. It would unify the whole concept of a repository and how it operates. If a class does not fit the specs of a repository, one could argue it shouldn't be called a repository. Especially when RepositoryProvider is literally just a wrapper around Provider, that does nothing but changes the name. Why wouldn't one just use vanilla Provider if one would not wan't to be opinionated at all?

tenhobi commented 1 year ago

I totally forgot that RepositoryProvider exists. πŸ€” Then I agree that if a provider for that exists, maybe having a Repository interface makes some sense.

Although I would rather remove the RepositoryProvider myself, as it does not provide any benefit in comparison to Provider, as you mention.

Gene-Dana commented 1 year ago

Hi there πŸ‘‹πŸΌ May you explain a little more how this could unify the whole concept of a repository and how it operates? I like the idea I'm just unsure of the approach

O-Hannonen commented 1 year ago

@Gene-Dana Sure!

The how

For the "how", the problem is that I'm unsure of the approach too πŸ˜ƒ. It could something similar to Cubit, opening some extra communication channels but still being lightweight. Or it could be something similar to the approach I've experimented with. Or something totally different...

The why

The "why" on the other hand is something I can open a bit more! Following the bloc architecture documentation, a repository is a wrapper around one or more data providers, that queries, filters and turns the data into domain models that the application layer can consume. Knowing this, I see that a repository has a quite specific task. So in the documentation, bloc library specifies very meticulously what a repository is, but when working with repositories, they can be literally anything. I could pass my neighbours cat to RepositoryProvider, and it would be happy to provide it 😸. Having the repository definition taken from the documentation into the actual code would also make it easier concept to understand and further document.

Repositories work directly with data providers, so they often times listen to streams or open other communication channels that shall be initialized/disposed. If there were to be an interface which all repositories inherit from, they could all have initialize/dispose methods. This way one would not have to keep track of which repositories should be initialized/disposed, when one could just initialize/dispose all of them (or RepositoryProvider could dot it).

What about the communication channels, why would they be necessary? For many use-cases and projects, they might not be, and in those use-cases one would not have to use them. But providing that functionality would bring extra verbosity to the package.

Lets take a look at an example where communication channels could be useful. I've found it a clean approach to define repositories as their own lightweight (local) dart packages, which can be referenced as local paths in pubspec.yaml. So in the root of the project, I have lib/, where all the views, blocs and app specific configurations live. In addition to that, I have packages/, where all the repositories are defined. Now lets say I have my logger configured within lib/, which leads to not being able to log anything from the repository. Another approach would be to define logger configurations in its own local package, but then it would have to be taken as dependency into all of the repositories. Now logger is just one functionality needed to be shared across the repositories, but imagine having dozens of more. This would lead to deeply nested local dependencies, which in my opinion are not good. With some way of communicating back from a repository to the application layer, we could keep loggers etc. within lib/ and avoid nesting local dependencies.

Another example where this could be useful is the one provided in the original comment. I don't like repositories depending on each other, but sometimes their functionalities align and an event from one repository should trigger some action on another repository. Using well designed communication channels, a repository only needs to define the events that need to be handled elsewhere, and it does not have to know how they are handled.

Gene-Dana commented 1 year ago

I don't like repositories depending on each other, but sometimes their functionalities align

You are thinking that perhaps we need to share information between repos? I can see how that may be the case.

A very common case is one where someone has two repos - an authentication repo and a todo's repo - where the todo's repo needed the User ID in order to get the specific todos for that User.

an event from one repository should trigger some action on another repository.

Isn't the bloc the controller of events?

Just to be clear the repo itself never stores any state - it's merely a 'transformer' which converts data from the network layer (JSON) into the application domain (models).

The bloc is the controller of that state and it's tightly coupled to the UI.

So in the root of the project, I have lib/, where all the views, blocs and app specific configurations live.

In a feature-driven architecture - in this case a multimodual monorepo - the lib/ is where all the features live. features may or may not have views, state mgmt, widgets - it all depends on the feature - although every single feature is rooted in a domain.

For example the auth example has login, signup, etc.. which all fall under the authentication repo. All of the features are related to this independent repo and there's no need for those features to interact with other repos .

So then how does the Todos Repository get the UserID to get the todos for that specific user?

You can simply pass the UserID through a feature that calls the event to get the todos for that specific user.

There is no need to have the repos interact since they don't store state. So then the next thought is perhaps the blocs themselves can maintain a communication channel between each other, which in some cases this makes sense. The 'TodosOverviewBloc' can maintain a stream of 'User' from the 'HomeBloc' although this is not an approach I suggest because it violates the separation of concerns and removes the composability of features (which I believe is the entire point of declarative UI)

Instead I do this in the presentation layer. I subscribe to the userID in the view and pass that into the blocs which need to share this information in order to proceed.

I'm not 100% sure I understand your suggestion although from the surface level I think it's important that we emphasize separation of concerns through this feature-driven approach.

felangel commented 1 year ago

The reason there is no base repository class is because the base class would be an empty abstract class since there is no universally accepted set of functions that must exist on all repositories. You are free to create your own repository abstraction and enforce that interface throughout your own applications but I'd prefer not to introduce a repository interface in bloc because:

  1. It would be a breaking change that would be extremely disruptive
  2. The value provided in doing so would be questionable
  3. As I mentioned, you can already define an enforce your own definition of a repository without any changes to the bloc library.

Hope that helps and closing for now but feel free to comment with additional thoughts/questions and I'm happy to continue the conversation πŸ‘