feat: implement FileReader

[KhafraDev]

Implements FileReader

Benefits:

• Allowed me to enable the idlharness tests for the FileAPI section (Blob, URL, File, etc.) - found 2 issues in File, 6 in Blob, and 2 in URL. The alternative was going through and disabling over 100 tests.
• Deno and every browser implement this. The same arguments for adding btoa and atob could be made here as well.

Also see: https://github.com/nodejs/undici/pull/1687#issuecomment-1272016124

The API, although outdated, is still perfectly usable in modern environments. Unlike XMLHttpRequest, does not come with any foot guns (synchronous requests).

[codecov-commenter]

Codecov Report

Base: 94.25% // Head: 89.55% // Decreases project coverage by -4.70% :warning:

Coverage data is based on head (824c0bb) compared to base (cc58f6e). Patch coverage: 11.11% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1690      +/-   ##
==========================================
- Coverage   94.25%   89.55%   -4.71%     
==========================================
  Files          53       58       +5     
  Lines        4965     5257     +292     
==========================================
+ Hits         4680     4708      +28     
- Misses        285      549     +264     

Impacted Files	Coverage Δ
lib/fileapi/encoding.js	2.32% <2.32%> (ø)
lib/fileapi/util.js	8.62% <8.62%> (ø)
lib/fileapi/filereader.js	9.00% <9.00%> (ø)
lib/fileapi/progressevent.js	23.52% <23.52%> (ø)
lib/fetch/webidl.js	93.65% <33.33%> (-0.98%)	:arrow_down:
index-fetch.js	100.00% <100.00%> (ø)
index.js	100.00% <100.00%> (ø)
lib/fetch/file.js	91.00% <100.00%> (+0.18%)	:arrow_up:
lib/fetch/index.js	83.36% <100.00%> (+0.03%)	:arrow_up:
lib/fileapi/symbols.js	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[targos]

Not objecting to it landing here, but I feel like it would be nice to have it in Node.js core independently of undici

[KhafraDev]

It would require a lot of work to port it to node core because it uses parts from the webidl and mime specs. Undici also implements File while node implements URL and Blob from the FileAPI spec.

~~I'm also not sure if node's WPT runner can handle idlharness tests as it currently does not do so.~~ edit: the WPTs just seem to be outdated for the FileAPI.

[jimmywarting]

think PR should rather be made to WPT to change the test to using the blobs new read methods instead... The FileReader should be a own isolated independent test.

[jimmywarting]

The Blob class in NodeJS core is kind of B imo, specially when it comes to streaming the content of the blob. (see https://github.com/nodejs/node/issues/42264) + it also lacks all those webidl stuff.

I think I also saw some changes/fixes you did to the Blob/File class that I think should rather be reflected onto the NodeJS Core Blob....

So I somewhat agree with @targos on this. either we have all the Blob/File stuff in NodeJS core, or everything is moved to from Core into Undici. But then we would have to move a bunch of things such as object URL and copy/postMessage stuff.

I also wish to see some form of "create a file backed up by the filesystem" to be implemented by NodeJS also. That would require the blob to keep track of certain file metadata. This is fetch-blob only main reason why it's still needed/around and why I haven't deprecated it yet and said: just use NodeJS built in stuff.

I also can't extend the NodeJS core Blob in fetch-blob b/c i want to create blobs backed up by the filesystem. wish is a bit sad b/c then my fetch-blob can't be transfered over with PostMessage + it's not an instance of NodeJS core Blob, and they can't work along with ObjectURLs or Worker 😞

As long as Blobs can only be constructed by some stuff in the memory, then they are pretty useless IMO. And you might just as well just use a ArrayBuffer/uint8array instead. Blob main purpose is kind of helping you to offload some of the data and have them being backed up by some file handle and offload some of the memory to a temporary location on the disc.

[KhafraDev]

it also lacks all those webidl stuff.

The problem with missing webidl is that it would be a pain to convert it to the node equivalent - it's simply easier to implement it in undici.

Regarding URL and Blob, I believe both of them interface with c++ at some point. It's not feasible to re-write both in js. Both File and FileReader are implemented in js (same with Deno).

---

If anything, File and FileReader could be implemented in node core directly eventually, however, considering that this FileReader implementation passes 95% of the WPTs already, it will incur very little maintenance burden (1 test with event ordering and 1 due to differences between a microtask and a html task queue).

[mcollina]

@KhafraDev this has quite a bit of conflicts.