feat: add KY data provider to simple-rest package #6544
Conversation
|
There was a problem hiding this comment.
Hey @OmkarBansod02 thanks for the PR!
Added some minor comments, generally it looks good. We'll know better when the tests are added. Make sure you use nock library to record new requests and create tests based on them.
Looking forward to your updates 🚀
| "nock": "^13.4.0", | ||
| "query-string": "^7.1.1", |
There was a problem hiding this comment.
Let's use qs package because it supports encoding query params with nested structures.
| @@ -48,9 +68,12 @@ | |||
| "@refinedev/cli": "^2.16.33", | |||
| "@refinedev/core": "^4.52.0", | |||
| "@types/jest": "^29.2.4", | |||
| "axios": "^1.6.2", | |||
There was a problem hiding this comment.
| "axios": "^1.6.2", |
We don't need this, it should be in dependencies already.
| export const kyInstance = ky.create({ | ||
| hooks: { | ||
| afterResponse: [ | ||
| async (_request, _options, response) => handleErrorResponse(response), | ||
| ], | ||
| }, | ||
| }); | ||
|
|
||
| export const createKyInstance = ( | ||
| baseUrl?: string, | ||
| options?: KyOptions, | ||
| ): KyInstance => { | ||
| return ky.create({ | ||
| prefixUrl: baseUrl, | ||
| ...options, | ||
| hooks: { | ||
| afterResponse: [ | ||
| async (_request, _options, response) => handleErrorResponse(response), | ||
| ], | ||
| ...options?.hooks, | ||
| }, | ||
| }); | ||
| }; |
There was a problem hiding this comment.
kyInstance is a default instance of ky with a pre-configured setup (e.g., error handling hooks). kyInstance is sufficient for refinedev usecase.
I added createKyInstance to allow more flexibility, like using multiple APIs or customizing hooks if needed. If this flexibility isn’t useful for refine.dev right now, we can remove it to keep things simpler.
There was a problem hiding this comment.
Where do we use createKyInstance?
|
Hey @OmkarBansod02 thanks for the PR. I'll need to close it and update the issue with more details, since it wasn't explained well. I'll discuss with core team to decide for necessary changes. |
No problem. If core team approve it create an issue with more details, i would love to work on it.🚀 |
PR Checklist
Please check if your PR fulfills the following requirements:
Bugs / Features
What is the current behavior?
The
@refinedev/simple-restpackage currently includes only an Axios-based data provider. Users do not have the option to use KY for handling HTTP requests.What is the new behavior?
The package now includes a KY-based data provider, providing users with an alternative lightweight HTTP client. The KY provider mirrors the Axios implementation's structure, ensuring consistency, type safety, and robust error handling.
fixes #6381
Notes for reviewers
i’ve added the KY data provider to the
@refinedev/simple-restpackage, following the same structure as the Axios provider. The key difference is KY’s simplicity and built-in query handling, which makes the implementation lighter and more efficient forcertain use cases. You can find more about KY here: KY Documentation.
Here is comparison :
Axios Example :
KY Example:
KY is a lightweight and simpler alternative to Axios. It uses searchParams for query parameters and simplifies response handling with .json().
I haven’t included tests yet because i wanted to get an initial review to ensure the implementation is solid. If the code looks strong, I will proceed to add test cases in a follow-up.
i need your input on whether this change should be categorized as minor or major for the changesets.
Looking forward to your feedback and suggestions for improvement!
Thanks!