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

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

Open
orioljp wants to merge 1 commit intomasterfrom
feature/frontend-8643-network-datasource-providers
Open

FRONTEND-8643 :: Feature :: Create NetworkDataSource repository providers#144
orioljp wants to merge 1 commit intomasterfrom
feature/frontend-8643-network-datasource-providers

Conversation

@orioljp
Copy link
Copy Markdown
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 Alex-DA, doup and urijp as code owners March 22, 2023 23:32
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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