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

Fix AWS S3 upload on React Native #3064

Merged
merged 4 commits into from Oct 4, 2021
Merged

Fix AWS S3 upload on React Native #3064

merged 4 commits into from Oct 4, 2021

Conversation

Cretezy
Copy link
Contributor

@Cretezy Cretezy commented Jul 31, 2021

This took me hours to figure out, but React Native's XMLHttpRequest has a bug where setting withCredentials to undefined crashes the app silently.

This is more of a patch, because withCredentials can never be set (from what I can tell).

@Cretezy
Copy link
Contributor Author

Cretezy commented Aug 2, 2021

I will address the comments soon, but this does require more work, as the React Native FormData to upload expects an internal URI instead of a blob. This is what was required to make it work:

diff --git a/node_modules/@uppy/aws-s3/lib/MiniXHRUpload.js b/node_modules/@uppy/aws-s3/lib/MiniXHRUpload.js
index e3c49aa..58187b2 100644
--- a/node_modules/@uppy/aws-s3/lib/MiniXHRUpload.js
+++ b/node_modules/@uppy/aws-s3/lib/MiniXHRUpload.js
@@ -108,13 +108,7 @@ module.exports = /*#__PURE__*/function () {
 
     this._addMetadata(formPost, file.meta, opts);
 
-    var dataWithUpdatedType = setTypeInBlob(file);
-
-    if (file.name) {
-      formPost.append(opts.fieldName, dataWithUpdatedType, file.meta.name);
-    } else {
-      formPost.append(opts.fieldName, dataWithUpdatedType);
-    }
+    formPost.append(opts.fieldName, { name: file.name, type: file.type, uri: file.meta.uri })
 
     return formPost;
   };
@@ -260,7 +254,7 @@ module.exports = /*#__PURE__*/function () {
       xhr.open(opts.method.toUpperCase(), opts.endpoint, true); // IE10 does not allow setting `withCredentials` and `responseType`
       // before `open()` is called.
 
-      xhr.withCredentials = opts.withCredentials;
+      // xhr.withCredentials = opts.withCredentials;
 
       if (opts.responseType !== '') {
         xhr.responseType = opts.responseType;

@arturi
Copy link
Contributor

arturi commented Aug 2, 2021

Thanks @Cretezy and thanks for your contribution! We are not actively updating React Native, so any work here is really appreciated.

@Cretezy
Copy link
Contributor Author

Cretezy commented Aug 2, 2021

Since React Native requires a URI, how would you recommend adding this functionality? In the patch above, I passed it in the meta, which works, but I don't know if there's a better way.

(Specifically, I'm taking about the uppy package, not @uppy/react-native, which was quite broken. I'm simply using the core with the S3 plugin but writing my own UI/handling code)

Cretezy and others added 2 commits August 2, 2021 21:28
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
// before `open()` is called.
xhr.withCredentials = opts.withCredentials
// before `open()` is called. It’s important to set withCredentials
// to a boolean, otherwise React Native crashes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit

Suggested change
// to a boolean, otherwise React Native crashes
// to a boolean, otherwise React Native crashes.

@arturi arturi merged commit 9840127 into transloadit:main Oct 4, 2021
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

Successfully merging this pull request may close these issues.

None yet

3 participants