-
Notifications
You must be signed in to change notification settings - Fork 608
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: support ArrayBufferView
and ArrayBuffer
as body
#1584
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1584 +/- ##
==========================================
- Coverage 94.91% 94.89% -0.03%
==========================================
Files 50 50
Lines 4724 4724
==========================================
- Hits 4484 4483 -1
- Misses 240 241 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a test
Yes, see extract:
|
This is true spec-wise, but seems costly in implementation. I wonder if there is a workaround with better performance, considering that most other implementations do not expose Anyway, even if it's possible to optimize later, working code and spec compliance goes first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
ping @ronag |
* fix: support `ArrayBufferView` and `ArrayBuffer` as body * test: ArrayBufferView as Request.body
* fix: support `ArrayBufferView` and `ArrayBuffer` as body * test: ArrayBufferView as Request.body
Implementation-wise, do we really need to make a copy?