jsdom icon indicating copy to clipboard operation
jsdom copied to clipboard

Implement `Blob `stream()`, `text()`, and `arrayBuffer()`

Open jimmywarting opened this issue 7 years ago • 34 comments

  • https://bugs.chromium.org/p/chromium/issues/detail?id=945893#c1
  • https://chromestatus.com/feature/6206980857266176
  • w3c/FileAPI#117
  • bitinn/node-fetch#604
  • bitinn/node-fetch#392

We over at node-fetch has a response.blob() method that allows you to get a blob like object, it's currently useless cuz you can't read it in any way except from doing:

new Response(blob).arrayBuffer()

We are planing on adding Blob.text() and Blob.arrayBuffer() at least, not sure about blob.stream() yet if it's going to be a node stream or a web stream we do not currently expose the constructor in any way but we would like to make this compatible with your FileReader. If you changed the FileReader api to read the content of the file using this methods you could read any blob/file like object not only coming from your own jsdom implementation.

fr.readAsBuffer() --> blob.arrayBuffer().then(dispatch) fr.readAsText() --> blob.text().then(dispatch) fr.readAsDataURL() --> blob.arrayBuffer().then(buffer2dataURL).then(dispatch)

jimmywarting avatar Apr 15 '19 17:04 jimmywarting

I'm not sure what the exact ask is here, but I'll note that jsdom, like browsers, reads the contents of things using internal APIs, so that you can't fool it with "non web platform" objects (i.e. non-jsdom objects). This is by design.

domenic avatar Apr 15 '19 17:04 domenic

There is now new ways to to read blob/files using a promise based api that exist as a method on the Blob class, mainly Blob.prototype.text and Blob.prototype.arrayBuffer when called both returns a promise. this is what i want you to implement

jimmywarting avatar Apr 15 '19 17:04 jimmywarting

What I'm also asking for is: That you change the way the FileReader how it reads the content. Using the new "internal public APIs" on the blob itself instead of reading it from blob._buffer

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

These are not internal APIs; they are public APIs. So this will not generally work, just like what you're asking for won't work in browsers.

domenic avatar Apr 15 '19 18:04 domenic

ups, meant public apis

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

Yeah, that would be going against the spec, so it's not an option for us.

domenic avatar Apr 15 '19 18:04 domenic

Blob.stream, Blob.text and Blob.arrayBuffer are speced, not just implemented anywhere yet

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

Right, but if you override them to throw an exception, then FileReader will still work (per spec), because FileReader uses internal data access, not the public APIs.

Your request is that if you override them to throw an exception, FileReader will call those public APIs and fail. We are not interested in violating the spec in that way.

domenic avatar Apr 15 '19 18:04 domenic

okey, so apart from my second request about changing the FileReader to use the public blob api. Would you consider adding the new public blob methods that allows you to get things with a promise?

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

For sure. Pull request welcome!

domenic avatar Apr 15 '19 18:04 domenic

Actually, when i read the readAsText spec now it says

  1. If fr ’s state is "loading" , throw an InvalidStateError DOMException .
  2. Set fr ’s state to "loading" .
  3. Set fr ’s result to null .
  4. Set fr ’s error to null .
  5. Let stream be the result of calling get stream on blob .
  6. Let reader be the result of getting a reader from stream .
  7. ...
  8. ...

so it states that the FileReader should read the content of the blob using the Public API's not some internal things?

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

get stream links to https://pr-preview.s3.amazonaws.com/w3c/FileAPI/117/795750f...42f8a45.html#blob-get-stream, not to https://pr-preview.s3.amazonaws.com/w3c/FileAPI/117/795750f...42f8a45.html#stream-method-algo

domenic avatar Apr 15 '19 18:04 domenic

thought https://pr-preview.s3.amazonaws.com/w3c/FileAPI/117/795750f...42f8a45.html#blob-get-stream was the same thing as calling https://pr-preview.s3.amazonaws.com/w3c/FileAPI/117/795750f...42f8a45.html#stream-method-algo

That the "A Blob blob has an associated get stream algorithm" which was the same thing as calling blob.stream()

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

Nope, #blob-get-stream is private, which two public methods stream() and readAsText() both call. Pretty standard pattern for factoring out "private methods" in spec text.

domenic avatar Apr 15 '19 18:04 domenic

oh well, still think you should use the public method to read the blob... would help some folks that use node-fetch that calls response.blob() to get a jsdom-like object - not sure why they do it if they are going to read it anyway with with fr.readAsWhatever(), could as well call response.arrayBuffer() directly or something like that

but if you are so strict, so be it. I'm not using jsdom or any combination of both node-fetch + jsdom. So it's not my problem

jimmywarting avatar Apr 15 '19 18:04 jimmywarting

Any chances Blob implementation could be extracted into separate library so both libraries could be made interoperable ? Right now both libraries have a strong opinions and as result they can't interop and combination of two is pretty much unusable.

Gozala avatar Apr 15 '19 19:04 Gozala

@domenic I think @jimmywarting is proposing very specific solution, although actual goal here is to make two libraries interoperable. Maybe we can shift discussion towards how two libraries can interop ?

Specific issue I've being running into in the context of https://github.com/WebMemex/freeze-dry/pull/44 is that project uses node-fetch and jsdom in nodejs where things seems to misbehave because blob instanceof Blob is false when one comes from jsdom and other from node-fetch because each bundles own implementation of Blob.

I believe what @jimmywarting was hoping is for both libraries to migrate from checks like blob instanceof Blob to blob.arrayBuffer(), which would resolve incompatibility issues.

@domenic would you please collaborate with us on a solution to this interop issues which would not require node-fetch to depend on jsdom ?

Gozala avatar Apr 16 '19 23:04 Gozala

would you please collaborate with us on a solution to this interop issues which would not require node-fetch to depend on jsdom ?

The problem here is that you include node-fetch at all. If your scripts ran in jsdom, your isomorphic libs should act as if they're in a browser (that is: detect the presence of XmlHttpRequest or similar) and get the "browser's" (jsdom's) native Blob implementation.

Sebmaster avatar Apr 17 '19 00:04 Sebmaster

The problem here is that you include node-fetch at all. If your scripts ran in jsdom, your isomorphic libs should act as if they're in a browser (that is: detect the presence of XmlHttpRequest or similar) and get the "browser's" (jsdom's) native Blob implementation.

I am not sure how did you arrive to that conclusion, but I respectfully disagree. Use only JSDOM or no JSDOM at all isn’t a great solution for the community, there are & will continue to be use cases that don’t fit this binary proposition

P.S.: My scripts aren’t run by JSDOM, it is used mostly to represent document in DOM API compatible API.

Gozala avatar Apr 17 '19 00:04 Gozala

Use only JSDOM or no JSDOM at all isn’t a great solution for the community

Not what I said.

there are & will continue to be use cases that don’t fit this binary proposition

Please do elaborate with specifics. It usually makes no sense to have a hybrid environment for testing, since that means you're not actually testing in any reasonably expected environment.

P.S.: My scripts aren’t run by JSDOM, it is used mostly to represent document in DOM API compatible API.

jest usually runs tests in jsdom by default unless you explicitly disable it. If you haven't configured that, the problem is in the detection of a "node environment" in the fetch implementation. It should detect that it's running in a browser.

If you did explicitly configure it otherwise, then you're mixing browser and non-browser environments which is something we explicitly recommend against (the pattern doesn't quite apply, but the results are the same) because of exactly these issues.

Sebmaster avatar Apr 17 '19 00:04 Sebmaster

I agree overall with @Sebmaster that you should be running your scripts inside the jsdom, and that using node-fetch (instead of the whatwg-fetch polyfill) is probably the core of the problem.

Stated another way, jsdom emulates a web browser---and that's a large project, enough to keep us quite busy. It seems like some folks in this issue thread are asking us to expand the project's scope beyond "just" emulating a web browser, to also emulate Electron, i.e. have mechanisms for bridging between Node-level primitives and the "web browser environment" jsdom provides. That is, whatwg-fetch runs in a browser; node-fetch can only run in a browser if that browser also grows all the Node-level capabilities that Electron has.

That's not really feasible for the project as it is currently scoped or envisioned. It may be an interesting project for someone else to pursue on top of jsdom, similar to how Electron is built on top of Chromium.

domenic avatar Apr 17 '19 01:04 domenic

Here's a simple scenario I think should be supported, but is not (due to the points made here, specifically by @jimmywarting and @Gozala): testing code that fetches an Image from a URL, uses FileReader to populate an Image, extracts width & height, and then proceeds to process the image data (save to S3, for instance).

  1. create a Blob via blob-utils dataURLToBlob and a small PNG data URI in my test file
  2. setup a response with fetch-mock for the given image URL to yield the Blob setup in step 1
  3. invoke my function under test

I was surprised to discover today the incompatibility between fetch-mock+node-fetch and jsdom in this area. I think that @Gozala has a great point-- let's find a way to make these two compatible.

Why is it so important for jsdom to use it's own Blob? Why couldn't it be extracted out into a separate package, usable by (in this case) both jsdom and node-fetch?

Why is it so important for jsdom's FileReader to access Blob via private variables _buffer instead of public interfaces?

The request here is not so unreasonable, and yet it seems like all effort of finding common ground has ceased. 2 ideas have already been put forth and rejected by one side... can the other side at least suggest some alternatives besides "you're doing it wrong"?

gap777 avatar Jun 16 '21 01:06 gap777

https://developer.mozilla.org/en-US/docs/Web/API/Blob

Array buffer is implemented in everything but IE, could we get an update on this?

cranesandcaff avatar Mar 16 '22 18:03 cranesandcaff

https://mobile.twitter.com/slicknet/status/782274190451671040

domenic avatar Mar 16 '22 18:03 domenic

A workaround for this for jest is to add a setupFilesAfterEnv script that does a

require("blob-polyfill");

tom-james-watson avatar Mar 23 '22 14:03 tom-james-watson

This works for File constructor.

nemanjam avatar Mar 27 '22 17:03 nemanjam

How do we actually access the contents of the Blob with JSDOM if none of these methods are implemented? Is it impossible?

Timmmm avatar Apr 07 '22 12:04 Timmmm

Didn't try @tom-james-watson's workaround (I should have and likely will)

But here's my, lets call it jsdom-native, workaround

const readFile = async (file: File) => {
  return new Promise<ArrayBuffer>((resolve, reject) => {
    const reader = new FileReader();
    reader.onload = function () {
      return resolve(reader.result as ArrayBuffer);
    };
    reader.onerror = function () {
      return reject(reader.error);
    };
    reader.readAsArrayBuffer(file);
  });
};

There's a good chance require("blob-polyfill"); does almost the exact same, but written by someone competent in the matter :)

EDIT: require("blob-polyfill"); works perfectly

DGollings avatar May 12 '22 20:05 DGollings

fyi, i still think you should be using the public api:s that exist on the blob to read the data from those instead in the FileReader

NodeJS v18 now has spec compatible FormData, Blob, File, fetch, Request, Response, Headers, whatwg streams and all of the things. I think you should use them instead whenever it's available and fallback to some polyfill if needed

NodeJS blob are more spec compliant than your own blob impl that still lacks stream() arraybuffer() and text() and implementing .stream() and whatwg stream is a lot of undertaking and bringing in web-stram-polyfills adds a lot of other complications too


to get a hold of the File class provided by NodeJS you would have to do:

const fd = new FormData()
fd.set('x', new Blob())
const File = fd.get('x').constructor

Right, but if you override them to throw an exception, then FileReader will still work (per spec), because FileReader uses internal data access, not the public APIs.

Your request is that if you override them to throw an exception, FileReader will call those public APIs and fail. We are not interested in violating the spec in that way.

I would not worry about someone patching Blob.prototype to do anything else badly... if you are then maybe you could do:

// cache the method
const Blob = globalThis.Blob
const read = Blob.prototype.arrayBuffer

FileReader.prototype.readAsArrayBuffer = function(blob) {
  read.call(blob).then(...)
}

jimmywarting avatar Aug 09 '22 10:08 jimmywarting

I was going to work on a PR for Blob.arrayBuffer() but looks like it will be a no-go for now as the wpt relies on TextEncoder which is not supported at this time ref: #2524.

exige81 avatar Oct 04 '22 08:10 exige81