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!: new RESTClient #52

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Conversation

HeadTriXz
Copy link
Contributor

@HeadTriXz HeadTriXz commented Mar 17, 2023

This pull request deprecates all REST methods of Client and moves them into a new class called RESTClient. This will allow for making requests without needing to create an instance of Client.

In addition, this PR introduces a completely new RequestHandler, which will handle requests made through RESTClient. The new RequestHandler provides better performance and less spaghetti code 🍝.

The implementation of RESTClient has been designed with full backwards compatibility in mind, so existing code should continue to work seamlessly. One significant improvement in this PR is the use of undici's fetch function for making HTTP requests.

As this is a significant change, extensive testing will be required to ensure its stability and correctness. It also heavily depends on #46. Therefore, this PR will remain a draft until all testing is completed and #46 is merged.

Please review the changes and provide feedback as needed.

The new files currently use a different coding style (see class fields) than the rest of the library. It may, or may not change in the future, depending on #46.

@TTtie TTtie added type: enhancement New feature or request scope: REST Issues or pull requests related to REST functionality scope: meta Meta changes labels Mar 17, 2023
@TTtie TTtie added this to the 0.2.0 milestone Mar 17, 2023
Copy link
Member

@TTtie TTtie left a comment

Choose a reason for hiding this comment

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

Here's a summary of my initial thoughts on this PR:

By the way, please rebase :(

lib/Client.js Outdated Show resolved Hide resolved
lib/Client.js Show resolved Hide resolved
lib/Client.js Show resolved Hide resolved
lib/rest/Request.js Outdated Show resolved Hide resolved
lib/rest/Request.js Outdated Show resolved Hide resolved
lib/Client.js Outdated Show resolved Hide resolved
lib/Client.js Outdated Show resolved Hide resolved
lib/rest/RESTClient.js Outdated Show resolved Hide resolved
lib/rest/RESTClient.js Show resolved Hide resolved
@HeadTriXz HeadTriXz marked this pull request as ready for review April 1, 2023 14:08
@HeadTriXz HeadTriXz requested a review from TTtie April 1, 2023 14:08
@TTtie TTtie changed the title Implement new RESTClient feat!: new RESTClient Jul 5, 2023
@TTtie TTtie modified the milestones: 0.2.0, next Jul 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: meta Meta changes scope: REST Issues or pull requests related to REST functionality type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants