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

Support for reading from Blob in Node.js #588

Merged
merged 3 commits into from Mar 31, 2023
Merged

Conversation

Borewit
Copy link
Collaborator

@Borewit Borewit commented Mar 29, 2023

Support for reading from Blob in Node.js context.

Resolves: #581

First merge:

@Borewit Borewit self-assigned this Mar 29, 2023
@Borewit Borewit marked this pull request as ready for review March 31, 2023 11:18
@sindresorhus sindresorhus merged commit 1c75cfb into main Mar 31, 2023
6 checks passed
@sindresorhus sindresorhus deleted the support-nodejs-blob branch March 31, 2023 18:58
@sindresorhus
Copy link
Owner

@Borewit Thanks. Maybe we can get rid of browser.js completely by moving the ReadableStream method into core and exposing it in Node.js if Node.js 18 (that's when it's a global) or later?

@Borewit
Copy link
Collaborator Author

Borewit commented Apr 3, 2023

The current implementation fileTypeFromStream() converts a Web-API readable stream into a Node.js readable stream, so essentially an adapter.

The fact that it implemented as an adapter makes it unsuitable at the moment to be moved into core.

Only when we are able utilize the Node.js ReadableStream it does make sense to move it into core. And actually only when this could be used to process both the

  1. Web-API readable stream and the
  2. Node.js ReadableStream

which should be in theory the same streams WHATWG streams, but are in reality, not necessary interchangeable:

  1. There are issues with different TypeScript definitions
  2. If I remember well, also the implementation was done differently hence the typings could not be blended.

Caveat on the file-type Blob function

The way the implementation is, and was, inconsistent with the other function, as it does not utilize the lazy reading we use with streams. As it converts the Blob to a buffer, use cases where a Blob (File) is still stream (like when you upload a file), the current mechanism will read the entire Blob (File) in memory.

Related to the previous discussion, utilizing the Blob.stream() this could be prevented. At the other hand, the user can do this as well, and provide that to the WHATWG stream read function we just discussed.

This was referenced Apr 4, 2023
@jimmywarting
Copy link
Contributor

if the hole file dose not need to be read in order to get magic numbers (head and footer)
then i suggest that you also slice the file

head = blob.slice(0, 1024)
footer = blob.slice(-1024)

you could also make a really small blob out of it: new Blob([head, footer]).stream()

Also worth mentioning that there now also is a fs.openAsBlob(path) method that could make reading files (partially) easier and more cross platform ergonomic. just using openAsBlob alone won't allocate the file into memory. it's read lazzily when you use any of the read methods...

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

Successfully merging this pull request may close these issues.

Importing fileTypeFromBlob in Node environment
3 participants