octet-stream / form-data

Spec-compliant FormData implementation for Node.js
https://www.npmjs.com/package/formdata-node
MIT License
142 stars 17 forks source link

File fields are not instances of Blob #14

Closed jaydenseric closed 3 years ago

jaydenseric commented 4 years ago

A File field should be an instance of Blob when read via FormData.get().

For example, in Chrome:

Screen Shot 2020-02-03 at 11 18 12 pm

I'm trying to assert in a project that a Blob instance (globally polyfilled via fetch-blob) gets correctly added to a FormData instance, and one of the things to check is that the type of the field that was added is a Blob.

octet-stream commented 4 years ago

Please read the docs:

Returns the first value associated with the given name. If the field has Blob, Buffer or any Readable and ReadableStream (and when options.size is set for this stream) value, the File-like object will be returned.

I don't polyfill anything, instead I use objects which have same methods and properties as those from browsers API. It allows you to use your own Blob and File implementations if they're compatible. If you want to check whether returning field is a Blob, File or ReadableStream, you must check it other way than just call instanceof. See: util/isWHATWGReadable.mjs and util/isBlob

Just to mention, this is how node-fetch handles the same checks: utils/is.js

jaydenseric commented 4 years ago

That may be how it works now, but it could be better.

octet-stream commented 4 years ago

Now that's sounds good to me. I will take a look at that. Thanks again!

octet-stream commented 4 years ago

hey, looks like fetch-blob has Symbol.hasInstance, so the myFile instanceof Blob should work.

See: https://github.com/node-fetch/fetch-blob/pull/46

jaydenseric commented 4 years ago

Hmm, this used to not work so maybe something changed 🤔

instanceof Blob is working now in my tests, so the original reason for this issue is resolved. I'll leave it up to you to close it or not here, because I'm not sure how anything other than the originally suggested fix could resolve other potential issues.

octet-stream commented 3 years ago

Hi @jaydenseric 👋 I think since Node.js has the first class Blob support, I can use fetch-blob in this package and then eventually add native Blob support.

octet-stream commented 3 years ago

So, this change available in https://github.com/octet-stream/form-data/releases/tag/v3.0.0. I believe this issue is now resolved.

octet-stream commented 3 years ago

Also, note that public API has been changed, so be careful and read release note before upgrading :)