-
Notifications
You must be signed in to change notification settings - Fork 5
Replace usages of Blob type with k6-compatible ArrayBuffer #32
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
base: main
Are you sure you want to change the base?
Conversation
| } else if (body.contentType === 'application/json') { | ||
| fetchBodyOption = `JSON.stringify(${body.implementation})` |
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.
Json serialization should be used only for application/json content type.
| const newFileContent = fileContent.replaceAll( | ||
| /(^|\W)Blob(?!\w)/g, | ||
| '$1ArrayBuffer' | ||
| ) | ||
|
|
||
| fs.writeFileSync(filePath, newFileContent) |
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.
Do we have any tests for this?
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 is the assertion that ensures Blob is replaces with ArrayBuffer https://github.com/grafana/openapi-to-k6/pull/32/files#diff-71baa70128ba18db2f8771ff948db5c4aaafd789460c654a47705da6e9015584R74
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.
This will actually replace "Blob" string from the documentation or other string occurrences as well - "foo Blob bar" will be replaced too.
Let not use regex replace for this and try to use ArrayBuffer, I will have to check with orval and code if that is possible thought
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.
The Blob type is hardcoded in orval and can not be easily overridden. I also tried redefining the Blob type as an alias to ArrayBuffer via a file header or footer, but this did not work for clients where the schema components are defined in a separate file.
I'm all for a cleaner solution.
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.
How about we allow taking the individual functions Blob type as input and when calling k6, convert it to ArrayBuffer?
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.
k6 does not support Blobs not only as a parameter to the request function, but as a runtime type in general. For example, instantiating a Blob in the script with new Blob() results in the following error:
ERRO[0000] ReferenceError: Blob is not defined
One way or another, the usage of Blobs should be replaced in the generated clients. It results in type declarations that can not be satisfied, defeating the whole point of using TypeScript.
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.
We do have a blob interface in the websockets library of k6 though - https://grafana.com/docs/k6/latest/javascript-api/k6-experimental/websockets/blob/
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.
@DefCon-007 websockets/Blob is an interface that is specific to the WebSocket.onmessage callback. It can not be instantiated directly in the script. Additionally, Blob and websockets/Blob are different types, so TypeScript will complain if we try to assign one to the other.
|
@fornfrey is this still relevant? |
From k6 docs:
This PR replaces usages of
Blobwith k6-compatibleArrayBufferand improves the request body generation logic.