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 https://github.com/facebook/react-native/issues/17974 & https://github.com/facebook/react-native/issues/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](https://github.com/facebook/react-native/issues/17974#issuecomment-368471737), 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 https://github.com/facebook/react-native/issues/17974#issuecomment-368471737.
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 https://github.com/facebook/react-native/pull/18114
Differential Revision: D7099570
Pulled By: hramos
fbshipit-source-id: 75b2dc468c1ed398a33eb00487c6aa14ae04e5c2
Summary:
This try to address #17672 and https://github.com/facebook/metro/issues/139. We should probably not include these folders in the released version of React Native, as they are used only for testing—am I incorrect?
cc MoOx, t4deu.
I ran:
```
npm pack
tar -t -f react-native-1000.0.0.tgz | less
```
Then verified that `__fixture__` was not part of the package.
https://github.com/facebook/react-native/pull/17672
[GENERAL] [BUGFIX] [.npmignore] - Do not publish test-specific files
Closes https://github.com/facebook/react-native/pull/18019
Differential Revision: D7098211
Pulled By: jeanlauliac
fbshipit-source-id: 0748ad8c064450bd2a9f4d6f49675a7f74fb300f
Summary:
Currently if you invoke `fetch()` with an invalid URL ("aaa" for
example) you cannot catch the error in javascript since it's not
reported. Instead the entire app crashes.
Fixes#7436 and #18087
Hopefully.
<!--
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 using fetch on Android with user generated input.
Added relevant unit test
`./scripts/run-android-local-unit-tests.sh` all pass
[ANDROID] [BUGFIX] [fetch] - Allow "unexpected url" exception to be caught on Android when using fetch
<!--
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
-->
Closes https://github.com/facebook/react-native/pull/18103
Differential Revision: D7097110
Pulled By: hramos
fbshipit-source-id: 69144e8a0f7404d9bcc7c71a94650de36a48c84a
Summary:
<!--
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!
-->
The scrollToOffset method of RCTScrollView for iOS does not include bound check for the scroll offset provided to the method. This can cause the whole view to scroll out of screen if the offset provided is greater than the bounds of the View.
The same does not happen on Android, where the scroll halts once the last item of the scrollView is in the viewport.
I have added bounds check so the offset resets to the MaxOffset which makes sure the view does not scroll out of the viewport.
The issue can be observed in the following snack:
https://snack.expo.io/H1363Uo8f
To test my changes I ran the code provided in the snack above with the react-native dependency pointing to my branch. As can be see in the video attached below, the scroll halts once it hits the end of the ScrollView even if the scroll offset provided is higher than the bounds of the ScrollView. It also does not scroll up for negative scroll offset.
[IOS] [BUGFIX] [ScrollView] - Added bounds check to prevent ScrollView from scrolling to an offset which is out of bounds of the ScrollView for iOS.
Closes https://github.com/facebook/react-native/pull/17927
Differential Revision: D7014466
Pulled By: shergin
fbshipit-source-id: a817738d08e1057a4c70f43373132f88fa1461c4
Summary:
I am testing with Jest a component that use [AppState.removeEventListener](https://facebook.github.io/react-native/docs/appstate.html#removeeventlistener) and I am currently facing a fatal error saying that `AppState.removeEventListener` is `undefined`.
Create a component that uses `AppState`, e.g.
```jsx
import React, { Component } from 'react';
import { AppState } from 'react-native';
class TestComponent extends Component {
componentDidMount() {
AppState.addEventListener('change', this.stateChangeListener);
}
componentWillUnmount() {
AppState.removeEventListener('change', this.stateChangeListener);
}
}
```
It should pass test using Jest (snapshots)
It's the continuation of #11199. This PR finish the mock for the native module `AppState`.
[ GENERAL ] [ BUGFIX ] [AppState] Add missing mock for Jest for `removeEventListener` method.
Closes https://github.com/facebook/react-native/pull/17908
Differential Revision: D7083144
Pulled By: shergin
fbshipit-source-id: eafa07f064f971c3d657f2ffc9c00766c0925bac
Summary:
<!--
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!
-->
This ensures that errors are actually noticed in case it is not
possible to create the directory. This will also throw an error when used with the upcoming Node.js 10.x because there is no callback.
Refs: https://github.com/nodejs/node/pull/18668
[CLI][MINOR][local-cli/library/library.js] - Use sync fs.mkdir for error handling
<!--
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
-->
Closes https://github.com/facebook/react-native/pull/18084
Differential Revision: D7083122
Pulled By: shergin
fbshipit-source-id: a13b64e57860e1ab2c15e0c008211dc0dca3b2d2
Summary:
When the `autogrow` prop was removed from `TextInput` on Android, the `_onLayout` helper method was removed. This helper method implemented the hook required to make `autogrow` work, then dispatched the `onLayout` event to the handler in `this.props`. This change points passes the `onLayout` handler from `this.props` directly to the inner component.
<!--
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!
-->
I was updating copied code in react-native-windows and noticed this bug.
Run jest tests.
N/A
<!--
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
-->
[ANDROID][MINOR][BUGFIX] [TextInput] - Fix `onLayout` prop for TextInput on Android
Closes https://github.com/facebook/react-native/pull/18040
Differential Revision: D7078736
Pulled By: hramos
fbshipit-source-id: 798530729d7f0ee1ebb59f698af4d4b6ff43928b
Summary:
We updated to Gradle 2.3.0 and our app's build failed. Our app doesn't provide "repositoryUrl" which is intended to be an optional gradle property. However, Gradle 2.3.0 blows up on findProperty('repositoryUrl') when "repositoryUrl" isn't provided:
````
* What went wrong:
A problem occurred configuring project ':ContextMenuAndroid'.
> Could not resolve all dependencies for configuration ':ContextMenuAndroid:_debugPublish'.
> A problem occurred configuring project ':ReactAndroid'.
> Could not get unknown property 'repositoryUrl' for project ':ReactAndroid' of type org.gradle.api.Project.
````
To fix this, we now use "project.hasProperty('repositoryUrl')" to safely detect the presence of the optional "repositoryUrl" property.
Since I cannot check it with your build environment, I've created a small demo to show that "project.hasProperty" properly detects the presence of the gradle property "repositoryUrl". I edited "getRepositoryUrl" to throw an exception if "repositoryUrl" is set:
````
def getRepositoryUrl() {
if (project.hasProperty('repositoryUrl')) throw new GradleException(property('repositoryUrl'))
return project.hasProperty('repositoryUrl') ? property('repositoryUrl') : 'https://oss.sonatype.org/service/local/staging/deploy/maven2/'
}
````
Then I ran gradle with "repositoryUrl" set like this (passing the property):
````
./gradlew -PrepositoryUrl=blah assembleDebug
````
As expected, it detected that "repositoryUrl" was set and threw an exception:
````
* What went wrong:
A problem occurred configuring project ':ContextMenuAndroid'.
> Could not resolve all dependencies for configuration ':ContextMenuAndroid:_debugPublish'.
> A problem occurred configuring project ':ReactAndroid'.
> blah
````
The same issue has been reported before - #14811, #14810
Minor changes in the Android build script
Closes https://github.com/facebook/react-native/pull/18075
Differential Revision: D7077788
Pulled By: hramos
fbshipit-source-id: ecfbab29d0632e7eecb3c6a247df39bc7616653e
Summary:
WebSocket uses the Platform module to check how many arguments for the `close` method should be used. In react-native-windows, we have the same number of arguments for `close` as Android, so we're prevented from using this module as-is because of the platform check (see https://github.com/Microsoft/react-native-windows/blob/master/Libraries/WebSocket/WebSocket.windows.js#L136).
By switching to an argument count check, this module becomes more useful cross-platform. If you'd like to keep the platform check, I'm also open to inverting the conditional, e.g., `if (Platform.OS !== 'ios')`.
<!--
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!
-->
I'd like to minimize the amount of code I need to copy over to react-native-windows for platform-specific overrides.
Run jest tests.
N/A
<!--
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
-->
[GENERAL][MINOR][ENHANCEMENT][Libraries/WebSocket/WebSocket.js] - Better enable cross-platform support of WebSocket.js
Closes https://github.com/facebook/react-native/pull/18056
Differential Revision: D7070380
Pulled By: TheSavior
fbshipit-source-id: 9bfd47654d0bf6f0e07fc799853a206721467525
Summary: Adds the configured metro host to `ReactMarker.logMarker(ReactMarkerConstants.RELOAD)`. This may be used for diagnostics in marker listeners
Reviewed By: bnham
Differential Revision: D7041086
fbshipit-source-id: 0b0777b1cd4c74b29b9245e925cf7851b24985fa
Summary:
This PR fixes regression introduced in #17896 with IllegalStateException being thrown in FrameBasedAnimationDriver.
After investigating it seemed that the root cause was the code responsible for looping animations that was setting next frame time by adding the frame interval to the current time. In some circumstances the next frame would run earlier than that and as a result the calculated frame index was negative.
Here is the stacktrace as reported by axemclion https://github.com/facebook/react-native/pull/17896/files#r170007224
```
Caused by: java.lang.IllegalStateException: Calculated frame index should never be lower than 0
at com.facebook.react.animated.FrameBasedAnimationDriver.runAnimationStep(FrameBasedAnimationDriver.java:60)
at com.facebook.react.animated.NativeAnimatedNodesManager.runUpdates(NativeAnimatedNodesManager.java:444)
at com.facebook.react.animated.NativeAnimatedModule$1.doFrameGuarded(NativeAnimatedModule.java:100)
at com.facebook.react.uimanager.GuardedFrameCallback.doFrame(GuardedFrameCallback.java:29)
```
Run native animated tests suite. Run RNTester and scroll to the loop animation and see it working correctly
[ANDROID][BUGFIX][Animated] - Fix exception thrown by timing animation when looping
Closes https://github.com/facebook/react-native/pull/18061
Differential Revision: D7059335
Pulled By: hramos
fbshipit-source-id: b08dfd1398d028eeeabeb11863743666379da374
Summary:
Hi! I would like to contribute to React Native, and I am just starting out. I forked the repo and found that it has quite a lot of ESLint warnings – many of which were automatically fixable. This PR is simply the result of running `yarn lint --fix` from the root folder.
Most changes are removing trailing spaces from comments.
Haven't really done any manual testing, since I haven't done any code changes manually. `yarn test` runs fine, `yarn flow` runs fine, `yarn prettier` is satisfied.
N/A
[INTERNAL][MINOR][] - Fix ESLint warnings
Closes https://github.com/facebook/react-native/pull/18047
Differential Revision: D7054948
Pulled By: hramos
fbshipit-source-id: d53e692698d1687de5821c3fb5cdb76a5e03b71e
Summary: This diff allows Metro to throw an error when a configuration file cannot be found, by using a new strict: true option.
Reviewed By: rafeca
Differential Revision: D6998613
fbshipit-source-id: b704633be18d5c007f1a022fa811d0a74c636fc9
Summary:
Previously, the Platform module was used to detect when RCTVirtualText should be used. Recently, this has changed to detecting the availability of a native virtual text component on the UIManager. The import for the Platform module can be deleted.
I was cleaning up source code copied in react-native-windows and noticed this reference is no longer used in Text.js.
Run jest tests.
N/A
<!--
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
-->
Closes https://github.com/facebook/react-native/pull/18039
Differential Revision: D7042473
Pulled By: hramos
fbshipit-source-id: d318cfdfd2d052bd1662ac469dd51633f6d59d17
Summary:
So, all initial operations can now be done synchronously (and on the main thread).
To do so, instancitate `RCTSurface` object and call `synchronouslyWaitForStage:timeout:` method like that:
RCTSurface *surface = [[RCTSurface alloc] initWithBridge:... moduleName:... initialProperties:...];
BOOL success = [surface synchronouslyWaitForStage:RCTSurfaceStageSurfaceDidInitialMounting timeout:timeout];
or
RCTSurfaceHostingView *surfaceHostingView = [[RCTSurfaceHostingView alloc] initWithBridge:... moduleName:... initialProperties:...];
BOOL success = [surfaceHostingView.surface synchronouslyWaitForStage:RCTSurfaceStageSurfaceDidInitialMounting timeout:timeout];
Reviewed By: fkgozali
Differential Revision: D7014178
fbshipit-source-id: c3c13904a3587ff2a222fa71623c40c8f30bc8af
Summary: In some embedding/interop cases (RN inside, something else outside), the interop layer has to have the ability to control (intercept, delay, perform synchronously with another stuff) mounting process. This API allows doing that.
Reviewed By: fkgozali
Differential Revision: D7014179
fbshipit-source-id: 04036095f7e60a5ff7e69025ff6066fea92eb361
Summary:
See the comment in code.
If we on the main thread on registering time, we can/will break sequentiality of registering and mounting processes doing registration asynchronously.
We need this to make sync mouting eventually possible.
Reviewed By: fkgozali
Differential Revision: D7014176
fbshipit-source-id: 110ad5e5d86e3422eac15c3b1bdb29ae04acd7e6
Josh Hargreaves
Jean Lauliac
Kevin Gozali
Jamie Curtis
Sebastian Markbage
Spencer Ahrens
Manman Ren
Ken Woo
Rafael Oleza
David Vacca
Miguel Jimenez Esun
Tim Yung
siddhantsoni
Maxime Thirouin
Ruben Bridgewater
Valentin Shergin
Eric Rozell
Sasha Nikiforov
Reem Helou
David Aurelio
Burak Yigit Kaya
Islam Magdy
Krzysztof Magiera
Andrew Chen (Eng)
Mats Byrkeland
Maël Nison
Alex Kotliarskyi
Brian Vaughn
