-
Notifications
You must be signed in to change notification settings - Fork 316
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
Allow upload strings as file content #488
Comments
If I understand this correctly, you are requesting a new feature and not reporting a bug. Nowhere to my knowledge, we are advertising that you can pass strings to the Upload constructor. Or am I missing something? To be honest, I also do not think we need to add this to tus-js-client and let users call |
Yes, It can be seen as an improvements, and although it's true users can call Buffer.from() on their own, it's so much common to be able to use both Buffers and strings for binary data or text, that's a bit annoying to need to do It yourself, that's why I ask to add it. |
I have never seen binary data being handled in strings, especially not inside Node.js where you have the Buffer class. But let's not discuss this here. After thinking a bit more about this, it might also be handy for our internals. We currently have an internal test helper which constructs buffers or blob out of strings: tus-js-client/test/spec/helpers/utils.js Lines 8 to 17 in b031f98
If we pull this functionality internally into tus-js-client we could also get rid of this function. Would you be interested in working on this? |
Yes, my propose is to don't lead users to have to deal with this kind of hacks themselves.
Sure, where can I start? Add the strings support and remove the internal |
PR available at #493. |
Describe the bug
At https://github.com/tus/tus-js-client/blob/master/lib/node/fileReader.js, it's being checked that the provided data is a Buffer instance or a Readable stream, but it's not possible to use a string, you need to convert it first to a Buffer yourself.
To Reproduce
Steps to reproduce the behavior:
Upload()
class constructorExpected behavior
Allow to use strings as data input, maybe converting it internally to a Buffer if needed, but I think it can be provided directly.
Setup details
Please provide following details, if applicable to your situation:
The text was updated successfully, but these errors were encountered: