Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49271
This error is somewhat expected, so causing the red box error popup is a bit too disruptive. Flip it to a no-crash exception.
Changelog: [Internal]
Reviewed By: Abbondanzo
Differential Revision: D69125274
fbshipit-source-id: 0dc7ac59ac8637bdabde25bd8886b1aebf175395
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49277
Replaces the `-1` magic number representing unset children with a named constant
Changelog: [Internal]
Reviewed By: zeyap
Differential Revision: D69324509
fbshipit-source-id: 64fb6c920a7715f5d15d3955564a8bf2b6ce404a
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49231
Those public modifier have no meaning as the class is internal. I'm removing them.
Changelog:
[Internal] [Changed] -
Reviewed By: javache
Differential Revision: D69252048
fbshipit-source-id: b51e5ac20338a01291d6cd04ee4c990cc8a6a755
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49229
This interface was converted to Kotlin, but the single method should have been converted to a `val`.
People kotlin consumers could call ReactOverflowView.overflow; now they need to call getOverflow().
Changelog:
[Internal] [Changed] - Undo a breaking change on ReactOverflowView
Reviewed By: NickGerleman
Differential Revision: D69250226
fbshipit-source-id: 5c7cca8c83f5c76a9cc1d254f8aa51409150c356
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49233
I'm converting the function inside ReactPointerEventsView from `fun` to `val`.
This Kotlin conversion resulted in a breakign change for Kotlin consumer which I believe can be prevented
if we do this change instead.
Changelog:
[Internal] [Changed] -
Reviewed By: alanleedev
Differential Revision: D69252562
fbshipit-source-id: b277c6720f3156ed532bf5f2253d54cd72e38050
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49230
Those public modifier have no meaning as the class is internal. I'm removing them.
Changelog:
[Internal] [Changed] -
Reviewed By: tdn120
Differential Revision: D69251378
fbshipit-source-id: 4c3747510d18330dcdb8a0798e92736c4ab65a03
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49146
Runtime Reference ShadowNode Update is enabled by default and no longer referenced in the RN holdout. This diff:
- removes the feature flag
- removes all references to the feature flag, enabling it
- updates the unit test for RSNRU
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D69061234
fbshipit-source-id: 0dab0b5cae99e83297f34645dee58ae9b3c0dc5f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49211
Add more control over view recycling behavior by splitting out each component that currently supports it.
Changelog:[Android][Added] Feature flags for recycling View, Text components separately
Reviewed By: sammy-SC, mdvacca
Differential Revision: D69190841
fbshipit-source-id: 6d85fee7103bf928e4f5bf6946bab3ff4cae4053
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49183
The feature has been released to all for quite some time now and the holdout group has finally been unlinked. This removes all references to the feature flag and a few indicators that the feature is enabled/disabled from RNTester
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D69146787
fbshipit-source-id: 8a7f01016a715e61541910630d8c3ceb84ec5c82
Summary:
Fixes a `ConcurrentModificationException` when iterating over `TextWatcher` `mListeners` array.
If you open Android open source code (`TextView` class), then we can see that Android iterates with `for/n` loop (not `for/:`):
```java
void sendAfterTextChanged(Editable text) {
if (mListeners != null) {
final ArrayList<TextWatcher> list = mListeners;
final int count = list.size();
for (int i = 0; i < count; i++) {
list.get(i).afterTextChanged(text);
}
}
notifyListeningManagersAfterTextChanged();
hideErrorIfUnchanged();
}
```
<hr>
We can catch the `ConcurrentModificationException` with old code, when for example we have 3 listeners:
- 0 is `EmojiTextWatcher` (seems like it's added by OS);
- 1 is `OnlyChangeIfRequiredMaskedTextChangedListener` (added by `react-native-text-input-mask`);
- 2 is a listener that attached by `react-native-keyboard-controller`.
On every afterTextChanged [input-mask-android](https://github.com/RedMadRobot/input-mask-android/tree/df452edc0c52a37e5082adcfc3d05d77b5aa34e8) [removes](https://github.com/RedMadRobot/input-mask-android/blob/df452edc0c52a37e5082adcfc3d05d77b5aa34e8/inputmask/src/main/kotlin/com/redmadrobot/inputmask/MaskedTextChangedListener.kt#L212) the listener and [adds](https://github.com/RedMadRobot/input-mask-android/blob/df452edc0c52a37e5082adcfc3d05d77b5aa34e8/inputmask/src/main/kotlin/com/redmadrobot/inputmask/MaskedTextChangedListener.kt#L231) it back.
The oversimplified version of the code can be next:
```java
public class MyClass {
public static void main(String args[]) {
ArrayList<Integer> mListeners = new ArrayList<>();
mListeners.add(0);
mListeners.add(1);
mListeners.add(2);
Iterator<Integer> iterator = mListeners.iterator();
while (iterator.hasNext()) {
Integer listener = iterator.next();
// Check if the listener is equal to 1
// 1 is OnlyChangeIfRequiredMaskedTextChangedListener and we simulate the behavior of this class
if (listener == 1) {
int i = mListeners.indexOf(listener);
if (i >= 0) {
mListeners.remove(i);
}
// Add the removed element at the end
mListeners.add(listener);
}
}
// Print the modified list
System.out.println(mListeners);
}
}
```
Key points are:
- if we have only [0, 1] listener, then it works well and `ConcurrentModificationException` will not be thrown, because we modify last element;
- if we have `[0, 1, 2]` then exception will be thrown.
So in this PR I decided to re-work code to match what Android has. With `for/n` approach `ConcurrentModificationException` will not be thrown, because we don't check array immutability in this case.
More information also can be found here: https://github.com/kirillzyusko/react-native-keyboard-controller/issues/324
## Changelog:
[ANDROID] [CHANGED] - avoid `ConcurrentModificationException` when iterating over `mListeners` `TextWatcher` array
Pull Request resolved: https://github.com/facebook/react-native/pull/49109
Reviewed By: cortinico
Differential Revision: D69050984
Pulled By: javache
fbshipit-source-id: 9c6a7a428467fa5e546d70549dfcc91d6b2e58d2
Summary:
Working on migrating some of the com.facebook.react.modules.network classes to Kotlin, I'm creating some test cases here for `CountingOutputStream` before migrating that class.
## Changelog:
[INTERNAL] - Add CountingOutputStream tests
Pull Request resolved: https://github.com/facebook/react-native/pull/49058
Test Plan:
```bash
yarn test-android
```
Reviewed By: cortinico
Differential Revision: D68903427
Pulled By: rshest
fbshipit-source-id: f71926cf526a65b2434aaa762007e0b4ca5dd1a4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49087
I'm moving the whole module to be in Kotlin and updating the BUCK file.
Those files also have 0 usages in OSS so not a breaking change.
Changelog:
[Internal] [Changed] -
Reviewed By: robhogan
Differential Revision: D68953731
fbshipit-source-id: d8238bf805661cbdd6fb070a60f3e32b44ec9832
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49062
Another round of cleanup for the `public` keyword that I found around.
Those are unnecessary here as those classes are `internal` and we should remove them.
Changelog:
[Internal] [Changed] -
Reviewed By: mdvacca
Differential Revision: D68894182
fbshipit-source-id: 6f7bac6051e17785a1bfb0d544950250429c71cb
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49074
This used to not be noticeable when we were clipping the background even without a border, after fixing that, we got line when the width/height was 0
This is again not an issue with new Background and Border since they take a slightly different approach
Diff that caused the issue D68279400
ie.
{F1974794589}
Changelog: [Internal]
Reviewed By: javache
Differential Revision: D68843649
fbshipit-source-id: a25ace46b604690e3385c49d6f4bb3a4163bc594
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49049
ReactBridge can be internalize, there are no usages in OSS
changelog: [internal] internal
Reviewed By: NickGerleman
Differential Revision: D68540710
fbshipit-source-id: ce7fe6ca52186414650dcc529c5891dc59cab51a
Summary:
The `maxFontSizeMultiplier` prop for `Text` and `TextInput` was not handled in Fabric / New Architecture as documented in https://github.com/facebook/react-native/issues/47499.
bypass-github-export-checks
## Changelog:
[GENERAL] [FIXED] - Fix `maxFontSizeMultiplier` prop on `Text` and `TextInput` components in Fabric / New Architecture
Pull Request resolved: https://github.com/facebook/react-native/pull/47614
Test Plan:
I have not added any automated tests for this change but try to do so if requested. I have however added examples to RN Tester for both the Text and TextInput components, as well as compared the behaviour with Paper / Old Architecture. Both on version 0.76.
Noticed now I didn't do exactly the same steps in both videos, oops! Be aware that reapplying changes made in the Settings are currently half-broken on the new architecture, thus I'm restarting the app on Android and iOS. But this issue is unrelated to my changes. I've tested on main branch and it has the same issue.
Here are comparison videos between Paper and Fabric on iOS *after* I've made my fix.
### Text
| Paper | Fabric |
| ------------- | ------------- |
| <video src="https://github.com/user-attachments/assets/f4fd009f-aa6d-41ab-92fa-8dcf1e351ba1" /> | <video src="https://github.com/user-attachments/assets/fda42cc6-34c2-42a7-a6e2-028e7c866075" /> |
### TextInput
| Paper | Fabric |
| ------------- | ------------- |
| <video src="https://github.com/user-attachments/assets/59b59f7b-25d2-4b5b-a8e2-d2054cc6390b" /> | <video src="https://github.com/user-attachments/assets/72068566-8f2a-4463-874c-45a6f5b63b0d" /> |
Reviewed By: Abbondanzo
Differential Revision: D65953019
Pulled By: cipolleschi
fbshipit-source-id: 90c3c7e236229e9ad9bd346941fafe4af8a9d9fc
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49033
Those methods are not used at all in the codebase, let's clean them up.
Changelog:
[Internal] [Changed] -
Reviewed By: cipolleschi
Differential Revision: D68826893
fbshipit-source-id: 36e2f0ae247ed72305c1d9d346c6cf32cef6f8f2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49034
This class was still in Java. I'm converting it to Kotlin + I'm making it internal.
As this class was inside the `com.facebook.react.internal.turbomodule.core` package,
we don't consider this a breaking change.
Changelog:
[Internal] [Changed] -
Reviewed By: cipolleschi
Differential Revision: D68826892
fbshipit-source-id: b1f7aea984ab333faea66a9e8ccbb1492767333e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49000
Just found those nits around while looking at the codebase.
This clears things out and should have no runtime impact.
Changelog:
[Internal] [Changed] -
Reviewed By: mdvacca
Differential Revision: D68768384
fbshipit-source-id: bd3a30f1792a6f662d1f5b25855c89b6d43e72bb
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/49004
This just converts yet another class to Kotlin.
Changelog:
[Internal] [Changed] -
Reviewed By: tdn120
Differential Revision: D68772336
fbshipit-source-id: 428cb3a0d54bf7a22f0e4eb07268cdc27ef6f2c3
Summary:
This is another attempt at fixing the Android HMR client for HTTPS proxied Metro instances. The previous one unintentionally [caused the following error](https://github.com/facebook/react-native/pull/48970#issuecomment-2617047184):
```
java.lang.AssertionError: Method overloading is unsupported: com.facebook.react.devsupport.HMRClient#setup
```
This PR removes the overloading, and only adds the `scheme` property as a parameter to the existing `.setup` method. Aligning with the exact behavior we have on iOS.
The alternative fix, which should NOT be backward breaking (if this is) - is to move this "infer the protocol from the bundle URL" to the JS side of the HMR client. Where we don't just always default to `http`, but instead default to `https IF port === 443, otherwise http`. It's a bit more hacky, but shouldn't cause any other issues. _**Ideally**_, we have the same working behavior on both Android and iOS without workarounds.
<details><summary>Alternative workaround</summary>
See [this change](https://github.com/facebook/react-native/compare/main...byCedric:react-native:patch-2).
<img width="1179" alt="image" src="https://github.com/user-attachments/assets/47c365bc-6df8-43e6-ad7d-5a667e350cd4" />
</details>
See full explanation on https://github.com/facebook/react-native/issues/48970
> We've noticed that the HMR on Android doesn't seem to be connecting when using a HTTPS-proxied Metro instance, where the proxy is hosted through Cloudflare. This is only an issue on Android - not iOS - and likely caused by the HMR Client not being set up properly on Android.
>
>- On Android, we run `.setup('android', <bundleEntryPath>, <proxiedMetroHost>, <proxiedMetroPort>, <hmrEnabled>)` in the [**react/devsupport/DevSupportManagerBase.java**](https://github.com/facebook/react-native/blob/53d94c3abe3fcd2168b512652bc0169956bffa39/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java#L689-L691) file.
>- On iOS, we run `[self.callableJSModules invokeModule:@"HMRClient" method:@"setup" withArgs:@[ RCTPlatformName, path, host, RCTNullIfNil(port), @(isHotLoadingEnabled), scheme ]];` in the [**React/CoreModules
/RCTDevSettings.mm**](https://github.com/facebook/react-native/blob/53d94c3abe3fcd2168b512652bc0169956bffa39/packages/react-native/React/CoreModules/RCTDevSettings.mm#L488-L491) file.
>
>Notice how Android does not pass in the scheme/protocol of the bundle URL, while iOS actually does? Unfortunately, because the default protocol (`http`) mismatches on Android when using HTTPS proxies, we actually try to connect the HMR client over `http` instead of `https` - while still using port 443 - which is rejected by Cloudflare's infrastructure even before we can redirect or mitigate this issue. And the rejection is valid, as we basically try to connect on `http://<host>:443` (the source URL is `https`, so the port is infered as `443`).
>
>This change adds scheme propagation to Android, exactly like we do on iOS for the HMR Client.
## Changelog:
[ANDROID] [FIXED] Pass the bundle URL protocol when setting up HMR client on Android
<!-- 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/48998
Test Plan:
See full explanation on https://github.com/facebook/react-native/issues/48970
> It's a little bit hard to test this out yourself, since you'd need a HTTPS-based proxy and reject HTTP connections for HTTPS/WSS Websocket requests.
>
>You can set this up through:
>- `bun create expo@latest ./test-app`
>- `cd ./test-app`
>- `touch .env`
>- Set `EXPO_PACKAGER_PROXY_URL=https://<proxied-metro-hostname>` in **.env**
>- Set `REACT_NATIVE_PACKAGER_HOSTNAME=<proxied-metro-hostname>` in **.env**
>- `bun run start`
>
>Setting both these envvars, the bundle URL in the manifest is set to `https://...` - which triggers this HMR issue on Android. You can validate the **.env** setup through:
>
>```bash
>curl "http://localhost:8081" -H "expo-platform: android" | jq .launchAsset.url
>```
>
>This should point the entry bundle URL towards the `EXPO_PACKAGER_PROXY_URL`.
Reviewed By: cortinico
Differential Revision: D68768351
Pulled By: javache
fbshipit-source-id: 49bf1dc60f11b2af6e57177141270632d62ab564