Skip to content

Conversation

@1st1
Copy link
Member

@1st1 1st1 commented Dec 18, 2024

This is a draft

ToDo:

  • Unify Session / Options objects
  • Implement withCodecs/decode
  • Implement withCodecs/encode
  • Add strict typing for withCode(spec)
  • Add docs
  • Cleanup more legacy protocol code
  • Kill setCustomCodecs
  • Enum is a bit weird -- add a test for enum types

@1st1 1st1 force-pushed the withCodecs branch 2 times, most recently from 339f2d2 to ed1786d Compare January 10, 2025 00:27
@1st1 1st1 marked this pull request as ready for review January 10, 2025 08:10
@1st1 1st1 requested review from jaclarke and scotttrinh January 10, 2025 08:10
Comment on lines +97 to +98
// TODO: Figure out if we can drop the dep
// on an external package for Float16Array.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unfortunately not: only Deno, Firefox, and Safari support native Float16Array

Copy link
Member Author

Choose a reason for hiding this comment

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

We can emulate is in a very very limited way the same way that third party library does. I just want us to "own" the type we'd be proxying through our API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, you mean basically vendor the existing library? We can also just use Float32Array until there is better support for Float16Array.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've looked at that implementation, it's huge and complicated :/ Vendoring is an option

@1st1 1st1 closed this Jan 11, 2025
@1st1 1st1 reopened this Jan 11, 2025
@1st1 1st1 merged commit df2faf9 into master Jan 11, 2025
9 checks passed
@1st1 1st1 deleted the withCodecs branch January 11, 2025 05:47
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.

4 participants