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 node native Blob #1159

Closed
zdm opened this issue May 15, 2021 · 4 comments
Closed

Use node native Blob #1159

zdm opened this issue May 15, 2021 · 4 comments

Comments

@zdm
Copy link

zdm commented May 15, 2021

Hi.
You can use native nodejs Blob class instead of your custom Blob.

https://nodejs.org/api/buffer.html#buffer_class_blob

@jimmywarting
Copy link
Collaborator

jimmywarting commented May 15, 2021

Hi. i'm aware of it already

There is some characters that makes native NodeJS blob unpleasant.
I have helped review theirs implementation and there are three things that stands out

  1. buffer.Blob don't support blob.stream() it's a bit important for handling large files. All doe this can be worked around by doing a slice-and-read blob.slice(0, 1024).arrayBuffer().then(continue_iterating). It's important that we keep supporting blobs that has a stream method when calling (await response.blob()).stream()
  2. NodeJS, Deno's and many older Blob polyfills implementation operates on a single buffer source. This essentially means that they keep all the data in the memory in some ArrayBuffer. They don't operate on a so called blob-part-sequence that is defined in the spec that can represent other chunks of data coming from different sources.
    So what will happen when you slice a piece of chunk with buffer.Blob is that they will copy part of the data that is kept in memory into the new blob and double the memory consumption. This is not how it works in the browser... When you slice something in the browser then it will create a dummy container that don't hold any data - instead it will just be a references point to some older blob with a other size and offset of where it should start and stop reading from. So slicing blobs in the browser is very cheep operation compare to buffer.Blob that clones the data internally
  3. b/c of what is stated in 2. they got no way of mixing blobs coming from other places... like from the filesystem for example.

b/c of point 2 and 3 this essentially renders buffer.Blob as a pretty useless container for holding any data. It's just essentially a dummy container wrapped around a ArrayBuffer that fills little use/benefits of what makes a blob a blob. You may just as well use ArrayBuffer instead.
The hole point of a blob is that it should be able to offload some of the memory to the disk. Neither fetch-blob, NodeJS or Deno dose this in theirs implementation (only browser got it right) but at least fetch-blob operates on blob parts instead so it can hold chunks of data that originate from a file path and dose not take up any extra space/memory so this kinds of blob are read lazy when needed, fetch-blob also handels slicing & referring other chunks better and more in line of how browser and the spec defines a Blob

Trust me I would like nothing more than everyone start using buffer.Blob instead but buffer.Blob is still very experimental and they have two issues I would like to see be fixed first before I would even like to consider using buffer.Blob instead

Then there is also the hole backward compatibility, they introduced it in v15.7 and got some tiny stuff wrong but got fixed later
we have to support older node versions as well.

@jimmywarting
Copy link
Collaborator

closing as dupe of #1079

@zdm
Copy link
Author

zdm commented May 15, 2021

  1. We can create class, inherited from Blob, with the stream method.
  2. Yes, api is experimental, need to wait for stable release.

@jimmywarting
Copy link
Collaborator

jimmywarting commented May 15, 2021

We can create class, inherited from Blob, with the stream method.

Yes, we could but i'm a bit sceptical about it. it could be possible.

fetch-blob is more generic as it has the ability to accept other blob like classes as well and "upgrade" them to a new Blob instance with a stream method

import buffer from 'node:buffer'
import FetchBlob from 'fetch-blob'
import blobFrom from 'fetch-blob/from.js'

const fsChunk = blobFrom('./package.json')
const nativeChunk = new buffer.Blob(['Hello world'])

new FetchBlob([ fsChunk, nativeChunk ])

The same is not true for buffer.Blob it don't know how to handle other blob like classes or how to read them.
In fetch-blob we would like to start using native blob as well, but something like nodejs/node#37340 is still missing and that depends on nodejs/node#37338 being fixed first

Guess that fetch-blob will become like safe-buffer once was.

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

2 participants