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

Update RCTSRWebSocket.m #1

Closed
wants to merge 32 commits into from
Closed

Update RCTSRWebSocket.m #1

wants to merge 32 commits into from

Conversation

chancehudson
Copy link

Update RCTSRWebSocket.m to the version at facebook#19489

grabbou and others added 30 commits February 12, 2018 13:10
Summary:
This commit removes special cases for linking iOS and Android platforms.

A previous commit opened up link and other commands for other platforms to provide their own behaviors. It left special cases in tact for iOS and Android. This PR removes the special case.

- Added jest tests related to the link command.
- Ran the `link` and `unlink` commands for iOS and Android and confirmed no changes.

facebook#17745

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][FEATURE][local-cli/link/link.js] - Removes special cases for linking in iOS and Android.
Closes facebook#17961

Differential Revision: D6975951

Pulled By: hramos

fbshipit-source-id: 8dd5da35619e2124ce4b3b18db8b694757792363
Summary:
Android equivalent of facebook#15066

Tested that download progress shows up properly when reloading the app.

[ANDROID] [FEATURE] [DevSupport] - Show bundle download progress on Android
Closes facebook#17809

Differential Revision: D6982823

Pulled By: hramos

fbshipit-source-id: da01e42b8ebb1c603f4407f6bafd68e0b6b3ecba
Summary:
Include a default blacklist into the build settings to prevent
processing of incorrect fixture files by Metro.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

Fix facebook#17610 issue, preventing metro from processing fixture files

1. Have a working demo
2. Install https://github.com/oblador/react-native-vector-icons
3. Use in a component
4. Start the app
5. The app starts successfully and display the icons

[ GENERAL  ]  [ BUGFIX ]  [local-cli/util/Config.js] - Add default file blacklist
Closes facebook#17672

Differential Revision: D7014627

Pulled By: hramos

fbshipit-source-id: 20974e6fdd0977eeeb1048c29c9d621c803c26e9
Reviewed By: cpojer

Differential Revision: D6976161

fbshipit-source-id: 0cf20f4b2372997a8aac41cc07a9bdd641a93ad4
Summary:
There appear to be two different types of crashes related to the recent addition of `onKeyPress` on Android introduce in `0.53`. This PR addresses the cause of both of them.

Firstly, it seems possible to get an `indexOutOfBoundsException` with some 3rd-party keyboards as observed in facebook#17974 & facebook#17922. I have simplified the backspace determining logic slightly, and also put in an explicit check for zero case so it is not possible to get an indexOutOfBoundsException & it should make sense in the context of the onKeyPress logic.

Secondly, it appears that `EditText#onCreateInputConnection` can return null. In this case, if we set `null` to be the target of our subclass of `ReactEditTextInputConnectionWrapper`, we will see the crashes as seen [here](facebook#17974 (comment)), whereby any of methods executed in the `InputConnection` interface can result in a crash. It's hard to reason about the state when `null` is returned from `onCreateInputConnection`, however I would might reason that any soft keyboard input cannot update the `EditText` with a `null` `input connection`, as there is no way of interfacing with the `EditText`. I'm am not sure, if there is a later point where we might return/set this input connection at a later point? As without the `InputConnection` onKeyPress will not work. But for now, this will fix this crash at least.

I have not managed to reproduce these crashes myself yet, but users have confirmed that the `indexOutOfBounds` exception is fixed with the 'zero' case and has been confirmed on the respective issues facebook#17974 (comment).

For the `null` inputConnection target case, I have verified that explicitly setting the target as null in the constructor of `onCreateInputConnection` results in the same stack trace as the one linked. Here is also a [reference](https://github.com/stripe/stripe-android/pull/392/files#diff-6cc1685c98457d07fd4e2dd83f54d5bb) to the same issue closed with the same fix for another project on github.

It is also important to verify that the behavior of `onKeyPress` still functions the same after this change, which can be verified by running the RNTesterProject and the `KeyboardEvents` section in `InputText`.
The cases to check that I think are important to check are:
- Cursor at beginning of input & backspace
- Return key & return key at beginning of input
- Select text then press delete
- Selection then press a key
- Space key
- Different keyboard types

This should not be a breaking change.

 [ANDROID] [BUGFIX] [TextInput] - Fixes crashes with TextInput introduced in 0.53.
Closes facebook#18114

Differential Revision: D7099570

Pulled By: hramos

fbshipit-source-id: 75b2dc468c1ed398a33eb00487c6aa14ae04e5c2
This reverts commit 8bdd98e.
Summary:
Android uses the name of the package, not the config, for the `isInstalled` check. Sending both parameters to `isInstalled` so we have a consistent API.

<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.

Help us understand your motivation by explaining why you decided to make this change.

You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html

Happy contributing!

-->

A bug was uncovered in the react-native link command where Android would not unlink because the wrong parameters were being sent to `isInstalled`.

Successfully linked and unlinked `react-native-fs` on Windows and Mac. Jest tests pass.

<!--
Help reviewers and the release process by writing your own release notes

**INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

  CATEGORY
[----------]        TYPE
[ CLI      ]   [-------------]      LOCATION
[ DOCS     ]   [ BREAKING    ]   [-------------]
[ GENERAL  ]   [ BUGFIX      ]   [-{Component}-]
[ INTERNAL ]   [ ENHANCEMENT ]   [ {File}      ]
[ IOS      ]   [ FEATURE     ]   [ {Directory} ]   |-----------|
[ ANDROID  ]   [ MINOR       ]   [ {Framework} ] - | {Message} |
[----------]   [-------------]   [-------------]   |-----------|

[CATEGORY] [TYPE] [LOCATION] - MESSAGE

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->

[CLI][BUGFIX][local-cli/link/link.js] - Fix issue with `isInstalled` check for Android
[CLI][BUGFIX][local-cli/link/unlink.js] - Fix issue with `isInstalled` check for Android
[CLI][BUGFIX][local-cli/link/ios/common/unregisterNativeModule.js] - Fix references to unregister implementations.
Closes facebook#18207

Differential Revision: D7180885

Pulled By: hramos

fbshipit-source-id: 5f479cd9d7b1ebd8626b461e9dc1f22988e2c61f
Differential Revision: D7112419

fbshipit-source-id: 1d80c0c13144dd19bbcd5535383befc6567cacf7
Summary:
React Native had an underlying problem connecting to Firestore (Google's latest database) from Android devices. You can follow the issue [here](firebase/firebase-js-sdk#283).
The main problem was in NetworkingModule.java. Please refer to section 3 of 4.5.6 in whatwg.org's guideline https://xhr.spec.whatwg.org/#dom-xmlhttprequest-send

In this [video](https://www.youtube.com/watch?v=tILagf46ys8), I am showing how the react native behaved before adding the new fix and how it worked after the new fix added.  The new fix starts at 50 seconds.

[ANDROID] [BUGFIX] [FIRESTORE][XMLHttpRequest][ReactAndroid/src/main/java/com/facebook/react/modules/network/NetworkingModule.java] - Fixes the connection to Firestore by following whatwg.org's XMLHttpRequest send() method
Closes facebook#17940

Differential Revision: D7173468

Pulled By: hramos

fbshipit-source-id: 354d36f03d611889073553b93a7c43c6d4363ff3
Reviewed By: flarnie

Differential Revision: D6965585

fbshipit-source-id: 48c20d0010f4daf83272a36b3bdaca94493ab8fa
Reviewed By: bvaughn

Differential Revision: D7077686

fbshipit-source-id: de39027bef1f9d48802202555a5c765999d7bfe7
Reviewed By: bvaughn

Differential Revision: D7256390

fbshipit-source-id: 9fe1324da93cb8f4a7f478e1037944774b9b95ff
Summary: See the comment it code.

Reviewed By: mmmulani

Differential Revision: D7074168

fbshipit-source-id: e6eda9a47552142ccb0ba8e7bd9a103b0cb4f9f9
Summary:
Resolve facebook#18333

CLI should ask users for params only once and waiting for response while linking plugin

Ran the `link` commands for iOS and Android and confirmed that params requested only once.

[CLI][FEATURE][local-cli/link/link.js] - Requesting link params only once for all platforms
Closes facebook#18349

Differential Revision: D7342181

Pulled By: hramos

fbshipit-source-id: a10f0f7f2170f067d78b30e5a5221634b77da577
Summary:
<!--
  Required: Write your motivation here.
  If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->

Commit facebook@7891805 broke the previous behavior of `ReadableNativeMap.toHashMap()` for nested maps and arrays. Previously, all nested `ReadableNativeMap`s and `ReadableNativeArray`s were recursively converted to `HashMap`s and `ArrayList`s, but this is lost when only `getLocalMap()` is returned.

<!--
  Required: Write your test plan here. If you changed any code, please provide us with
  clear instructions on how you verified your changes work. Bonus points for screenshots and videos!
-->

Call `ReadableNativeMap.toHashMap()` on a map with values of type `ReadableNativeMap` and `ReadableNativeArray`. Verify the returned hash map has these converted to `HashMap` and `ArrayList`, respectively.

<!--
  Does this PR require a documentation change?
  Create a PR at https://github.com/facebook/react-native-website and add a link to it here.
-->

<!--
  Required.
  Help reviewers and the release process by writing your own release notes. See below for an example.
-->

[ANDROID] [BUGFIX] [ReadableNativeMap] - Fix toHashMap() for nested maps and arrays

<!--
  **INTERNAL and MINOR tagged notes will not be included in the next version's final release notes.**

    CATEGORY
  [----------]      TYPE
  [ CLI      ] [-------------]    LOCATION
  [ DOCS     ] [ BREAKING    ] [-------------]
  [ GENERAL  ] [ BUGFIX      ] [ {Component} ]
  [ INTERNAL ] [ ENHANCEMENT ] [ {Filename}  ]
  [ IOS      ] [ FEATURE     ] [ {Directory} ]   |-----------|
  [ ANDROID  ] [ MINOR       ] [ {Framework} ] - | {Message} |
  [----------] [-------------] [-------------]   |-----------|

 EXAMPLES:

 [IOS] [BREAKING] [FlatList] - Change a thing that breaks other things
 [ANDROID] [BUGFIX] [TextInput] - Did a thing to TextInput
 [CLI] [FEATURE] [local-cli/info/info.js] - CLI easier to do things with
 [DOCS] [BUGFIX] [GettingStarted.md] - Accidentally a thing/word
 [GENERAL] [ENHANCEMENT] [Yoga] - Added new yoga thing/position
 [INTERNAL] [FEATURE] [./scripts] - Added thing to script that nobody will see
-->
Closes facebook#18455

Reviewed By: kathryngray

Differential Revision: D7347344

Pulled By: mdvacca

fbshipit-source-id: af2bca9dec6c0cb8a7da099b6757434fcc3ac785
Summary:
We currently handle empty body poorly in the iOS blob implementation, this happens because of an early return that cause the blob response to not be processed by the blob module, resulting in an empty string as the body instead of a blob object. We also need to make sure to create an empty blob object when data is nil (empty body) as per the XMLHttpRequest spec. The Android implementation was already handling this properly.

Fixes facebook#18223

Send a HEAD request

```js
fetch('https://apipre.monkimun.com/whoami', {
  body: null,
  method: 'HEAD',
  headers: {
    Accept: 'application/json',
    'Content-Type': 'application/json',
  },
})
```

[IOS][BUGFIX][Blob] - Fix blob response parsing for empty body
Closes facebook#18547

Differential Revision: D7415950

Pulled By: hramos

fbshipit-source-id: 56860532c6171255869f02a0960f55d155184a46
@chancehudson chancehudson changed the base branch from 0.54-stable to 0.55-stable August 12, 2018 02:57
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.