Skip to content
This repository has been archived by the owner on Sep 5, 2023. It is now read-only.

FRONTEND-8643 :: Feature :: Create NetworkDataSource repository providers #144

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

orioljp
Copy link
Contributor

@orioljp orioljp commented Mar 22, 2023

Asana task link:
https://app.asana.com/0/1109863238977521/1204241389098643

Merge / Pull request information:

  • Network DataSource provider has been splited in different providers per action.
  • New repository providers that use the previous mentioned network datasource providers to instanciate generic single network datasource repositories.

@orioljp orioljp requested review from urijp, doup and Alex-DA as code owners March 22, 2023 23:32
Copy link
Contributor

@doup doup left a comment

Choose a reason for hiding this comment

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

Approving, but check the comment, I don't think the generics are correct.

import { DataSourceMapper, GetDataSourceMapper, PutDataSourceMapper } from "./data-source-mapper";
import { ArrayMapper, BlankMapper, JsonDeserializerMapper } from "../mapper/mapper";

export function provideGetArrayNetworkDataSource<T extends unknown | void>(
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you mean by T extends unknown | void? Isn't this the same as just T? Also, regarding the void, does GetDataSource<void[]> make sense? We can probably leave it as:

Suggested change
export function provideGetArrayNetworkDataSource<T extends unknown | void>(
export function provideGetArrayNetworkDataSource<T>(

Copy link
Contributor Author

@orioljp orioljp Mar 27, 2023

Choose a reason for hiding this comment

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

@doup The thing is that the generic type should allow void as a type, and having T you cannot create a GetDataSource<void>.

Since this is a generic library, I didn't want to restrict the usage because the API might not be ours and might return nothing at some endpoint with a call of type GET. What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense to me

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

Successfully merging this pull request may close these issues.

4 participants