search

tus / tus-js-client

A pure JavaScript client for the tus resumable upload protocol
https://tus.io/
MIT License
2.11k stars 316 forks source link

Allow upload strings as file content #488

Open piranna opened 2 years ago

piranna commented 2 years ago

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:

  1. Provide a string as data in the Upload() class constructor
  2. Start upload '...'
  3. See error

Expected 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:

Acconut commented 2 years ago

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 Buffer.from on their own to convert the string into a buffer. This way they also have full control over the encoding and it does not require a lot of work from the users.

piranna commented 2 years ago

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.

Acconut commented 2 years ago

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: https://github.com/tus/tus-js-client/blob/b031f98d9fe71dc28d3b162ec47a22163db8b8d0/test/spec/helpers/utils.js#L8-L17

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?

piranna commented 2 years ago

We currently have an internal test helper which constructs buffers or blob out of strings:

Yes, my propose is to don't lead users to have to deal with this kind of hacks themselves.

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?

Sure, where can I start? Add the strings support and remove the internal getBlob() function?

piranna commented 2 years ago

PR available at https://github.com/tus/tus-js-client/pull/493.