'Repro' for https://github.com/facebook/react-native/issues/52272 #52273
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#publish-packages-to-npm&next
Summary: Pull Request resolved: facebook#48656 While working on 0.78, I realize we were not testing the template app with JSC. This change should fix this. ## Changelog: [Internal] - Disable Hermes for the JSC E2E tests with Maestro Reviewed By: cortinico, fabriziocucci Differential Revision: D68147849 fbshipit-source-id: 4fbe005b5d04d6163a37041d1bd57fd48a9dfda8
Summary: This change improves the E2E testing by downloading the iOS RNTesterApp that is built in CI instead of building it locally. This should let us save 10 to 20 minutes when we test a new release. ## Changelog: [Internal] - Use the RNTester app built in CI for release testing on iOS Pull Request resolved: facebook#48637 Test Plan: - build the app in ci - run `yarn test-e2e-local -c <my-token>` and `yarn test-e2e-local -h false -c <my-token>` and verify that the iOS app is not built, but run in the simulator Reviewed By: cortinico Differential Revision: D68161477 Pulled By: cipolleschi fbshipit-source-id: 577d110f9ff0197a2d3348a08a60e60a4d0a752b
Summary: Following the suggestions [here](https://stackoverflow.com/questions/79360526/uninitialized-constant-activesupportloggerthreadsafelevellogger-nameerror), it seems that concurrent-ruby has been released tonight and it is bugged. Let's pin it to the right version. ## Changelog: [iOS][Changed] - Pin 'concurrent-ruby' to a working version Pull Request resolved: facebook#48721 Test Plan: GHA Reviewed By: robhogan Differential Revision: D68262719 Pulled By: cipolleschi fbshipit-source-id: fc6410e28cc96f9d3769d3082a77cac0a3efe6db
Summary: Pull Request resolved: facebook#48672 ## Changelog: [General] [Fixed] - Buttons becoming unresponsive when transform is animated # The problem D67872307 changes when `ensureUpdateSubscriptionExists` is called to in `__attach`. This breaks the functionality because `__attach` is called before flag `__isNative` is set and subscriptions are never setup. # Fix The diff sets up subscriptions in `__makeNative` method. Reviewed By: yungsters Differential Revision: D68154908 fbshipit-source-id: e2ac108b064a66dda08902653d6bd20286f92458
…book#48678) Summary: Pull Request resolved: facebook#48678 While diagnosing a recent issue in which `AnimatedValue` instances were not being correctly updated as expected, the insertion effects feature flag was identified as a root cause. Upon further investigation, it appears that this is because the `onUserDrivenAnimationEnded` listener was not implemented the same way in the two feature flag states: - When `useInsertionEffectsForAnimations` is disabled, `useAnimatedProps` listens to `onUserDrivenAnimationEnded` in a passive effect, after all nodes have been attached. - When `useInsertionEffectsForAnimations` is enabled, `useAnimatedProps` listens to `onUserDrivenAnimationEnded` in an insertion effect when attaching nodes. The bugs occurs because `useAnimatedProps` checks whether native driver is employed to decide whether to listen to `onUserDrivenAnimationEnded`. However, we do not know whether native driver will be employed during the insertion effect. (Actually, we do not necessarily know that in a passive effect, either... but that is a separate matter.) This fixes the bug when that occurs when `useInsertionEffectsForAnimations` is enabled, by moving the listening logic of `onUserDrivenAnimationEnded` into a passive effect. This is the same way that it is implemented when `useInsertionEffectsForAnimations` is disabled. Changelog: [Internal] Reviewed By: javache, sammy-SC Differential Revision: D68171721 fbshipit-source-id: 50b23348fd4641580581cacebc920959651f96a7
…48715) Summary: Pull Request resolved: facebook#48715 {D68154908} fixed a problem with the `onAnimatedValueUpdate` listener not being correctly attached if `__attach` were called before `__makeNative` (which sets `__isNative` to true). We're potentially seeing production symptoms of stuttering interactions and user responsiveness, after queuing up many operations. Our hypothesis is that in scenarios where `ensureUpdateSubscriptionExists` is being called during `__makeNative` (instead of during `__attach`), a backup of operations occurs leading to these symptoms. This diff attempts to validate and mitigate this hypothesis by deferring `ensureUpdateSubscriptionExists` to when an `AnimatedValue` instance has had both `__attach` and `__makeNative` invoked. Changelog: [Internal] Differential Revision: D68236594 fbshipit-source-id: 2089100a773ebfc161fb5b567123eb58a893939f
#publish-packages-to-npm&next
Changelog: [Internal]
Summary: Pull Request resolved: facebook#48888 We have a report from OSS where Images are not displayed properly in case they are saved on disk with no extension. We previously had a fix attempt iwith [this pr](facebook#46971), but this was breaking some internal apps. This second attempt should work for both cases. ## Changelog: [iOS][Fixed] - Load images even when the extension is implicit Reviewed By: cortinico Differential Revision: D68555813 fbshipit-source-id: bc25970aafe3e6e5284163b663d36e00b3df3d82
#publish-packages-to-npm&next
Changelog: [Internal]
… Android (facebook#48998) Summary: This is another attempt at fixing the Android HMR client for HTTPS proxied Metro instances. The previous one unintentionally [caused the following error](facebook#48970 (comment)): ``` 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](facebook/react-native@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 facebook#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: facebook#48998 Test Plan: See full explanation on facebook#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
Summary: `dev-middleware` uses `invariant` but does not declare it as a dependency. Under certain hoisting scenarios, or when using pnpm, this will cause `dev-middleware` to fail while being loaded. ## Changelog: [GENERAL] [FIXED] - add missing `invariant` dependency Pull Request resolved: facebook#49047 Test Plan: n/a Reviewed By: cortinico Differential Revision: D68835789 Pulled By: huntie fbshipit-source-id: 13718f4970ed55e6e062b7c2bd719be977abdd0c
… in new architecture (facebook#47614) Summary: The `maxFontSizeMultiplier` prop for `Text` and `TextInput` was not handled in Fabric / New Architecture as documented in facebook#47499. bypass-github-export-checks ## Changelog: [GENERAL] [FIXED] - Fix `maxFontSizeMultiplier` prop on `Text` and `TextInput` components in Fabric / New Architecture Pull Request resolved: facebook#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
facebook#48995) Summary: Pull Request resolved: facebook#48995 This change adds an extra parameter to the codegen script that allow our users to trigger codegen for Apps or for Libraries. When running codegen for Apps, we have to generate some extra files that are not needed by the Libraries. This is causing issues to our library maintainers and this change will provide more flexibility in the DevX of libraries. The default value is App, so if the new parameter is not passed, nothing will change in the current behavior. [iOS][Added] - Add the `source` parameter to generate-codegen-artifacts to avoid generating files not needed by libraries. Reviewed By: cortinico Differential Revision: D68765478 fbshipit-source-id: 8030b4472ad4f5058e58b1c91089de5122a4f60a
#publish-packages-to-npm&next
…book#49072) Summary: Pull Request resolved: facebook#49072 We have instance of apps crashing when enabling the New Architecture because of the TurboModule interop layer. What's happening is that when the module is loaded, the TM Interop Layer tries to parse the method definition to expose them in JS. However, for some libraries in the Legacy Architecture, it is possible to define a method in Objective-C and to define a different signature in Swift. For example, the [`RNBluetoothClassic` library](https://github.com/kenjdavidson/react-native-bluetooth-classic) defines a selector in objective-c which [has the signature](https://github.com/kenjdavidson/react-native-bluetooth-classic/blob/main/ios/RNBluetoothClassic.m#L134-L136) ``` RCT_EXTERN_METHOD(available: (NSString *)deviceId resolver: (RCTPromiseResolveBlock)resolve rejecter: (RCTPromiseRejectBlock)reject) ``` And the method is inmplemented in Swift with [the signature](https://github.com/kenjdavidson/react-native-bluetooth-classic/blob/main/ios/RNBluetoothClassic.swift#L502-L505): ``` func availableFromDevice( _ deviceId: String, resolver resolve: RCTPromiseResolveBlock, rejecter reject: RCTPromiseRejectBlock ) ``` When the TurboModule interop layer tries to parse the method, it receives the `accept:resolver:rejecter:` signature, but that signature is not actually defined in as a method in the module instance, and it crashes. This crash was not happening in the Old Architecture, which was handling this case gracefully. Notice that the specific method from the example is not working in the Old Architecture either. However, the app is not crashing in the old architecture. This change adds the same graceful behaviors plus it adds a warning in development to notify the developer about which methods couldn't be found in the interface. Fixes: - facebook#47587 - facebook#48065 ## Changelog: [iOS][Fixed] - Avoid crashing the app when the InteropLayer can't find some methods in the native implementation. Reviewed By: javache Differential Revision: D68901734 fbshipit-source-id: 844d1bf29423d5c601b583540e86d57dfffd1428
Summary: Pull Request resolved: facebook#48736 Pull Request resolved: facebook#48735 This icon was broken by D65457771, identified in [OSS testing for the 0.77 release](reactwg/react-native-releases#724). By explicitly setting the image for the `Disabled` state to the same as `Normal`, we get the same behaviour as the deprecated [`adjustsImageWhenDisabled = NO`](https://developer.apple.com/documentation/uikit/uibutton/adjustsimagewhendisabled?language=objc) without the need for `configurationUpdateHandler`. Changelog: [iOS][Fixed] Restore "Paused in debugger" overlay icon Reviewed By: cipolleschi Differential Revision: D68274336 fbshipit-source-id: 3f4b84eb7cfb518ca953c721da9885df8f98b437
Summary: Pull Request resolved: facebook#48823 This diff is fixing the execution of Events that are sent early in the rendering of surfaces. This diff fixes a bug in the queueing of events that are built with not surfaceId (-1), the fixes is to call getSurfaceManagerForView() to retrieve the proper surfaceId (as we do in the execution of events) calling getSurfaceManagerForView() has a perf hit, we believe this won't be a problem because this method will only be called in edge cases (no surfaceId and early execution of events) changelog: [Android][Fixed] Fix execution of early InteropEvents Reviewed By: shwanton, lenaic Differential Revision: D68454811 fbshipit-source-id: a79be0b392004e645c48d1683bba774b6b597ca0
…on (facebook#49233) Summary: Pull Request resolved: facebook#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: facebook#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: facebook#49247 This was incorrectly made internal in https://www.internalfb.com/diff/D66724567 Changelog: [Android][Removed] Made ReactCookieJarContainer internal. Reviewed By: cortinico, andrewdacenko Differential Revision: D69254203 fbshipit-source-id: 5c4ba9b4f9a8e53002df25b55f0c8762874e6736
#publish-packages-to-npm&next
Changelog: [Internal]
Summary: Make UIBlock.execute params non nullable to avoid breaking changes for kotlin usages in OSS changelog: [internal] internal Reviewed By: NickGerleman Differential Revision: D69407325 fbshipit-source-id: 5fc01fba9baba97e21a388d02cf98d5aebb5ac20
Summary: Pull Request resolved: facebook#49300 With the refactor of the AppDelegate in favor ReactNativeFactory, the users can now instantiate multiple instances of react native. However, currently, if you try to run multiple instances, the app will crash with the message: ``` libc++abi: terminating due to uncaught exception of type std::runtime_error: Feature flags cannot be overridden more than once ``` This happens also when the feature flags we would like to set are the same that we already applied. This should be an allowed scenario because reapplying the excatly same features flags should have no effect on React native and that's not the use case we want to forbid. With this change, we are creating a static variable that checks whether we already apply that set of feature flags and it allows you to create multiple instances by keeping the same flags ## Changelog: [iOS][Fixed] - Allow multiple RN instances to run at the same time Reviewed By: rubennorte Differential Revision: D69398441 fbshipit-source-id: a377c6a1402d38d66d348fa8c6a65e645973aadc
Summary: This change adds an opt-in to restore JavaScript log streaming via the Metro dev server, [removed from React Native core in 0.77](https://reactnative.dev/blog/2025/01/21/version-0.77#removal-of-consolelog-streaming-in-metro). Users can opt into this legacy behaviour by adding the `--client-logs` flag to `npx react-native-community/cli start`. - The default experience remains without streamed JS logs. - The existing "JavaScript logs have moved! ..." notice is printed in all cases, and we do not advertise the new flag for new users. - Under non-Community CLI dev servers (i.e. Expo), log streaming is restored implicitly. We will clean up this functionality again when we eventually remove JS log streaming over `HMRClient`, tasked in T214991636. **Implementation notes** - Logs are always sent over `HMRClient` (previous status quo), even with log streaming off in the dev server. This is a necessary evil to be able to flag this functionality in a user-accessible place, and to move fast for 0.78. - Necessarily, emitting `fusebox_console_notice` moves to the dev server itself, on first device (Fusebox) connection. Changelog: [General][Added] - Add opt in for legacy Metro log streaming via `--client-logs` flag
#publish-packages-to-npm&next
Changelog: [Internal]
#publish-packages-to-npm&latest
|
Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
|
This is not a valid reproducer. See more on this here: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Repro for #52272
Forked from v0.78.0 tag, no code changes.