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

fix: change serialization format from FormData to MessagePack #5

Merged
merged 9 commits into from Jan 13, 2023

Conversation

angeloashmore
Copy link
Member

@angeloashmore angeloashmore commented Jan 13, 2023

Types of changes

  • Chore (a non-breaking change which is related to package maintenance)
  • Bug fix (a non-breaking change which fixes an issue)
  • New feature (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This PR replaces FormData with MessagePack as r19's serialization format. MessagePack is implemented using the @msgpack/msgpack package.

FormData is implemented differently depending on the runtime environment. Browsers, various Node.js versions, polyfills, and JS browser-like environments each have quirks in their implementations. Quirks lead to unexpected incompatiblities.

Rather than use a format that may not always serialize or deserialize in a consistent manner, we can use a standardized format, like MessagePack. MessagePack is an efficient binary serialization format that supports native JavaScript data, like objects, Dates, and more. Binary data, like images, can be transferred easily as well.

FormData was originally selected to minimize the package's size. Using native browser code means r19 can be shipped with less code. Unfortunately, limited Node.js support for FormData at this time makes it an unsuitable choice.

Checklist:

  • My change requires an update to the official documentation.
  • All TSDoc comments are up-to-date and new ones have been added where necessary.
  • All new and existing tests are passing.

@github-actions
Copy link

github-actions bot commented Jan 13, 2023

size-limit report 📦

Path Size
./dist/index.cjs 17.08 KB (-51.46% 🔽)
./dist/index.js 10.51 KB (-60.52% 🔽)
./dist/client/index.cjs 7.29 KB (+88.4% 🔺)
./dist/client/index.js 7.43 KB (+216.44% 🔺)

@angeloashmore angeloashmore changed the title refactor: change serialization format from FormData to MessagePack fix: change serialization format from FormData to MessagePack Jan 13, 2023
@codecov-commenter
Copy link

Codecov Report

Base: 90.10% // Head: 86.90% // Decreases project coverage by -3.20% ⚠️

Coverage data is based on head (4b3bb79) compared to base (58141fb).
Patch coverage: 83.13% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master       #5      +/-   ##
==========================================
- Coverage   90.10%   86.90%   -3.21%     
==========================================
  Files          17        9       -8     
  Lines        1021      649     -372     
  Branches       78       55      -23     
==========================================
- Hits          920      564     -356     
+ Misses        100       84      -16     
  Partials        1        1              
Impacted Files Coverage Δ
src/handleRPCRequest.ts 64.28% <68.75%> (-6.84%) ⬇️
src/lib/replaceLeaves.ts 81.63% <81.63%> (ø)
src/client/createRPCClient.ts 94.26% <100.00%> (+1.52%) ⬆️
src/createRPCMiddleware.ts 100.00% <100.00%> (ø)
src/types.ts 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@angeloashmore angeloashmore merged commit 408c5cb into master Jan 13, 2023
@angeloashmore angeloashmore deleted the aa/msgpack branch January 13, 2023 04:48
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.

None yet

2 participants