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

Implement FileReader.readAsArrayBuffer #30769

Conversation

acostalima
Copy link

Summary

The proposed implementation is not an efficient solution as is requires to encode the raw binary of a Blob into base64 natively and then decode the base64 into an ArrayBuffer in JS land. We have to do this for the time being because it's not yet possible to share/manipulate native memory or object references with JS. However, it works as a workaround for the short term.

Related: #21209

Changelog

[JavaScript] [Added] - Implement FileReader.readAsArrayBuffer

Test Plan

Run the following command at the root of React Native's directory:

npm t -- FileReader-test.js

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 20, 2021
@analysis-bot
Copy link

analysis-bot commented Jan 20, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: d2e8e7d
Branch: main

@hugomrdias
Copy link

Any chance we can get some feedback with this ?

@react-native-bot react-native-bot added the Type: Enhancement A new feature or enhancement of an existing feature. label Oct 1, 2021
@gkjohnson
Copy link

gkjohnson commented Dec 16, 2021

Would it be possible to get any progress on this? The lack of implementation for readAsArrayBuffer means that using fetch's response.arrayBuffer throws an error and is completely unusable since the Github fetch polyfill relies on FileReader.readAsArrayBuffer when returning an array buffer.

The inability to use response.arrayBuffer in React Native causes issues with compatibility for other libraries and limits their ability to use fetch. In this case three.js has recently upgraded its loader infrastructure to use fetch instead of XMLHttpRequest meaning react-native now throws an error when trying to load models. It would be great to get this fixed so three can continue to be used without much fuss in react-native.

And from what I can tell the CI failure is unrelated to any change in this PR.

@glenvollmer
Copy link

glenvollmer commented Mar 29, 2022

@acostalima, lets get this test fixed so we can get a PR review and merge this in!

@acostalima
Copy link
Author

@glenvollmer the CI error does not seem to be related to the changes I've made 🤔

@glenvollmer
Copy link

@acostalima, any idea what it could be? let me know if there is anything I can do to help out with getting this fixed

@acostalima
Copy link
Author

acostalima commented Mar 29, 2022

@glenvollmer I believe that a dependency failed to download according to the log: https://app.circleci.com/pipelines/github/facebook/react-native/7824/workflows/4633f47d-fb4a-45e4-b986-fb329475ee6b/jobs/185596/parallel-runs/0/steps/0-114:

Unable to download: mvn:androidx.customview:customview:aar:1.0.0

Maybe re-running the job might fix the issue...?

@glenvollmer
Copy link

glenvollmer commented Mar 29, 2022

@acostalima, did a little digging. I think the url or the sha1 in this file may be broken
https://github.com/facebook/react-native/blob/main/ReactAndroid/src/main/third-party/android/androidx/BUCK#L601

@acostalima acostalima force-pushed the implement-filereader-readasarraybuffer branch from 3a51a6e to 497846d Compare March 29, 2022 17:52
@acostalima
Copy link
Author

acostalima commented Mar 29, 2022

@glenvollmer I've just rebased my branch with main (as I have not touched this branch/PR for over a year now) and now the error is different. However, I've checked a few of the most recent PRs and I can see the CI failing in those too with exactly the same error.

@minhchienwikipedia
Copy link

It works as well, can we merge it?

@SheetJSDev
Copy link
Contributor

The changes from this PR (thanks @acostalima !) to Libraries/Blob/FileReader.js can be directly applied in RN projects by modifying node_modules/react-native/Libraries/Blob/FileReader.js. The patching process can be automated with patch-package. Hopefully this unblocks anyone waiting for an implementation.

hawkrives added a commit to StoDevX/AAO-React-Native that referenced this pull request Jan 20, 2023
@SheetJSDev
Copy link
Contributor

The relevant script has switched to ESM. The new patch (tested in 0.71.3) is:

--- a/Libraries/Blob/FileReader.js
+++ b/Libraries/Blob/FileReader.js
@@ -11,6 +11,7 @@
 import type Blob from './Blob';
 
 import NativeFileReaderModule from './NativeFileReaderModule';
+import { toByteArray } from 'base64-js';
 
 const EventTarget = require('event-target-shim');
 
@@ -74,8 +75,35 @@ class FileReader extends (EventTarget(...READER_EVENTS): any) {
     }
   }
 
-  readAsArrayBuffer(): any {
-    throw new Error('FileReader.readAsArrayBuffer is not implemented');
+  readAsArrayBuffer(blob: ?Blob): void {
+    this._aborted = false;
+
+    if (blob == null) {
+      throw new TypeError(
+        "Failed to execute 'readAsArrayBuffer' on 'FileReader': parameter 1 is not of type 'Blob'",
+      );
+    }
+
+    NativeFileReaderModule.readAsDataURL(blob.data).then(
+      (text: string) => {
+        if (this._aborted) {
+          return;
+        }
+
+        const base64 = text.split(',')[1];
+        const typedArray = toByteArray(base64);
+
+        this._result = typedArray.buffer;
+        this._setReadyState(DONE);
+      },
+      error => {
+        if (this._aborted) {
+          return;
+        }
+        this._error = error;
+        this._setReadyState(DONE);
+      },
+    );
   }
 
   readAsDataURL(blob: ?Blob): void {

facebook-github-bot pushed a commit that referenced this pull request Mar 14, 2023
Summary:
Fixes a number of issues with third party libraries that use `Blob#arrayBuffer` or `FileReader#readAsArrayBuffer`, including #30769 #34402 #20091 #21209

## Changelog

[INTERNAL] [FIXED] - Implemented FileReader#readAsArrayBuffer

Pull Request resolved: #36332

Test Plan: Added a test which fails against current release but passes after code changes.

Reviewed By: christophpurrer

Differential Revision: D43907171

Pulled By: javache

fbshipit-source-id: 73d622ec569a282b6394732b9a0dc687b447fb62
@acostalima
Copy link
Author

The relevant script has switched to ESM.

When this PR gets the attention of the RN team, I'll push the fix. Thanks @SheetJSDev!

@darde
Copy link

darde commented Apr 25, 2023

At first, thanks @acostalima for this contribution. I'm trying to use this PR locally, however, I'm getting an empty buffer on this line

this._result = typedArray.buffer;
console.log('buffer: ', this._result); // outputs: buffer []

I checked the typedArray and it's fulfilled correctly.

OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
Summary:
Fixes a number of issues with third party libraries that use `Blob#arrayBuffer` or `FileReader#readAsArrayBuffer`, including facebook#30769 facebook#34402 facebook#20091 facebook#21209

## Changelog

[INTERNAL] [FIXED] - Implemented FileReader#readAsArrayBuffer

Pull Request resolved: facebook#36332

Test Plan: Added a test which fails against current release but passes after code changes.

Reviewed By: christophpurrer

Differential Revision: D43907171

Pulled By: javache

fbshipit-source-id: 73d622ec569a282b6394732b9a0dc687b447fb62
@alexstanbury
Copy link

I had to change the line

this._result = typedArray.buffer

to

this._result = typedArray

to get this to work.

Copy link

github-actions bot commented Jan 1, 2024

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 1, 2024
Copy link

github-actions bot commented Jan 8, 2024

This PR was closed because it has been stalled for 7 days with no activity.

@github-actions github-actions bot closed this Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Stale There has been a lack of activity on this issue and it may be closed soon. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.