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

[BREAKING on iOS] Add blob implementation with WebSocket integration #11417

Closed
wants to merge 2 commits into from
Closed

[BREAKING on iOS] Add blob implementation with WebSocket integration #11417

wants to merge 2 commits into from

Conversation

satya164
Copy link
Contributor

@satya164 satya164 commented Dec 12, 2016

This is the first PR from a series of PRs @grabbou and me will make to add blob support to React Native. The next PR will include blob support for XMLHttpRequest.

I'd like to get this merged with minimal changes to preserve the attribution. My next PR can contain bigger changes.

Motivation

Blobs are used to transfer binary data between server and client. Currently React Native lacks a way to deal with binary data. The only thing that comes close is uploading files through a URI.

Current workarounds to transfer binary data includes encoding and decoding them to base64 and and transferring them as string, which is not ideal, since it increases the payload size and the whole payload needs to be sent via the bridge every time changes are made.

The PR adds a way to deal with blobs via a new native module. The blob is constructed on the native side and the data never needs to pass through the bridge. Currently the only way to create a blob is to receive a blob from the server via websocket.

The PR is largely a direct port of https://github.com/silklabs/silk/tree/master/react-native-blobs by @philikon into RN (with changes to integrate with RN), and attributed as such.

Note: This is a breaking change for all people running iOS without CocoaPods. You will have to manually add RCTBlob.xcodeproj to your Libraries and then, add it to Build Phases. Just follow the process of manual linking. We'll also need to document this process in the release notes.

Related discussion - #11103

Known issues

  • Image can't show image when URL.createObjectURL is used with large images on Android

Test plan

The websocket integration can be tested via a simple server,

const fs = require('fs');
const http = require('http');

const WebSocketServer = require('ws').Server;

const wss = new WebSocketServer({
  server: http.createServer().listen(7232),
});

wss.on('connection', (ws) => {
  ws.on('message', (d) => {
    console.log(d);
  });

  ws.send(fs.readFileSync('./some-file'));
});

Then on the client,

var ws = new WebSocket('ws://localhost:7232');

ws.binaryType = 'blob';

ws.onerror = (error) => {
  console.error(error);
};

ws.onmessage = (e) => {
  console.log(e.data);
  ws.send(e.data);
};

cc @brentvatne @ide

@facebook-github-bot
Copy link
Contributor

By analyzing the blame information on this pull request, we identified @antoinerousseau and @dlowder-salesforce to be potential reviewers.

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Dec 12, 2016
@grabbou
Copy link
Contributor

grabbou commented Dec 12, 2016

Also if you want to test it, here's demo: https://github.com/callstack-io/react-native-blob-test

@satya164
Copy link
Contributor Author

cc @bestander need some help on writing tests for this :D

@grabbou
Copy link
Contributor

grabbou commented Dec 12, 2016

I'll update the example app layout later to fix the failing iOS tests

@grabbou
Copy link
Contributor

grabbou commented Dec 12, 2016

Also note that some of the changes to .xcodeproj are because of other changes made outside of this pull request. I've only added RCTBlob and libRCTBlob.a. Looks like someone changed native structure in master but forgot to open .xcodeproj.

@satya164
Copy link
Contributor Author

satya164 commented Dec 16, 2016

@grabbou failing iOS tests look like the same error I got when building the example app.

@lacker
Copy link
Contributor

lacker commented Dec 16, 2016

I think we should have unit or integration tests that cover this, too - it's a really big chunk of functionality so it seems quite testworthy.

@satya164
Copy link
Contributor Author

@lacker I'll look into how to write tests for this. Will likely need help :D

@douglowder
Copy link
Contributor

If you go ahead and import #11433 , you'll have a basic WebSocket test infrastructure that you can add blob tests to.

@grabbou
Copy link
Contributor

grabbou commented Dec 19, 2016

iOS tests fixed - the issue started to happening after the rebase, that includes the breaking change of imports on the native side. Fixed.

@satya164
Copy link
Contributor Author

Me and @Kureev were talking about this yesterday about the current behaviour of close. Currently it'll release the blob for all sliced blobs in addition to the parent blob. It's not probably what we want and likely not spec-compliant.

I'll fix this behaviour in my next PR. But there is one problem with this. Libraries like Firebase slice the blobs internally. There's no way to close those blob slices since we don't have a reference to them. The only way I can think of is to have something like BlobManager.closeSlices(blob) to achieve this. Any suggestions will be nice.

@Kureev
Copy link
Contributor

Kureev commented Dec 20, 2016

I propose to use BlobManager.close(blob). It can be any kind of blob: a sliced or an original one.

@brentvatne
Copy link
Collaborator

What does the spec say about close? It doesn't look like any browsers support it yet (source: "Browser compatibility table in MDN docs".

@satya164
Copy link
Contributor Author

@brentvatne unfortunately I couldn't find much documentation on the behaviour of close. and since browsers haven't implemented it, I couldn't test it either. But it's likely that closing the parent blob when sliced blob is closed and vice-versa is non-spec compliant, since it doesn't sound very intuitive.

@ide
Copy link
Contributor

ide commented Dec 25, 2016

I would expect slices to provide the illusion that memory is copied (whether or not that actually happens is up to the implementation -- if you make a small slice you might want to not copy at first but then make a copy if the large parent is later freed -- this is all below the level of abstraction). This matches my reading of the spec https://www.w3.org/TR/FileAPI/#dfn-close

Does the browser free Blobs when they are GC'd? If that's the case we might want to build a synchronous implementation that interacts with the native JSC API and frees native memory when JS GCs it. This would also pave the way towards moving the implementation to the TypedArray API in iOS 10 for even more efficient Blobs -- this is what we do in our OpenGL implementation.

My main concern is that we're aiming to implement an existing API so that people can use existing code but we might not be providing existing semantics around freeing memory. Or if I'm wrong about this that solves the problem too =P

@satya164
Copy link
Contributor Author

@ide

I would expect slices to provide the illusion that memory is copied (whether or not that actually happens is up to the implementation -- if you make a small slice you might want to not copy at first but then make a copy if the large parent is later freed -- this is all below the level of abstraction).

Yeah, I'm planning to implement this behaviour in my next PR

Does the browser free Blobs when they are GC'd? If that's the case we might want to build a synchronous implementation that interacts with the native JSC API and frees native memory when JS GCs it.

I think so. I'm not very familiar with JSC and C++ to be able to implement this unfortunately. Can someone help?

My main concern is that we're aiming to implement an existing API so that people can use existing code but we might not be providing existing semantics around freeing memory.

Yeah, we should totally provide existing semantics.

Copy link
Contributor

@ericvicenti ericvicenti left a comment

Choose a reason for hiding this comment

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

I would really like to see this landed! A blob implementation could be super super versatile

Can you go into more detail about the nature of the breaking change?

cc @javache for the native changes on iOS (and the wall of xcode project changes 🙄 )

cc @AaaChiuuu and @mkonicek to give the Android code a lookover

cc @mhorowitz regarding JSC/C++ .

let size = 0;
parts.forEach((part) => {
if (!(part instanceof Blob)) {
throw new Error('Can currently only create a Blob from other Blobs');
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use invariant here, for consistency

function createUUID(): string {
/* eslint-disable no-bitwise */
return UUID4.replace(/[xy]/g, (pat) => {
let nibble = Math.random() * 16 | 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting name! Reminds me of nibbler from Futurama

@satya164
Copy link
Contributor Author

satya164 commented Jan 2, 2017

@evv only breaking change is that iOS users have to manually link this for the app to compile. The behaviour stays the same.

@ericvicenti
Copy link
Contributor

I see, so that should be covered by react-native-git-upgrade or react-native upgrade.

Copy link
Member

@javache javache left a comment

Choose a reason for hiding this comment

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

Xcode project changes look ok to me, as they mostly concern the example projects.

*/
'use strict';

const { BlobModule } = require('react-native').NativeModules;
Copy link
Member

Choose a reason for hiding this comment

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

Inside RN, use require('NativeModules')

/**
* This method is in the standard, but not actually implemented by
* any browsers at this point. It's important for how Blobs work in
* React Native, however, since we cannot de-allocate resources automatically,
Copy link
Member

Choose a reason for hiding this comment

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

You could hook into JSC's finalizer callback to clean this up, but we currently don't have a good cross-platform approach to creating custom JS objects. It also would be an issue with the Websocket executor.

358F4EE11D1E81A9004DF814 /* Debug */ = {
isa = XCBuildConfiguration;
buildSettings = {
HEADER_SEARCH_PATHS = (
Copy link
Member

Choose a reason for hiding this comment

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

There's no longer any need to customize HEADER_SEARCH_PATHS. Can you remove the override from your xcodeproj?


@interface RCTBridge (RCTBlobManager)

@property (nonatomic, readonly) RCTBlobManager *blobs;
Copy link
Member

Choose a reason for hiding this comment

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

nit: I'd prefer the method name to be blobManager

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct.


RCT_EXPORT_MODULE(BlobModule)

- (NSDictionary<NSString *, id> *)constantsToExport
Copy link
Member

Choose a reason for hiding this comment

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

Export constants causes modules to be initialized / allocated eagerly. Since these are just constants, perhaps you could just have these strings in JS?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would introduce two places where the string is defined, one on native side (we need to refer to BLOB_URI_SCHEME and second on the Javascript side). Also, on Android it has a different value.

@satya164 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BLOB_URI_HOST is determined from package name, so not something we can hardcode

if (!data) {
return nil;
}
if (offset != 0 || (size != -1 && size != data.length)) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not have the caller take care of this and keep your interface simple? Using magic numbers to mark certain arguments as unused is not best practice

Copy link
Contributor

@grabbou grabbou Jan 9, 2017

Choose a reason for hiding this comment

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

I think it's just a convenience. If you will look at the caller, it's -resolve: blob that refers to it. And that method is just a proxy, that extracts arguments from a passed dictionary and applies default parameters.

Since both offset and size can be optional, I cannot think of a better alternative to that.

@@ -223,7 +223,7 @@
MTL_ENABLE_DEBUG_INFO = YES;
ONLY_ACTIVE_ARCH = YES;
SDKROOT = iphoneos;
SKIP_INSTALL = YES;
USER_HEADER_SEARCH_PATHS = "$(SRCROOT)/../Blob";
Copy link
Member

Choose a reason for hiding this comment

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

Undo. Instead configure the copy files/headers phase correctly, or specify the dependencies correctly in xcode.

@@ -173,6 +173,8 @@ defineProperty(global, 'Headers', () => require('fetch').Headers);
defineProperty(global, 'Request', () => require('fetch').Request);
defineProperty(global, 'Response', () => require('fetch').Response);
defineProperty(global, 'WebSocket', () => require('WebSocket'));
defineProperty(global, 'Blob', () => require('Blob'));
defineProperty(global, 'URL', () => require('URL'));
Copy link
Member

Choose a reason for hiding this comment

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

These both have a potential impact on the initial bundle size. Are we defining them as globals for compatibility with the web spec?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javache yes, they have to be global

Copy link
Member

Choose a reason for hiding this comment

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

How important is that really? We're fighting really hard to reduce bridge startup time, and all of these global modules reduce the amount of work we can do on-demand / lazily.

@@ -279,6 +279,10 @@
buildSettings = {
EXECUTABLE_PREFIX = lib;
GCC_TREAT_WARNINGS_AS_ERRORS = NO;
HEADER_SEARCH_PATHS = (
Copy link
Member

Choose a reason for hiding this comment

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

Remove

@@ -85,6 +86,27 @@ - (void)dealloc
[_sockets[socketID] send:message];
}

RCT_EXPORT_METHOD(sendBlob:(NSDictionary *)blob socketID:(nonnull NSNumber *)socketID)
{
RCTBlobManager *blobManager = [[self bridge] moduleForClass:[RCTBlobManager class]];
Copy link
Member

Choose a reason for hiding this comment

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

Why not use the category method?

Copy link
Contributor

Choose a reason for hiding this comment

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

How that could work in practice? I mean not sure how category could be used to export method. If possible - that's great and quite reasonable as long as we assume Blob is opt-in module.

@grabbou
Copy link
Contributor

grabbou commented Jan 7, 2017

I have applied some of the tweaks, mainly related to the xcodeproj. Will continue with the rest now.

@lacker lacker removed the Core Team label Feb 8, 2017
@LorienHW
Copy link

Any updates on this? Blob upload via XMLHttpRequest would be super helpful.

@satya164
Copy link
Contributor Author

@SpecialDOS I'm waiting for someone from the core team to review the Java code. css @lacker

import type { BlobProps } from './BlobTypes';

const UUID4 = 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx';
function createUUID(): string {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a tiny npm module you could use for this? Is the implementation copied from somewhere?

* Constructor for JS consumers.
* Currently we only support creating Blobs from other Blobs.
*/
constructor(parts: Array<Blob>, options: any) {
Copy link
Contributor

@mkonicek mkonicek Mar 23, 2017

Choose a reason for hiding this comment

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

How are the parts used? Are they concatenated? Would be worth documenting the parts argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, basically follows the browser's Blob API. Will add a link.

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Jun 6, 2017
@facebook-github-bot
Copy link
Contributor

@sahrens has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@sahrens sahrens left a comment

Choose a reason for hiding this comment

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

This is failing travis and internal tests - can you rebase and get things passing?

@satya164
Copy link
Contributor Author

satya164 commented Jun 8, 2017

@sahrens will check asap!

@@ -22,6 +23,20 @@ const base64 = require('base64-js');

import type EventSubscription from 'EventSubscription';

Choose a reason for hiding this comment

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

no-unused-vars: 'ArrayBufferView' is defined but never used.

@@ -22,6 +23,20 @@ const base64 = require('base64-js');

import type EventSubscription from 'EventSubscription';

Choose a reason for hiding this comment

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

no-unused-vars: 'ArrayBufferView' is defined but never used.

@@ -22,6 +23,20 @@ const base64 = require('base64-js');

import type EventSubscription from 'EventSubscription';

Choose a reason for hiding this comment

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

no-unused-vars: 'ArrayBufferView' is defined but never used.

@satya164
Copy link
Contributor Author

satya164 commented Jun 9, 2017

Hey @sahrens. Fixed what I could. The tests that are still failing are failing on master too and don't seem related :(

@facebook-github-bot
Copy link
Contributor

@sahrens has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@@ -99,6 +100,27 @@ - (void)dealloc
[_sockets[socketID] send:message];
}

RCT_EXPORT_METHOD(sendBlob:(NSDictionary *)blob socketID:(nonnull NSNumber *)socketID)
{
RCTBlobManager *blobManager = [[self bridge] moduleForClass:[RCTBlobManager class]];
Copy link
Member

Choose a reason for hiding this comment

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

I think we should revisit this: it would be nice for this module to be completely optional, by linking it here, you're basically required to adopt this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javache is there any advantage for having it as an optional module? any ideas/tips how else we can do it?

Copy link
Member

Choose a reason for hiding this comment

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

It means we keep the core of the bridge small, and you only need to include the modules you require. It seem like most of the methods here could live on RCTBlobManager itself, if we exposed a way for it to hook into WebSocketModule, e.g. an API on WebSocketModule: setContentHandler:.. forSocketId:...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javache okies. let's talk about how to do this on slack? I'd really prefer not to do it in the current PR and do a followup for that since this PR and the follow up PR to this are quite big.

also probably worth noting that in my mind eventually more native modules are going to support blob, (there's an open PR for Networking module), e.g. - maybe image module should be able to render a blob, camera roll module should return blobs etc.

@javache
Copy link
Member

javache commented Jul 26, 2017

Woohoo, this finally landed (sorry for the delay!) I changed the approach slightly so this is all opt-in now. Please let me know if you have any feedback.

@timwangdev
Copy link
Contributor

This BlobModule doesn't have a jest mock, which will break tests in 0.48.0.
Related issue: #15810

facebook-github-bot pushed a commit that referenced this pull request Sep 7, 2017
Summary:
React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed.

Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java)

Related PR: #11417
Related issue: #15810

Passed CI tests.
Need render a component in jest with WebSocketModule as dependency.
Closes #15843

Differential Revision: D5783263

Pulled By: shergin

fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Sep 11, 2017
Summary:
This is the first PR from a series of PRs grabbou and me will make to add blob support to React Native. The next PR will include blob support for XMLHttpRequest.

I'd like to get this merged with minimal changes to preserve the attribution. My next PR can contain bigger changes.

Blobs are used to transfer binary data between server and client. Currently React Native lacks a way to deal with binary data. The only thing that comes close is uploading files through a URI.

Current workarounds to transfer binary data includes encoding and decoding them to base64 and and transferring them as string, which is not ideal, since it increases the payload size and the whole payload needs to be sent via the bridge every time changes are made.

The PR adds a way to deal with blobs via a new native module. The blob is constructed on the native side and the data never needs to pass through the bridge. Currently the only way to create a blob is to receive a blob from the server via websocket.

The PR is largely a direct port of https://github.com/silklabs/silk/tree/master/react-native-blobs by philikon into RN (with changes to integrate with RN), and attributed as such.

> **Note:** This is a breaking change for all people running iOS without CocoaPods. You will have to manually add `RCTBlob.xcodeproj` to your `Libraries` and then, add it to Build Phases. Just follow the process of manual linking. We'll also need to document this process in the release notes.

Related discussion - facebook/react-native#11103

- `Image` can't show image when `URL.createObjectURL` is used with large images on Android

The websocket integration can be tested via a simple server,

```js
const fs = require('fs');
const http = require('http');

const WebSocketServer = require('ws').Server;

const wss = new WebSocketServer({
  server: http.createServer().listen(7232),
});

wss.on('connection', (ws) => {
  ws.on('message', (d) => {
    console.log(d);
  });

  ws.send(fs.readFileSync('./some-file'));
});
```

Then on the client,

```js
var ws = new WebSocket('ws://localhost:7232');

ws.binaryType = 'blob';

ws.onerror = (error) => {
  console.error(error);
};

ws.onmessage = (e) => {
  console.log(e.data);
  ws.send(e.data);
};
```

cc brentvatne ide
Closes facebook/react-native#11417

Reviewed By: sahrens

Differential Revision: D5188484

Pulled By: javache

fbshipit-source-id: 6afcbc4d19aa7a27b0dc9d52701ba400e7d7e98f
rquigley pushed a commit to rquigley/react-native that referenced this pull request Sep 14, 2017
Summary:
React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed.

Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java)

Related PR: facebook#11417
Related issue: facebook#15810

Passed CI tests.
Need render a component in jest with WebSocketModule as dependency.
Closes facebook#15843

Differential Revision: D5783263

Pulled By: shergin

fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
ide pushed a commit to expo/react-native that referenced this pull request Sep 19, 2017
Summary:
React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed.

Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java)

Related PR: facebook#11417
Related issue: facebook#15810

Passed CI tests.
Need render a component in jest with WebSocketModule as dependency.
Closes facebook#15843

Differential Revision: D5783263

Pulled By: shergin

fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
ide pushed a commit that referenced this pull request Sep 19, 2017
Summary:
React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed.

Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java)

Related PR: #11417
Related issue: #15810

Passed CI tests.
Need render a component in jest with WebSocketModule as dependency.
Closes #15843

Differential Revision: D5783263

Pulled By: shergin

fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
ide pushed a commit that referenced this pull request Sep 19, 2017
Summary:
React Native v0.48.0 shipped `WebSocketModule` support with `BlobModule` as dependency. But `BlobModule` is not mocked in jest which will cause render tests failed.

Reference implantation: [BlobModule.java](https://github.com/facebook/react-native/blob/ed903099b42259958d8a6eb3af1a6460c1bc8b2c/ReactAndroid/src/main/java/com/facebook/react/modules/blob/BlobModule.java)

Related PR: #11417
Related issue: #15810

Passed CI tests.
Need render a component in jest with WebSocketModule as dependency.
Closes #15843

Differential Revision: D5783263

Pulled By: shergin

fbshipit-source-id: 2386692f4a644796da2fd66b3135da9f5911663e
facebook-github-bot pushed a commit that referenced this pull request Jan 26, 2018
Summary:
This PR is a followup to #11417 and should be merged after that one is merged.

  1. Add support for creating blobs from strings, not just other blobs
  1. Add the `File` constructor which is a superset of `Blob`
  1. Add the `FileReader` API which can be used to read blobs as strings or data url (base64)
  1. Add support for uploading and downloading blobs via `XMLHttpRequest` and `fetch`
  1. Add ability to download local files on Android so you can do `fetch(uri).then(res => res.blob())` to get a blob for a local file (iOS already supported this)

  1. Clone the repo https://github.com/expo/react-native-blob-test
  1. Change the `package.json` and update `react-native` dependency to point to this branch, then run `npm install`
  1. Run the `server.js` file with `node server.js`
  1. Open the `index.common.js` file and replace `localhost` with your computer's IP address
  1. Start the packager with `yarn start` and run the app on your device

If everything went well, all tests should pass, and you should see a screen like this:

![screen shot 2017-06-08 at 7 53 08 pm](https://user-images.githubusercontent.com/1174278/26936407-435bbce2-4c8c-11e7-9ae3-eb104e46961e.png)!

Pull to rerun all tests or tap on specific test to re-run it

  [GENERAL] [FEATURE] [Blob] - Implement blob support for XMLHttpRequest
Closes #11573

Reviewed By: shergin

Differential Revision: D6082054

Pulled By: hramos

fbshipit-source-id: cc9c174fdefdfaf6e5d9fd7b300120a01a50e8c1
rozele pushed a commit to microsoft/react-native-windows that referenced this pull request Mar 1, 2018
Summary:
This PR is a followup to facebook/react-native#11417 and should be merged after that one is merged.

  1. Add support for creating blobs from strings, not just other blobs
  1. Add the `File` constructor which is a superset of `Blob`
  1. Add the `FileReader` API which can be used to read blobs as strings or data url (base64)
  1. Add support for uploading and downloading blobs via `XMLHttpRequest` and `fetch`
  1. Add ability to download local files on Android so you can do `fetch(uri).then(res => res.blob())` to get a blob for a local file (iOS already supported this)

  1. Clone the repo https://github.com/expo/react-native-blob-test
  1. Change the `package.json` and update `react-native` dependency to point to this branch, then run `npm install`
  1. Run the `server.js` file with `node server.js`
  1. Open the `index.common.js` file and replace `localhost` with your computer's IP address
  1. Start the packager with `yarn start` and run the app on your device

If everything went well, all tests should pass, and you should see a screen like this:

![screen shot 2017-06-08 at 7 53 08 pm](https://user-images.githubusercontent.com/1174278/26936407-435bbce2-4c8c-11e7-9ae3-eb104e46961e.png)!

Pull to rerun all tests or tap on specific test to re-run it

  [GENERAL] [FEATURE] [Blob] - Implement blob support for XMLHttpRequest
Closes facebook/react-native#11573

Reviewed By: shergin

Differential Revision: D6082054

Pulled By: hramos

fbshipit-source-id: cc9c174fdefdfaf6e5d9fd7b300120a01a50e8c1
grabbou pushed a commit to react-native-community/cli that referenced this pull request Sep 26, 2018
Summary:
This is the first PR from a series of PRs grabbou and me will make to add blob support to React Native. The next PR will include blob support for XMLHttpRequest.

I'd like to get this merged with minimal changes to preserve the attribution. My next PR can contain bigger changes.

Blobs are used to transfer binary data between server and client. Currently React Native lacks a way to deal with binary data. The only thing that comes close is uploading files through a URI.

Current workarounds to transfer binary data includes encoding and decoding them to base64 and and transferring them as string, which is not ideal, since it increases the payload size and the whole payload needs to be sent via the bridge every time changes are made.

The PR adds a way to deal with blobs via a new native module. The blob is constructed on the native side and the data never needs to pass through the bridge. Currently the only way to create a blob is to receive a blob from the server via websocket.

The PR is largely a direct port of https://github.com/silklabs/silk/tree/master/react-native-blobs by philikon into RN (with changes to integrate with RN), and attributed as such.

> **Note:** This is a breaking change for all people running iOS without CocoaPods. You will have to manually add `RCTBlob.xcodeproj` to your `Libraries` and then, add it to Build Phases. Just follow the process of manual linking. We'll also need to document this process in the release notes.

Related discussion - facebook/react-native#11103

- `Image` can't show image when `URL.createObjectURL` is used with large images on Android

The websocket integration can be tested via a simple server,

```js
const fs = require('fs');
const http = require('http');

const WebSocketServer = require('ws').Server;

const wss = new WebSocketServer({
  server: http.createServer().listen(7232),
});

wss.on('connection', (ws) => {
  ws.on('message', (d) => {
    console.log(d);
  });

  ws.send(fs.readFileSync('./some-file'));
});
```

Then on the client,

```js
var ws = new WebSocket('ws://localhost:7232');

ws.binaryType = 'blob';

ws.onerror = (error) => {
  console.error(error);
};

ws.onmessage = (e) => {
  console.log(e.data);
  ws.send(e.data);
};
```

cc brentvatne ide
Closes facebook/react-native#11417

Reviewed By: sahrens

Differential Revision: D5188484

Pulled By: javache

fbshipit-source-id: 6afcbc4d19aa7a27b0dc9d52701ba400e7d7e98f
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. Import Started This pull request has been imported. This does not imply the PR has been approved. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.