Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(graphql): add getData and getCount options #6357

Open
wants to merge 1 commit into
base: releases/october
Choose a base branch
from

Conversation

BatuhanW
Copy link
Member

@BatuhanW BatuhanW commented Sep 19, 2024

PR Checklist

Please check if your PR fulfills the following requirements:

Bugs / Features

What is the current behavior?

What is the new behavior?

fixes (issue)

Notes for reviewers

@BatuhanW BatuhanW requested a review from a team as a code owner September 19, 2024 10:11
Copy link

changeset-bot bot commented Sep 19, 2024

🦋 Changeset detected

Latest commit: 3cb0e82

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@refinedev/graphql Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Member

@aliemir aliemir left a comment

Choose a reason for hiding this comment

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

Left couple of comments for types, typos and conventions. Thank you 🙏

Comment on lines +33 to +36
export type GraphQLDataProviderOptions = {
getData: GraphQLGetDataFunction;
getCount: GraphQLGetCountFunction;
};
Copy link
Member

Choose a reason for hiding this comment

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

This type is also used in the dataProvider export and forces user to define both of the functions. It should allow partial assignments.

import camelCase from "camelcase";
import pluralize from "pluralize";

"asd".toUpperCase;
Copy link
Member

Choose a reason for hiding this comment

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

leftover code here

| { method: "deleteOne"; params: DeleteOneParams }
);

type GraphQLGetDataFunction = (params: GraphQLGetDataFunctionParams) => any;
Copy link
Member

Choose a reason for hiding this comment

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

Return type can be inferred from params.method by checking it like below:

type ResponseMap = {
  getList: GetListResponse<any>;
  getOne: GetOneResponse<any>;
  /* ... */
};

type InferResponse<T extends GraphQLGetDataFunctionParams> = T extends { method: infer M }
  ? M extends keyof ResponseMap
    ? ResponseMap[M]
    : never
  : never;
  
type GraphQLGetDataFunction = (params: GraphQLGetDataFunctionParams) => InferResponse<typeof params>;

getCount: GraphQLGetCountFunction;
};

export const defaultGetDataFunc: GraphQLGetDataFunction = ({
Copy link
Member

Choose a reason for hiding this comment

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

I think Func suffix here doesn't add any context, let's remove the Func or don't try to cut 4 letters 🤣

const camelCreateName = camelCase(`create-${singularResource}`);
const operation = params.meta?.operation ?? camelCreateName;

// console.log(response, operation, singularResource);
Copy link
Member

Choose a reason for hiding this comment

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

Leftover here too

Comment on lines +26 to +29
options: GraphQLDataProviderOptions = {
getCount: defaultGetCountFunc,
getData: defaultGetDataFunc,
},
Copy link
Member

Choose a reason for hiding this comment

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

This marks the options as optional but requires both methods to be defined when passing it. getCount and getData should have default values individually.

Suggested change
options: GraphQLDataProviderOptions = {
getCount: defaultGetCountFunc,
getData: defaultGetDataFunc,
},
{
getCount = defaultGetCountFunc,
getData = defaultGetDataFunc,
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants