-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
More ArrayBuffer support #1841
Merged
Merged
More ArrayBuffer support #1841
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
89ff054
Return ArrayBuffer from open()
ceac03b
Fix some scopelint issues in tests
819e0f2
Return ArrayBuffer from binary response bodies
824e6cf
Handle ArrayBuffer conversion from http.batch()
cc39643
Return ArrayBuffer from crypto.randomBytes()
d30c7b0
Return ArrayBuffer from hasher.digest('binary')
a4927b6
Return ArrayBuffer from encoding.b64decode() by default
71b02fc
Use ToBytes to convert response bodies in Response.HTML()
23af28f
Move JSON response body processing to JS http module
c211b9e
Add ToString function to avoid unnecesary conversion from []byte
d8e8345
Add ArrayBuffer support in k6/ws module
3d8b343
Send binary WS message using a separate method
77f8530
Handle binary WS message using a separate event handler
95337c0
Call WS message handlers depending on the message type
777155d
Check WS SendBinary argument and raise error if not ArrayBuffer
d3dcbcd
Fix and silence linter issues
9f623d8
Error with JS type in WS SendBinary
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
here @imiric , there is no point in having
ab
;)This (for ArrayBuffer) is done in another 5 places probably
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.
Hhmm how would I get the pointer to it otherwise? Parenthesis wouldn't work in this case.
Though I did also spot
msg := message.Export()
which is not needed.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.
I also forget that you can't directly get the pointer from the function call 🤦
On the other hand it seems like ... you don't need to? I mean do you really need to give a pointer?
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.
I err'd on the assumption that it's better to pass around pointers to this data than incur copying penalties, but considering the internal implementation is a pointer I guess it would be safe to return the struct itself.
Do you think this is worthwhile changing?
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.
as my original comment suggests, I don't think it is a requirement.
In general, this means that it will now need to dereference 2 pointers in order to get to the actual data which is objectively worse, but not enough to matter in the grand scheme of things