-
Notifications
You must be signed in to change notification settings - Fork 25k
Allow uploading a native file (e.g. photo) via XMLHttpRequest #1357
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
Conversation
|
cc @lwansbrough, you might find this interesting |
|
@ide Very interesting! I will definitely have a use for this in my own app. But it's going to need file protocol support before it can be used by the camera module. |
Libraries/Network/RCTDataManager.m
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we add more things in RCTImageLoader we'll have to add it here too which kind of sucks as they are going to diverge. Is there anyway we can avoid listing those here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the very least we could have an [RCTImageLoader canLoadURI:uri] helper, so that these prefixes remain an implementation detail of RCTImageLoader. At some point in the future I imagine we'll have other kinds of URIs (e.g. file:///) with other ways of resolving them, at which point we might want to abstract resolving URIs altogether...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
canLoadURI works with me
Can we make CameraRoll vend NativeFile objects instead? This seems like a sucky API to have to require the developer to wrap the assetURI in a NativeFile before sending it. Also, we absolutely want to conserve as many metadata as possible from the files than just the URI. Maybe what you can do instead of having a NativeFile tag is to check if it's an object and has a uri attribute, then it's considered to be an image and you use RCTLoadImage instead of sending the object. I'm not a big fan of using instanceof to check if it's a good type, I much prefer duck typing, this way it can be serialized and unserialized. |
|
Ok, so my proposal for you is:
It should make the implementation simpler. Tell me what you think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what's going on with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we need to add Libraries/Image/ to the header search path so we can #import "RCTImageLoader.h"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make sure it also works with our buck rules inside of fb
Yes, I think we should... eventually. Currently the API mimics GraphQL APIs, which seems oddly FB-centric. I think we should return something more basic, and then build the GraphQL part around it using JS.
Like what? I can only think of the Content-Type, and the
I understand the desire to duck-type. I'd be ok with this, though in terms of the documentation, I think it'd be good to make |
Width and height, aspect ratio, multiple versions of the same file @2x, @3x. Also, one thing that I really want to support is representing a slice of an image. {uri: 'image.jpg', crop_rect: [0, 0.5, 0, 0.5]}. This would be useful for image spriting. |
It seems you're conflating system image assets used in the app with camera roll assets.
Again, this seems useful for image assets used by the app. If you want a photo (from camera roll or the web or wherever) cropped or other transformed (like Ads Manager app), you're going to have to perform that operation in native code, which leaves you with a new URI to a new image asset, which you can then pass to XHR to upload it. |
|
@philikon iOS supports displaying a sub-region of an image at the view level without having to actually chop up the image. It would be more efficient to do it that way for most purposes, although that would limit the use of the cropped image to being displayed inside an |
|
I'm ok with making the interface that |
|
Also, one thing that we could do is to make non-destructive transforms. {uri: 'baseImage', transforms: ['blur:20%', 'instagram-filter#5']} and don't save the temporary images. Same we could have resize, rotate... And, eventually when you actually want to upload it, you are doing all the processing and send it. I don't think that we want to dissociate sending images vs displaying images. |
Fair enough. Let's take it one step at a time. I'll get rid of |
|
Incidentally, I've been working on expanding the implementation of [RCTConvert data:] to make it easier to construct multipart mime uploads. I was thinking that instead of just string data, it could support a structure something like this: So then you could construct an entire request body on the JS side, either passing the data yourself or referencing data known to the native side by using tags/uris. This might be a more flexible solution than what you're planning with the separate dataURI. The only problem is that it might be tricky to make the NSData blob construction asynchronous, due to the design of RCTConvert, in which case those uris might be less generally useful than they would be in your design (although most of the uris you'd want to access would be local, and the construction would happen on a background thread anyway, so it might not be too bad). |
|
The other thing I wanted to do was basically unify the concept of a source (in the sense of This would then be processed into an NSURLRequest, which could then be used to retrieve any sort of data in any situation, whether it's loading an image, making an XHR request, setting a WebView target, etc. The source dictionary could also include whatever other data you wanted which would be handled by the receiver, so for image sources it might have width/height and cropping information, for RCTDataManager it might include a queryType, etc. |
Yes, that is exactly my goal, I've already started work on a |
|
It seems to me that the analogy between uploading native files and form input files (as in HTML) is weak. Instead of stuffing file handling into the same method (i.e. in which case, perhaps this functionality should live elsewhere, not in react-native core. |
|
@facebook-github-bot import |
|
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/897906616934455/int_phab to review. |
|
@rt2zz I'm not sure why you don't think that The interesting one is |
Summary:
With this in place, it's possible to upload a picture from the `CameraRoll` to Parse, for instance:
xhr = new XMLHttpRequest();
xhr.onload = function() {
data = JSON.parse(xhr.responseText);
var parseFile = new Parse.File(data.name);
parseFile._url = data.url;
callback(parseFile);
};
xhr.setRequestHeader('X-Parse-Application-Id', appID);
xhr.setRequestHeader('X-Parse-JavaScript-Key', appKey);
xhr.open('POST', 'https://api.parse.com/1/files/image.jpg');
// assetURI as provided e.g. by the CameraRoll API
xhr.send(new NativeFile(assetURI));
Closes facebook#1357
Github Author: Philipp von Weitershausen <[email protected]>
Test Plan: Imported from GitHub, without a `Test Plan:` line.
|
Could you provide some quick example how to correctly setup FormData and use it with fetch? I'm trying to use it but getting error Unsupported BodyInit type in support.FormData. |
|
Phil has added a nice upload example to the XHRExample in UIExplorer. It should be included in the next update, but here's the code if you want to paste it into XHRExample.js yourself. |
|
Did this get implemented into Android? I have this working on ios fine, but in Android, the request made misses the image. I see the commits in this pull refer to |
|
Yes, I added the same machinery to Android in a separate commit. It lives here: https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java If it's not working for you, I suggest you open a separate issue and describe your problem in detail (including how exactly you're getting the image's URI and adding it to the request.) Thanks! |
|
@philikon Is it possible to upload through a uri pointing to the image. I have resized the image and have the uri. Something like |
|
@skyride99 yes, that should work (although you'd need the |
|
Thanks formdata.append('image', {uri: pic.uri}); but it returns status of 0. I can upload the base64 like this formdata.append('file', pic.data); Really need to upload the binary data not sure how to do it with xhr.... |
|
@skyride99 I recently put some code in this issue (make sure you leave out the |
|
Thanks much but different error now. |
|
That seems like a server error, you should ask on stackoverflow for help |
|
@ashleydw You are correct. The server wants 'file' always file. Not 'image' |
With this in place, it's possible to upload a picture from the
CameraRollto Parse, for instance: