Repurpose DataSource/Repository base interface to DataSource<T>/Repository<T> get/put/delete type aliases

[doup]

See agreed proposal: #88 (comment)

---

Original Message:

We have an empty base Repository interface which all three repositories implement. On a provider is not very useful as the generic interactors will complain that the provided repository doesn't implement the interface… so something like this doesn't work:

const repo: Repository = ...;
const put = new PutInteractor(repo);

Instead, we need to do this:

const repo: GetRepository<MyModel> & PutRepository<MyModel> = …;
const put = new PutInteractor(repo);

Do we really need Repository?
Maybe we need some helper types that join these? Something like: GetPutRepository<MyModel>, FullRepository<MyModel>…

[orioljp]

@doup I've never seen the need of generate a repository as a generic one like you're proposing. The provider defines the specific repository that is being used. SingleDataSurceRepository, GetRepositoryMapper, .... all these implementations implement one or all the interfaces.

Could you show an example of this need?

[joselufo]

@doup we only have it in harmony-swift because it allows us to not provide the 3 different repositories or data sources in the constructor thanks to a swift generic feature.

Here you have an example of being used in harmony-swift in the cache repository. We are providing one instance type for the main and another for the cache datasources.

As we can't do it on kotlin, we do not have that interface created in harmony-kotlin.

I would suggest removing the interface from the code base as it's not useful.

@doup or @orioljp can you take care?

[doup]

I suggest we repurpose it to the following:

type DataSource<T> = GetDataSource<T> & PutDataSource<T> & DeleteDataSource;
type Repository<T> = GetRepository<T> & PutRepository<T> & DeleteRepository;

In a provider, instead of using a concrete type (e.g. RepositoryMapper<T, U>) we can use this type instead: Repository<T>. We have the same exact issue with the DataSource type. Instead of listing all get/put/delete that a datasource needs to implement we can just class XYZ implements DataSource<T>.

---

@orioljp this is an example: c004ebd

• In that commit we create a getUserRepository() method
• The return type has to be either:
  • RepositoryMapper<T, U>
  • GetRepository<UserModel> & PutRepository<UserModel> & DeleteRepository

I think the second option is the correct one. If we repurpose Repository, we could just express the return type as Repository<UserModel>.

[doup]

Btw, we could also add the following helper types (same applies for DataSource):

• GetPutRepository<T>
• GetDeleteRepository<T>
• PutDeleteRepository<T>

So we would have:

• GetRepository<T>
• PutRepository<T>
• DeleteRepository
• GetPutRepository<T>
• GetDeleteRepository<T>
• PutDeleteRepository<T>
• Repository<T>

[joselufo]

The type for the DataSource<T> and Repository<T> makes sense as it's replicating what we did in swift.

Nevertheless, applying the same concept for all types of combinations is overkill, and it's already complex to explain things to the team, so adding more wrapper types will be more confusing.

[lucianosantana]

Sorry for joining late. Only after getting notified about the PR, I've noticed the discussion.

It bothers me that only the case that implements all is contemplated.

What if we do something like ?

type Repository<T> = (GetRepository<T> & PutRepository<T> & DeleteRepository) 
| (GetRepository<T> & PutRepository<T>) 
| (GetRepository<T> & DeleteRepository) 
| (PutRepository<T> & DeleteRepository) 
| GetRepository<T> 
| PutRepository<T>
| DeleteRepository;

[doup]

@lucianosantana @joselufo I don't think this is a good idea. I see two problems:

1. By using an union we can't use the type as an interface, so no more class XYZ implements Repository<T>. This is not possible because TypeScript doesn't know which of the interfaces you want to implement.
2. Similarly, when you get a Repository<T> value, you might be getting a full Get+Put+Delete implementation… or just a Delete one. Since none of the methods is common to all the union types you need to narrow the type before using the value (e.g. if ('getAll' in value) { /* now is narrowed to all the ones that impl Get */ }).

See TS Playground example:

1. Problem 1: Line 31.
2. Problem 2: Line 60.

---

How were you thinking to use this union type? As an alternative, maybe we can reconsider the addition of these?

• GetPut*<T>
• GetDelete*<T>
• PutDelete*<T>

[lucianosantana]

For the record, I've discussed with @doup and the "catch all" union type wouldn't be really helpful, as it gives no assurance about the expected methods to be implemented, forcing us to do some narrowing to use it.

The motivation for my suggestion was that I find confusing using the name "Repository" only for the case that implements all 3 types of Repository. While, for example, a PutRepository in this case wouldn't be of type Repository.

But we actually have this pattern in other places. SingleDataSourceRepository which implements Get, Put and Delete, while SingleGetDataSourceRepository implements only Get. Same for RepositoryMapper. It's not the most intuitive IMO, but once you learn it you just get used to it.

The alternative would be to write union specific types, without any wrapper type. But this would be too verbose and less efficient.

So in the end, I'm convinced to use it this way to be consistent with the rest of the code base.