Skip to content
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

Use native Blob on Node.js 15.7.0 #1079

Open
szmarczak opened this issue Jan 27, 2021 · 8 comments
Open

Use native Blob on Node.js 15.7.0 #1079

szmarczak opened this issue Jan 27, 2021 · 8 comments

Comments

@szmarczak
Copy link

@szmarczak szmarczak changed the title Use native Blob on Node.js Use native Blob on Node.js 15.7.0 Jan 27, 2021
@Richienb
Copy link
Member

Richienb commented Jan 28, 2021

Fixed in node-fetch/fetch-blob@689bee1

@xxczaki @jimmywarting Can one of you do a release? reverted for now

@szmarczak
Copy link
Author

Are you sure fetch-blob is the right place to make the fix? In my opinion, an option inside node-fetch would suit this better. For example, you can pass a fetch option in Ky: https://github.com/sindresorhus/ky/#fetch

@Richienb
Copy link
Member

We need to make the API just work. Adding an extra option that deviates from the fetch spec might not be a good idea unless there's a problem we need to avoid.

@szmarczak
Copy link
Author

Tests are failing.

@Richienb
Copy link
Member

Good point. I think we'll either want to make Node.js Blobs pass the tests and adjust fetch-blob to work more closely as a polyfill or the other way round.

@jimmywarting
Copy link
Collaborator

jimmywarting commented Jan 28, 2021

Oh sweet, native blob support!

well, one thing that was brought up was

// eventually
const blob1 = await fs.promises.readAsBlob('path/to/some/file1')
// blob1 identifies a persistent memory location where blob data is stored.

We have that support already, we made it like so: const blob = require('blob/from.js')(path)
I first thought about putting it on Blob.of(path) or Blob.from(path) but wanted it to be more closer to the spec


Seems to live at buffer.Blob I thought that was a weird place to put it... think it should have been put in utils like TextDecoder was and eventually move it into the globalThis, but whatever.


I guess Node's Blob don't yet support .stream()? cuz of whatwg stream is missing?
and it's missing fs.readAsBlob also?
and the only way to get a Blob instance is by constructing them with data you already have in the memory - we also only supported that in the past


We have now based ours on blob parts instead a being a buffer container containing all data like it was before when we didn't have blob-from-path. And because of that our fetch-blob can support other blob-like parts that isn't the same instanceof itself as long as it behaves as a Blob and have a way to read them. this is thanks to that it now operates on parts and not a single buffer container like how node's buffer.Blob implementation is at the moment.

So it could be possible to wrap them in such a way like this if we would like to do that:

import FetchBlob from 'fetch-blob'
import blobFromPath from 'fetch-blob/from.js'
import { Blob } from 'buffer'

var native = new Blob()
var fetchBlob = new FetchBlob([native, blobFromPath('./package.json')])

If you do want to upload something big, then you want to read the blob as a stream, hence why Blob.prototype.stream() exist. and you probably would want to use fetch-blob/from since there isn't any fs.promises.readAsBlob yet that can references a 2+ gb large file

fetch(url, {
  method: 'POST',
  body: blob_2gb
})

There is some pro and cons to nodes and fetchs blob

  • fetch-blob have blob/from that can handle very large files that makes it more superior to handle large data
    • nodes blob can yet only be constructed and behave more like a (Array)Buffer for now
  • fetch-blob have have .stream() support (all doe it's a node stream and not a whatwg stream)
    • we could technically polyfill stream by patching Blob.prototype.stream if it don't exist or create some "make-chunked-node-stream-from-blob" function that slices and read from slice(start, end).arrayBuffer() as a fallback if .stream() don't exist
  • Node's blob can be sent via MessagePort to multiple destinations without transfering or immediately copying the data that makes it more superior than fetch-blob
    • but since they can only be constructed for now what makes it more useful than using the transferable argument?
      postMessage(arrayBuffer, [arraybuffer]) sure you can send it to multiple destinations but i don't see that as normal huge usecase, you either slice and send or only use one destination
  • fetch-blob constructor can accept a buffer.Blob part, buffer.Blob can't accept anything other than buffer.Blob?

So you have to weight in if you either want to have support for large data with blob/from or if you want to be able to send them over postMessage to multiple destinations


IMO i think large data have a better usecase for now. We have talked about it before and Blob & Files have yet no real usecase in the NodeJS land if they can only be constructed with other in memory variables. then you can as well stick with the buffer or string that you already have declared as a variable. and do things with it. the really good usecase starts to come when you must handle large data and have to do so with blob's backed up by the filesystem, the 2nd good usecase is to create workers from URL.createObjectURL(blob) but node haven't supported it yet 3th usecase would be as mention earlier, sending postMessage to multiple destinations without copying data, but that is only useful if you want to send to multiple destinations and don't want to transfer data and loose the information

A Blob was first invented to hold data that dosen't exactly have to live in the memory so that you could read data from <input type="file">


since buffer.Blob are currently experimental and lacks support for .stream() and blob-from-path then i would want to hold off from using it currently. I would like to at least first see support for fs.promises.readAsBlob before considering replacing fetch-blob with native buffer.Blob

jimmywarting referenced this issue in node-fetch/fetch-blob Jan 28, 2021
@jimmywarting
Copy link
Collaborator

I think we can start of with the easiest thing first to support accepting buffer.Blob as an input for new Request(url, {body: Blob}) and new Response(blob). then we can decide if (request || response).blob() should return a buffer.Blob or a fetch-blob at the very least it has to return a blob with a .stream() functionality.

@jimmywarting
Copy link
Collaborator

jimmywarting commented Feb 11, 2021

PR/Issue for supporting native blob with any kind of stream or async iterator as input have been made
#1091 -> #1092

Still doe, Buffer.Blob.prototype.stream are still missing, whatever kind of stream they implement we will support it in the feature

EDIT made it support none streaming blob also by slicing/reading
buffer.Blob is a bit buggy as of now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants