Summary:
From the video below, we can see that the UI thread has dropped many frames, and it would become worse if there are multiple images.
If an image is located in the sandbox of the disk, we cannot load it using `RCTLocalAssetImageLoader` because `RCTLocalAssetImageLoader.requiresScheduling` is set to true, which loads the data on the UI thread and causes main thread stuttering. This will affect libraries such as `react-native-code-push` and others that save images to the sandbox of the disk.
Therefore, we should replace `RCTLocalAssetImageLoader.canLoadImageURL` from `RCTIsLocalAssetURL(url)` to `RCTIsBundleAssetURL(url)`. Similarly, we should rename the entire `RCTLocalAssetImageLoader` file with `RCTBundleAssetImageLoader`, which ignores images in the disk sandbox. And finally these images will be loaded from `NSURLRequest`, and our UI thread will run smoothly again.
https://user-images.githubusercontent.com/20135674/236368418-8933a2c6-549c-40d3-a551-81b492fe41d5.mp4
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[IOS] [Breaking] - Replace `RCTLocalAssetImageLoader` to `RCTBundleAssetImageLoader`
Pull Request resolved: https://github.com/facebook/react-native/pull/37232
Test Plan:
Test Code:
```javascript
constructor(props) {
super(props)
this.state = {
bundle_image: require('./large_image.png'),
sandbox_image: '',
source: null,
isLoading: false,
}
}
render() {
console.log('render', this.state)
return (
<View style={{ flex: 1, padding: 50, backgroundColor: 'white'}}>
<View style={{ flexDirection: 'row', alignItems: 'center', height: 70}}>
{
[{ title: 'Save Image To SandBox', onPress: () => {
let image = Image.resolveAssetSource(this.state.bundle_image)
console.log(image.uri)
this.setState({ isLoading: true })
RNFetchBlob.config({ fileCache: true, appendExt: "png" }).fetch("GET", image.uri).then(response => {
let path = response.path()
path = /^file:\/\//.test(path) ? path : 'file://' + path
console.log(path)
this.state.sandbox_image = path
}).finally(() => this.setState({ isLoading: false }))
}}, { title: 'Load From SandBox', onPress: () => {
this.setState({ source: { uri: this.state.sandbox_image } })
}}, { title: 'Clear', onPress: () => {
this.setState({
source: null,
isLoading: false
})
}}, { title: 'Load From Bundle', onPress: () => {
this.setState({ source: this.state.bundle_image })
}}].map((item, index) => {
return (
<Pressable
key={index}
style={{ height: '100%', justifyContent: 'center', flex: 1, borderWidth: 1, borderColor: 'black', marginLeft: index > 0 ? 15 : 0 }}
onPress={item.onPress}
>
<Text style={{ textAlign: 'center' }}>{item.title}</Text>
</Pressable>
)
})
}
</View>
<ActivityIndicator style={{ marginTop: 10 }} animating={this.state.isLoading} />
<Image
key={`${this.state.source}`}
style={{ marginTop: 20, width: 200, height: 200 }}
source={this.state.source}
onProgress={({ nativeEvent }) => console.log(nativeEvent)}
onLoadStart={() => this.setState({ isLoading: true })}
onLoadEnd={() => this.setState({ isLoading: false })}
/>
</View>
)
}
```
It needs to be tested in three environments: [Simulator_Debug, RealDevice_Debug, RealDevice_Release]
1. Open `Perf Monitor` (RealDevice_Release can be skipped)
2. Click `Save Image to SandBox`
3. Wait for the loading to end and click `Load From SandBox`
4. Verify that the image can be loaded successfully
5. Verify that the `UI thread` keeps `60 FPS` (RealDevice_Release can be skipped)
6. Click `Clear`
7. Repeat steps [3, 4, 5, 6] several times
8. Click `Load From Bundle` to verify that the bundle image can be loaded successfully
Simulator_Debug
https://user-images.githubusercontent.com/20135674/236369344-ee1b8ff1-2d49-49f3-a322-d973f4adf3e7.mp4
RealDevice_Debug
https://user-images.githubusercontent.com/20135674/236369356-fe440b2b-f72a-49be-b63c-b4bf709dac8c.mp4
RealDevice_Release
https://user-images.githubusercontent.com/20135674/236369365-8a6a5c2f-09ad-4c90-b6bd-41e8a5e3aa7f.mp4
Reviewed By: rshest
Differential Revision: D46441513
Pulled By: dmytrorykun
fbshipit-source-id: 652febd4147dbff6c1ceef03d84ce125b8c66770
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/37730
This diff makes unschematized native components available in bridgeless mode. In case there is no static view config, `BridgelessUIManager` calls `RN$LegacyInterop_UIManager_getConstants`, and gets native view config form the constants.
Changelog: [Internal] - Use RN$LegacyInterop_UIManager_getConstants in BridgelessUIManager
Reviewed By: sammy-SC
Differential Revision: D45154396
fbshipit-source-id: 32b3718841b59a8b6fb22022c9d9edc17dad877f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38501
char8_t is a distinct type in C++20, which would need to be propagated
everywhere else (e.g. using `std::u8string` instead of `std::string`).
The code was already assuming that char is UTF-8, so we can just cast
accordingly (which works on all compilers: https://godbolt.org/z/9cv4c48o4).
Reviewed By: javache
Differential Revision: D47537998
fbshipit-source-id: ba045483361463f1754e02791114b78f51932a56
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38153
This diff adds Android specific implementation of `RN$LegacyInterop_UIManager_getConstants` and binds it to JS runtime. It is supposed to be used as a substitute to UIManager.getConstants in bridgeless mode.
Changelog:
[Internal] - Introduce RN$LegacyInterop_UIManager_getConstants in Android.
Reviewed By: RSNara
Differential Revision: D45773342
fbshipit-source-id: 194aa5e940743b4d2c242798764a4207e8b1334f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38511
## Changelog
[Internal] -
This allows to override `Platform.isDisableAnimations` for the Android platform, in the same way as it's done with `IS_TESTING` (but similarly optionally doing `IS_DISABLE_ANIMATIONS` in addition to/instead).
See for more context: https://github.com/facebook/react-native/pull/38490
Reviewed By: cortinico
Differential Revision: D47530731
fbshipit-source-id: b90300124b2a8bac97fae78a94e8a2cc9d7fd5bc
Summary:
A simple sorting of the plugins list in each map. This is to sync with Meta internal tools update.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D47581872
fbshipit-source-id: 522f845eaee63c2812315eb4b8c25237d114ccd4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38506
There are use cases where an app needs to install a custom font in a separate infra/tool. Instead of bloating those infra/tool with deep React Native dependencies, this allows it to depend on only the pieces that matter.
To ensure backward compatibility, the existing ReactFontManager will proxy method calls to the new impl going forward.
Changelog: [Changed][Android] Moved ReactFontManager to a common package
Reviewed By: mdvacca
Differential Revision: D47569319
fbshipit-source-id: 18ba6617d6c3f68823bdccaacfd8cc961cd6ce34
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38325
This diff changed how we log app startup time by leveraging ReactMarker `logMarker` API, instead of the custom `setAppStartTime` API.
Changelog:
[Android][Internal] - Refactor how app should notify C++ about the app startup time.
Reviewed By: mdvacca
Differential Revision: D43863975
fbshipit-source-id: f80bcdb55fae82abce08eb2eff689985f90f1213
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38343
We'd like to run experiments on removing background executor in RN Fabric. This will move layout computing from the background thread (fabric_bg) to the JS thread. It will simplify the RN threading model and unblock new performance tracking features like Visual Completion Tracker and MutationObserver.
Changelog:
[Android][Internal] - Experiment with disabling background executor in VR panel apps
Reviewed By: javache
Differential Revision: D47452561
fbshipit-source-id: 554fef4ace2a967cdf0748d9148257d5d6bce987
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38327
This diff caches the react marker sent from the android host platform before JNI library is loaded and sent out once it's ready. This way the C++ side will receive those markers as expecetd.
Changelog:
[Android][Internal] - Cache react marker timings before JNI library is loaded.
Reviewed By: mdvacca
Differential Revision: D43863973
fbshipit-source-id: 6d0d41d803d39e66a793f86a21ee11133a631bf7
Summary:
This improves the strictness of TS typings for `transform` on `View`'s `style` prop. Consider the following example, with what TS reports in case of errors, using RN 0.72.3. The ❌ / ✅ symbols indicate whether TS is happy with the code
```tsx
❌ <View style={{ transform: [{ scale: undefined }] }} /> // TS2769: No overload matches this call.
❌ <View style={{ transform: [{ something: 1 }] }} /> // TS2769: No overload matches this call.
✅ <View style={{ transform: [{ scale: 1 }, { rotate: '90deg' }] }} />
✅ <View style={{ transform: [{ scale: 1, translateX: 1 }] }} /> // this is WRONG, corrected in the next row
✅ <View style={{ transform: [{ scale: 1 }, { translateX: 1 }] }} />
```
With this change, TS will report an error even for line 4
```tsx
❌ <View style={{ transform: [{ scale: undefined }] }} /> // TS2769: No overload matches this call.
❌ <View style={{ transform: [{ something: 1 }] }} /> // TS2769: No overload matches this call.
✅ <View style={{ transform: [{ scale: 1 }, { rotate: '90deg' }] }} />
❌ <View style={{ transform: [{ scale: 1, translateX: 1 }] }} /> // TS2769: No overload matches this call.
✅ <View style={{ transform: [{ scale: 1 }, { translateX: 1 }] }} />
```
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
[GENERAL] [CHANGED] - stricter TS check for transform style
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: https://github.com/facebook/react-native/pull/38348
Test Plan: tested locally with the example given above; also added a TS type test
Reviewed By: rshest
Differential Revision: D47526366
Pulled By: NickGerleman
fbshipit-source-id: 5bd007ce29509ccdfce74c3864dee24290d9a175
Summary:
Added code previously added in the following PR: https://github.com/facebook/react-native/pull/28659.
The above PR was accidentally scrapped due to the renaming of the master branch on the React Native repo. As advised in this comment: https://github.com/facebook/react-native/issues/37770#issuecomment-1582476332, I'm opening a new PR with the same code to get this merged into master / main.
Currently, we need to run a local fork of React Native and manually apply these changes ourselves. This then causes additional issues, as it's currently _**impossible**_ to build React Native from source when running on a Windows machine, as evidenced in https://github.com/facebook/react-native/issues/37770 and the other linked issues nested inside of this issue.
**Original summary is as follows:**
With `NetworkModule.setCustomClientBuilder` we can customize our OkHttpClient to all requests made by react-native, it's very useful when you do `SSL Pinning` or change some OkHttpClient configuration at all. I've added a similar function to websocket, it allow us do some configurations on Websocket OkHttpClient.
## Changelog:
[Android] [Added] - Websocket Module setCustomClientBuilder
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: https://github.com/facebook/react-native/pull/37798
Test Plan:
**From the original PR:**
You can just set a custom `CustomClientBuilder` on `MainActivity` `onCreate`:
```
import okhttp3.OkHttpClient;
import java.util.concurrent.TimeUnit;
import com.facebook.react.modules.network.CustomClientBuilder;
import com.facebook.react.modules.websocket.WebsocketModule;
public class MainActivity extends ReactFragmentActivity {
Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
RNBootSplash.init(R.drawable.launch_screen, MainActivity.this);
WebsocketModule.setCustomClientBuilder(new CustomClientBuilder() {
Override
public void apply(OkHttpClient.Builder builder) {
builder.connectTimeout(0, TimeUnit.MILLISECONDS);
}
});
}
...
```
Reviewed By: cortinico
Differential Revision: D47468613
Pulled By: javache
fbshipit-source-id: ad97fb18ba5784d8abe157f5ccd29201b8b0fe84
Summary:
This PR is a modified version of https://github.com/facebook/react-native/issues/38060 which was reverted because it may sometimes cause sensitive info to be logged. This version no longer includes any parameters in the thrown errors.
### Motivation
I had a crash-causing error in our error reporting console (Bugsnag) that was extremely hard to debug due to the lack of specificity in the thrown error.
```
java.lang.NullPointerException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference
at com.facebook.react.bridge.ReadableNativeArray.getDouble(ReadableNativeArray.java:92)
at com.facebook.react.bridge.JavaMethodWrapper$4.extractArgument(JavaMethodWrapper.java:64)
at com.facebook.react.bridge.JavaMethodWrapper$4.extractArgument(JavaMethodWrapper.java:60)
at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:356)
at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
at android.os.Handler.handleCallback(Handler.java:883)
at android.os.Handler.dispatchMessage(Handler.java:100)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
at android.os.Looper.loop(Looper.java:214)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
at java.lang.Thread.run(Thread.java:919)
```
I noticed that `invoke` in `JavaMethodWrapper` tacks on the JS method name on other errors, so wanted to change this to handle NullPointerExceptions more gracefully too.
This helps make it easier to debug issues like https://github.com/facebook/react-native/issues/23126, https://github.com/facebook/react-native/issues/19413, https://github.com/facebook/react-native/issues/27633, https://github.com/facebook/react-native/issues/23378, https://github.com/facebook/react-native/issues/29250, https://github.com/facebook/react-native/issues/28262, https://github.com/facebook/react-native/issues/34001 and likely many more.
### After adding NullPointerException
Even after adding the NullPointerException, I got errors like this:
```
com.facebook.react.bridge.NativeArgumentsParseException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference (constructing arguments for UIManager.dispatchViewManagerCommand at argument index 0) with parameters [null, 4.0, []]
at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
at android.os.Handler.handleCallback(Handler.java:942)
at android.os.Handler.dispatchMessage(Handler.java:99)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
at android.os.Looper.loopOnce(Looper.java:226)
at android.os.Looper.loop(Looper.java:313)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
at java.lang.Thread.run(Thread.java:1012)
```
This helped, but still didn't help me easily find which library calling `dispatchViewManagerCommand` was causing this.
## Changelog:
[ANDROID] [CHANGED] - Wrap NullPointerExceptions when thrown by native code called from JS for readability
[GENERAL] [CHANGED] - Throw Error in dispatchViewManagerCommand when non-numeric tag is passed for easier debugging
Pull Request resolved: https://github.com/facebook/react-native/pull/38444
Test Plan:
Test change on our app via a beta release.
With these changes, we got better stacktraces like these:
### Java stacktrace
```
com.facebook.react.bridge.NativeArgumentsParseException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference (constructing arguments for UIManager.dispatchViewManagerCommand at argument index 0) with parameters [null, 4.0, []]
at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
at android.os.Handler.handleCallback(Handler.java:942)
at android.os.Handler.dispatchMessage(Handler.java:99)
at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
at android.os.Looper.loopOnce(Looper.java:226)
at android.os.Looper.loop(Looper.java:313)
at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
at java.lang.Thread.run(Thread.java:1012)
```
### JS stacktrace (after dispatchViewManagerCommand change)
```
Error dispatchViewManagerCommand: found null reactTag with args []
node_modules/react-native/Libraries/ReactNative/PaperUIManager.js:120:21 dispatchViewManagerCommand
node_modules/react-native-maps/lib/MapMarker.js:68:67 _runCommand
node_modules/react-native-maps/lib/MapMarker.js:60:24 redraw
templates/Components/MapViewWithMarkers/PlaceMarker.tsx:88:32 anonymous
(native) apply
node_modules/react-native/Libraries/Core/Timers/JSTimers.js:213:22 anonymous
node_modules/react-native/Libraries/Core/Timers/JSTimers.js:111:14 _callTimer
node_modules/react-native/Libraries/Core/Timers/JSTimers.js:359:16 callTimers
(native) apply
node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:427:31 __callFunction
node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:113:25 anonymous
node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:368:10 __guard
node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:112:16 callFunctionReturnFlushedQueue
```
Reviewed By: javache
Differential Revision: D47546672
Pulled By: cortinico
fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
Summary:
Use Apple Silicon instead of M1 because we have M2 and other variations too.
I did not change the name of `__apply_Xcode_12_5_M1_post_install_workaround` as it's in use.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[GENERAL] [FIXED] - Use Apple Silicon instead of M1
Pull Request resolved: https://github.com/facebook/react-native/pull/38344
Test Plan: None as no behavioral changes
Reviewed By: christophpurrer
Differential Revision: D47466540
Pulled By: javache
fbshipit-source-id: d770189e065899181a8b4ea17465beef09607d2f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38476
The TurboModule interop layer test asserts that SampleLegacyModule.const2 == 390.
But SampleLegacyModule.const2 was actually the screen size of device, which is different locally vs sandcastle.
So, while the test passed locally (b/c screen size = 390), it failed on Sandcastle (b/c screen size = 375).
So, this diff hardcodes const2 to be 390.
Created from CodeHub with https://fburl.com/edit-in-codehub
Reviewed By: christophpurrer
Differential Revision: D47379531
fbshipit-source-id: 0abf079400d376d4816fecf1e7dac6db6fc688c1
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38350
Found subtle differences between how JSPointerDispatcher is configured across bridgeless.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D47468666
fbshipit-source-id: 730cd93ea426e50eb67a71feacc8d3eb477c4c96
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38490
## Changelog
[Internal] -
The internal `Platform.isTesting` is tightly coupled to animations being disabled, which in turn can lead to subtle problems in some of the corner cases:
* Since `isTesting` is force override to `false` on JS side in non-dev builds, it means that e2e tests, that would like to use release builds, are out of luck when animation disabling is desired
* Conversely, some of the e2e tests may actually rely on animations being enabled in order to work, which means they are also out of luck if trying to test a dev build
* Finally, we have cases of hybrid builds, which are build in release on native side, but also have `__DEV__=true` on the native side
To both cover the above scenarios, but also to be backwards compatible to all the existing once, this change introduces another flag, `Platform.isDisableAnimations`.
The way it works is:
* If it's not specified, the e2e tests behaviour will be exactly the same as before, since by default `isDisableAnimations` will be true when `isTesting` is true
* If it's specified and is equal to `false`, it means that animations will be still enabled, even if `isTesting` is true (for those e2e tests that rely on animations being enabled)
* If it's specified and is equal to 'true', it means that animations will be force disabled, no matter whether we test a release or a dev build
Note that this only specifies the JS side of things, defaulting `Platform.isDisableAnimations` to "not specified" (i.e. all the tests will behave as before).
Pulling it through for different platforms is done as a separate follow-up.
Differential Revision: D47516800
fbshipit-source-id: efcb78f68f9102fec025ecce9a475c9c0bc1ecca
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38475
We added `scrollEventThrottle` for android in D35735978, but the experiment was never executed and the flag got removed in D39449184.
Since the same feature is on iOS and the implementation here is the same to iOS (https://fburl.com/code/htcuhq4w), it should be safe to support.
Changelog:
[Android][Add] - Add scrollEventThrottle prop support for android
Reviewed By: cortinico
Differential Revision: D47492259
fbshipit-source-id: 09a2bead652bfef4a2c70b996cf66f6983604db2
Summary:
These changes are intended to resolve https://github.com/facebook/react-native/issues/11068.
## Changelog:
[Android] [Fixed] - Fix letters duplication when using autoCapitalize
Pull Request resolved: https://github.com/facebook/react-native/pull/35929
Test Plan:
I took the `RewriteExample` from `TextInputSharedExamples.js` duplicated it, updated the labels, attached to the same screen. Modified its `onChangeText` function, from `text = text.replace(/ /g, '_');` to `text = text.toLowerCase();` then tested via rn-tester as shown in the video:
- No duplicate characters
- Characters are updated to be lowercase
- Long pressing delete properly deletes, doesn’t stop after deleting one character
- Suggestions (selected from keyboard) work and are updated to lowercase when it becomes part of the input text
- Moving the cursor and typing works, cursor position is kept as it should
- Moving the cursor and deleting works
- Selection portion and deleting it works, cursor position is kept as it should
https://user-images.githubusercontent.com/14225329/213890296-2f194e21-2cf9-493f-a516-5e0212ed070e.mp4
Note: I have tested manually with 0.67.4, because later versions failed on my machine with cmake and argument errors when building the rn-tester from Android Studio to any device.
Help regarding that would be appreciated.
## Possible Future Improvements
As it can be seen the video, the letter duplication is resolved, however since the lowercase modification happens on the Javascript side, it takes a couple milliseconds and the Uppercase can still be shown momentarily while typing.
## Technical details, why the solution works
I've debugged a simple AppCompatEditText with calling the same `getText().replace` in `doAfterTextChanged` with a bit of delay and noticed a difference to the `ReactEditText`.
The ReactEditText removes the `android.view.inputmethod.ComposingText` Span in `manageSpans` before calling replace (ComposingText is `Spanned.SPAN_EXCLUSIVE_EXCLUSIVE`).
That `ComposingText` Span is used in `android.view.inputmethod.BaseInputConnection` `private replaceText` to find from what point the text should be replaced from when applying suggestions or typing new letters. Without that Span, it defaults to the selection position, which is usually the end of the text causing duplication of the old "word".
**In simple terms, while typing with suggestions on the keyboard, each new letter is handled similarly as clicking a suggestion would be, aka replacing the current "word" with the new "word". (let's say "Ar" word with "Are" word)**
Another way to describe it:
While typing with suggestions, with the ComposingText Span the TextView keeps track of what word completions are suggested for on the keyboard UI. When receiving a new letter input, it replaces the current "word" with a new "word", and without the Span, it replaces nothing at the end (selection point) with the new word which results in character duplication.
It also seems to make sense then why without suggestions (like password-visible and secureTextEntry) the issue hasn't occurred.
### Examples
How the issue happened:
> - User types: A (ComposingText Span becomes (0,1), composingWord: "A")
> - Javascript replaced A with a, ComposingText Span was removed from getText()
> - User types a new character: r (ComposingText, defaulting to selection, from selection, empty string is replaced with word "Ar")
> => Complete text: aAr => letter duplication.
How it works with the changes applied:
> - User types: A (ComposingText Span becomes (0,1), composingWord: "A")
> - Javascript replaces A with a, (ComposingText Span (0,1) is re-set after replace)
> - User types a new character: r (ComposingText (0,1), "a" word is replaced with word "Ar". ComposingText becomes (0,2) "Ar")
> - Javascript replaced Ar with ar, (ComposingText Span (0,2) is re-set after replace)
> => Complete text: ar => no letter duplication
> - User selects suggestion "Are" (ComposingText Span (0,2) "ar" is replaced with new word and space "Are ")
> - CompleteText: "Are "
> - Javascript replaces "Are " with "are " (ComposingText Span doesn't exist, no string after space " ")
Note: the Editable.replace also removes the ComposingText, if it doesn't cover the whole text, that's why we need to re-setSpan them even if we wouldn't remove them in `manageSpans`.
## Note
This is my first attempt to contribute so if I missed something or messed up, please point out and I will try to adapt.
Reviewed By: NickGerleman
Differential Revision: D47243817
Pulled By: lunaleaps
fbshipit-source-id: 5f3551d17466f2c7cd1aa89ffb09af50e065b52e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38321
This diff adds the native method in `ReactMarker.java` file so that any logs in the android side will call into C++. Given that currently the C++ uses the native implementation from hosting platforms, this allows any call (either from C++ or android) will end up calling the C++ method after logging is done.
The motivation of this change is to allow us to collect timing information from hosting platforms, and report back to JS performance startup API. We will have items that are logged before the JNI part is loaded, and those will be cached and sent over in the next diff.
Changelog:
[Android][Internal] - Implemented native method for Android LogMarker API to report timing values to C++.
Reviewed By: mdvacca
Differential Revision: D43806115
fbshipit-source-id: 11497bdc0af8d1b0299a797c636bb7af7a6ddda4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38356
fix the correct case of this event
Changelog:
[General][Fixed] - fix the correct case of this event
Reviewed By: javache
Differential Revision: D47480741
fbshipit-source-id: a600f4921c59c571f2da251b36674a98c8c7e51f
Summary:
I was getting the following error when trying to run RN Tester on Android.
```
A problem occurred configuring project ':packages:react-native:ReactAndroid'.
> Could not determine the dependencies of task ':react-native-gradle-plugin:compileKotlin'.
> No matching toolchains found for requested specification: {languageVersion=17, vendor=any, implementation=vendor-specific} for MAC_OS on aarch64.
> No locally installed toolchains match and toolchain download repositories have not been configured.
```
This is fixed by adding the `org.gradle.toolchains.foojay-resolver-convention` plugin in settings.gradle, this was done already for the root one, but not in the one in `react-native-gradle-plugin`.
## Changelog:
[INTERNAL] [FIXED] - Add missing toolchains plugin in react-native-gradle-plugin
Pull Request resolved: https://github.com/facebook/react-native/pull/38292
Test Plan: Build RN Tester on android
Reviewed By: mdvacca
Differential Revision: D47371772
Pulled By: cortinico
fbshipit-source-id: 8eaa5de9559720a7b37d11b3ddceb5fb84753a40
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38357
## Context
On iOS, NSNotificationCenter facilitates event dispatch:
- People can dispatch events **to** objects.
- People can subscribe to events coming **from** objects.
(By specifying objects, NSNotificationCenter also implements event filtering).
The TuboModule system uses NSNotificationCenter to implement module invalidation:
- The bridge dispatches invalidation notifications to its parentBridge object.
- The TurboModuleManager listens to those invalidation notifications from the parentBridge object.
## Problem
In some apps, the TurboModuleManager never invalidates modules.
The bridge dispatches its invalidation notifications to nil: the parentBridge gets deallocated before bridge invalidation finishes.
But, the TurboModuleManager never receives those invalidation notifications: it is listening to invalidation notifications coming from a non-nil parentBridge.
## Fix
Make the TurboModuleManager listen to invalidation notifications from all objects. It will just manually noop invalidation if the notification's bridge object doesn't match its own bridge object. (This is existing logic).
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D47485910
fbshipit-source-id: 163403ef01f7d164a0e482f0f770a801d572503b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38349
## Changelog:
[Internal] -
This backs out an earlier commit because we have two separate internal tests' setups that broke as a side effect of it.
In both cases the setup was arguably inconsistent, however the plan is to temporarily revert this nevertheless, fix the tests first and then resubmit.
Reviewed By: javache
Differential Revision: D47469176
fbshipit-source-id: 532253b032b010b037d7b6f6581147f5531f7ee1
Summary:
I noticed that `onMomentumScrollEnd` is called multiple times on Android.
1. It is always called three times with the last value
2. It is sometimes called many times befire the scrolling stops when the pagingEnabled prop is true
See:
<img src="https://user-images.githubusercontent.com/17070498/137640334-301b32a7-3f59-403f-ba7e-a898666aaf3e.png" width="400"/>
I used the following code to get the logs:
```
import React from 'react';
import {SafeAreaView, ScrollView, Text, View} from 'react-native';
const App = () => {
const onMomentumScrollEnd = ({nativeEvent}) => {
console.log(
'onMomentumScrollEnd',
nativeEvent.contentOffset.x,
nativeEvent.contentOffset.y,
);
};
const onMomentumScrollBegin = ({nativeEvent}) => {
console.log(
'onMomentumScrollBegin',
nativeEvent.contentOffset.x,
nativeEvent.contentOffset.y,
);
};
return (
<SafeAreaView>
<ScrollView
horizontal
pagingEnabled
onMomentumScrollEnd={onMomentumScrollEnd}
onMomentumScrollBegin={onMomentumScrollBegin}>
{new Array(10).fill(0).map((_, index) => {
return (
<View
style={{width: 400, height: 400, backgroundColor: 'red'}}
key={index}>
<Text>{index}</Text>
</View>
);
})}
</ScrollView>
</SafeAreaView>
);
};
export default App;
```
From what I understood:
1. We do not check that `mStableFrames` is >= 3 when emitting the event (line 798) and we keep executing the runnable, so it is emitted until `mStableFrames` equals 3. When `mStableFrames` equals 3 we stop executing the runnable (line 809). That's why it gets called 3 times.
2. When `pagingEnabled` is true, the `postOnAnimationDelayed` method is called twice (line 794 and line 806). I believe it causes the runnable to be executing too often, and the `onMomentumScrollEnd` event to be emitted too many times.
I updated the code so:
1. The event is emitted only once, and at the same time we stop executing the runnable
2. The `postOnAnimationDelayed` method is called at most once per execution of the runnable
## Changelog
[Android] [Fixed] - Fix ScrollView's onMomentumScrollEnd being called multiple times on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/32433
Test Plan: I tested using the code above with every combination of `horizontal` and `pagingEnabled` values.
Reviewed By: NickGerleman
Differential Revision: D47297163
Pulled By: ryancat
fbshipit-source-id: 7c31175d941ff13bed20dac03fb92d2b56e94dec
Summary:
This PR is a result of this PR, which got merged but then reverted:
- https://github.com/facebook/react-native/pull/37913
We are trying to implement a workaround for https://github.com/facebook/react-native/issues/35350, so react-native users on android API 33+ can use `<FlatList inverted={true} />` without running into ANRs.
This is the native part, where we add a new internal prop named `isInvertedVirtualizedList`, which can in a follow up change be used to achieve the final fix as proposed in https://github.com/facebook/react-native/pull/37913
However as NickGerleman pointed out, its important that we first ship the native change.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[ANDROID] [ADDED] - Native part of fixing ANR when having an inverted FlatList on android API 33+
Pull Request resolved: https://github.com/facebook/react-native/pull/38071
Test Plan:
- Check the RN tester app and see that scrollview is still working as expected
- Add the `isInvertedVirtualizedList` prop as test to a scrollview and see how the scrollbar will change position.
Reviewed By: rozele
Differential Revision: D47062200
Pulled By: NickGerleman
fbshipit-source-id: d20eebeec757d9aaeced8561f53556bbb4a492e4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38339
## Changelog:
[Internal] -
In D13050583, the `Platform.isTesting` was unconditionally forced to `false` in `__DEV__` on JS side.
On some platforms, we may want to run tests in the release mode.
This hoists these gatings down to the corresponding platforms' native modules.
Reviewed By: ryancat
Differential Revision: D47438069
fbshipit-source-id: 5bae3df9873f659f65179df93e727108d0062047
Summary:
Bumped Flipper version from `0.182.0` to `0.201.0` (which is currently latest version). New version contain NDK 25, which is necessarily for us since we would like to bump NDK in React Native to 25, see [here](https://github.com/facebook/react-native/pull/37974) for more context.
## Changelog:
[General] [Changed] - Bump Flipper to 0.204.0
Pull Request resolved: https://github.com/facebook/react-native/pull/38260
Test Plan: CI Green ✅
Reviewed By: NickGerleman, mdvacca
Differential Revision: D47373525
Pulled By: cortinico
fbshipit-source-id: d1d5f03cb2f00bc8b9064af986b7c3b6e7ccae3c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38319
The TurboModule interop layer's bridge proxy emits logs.
These logs aren't very actionable (yet). (We will make them more useful eventually).
So, this diff adds bridge proxy log-levels (backed by a server-side flag).
Longer term, **all of** React Native's backwards compatibility layers will output logs (not just the bridge proxy). So, we might replace this specific control with something more general.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D47407548
fbshipit-source-id: 2bd79d2a8d52a46d25852a6a23f9cebea0580f9c
Summary:
Adds changelog for new patch.
## Changelog:
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Internal] [Changed] - add changelog entry for 0.72.3
Pull Request resolved: https://github.com/facebook/react-native/pull/38312
Test Plan: N/A
Reviewed By: cortinico
Differential Revision: D47402979
Pulled By: lunaleaps
fbshipit-source-id: c126ed5af26301a4f21a15a186db9b178d70b2ae
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38302
X-link: https://github.com/facebook/hermes/pull/1052
Make Runtime a friend of these types so we can use `Runtime::make` with
all of them. Simplify the Hermes API code to do this.
Changelog: [Internal]
Reviewed By: avp
Differential Revision: D47373807
fbshipit-source-id: c9530b6c159592e36bb0929badba0542f0c7db78
hublot
Intl Scheduler
Dmitry Rykun
Shoaib Meenai
Arushi Kesarwani
Lulu Wu
Deepak Jacob
Ruslan Shestopalyuk
kacperkapusciak
Kevin Gozali
Xin Chen
Vojtech Novak
MO-Lewis
Constantine Fry
Harry Yu
Nicola Corti
Ken Tominaga
Ramanpreet Nara
Pieter De Baets
Gergely Hegedus
Juan Luis Herrero Estrada
Janic Duplessis
AntoineDoubovetzky
Hanno J. Gödecke
szymonrybczak
Nick Gerleman
Lorenzo Sciandra
Neil Dhar
Samuel Susla
Fabrizio Cucci
