Conversation
Not sure that the id’s types are necessarily correct here…
facebook-github-bot
added
CLA Signed
react-native-bot
added
the
Type: Enhancement
RSNara
left a comment
RSNara
left a comment
There was a problem hiding this comment.
Looks good, but I think there's a mistake in the flow-typing.
analysis-bot
left a comment
analysis-bot
left a comment
There was a problem hiding this comment.
Code analysis results:
flowfound some issues.
| @@ -92,7 +92,7 @@ class BlobManager { | |||
| } | |||
| }, 0); | |||
|
|
|||
There was a problem hiding this comment.
Cannot call NativeBlobModule.createFromParts because property createFromParts is missing in null or undefined [1].
| @@ -131,38 +131,38 @@ class BlobManager { | |||
| if (BlobRegistry.has(blobId)) { | |||
| return; | |||
There was a problem hiding this comment.
Cannot call NativeBlobModule.release because property release is missing in null or undefined [1].
| /** | ||
| * Inject the blob content handler in the networking module to support blob | ||
| * requests and responses. | ||
| */ |
There was a problem hiding this comment.
Cannot call NativeBlobModule.addNetworkingHandler because property addNetworkingHandler is missing in null or undefined [1].
| /** | ||
| * Indicate the websocket should return a blob for incoming binary | ||
| * messages. | ||
| */ |
There was a problem hiding this comment.
Cannot call NativeBlobModule.addWebSocketHandler because property addWebSocketHandler is missing in null or undefined [1].
| /** | ||
| * Indicate the websocket should no longer return a blob for incoming | ||
| * binary messages. | ||
| */ |
There was a problem hiding this comment.
Cannot call NativeBlobModule.removeWebSocketHandler because property removeWebSocketHandler is missing in null or undefined [1].
|
|
||
| /** | ||
| * Send a blob message to a websocket. | ||
| */ |
There was a problem hiding this comment.
Cannot call NativeBlobModule.sendOverSocket because property sendOverSocket is missing in null or undefined [1].
RSNara
left a comment
RSNara
left a comment
There was a problem hiding this comment.
Awesome! Thanks for the fixes.
facebook-github-bot
left a comment
facebook-github-bot
left a comment
There was a problem hiding this comment.
@RSNara has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@RSNara flow is quite angry, i am attempting to fix |
|
This pull request was successfully merged by @ericlewis in 342c81d. When will my fix make it into a release? | Upcoming Releases |
Summary: Part of facebook#24875. Not sure that the id’s types are necessarily correct here… ## Changelog [General] [Added] - Add TM spec for BlobModule Pull Request resolved: facebook#24909 Reviewed By: fkgozali Differential Revision: D15433753 Pulled By: RSNara fbshipit-source-id: 68193d1a82fc7c66d6cc7ba4f22a0d3786987599
6 participants
Summary
Part of #24875. Not sure that the id’s types are necessarily correct here…
Changelog
[General] [Added] - Add TM spec for BlobModule
Test Plan
Flow passes