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: added snappy compression support #584

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

Conversation

m0lc14kk
Copy link

Added support for snappy compression algorithm.

Compression is provided by snappy package, which is lightweight and the fastest way to use this type of compression.

@extremeheat
Copy link
Member

This is alright, but the problem is that it's a C++ library and we'd like to move away from requiring any native dependencies at all to avoid needing to install a bunch of native build tools and other complexity that comes along with it. A pure JS or WASM solution would be more ideal, but we can offer a native option too ideally just not a required dep in package.json (like how we support different raknet backends).

@m0lc14kk
Copy link
Author

okay, so if I would use a pure JS snappy solution and use it in bedrock-protocol, you would be okay with it and applied it to the next version?

I just found snappyjs minified file, which bedrock-protocol can use to handle snappy compressions

what do you think about copying this minified file or maybe just using this dependency?

@extremeheat
Copy link
Member

Yes, you can try https://www.npmjs.com/package/snappyjs. Adding a test would be good.

Changed dependency from snappy to snappyjs, to have a pure implementation of an algorithm.
Added a Snappy example in tests.
Copy link

socket-security bot commented Feb 28, 2025

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/[email protected] None 0 21.6 kB zhipengjia

View full report↗︎

@m0lc14kk
Copy link
Author

I just added a small test and changed the library, you can tell me if that's enough to add this to bedrock-protocol.

@extremeheat
Copy link
Member

CI is failing

And for the test you can update one of the internal server tests to set snappy as the compressor

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

Successfully merging this pull request may close these issues.

2 participants