-
Notifications
You must be signed in to change notification settings - Fork 4
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/
From k6 docs:
This PR replaces usages of
Blob
with k6-compatibleArrayBuffer
and improves the request body generation logic.