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

brotli-decompress received binary websocket messages #41

Merged
merged 1 commit into from
Mar 29, 2024

Conversation

gefjon
Copy link
Contributor

@gefjon gefjon commented Mar 27, 2024

Description of Changes

Uses brotli.js to decompress incoming binary messages before Protobuf deserializing.

API

  • This is an API breaking change to the SDK

If the API is breaking, please state below what will break

Requires SpacetimeDB PRs

@RReverser
Copy link
Member

RReverser commented Mar 27, 2024

Let's switch to brotli-dec-wasm instead, should be faster & smaller (1.5 MB for brotli.js vs 350KB for brotli-dec-wasm).

@RReverser
Copy link
Member

That way we'll be able to get rid of extra Buffer dependency too, which really shouldn't be necessary on the Web.

@gefjon
Copy link
Contributor Author

gefjon commented Mar 29, 2024

@RReverser 's version would have been better, but it turns out that making it work across different platforms (incl. the test platform) is a pain. We'll merge this now, and make improvements later.

@RReverser
Copy link
Member

I was also wrong about 1.5MB - because you imported /decompress submodule directly, it's closer to 500KB, so it's not as bad as the entire module looked.

But yeah we'll revisit in future.

Copy link
Contributor

@bfops bfops left a comment

Choose a reason for hiding this comment

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

"LGTM"; this doesn't seem to have much surface area besides obviously-needed.

Copy link
Contributor

@kazimuth kazimuth left a comment

Choose a reason for hiding this comment

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

Yeah, LGTM.

@gefjon gefjon merged commit 390c933 into main Mar 29, 2024
2 checks passed
@joshua-spacetime joshua-spacetime linked an issue Apr 9, 2024 that may be closed by this pull request
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.

Decompress protobuf messages
4 participants