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(ws): Allow passing in CompressionMethod to ClientOptions to use native node zlib #10411

Closed
wants to merge 18 commits into from

Conversation

JMTK
Copy link
Contributor

@JMTK JMTK commented Jul 26, 2024

Please describe the changes this PR makes and why it should be merged:
#10316 added support for native zlib support to @discordjs/ws, but there's not currently a way to pass it in to a client(see

compression: zlib ? CompressionMethod.ZlibStream : null,
).

This PR now first checks if you passed in a CompressionMethod, and if not checks if you have zlib-sync installed, if not then no compression

Status and versioning classification:
Minor

@JMTK JMTK requested a review from a team as a code owner July 26, 2024 17:38
Copy link

vercel bot commented Jul 26, 2024

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

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Jul 26, 2024 5:43pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jul 26, 2024 5:43pm

@Qjuh
Copy link
Contributor

Qjuh commented Jul 27, 2024

While that PR was merged it was done in a semver major way and @discordjs/ws@2.0.0 isn‘t released. So this PR needs to

  • be blocked until that happens
  • address any other changes that major update might bring, including the event change already merged in it
  • best case that being done in a non-breaking way because otherwise if it would be a semver major the /ws incorporation would be redone entirely anyway

@almeidx
Copy link
Member

almeidx commented Aug 20, 2024

Achieving this in a non-breaking way doesn't seem feasible, meaning this pull request is superseded by #10420

@almeidx almeidx closed this Aug 20, 2024
@almeidx almeidx removed the blocked label Aug 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants