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.

Sign up for GitHub

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

Jump to bottom

refactor(REST): remove double classing #9722

Merged
merged 2 commits into from
Jul 25, 2023
Merged

refactor(REST): remove double classing #9722

merged 2 commits into from
Jul 25, 2023

Conversation

ckohen
Copy link
Member

@ckohen ckohen commented Jul 17, 2023

Please describe the changes this PR makes and why it should be merged:

We weren't exactly sure why this existed, it added extra complexity for no real reason. The diff is a little unfortunate, sorry.
In order to help a little bit with this and my own sanity when doing this, all types were moved to a types util file.

BREAKING CHANGE: REST and RequestManager have been combined, most of the properties, methods, and events from both classes can now be found on REST
BREAKING CHANGE: REST#raw has been removed in favor of REST#queueRequest
BREAKING CHANGE: REST#getAgent has been removed in favor of REST#agent

This also fixes some issues that popped up in node 16 due to globals not being present.

Status and versioning classification:

  • Code changes have been tested against the Discord API, or there are no code changes
  • I know how to update typings and have done so, or typings don't need updating
  • This PR changes the library's interface (methods or parameters added)
  • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)

@ckohen ckohen requested review from a team as code owners July 17, 2023 08:54
@vercel
Copy link

vercel bot commented Jul 17, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Ignored Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Jul 24, 2023 9:52am
discord-js-guide ⬜️ Ignored (Inspect) Jul 24, 2023 9:52am

@Jiralite Jiralite added this to the rest 2.0.0 milestone Jul 17, 2023
vladfrangu
vladfrangu requested changes Jul 17, 2023
View reviewed changes
packages/rest/src/index.ts Show resolved Hide resolved
packages/rest/src/lib/utils/types.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/REST.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/REST.ts Outdated Show resolved Hide resolved
packages/rest/src/lib/REST.ts Outdated Show resolved Hide resolved
ckohen added 2 commits July 24, 2023 02:52
@ckohen
refactor(REST): remove double classing
54b39bf 
BREAKING CHANGE: `REST` and `RequestManager` have been combined, most of the properties, methods, and events from both classes can now be found on `REST`
BREAKING CHANGE: `REST#raw` has been removed in favor of `REST#queueRequest`
BREAKING CHANGE: `REST#getAgent` has been removed in favor of `REST#agent`
@ckohen
chore: update for /rest changes
2b1208b
@codecov
Copy link

codecov bot commented Jul 24, 2023

Codecov Report

Merging #9722 (2b1208b) into main (d26e022) will decrease coverage by 0.16%.
The diff coverage is 92.50%.

@@            Coverage Diff             @@
##             main    #9722      +/-   ##
==========================================
- Coverage   57.66%   57.51%   -0.16%     
==========================================
  Files         232      232              
  Lines       15093    15030      -63     
  Branches     1135     1126       -9     
==========================================
- Hits         8703     8644      -59     
+ Misses       6350     6346       -4     
  Partials       40       40
Flag Coverage Δ
proxy 76.31% <100.00%> (ø)
rest 92.69% <92.49%> (-0.03%) ⬇️
ws 52.38% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
packages/rest/src/lib/REST.ts 87.98% <84.74%> (-10.90%) ⬇️
packages/proxy/src/handlers/proxyRequests.ts 77.04% <100.00%> (ø)
packages/rest/src/lib/errors/DiscordAPIError.ts 96.42% <100.00%> (ø)
packages/rest/src/lib/errors/HTTPError.ts 100.00% <100.00%> (ø)
packages/rest/src/lib/errors/RateLimitError.ts 100.00% <100.00%> (ø)
packages/rest/src/lib/handlers/BurstHandler.ts 91.03% <100.00%> (ø)
...ackages/rest/src/lib/handlers/SequentialHandler.ts 87.71% <100.00%> (ø)
packages/rest/src/lib/handlers/Shared.ts 88.23% <100.00%> (ø)
packages/rest/src/lib/utils/constants.ts 100.00% <100.00%> (ø)
packages/rest/src/lib/utils/types.ts 100.00% <100.00%> (ø)
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@kodiakhq kodiakhq bot merged commit 8f4256d into discordjs:main Jul 25, 2023
@ckohen ckohen deleted the feature/djs-34 branch July 25, 2023 08:54
@N1ckPro N1ckPro mentioned this pull request Jul 31, 2023
Merged
Vylpes pushed a commit to Vylpes/vylbot-app that referenced this pull request Aug 7, 2023
@Vylpes
Update dependency @discordjs/rest to v2 (#327)
9968e2e 
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@discordjs/rest](https://discord.js.org) ([source](https://github.com/discordjs/discord.js)) | dependencies | major | [`^1.1.0` -> `^2.0.0`](https://renovatebot.com/diffs/npm/@discordjs%2frest/1.7.1/2.0.0) |

---

### Release Notes

<details>
<summary>discordjs/discord.js</summary>

### [`v2.0.0`](https://github.com/discordjs/discord.js/blob/HEAD/packages/rest/CHANGELOG.md#discordjsrest200-httpsgithubcomdiscordjsdiscordjscomparediscordjsrest171discordjsrest200---2023-07-31)

[Compare Source](https://github.com/discordjs/discord.js/compare/@discordjs/rest@1.7.1...@discordjs/rest@2.0.0)

#### Features

-   No-de-no-de, now with extra buns ([#&#8203;9683](discordjs/discord.js#9683)) ([386f206](discordjs/discord.js@386f206))
    -   **BREAKING CHANGE:** The REST and RequestManager classes now extend AsyncEventEmitter
        from `@vladfrangu/async_event_emitter`, which aids in cross-compatibility
        between Node, Deno, Bun, CF Workers, Vercel Functions, etc.
    -   **BREAKING CHANGE:** DefaultUserAgentAppendix has been adapted to support multiple
        different platforms (previously mentioned Deno, Bun, CF Workers, etc)
    -   **BREAKING CHANGE:** the entry point for `@discordjs/rest` will now differ
        in non-node-like environments (CF Workers, etc.)
    -   **Co-authored-by:** Suneet Tipirneni <77477100+suneettipirneni@users.noreply.github.com>
    -   **Co-authored-by:** Jiralite <33201955+Jiralite@users.noreply.github.com>
    -   **Co-authored-by:** suneettipirneni <suneettipirneni@icloud.com>
-   User avatar decorations ([#&#8203;8914](discordjs/discord.js#8914)) ([8d97017](discordjs/discord.js@8d97017))
-   Support new username system ([#&#8203;9512](discordjs/discord.js#9512)) ([1ab60f9](discordjs/discord.js@1ab60f9))

#### Refactor

-   **REST:** Remove double classing ([#&#8203;9722](discordjs/discord.js#9722)) ([8f4256d](discordjs/discord.js@8f4256d))
    -   **BREAKING CHANGE:** `REST` and `RequestManager` have been combined, most of the properties, methods, and events from both classes can now be found on `REST`
    -   **BREAKING CHANGE:** `REST#raw` has been removed in favor of `REST#queueRequest`
    -   **BREAKING CHANGE:** `REST#getAgent` has been removed in favor of `REST#agent`

<!---->

-   chore: update for /rest changes

<!---->

-   **rest:** Switch api to fetch-like and provide strategies ([#&#8203;9416](discordjs/discord.js#9416)) ([cdaa0a3](discordjs/discord.js@cdaa0a3))
    -   **BREAKING CHANGE:** NodeJS v18+ is required when using node due to the use of global `fetch`
    -   **BREAKING CHANGE:** The raw method of REST now returns a web compatible `Respone` object.
    -   **BREAKING CHANGE:** The `parseResponse` utility method has been updated to operate on a web compatible `Response` object.
    -   **BREAKING CHANGE:** Many underlying internals have changed, some of which were exported.
    -   **BREAKING CHANGE:** `DefaultRestOptions` used to contain a default `agent`, which is now set to `null` instead.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC43NC4yIiwidXBkYXRlZEluVmVyIjoiMzQuNzQuMiJ9-->

Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/vylbot-app/pulls/327
Co-authored-by: Renovate Bot <renovate@vylpes.com>
Co-committed-by: Renovate Bot <renovate@vylpes.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdanialraza sdanialraza sdanialraza approved these changes

@suneettipirneni suneettipirneni suneettipirneni approved these changes

@Jiralite Jiralite Jiralite approved these changes

@didinele didinele didinele approved these changes

@vladfrangu vladfrangu vladfrangu approved these changes

@iCrawl iCrawl Awaiting requested review from iCrawl iCrawl is a code owner automatically assigned from discordjs/rest

@kyranet kyranet Awaiting requested review from kyranet kyranet is a code owner automatically assigned from discordjs/rest

Assignees
No one assigned
Labels
packages:proxy packages:rest refactor semver:major
Projects
discord.js
Archived in project
Milestone
rest 2.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@ckohen @vladfrangu @didinele @Jiralite @suneettipirneni @sdanialraza