Merge up to 0.76.3 #2295
Open
Merge up to 0.76.3 #2295
Conversation
This file contains 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
Summary: For users who may have node installed in a path with a space, this requires escaping. For example: ``` NODE_BINARY=/Users/blakef/Library/Application Support/fnm/node-versions/v20.12.0/installation/bin/node ``` Needs to be: ``` NODE_BINARY=/Users/blakef/Library/Application\ Support/fnm/node-versions/v20.12.0/installation/bin/node ``` # Changelog [iOS][Fixed] Generated NODE_BINARY in .xcode.env.local now supports paths with a space Reviewed By: cipolleschi Differential Revision: D64080118 fbshipit-source-id: 1045473e4fd284fc570fa538984618630be1af6d
…ComponentsProvider (facebook#47176) Summary: Pull Request resolved: facebook#47176 While writing the guide for the New Architecture, we realized that we need to exclude the generation of the Cls function in the RCTThirdPartyFabricComponentsProvider for components defined in the app. This is needed because a component that is defined in the app will have those function defined in the app project. However, the RCTThirdPartyFabricComponentsProvider is generated in Fabric, inside the Pods project. The pod project needs to build in isolation from the app and cocoapods then link the app to the pods project. But the compilation of the pods project fails if one of the symbol needed by the pods lives in the app. By disabling the generation of that function in th RCTThirdPartyFabricComponentsProvider, we can successfully build the app. The downside is that the user needs to register the component manually, but this is not an issue because if they are writing a component in the app space, they have all the information tomanually register it in the AppDelegate ## Changelog [iOS][Fixed] - Do not generate the ComponentCls function in the RCTThirdPartyFabricComponentsProvider for components deined in the app. Reviewed By: cortinico, blakef Differential Revision: D64739896 fbshipit-source-id: 0eca818ea0198532a611377d14a3ff4c95cb5fe3
Summary: The JsBundleFilePath has been ignored when converting DefaultReactNativeHost to ReactHost Changelog: [Internal] [Changed] - Add jsBundleFile to DefaultReactNativeHost.kt Pull Request resolved: facebook#47188 Reviewed By: javache Differential Revision: D64914149 Pulled By: cortinico fbshipit-source-id: d437ca81df5a170e0c5f01a22ccda83f43a09dd2
Summary: Pull Request resolved: facebook#47101 See facebook#47011 borders do not have this problem because they call `removeAllAnimations` on the layer after changing it, which is what I am doing here Changelog: [iOS] [fixed] - Fixed bug where background colors would sometimes animate when changing on Views Reviewed By: cipolleschi Differential Revision: D64493968 fbshipit-source-id: cf81549f21b124b67c6e7647c6ae827bfe80a9cf
Summary: Pull Request resolved: facebook#47237 The Xcodeproj gem has been released yesterday to version 1.26.0 and it broke the CI pipeline of react native. This should fix the issue ## Changelog [Internal] - Pin Xcodeproj gem to 1.26.0 Reviewed By: blakef Differential Revision: D65057797 fbshipit-source-id: f4035a1d3c75dd4140eb1646ab2aa0ccb08fb16b
…cebook#47236) Summary: Fixes facebook#47234. regression from facebook@391680f#diff-b7fda5d350ac535115fa683faa7317b43aa11f3448f95266ef9ff051c3753a6fL63 bypass-github-export-checks ## Changelog: [IOS] [FIXED] - Fixes regression of RCTWindowFrameDidChangeNotification not fired Pull Request resolved: facebook#47236 Test Plan: Demo in facebook#47234. Reviewed By: blakef Differential Revision: D65058105 Pulled By: cipolleschi fbshipit-source-id: 0e286182ed93f289cb853710e2e00801ef2d4f73
Summary: AppRegistry was not treated as a Callable Module in bridgeless mode. This is breaking headless tasks on Android. Fixes: - facebook#46050 ## Changelog: [ANDROID] [FIXED] - Made AppRegistry callable from Native code in Bridgeless (fixes headless tasks) Pull Request resolved: facebook#46480 Test Plan: Used repro from linked issue Reviewed By: javache Differential Revision: D62637486 Pulled By: cortinico fbshipit-source-id: 756527003ac6d712e76c02c188e280d15c010068
This step called an old reference, this function identifier was updated. Changelog: [Internal]
#publish-packages-to-npm&latest
Changelog: [Internal]
* Re-enable integration tests (facebook#46639) Summary: Pull Request resolved: facebook#46639 These tests were skipped when we were switching to component stacks, which also hid a bug later in the stack. Re-enable them. Changelog: [Internal] Reviewed By: javache Differential Revision: D63349616 fbshipit-source-id: ccde7d5bb3fcd9a27adf4af2068a160f02f7432a * Add integration tests for console errors + ExceptionManager (facebook#46636) Summary: Pull Request resolved: facebook#46636 Adds more integration tests for LogBox (currently incorrect, but fixed in a later diff). Changelog: [Internal] Reviewed By: javache Differential Revision: D63349614 fbshipit-source-id: 8f5c6545b48a1ed18aea08d4ecbecd7a6b9fa05a * Refactor LogBox tests to spies (facebook#46638) Summary: Pull Request resolved: facebook#46638 This is annoying, but in the next diff that fixes a bug I need to test using the default warning filter instead of a mock (really, all this mocking is terrible, idk why I did it this way). Unfortunately, in Jest you can't just reset mocks from `jest.mock`, `restoreMocks` only resets spies and not mocks (wild right). So in this diff I converted all the `jest.mock` calls to `jest.spyOn`. I also corrected some of the mocks that require `monitorEvent: 'warning',` like the warning filter sets. I also added a test that works without the fix. Changelog: [Internal] Reviewed By: javache Differential Revision: D63349615 fbshipit-source-id: 4f2a5a8800c8fe1a10e3613d3c2d0ed02fca773e * Fix errors with component stacks reported as warnings (facebook#46637) Summary: Ok so this is a doozy. ## Overview There was a report that some console.error calls were being shown as warnings in LogBox but as console.error in the console. The only time we should downlevel an error to a warning is if the custom warning filter says so (which is used for some noisy legacy warning filter warnings internally). However, in when I switched from using the `Warning: ` prefix, to using the presence of component stacks, I subtly missed the default warning filter case. In the internal warning filter, the `monitorEvent` is always set to something other than `unknown` and if it's set to `warning_unhandled` then `suppressDialog_LEGACY` is always false. However, the default values for the warning filter are that `monitorEvent = 'unknown'` and `suppressDialog_LEGACY = true`. In this case, we would downlevel the error to a warning. ## What's the fix? Change the default settings for the warning filter. ## What's the root cause? Bad configuration combinations in a fragile system that needs cleaned up, and really really bad testing practices with excessive mocking and snapshot testing (I can say that, I wrote the tests) ## How could it have been caught? It was, but I turned off the integration tests while landing the component stack changes because of mismatches between flags internally and in OSS, and never turned them back on. Changelog: [General] [Fixed] - Fix logbox reporting React errors as Warnings Pull Request resolved: facebook#46637 Reviewed By: huntie Differential Revision: D63349613 Pulled By: rickhanlonii fbshipit-source-id: 32e3fa4e2f2077114a6e9f4feac73673973ab50c * [LOCAL] using older version of React Dev Tools - Older version has old URL, updated tests - Comments on test don't match what's being tested. Updated. --------- Co-authored-by: Rick Hanlon <[email protected]>
Summary: Pull Request resolved: facebook#46484 We recently realized that we don't have TS types for Codegen. These are needed to let our users use these types when writing Specs in TS ## Changelog [General][Added] - Add CodegenTypes for TS Reviewed By: christophpurrer Differential Revision: D62644516 fbshipit-source-id: 92bb7e8998d31806f6eb63319fb6d406fcd65ad8
Summary: Pull Request resolved: facebook#47018 This change makes it so that newly typed text in a TextInput will include the existing attributes present based on cursor position. E.g. if you type after an inner fragment with blue text, the next character will be blue (or, an event emitter specific to an inner fragment will also be expanded). This is a behavior change for the (admittedly rare) case of uncontrolled TextInput with initially present children AttributedText, but more often effect controlled components, before state update (we are after, less likely to need to reset AttributedString because of mismatch). Originally included this in D64121570, but it's not needed to fix the common case since we include paragraph-level event emitter as part of default attributes, and has some of its own risk, so I decided it is better separate. Changelog: [iOS][Changed] - Include existing attributes in newly typed text Reviewed By: cipolleschi Differential Revision: D64352310 fbshipit-source-id: 90ef8c49f50186eadf777e81cf6af57e1aada207
Summary: Pull Request resolved: facebook#47195 When a user wants to create a Fabric Component i their app (not in a separate library) the app fails to build because: - The custom component has to inherit from `RCTViewComponentView` - `RCTViewComponentView` imports `ViewProps.h` - `ViewProps.h` imports `HostPlatformViewProps.h` - `HostPlatformViewProps.h` imports `BaseViewProps.h` - `BaseViewProps.h` imports `YogaStylableProps.h` which is a Yoga private header and the App has not visibility over it. It is also not possible to fix this issue with forward declaring the `YogaStylableProps`, because `BaseViewProps` inherit from the yoga's props, so the compiler needs the full declaration of `YogaStylableProps` to work This needs to be picked in 0.76 ## Changelog [iOS][Fixed] - Give apps access to Yoga headers Reviewed By: blakef Differential Revision: D64925222 fbshipit-source-id: e724076bbfb0a678948340dfab2ce609e6509533
…n controlled single line TextInput on iOS (New Arch) (facebook#46970) Summary: Pull Request resolved: facebook#46970 Fixes facebook#44157 This one is a bit of a doozy... During auto-correct in UITextField (used for single line TextInput) iOS will mutate the buffer in two parts, non-atomically. After the first part, after iOS triggers `textFieldDidChange`, selection is in the wrong position. If we set full new AttributedText at this point, we propagate the incorrect cursor position, and it is never restored. In the common case, where we are not mutating text in the controlled component, we shouldn't need to be setting AttributedString in the first place, and we do have an equality comparison there currently. But it is defeated because attributes are not identical. There are a few sources of that: 1. NSParagraphStyle is present in backing input, but not the AttributedString we are setting. 2. Backing text has an NSShadow with no color (does not render) not in the AttributedText 3. Event emitter attributes change on each update, and new text does not inherit the attributes. The first two are part of the backing input `typingAttributes`, even if we set a dictionary without them. To solve for them, we make attribute comparison insensitive to the attribute values in a default initialized control. There is code around here fully falling back to attribute insensitive comparison, which we would ideally fix to instead role into this "effective" attribute comparison. The event emitter attributes being misaligned is a real problem. We fix in a couple ways. 1. We treat the attribute values as equal if the backing event emitter is the same 2. We set paragraph level event emitter as a default attribute so the first typed character receives it After these fixes, scenario in facebook/react-native-website#4247 no longer repros in new arch. Typing in debug build also subjectively seems faster? (we are not doing second invalidation of the control on every keypress). Changes which do mutate content may be susceptible to the same style of issue, though on web/`react-dom` in Chrome, this seems to not try to preserve selection at all if the selection is uncontrolled, so this seems like less of an issue. I haven't yet looked at old arch, but my guess is we have similar issues there, and could be fixed in similar ways (though, we've been trying to avoid changing it as much as possible, and 0.76+ has new arch as default, so not sure if worth fixing in old impl as well if this is very long running issue). Changelog: [iOS][Fixed] - Fix cursor moving in iOS controlled single line TextInput on Autocorrection (New Arch) Reviewed By: javache, philIip Differential Revision: D64121570 fbshipit-source-id: 2b3bd8a3002c33b68af60ffabeffe01e25c7ccfe
…nent value specified using `value` instead of `children` (facebook#47269) Summary: Pull Request resolved: facebook#47269 There were [reports](reactwg/react-native-releases#595) that patching in the fixes for iOS controlled input did not work as expected. I think tracked this down to a difference in how I tested, where the controlled component I used passed value as a child of the `TextInput`, instead of via `value`. Passing via `value` triggers a secondary bug, where we don't correctly pass a reference to correct ShadowView when creating attributedstring, specifically in the iOS TextInputShadowNode impl. We previously passed nothing for the ShadowView (only the first two struct fields). This was exposed in D52589303 which enabled `-Wextra`, but there, I went with same behavior of passing empty ShadowView, instead of the correct behavior (like Android impl) of passing a ShadowView of the current ShadowNode. After fixing this, we now correctly create event emitters in the passed attributedstring, which matches expectations for pargraph-level eventemitter now in typing attributes. We don't seem actually use this on iOS for TextInput right now (just Text), but this is likely the right foundation for events regardless. Changelog: [iOS][Fixed] - Fix missing emitter attributes on iOS TextInput when controlled component value specified using `value` instead of `children` Reviewed By: cipolleschi Differential Revision: D65108163 fbshipit-source-id: 499fe28439fabd2579eca6ded7fd13fd8ea2e43e
…facebook#47339) Summary: Pull Request resolved: facebook#47339 Fixing issue raised in facebook#47307 This is a follow up from D62286026. It appears there was a line that went missing while trying to refactor the code. `fitsSystemWindows = true` is needeod for < API 30 to avoid content rendering under the system bars when Modal is shown with Activity that is edge-to-edge. Changelog: [Android][Fixed] Fix Regression - Modal content rendering below system bar on < API 30 when activity is edge-to-edge Reviewed By: cortinico Differential Revision: D65280014 fbshipit-source-id: 616ff739be55635f1295ef3bf8b997a27ef769ae
Summary: Pull Request resolved: facebook#47388 Fixes facebook#47364 Fixes facebook#47377 Fixes facebook#37124 We're having problems is a path contains a space ' ' because when autolinking, the `add_subdirectory()` function of CMake consider the path with space as 2 parameters. This fixes it by properly quoting the path. Changelog: [Android] [Fixed] - Properly handle paths with spaces in autolinking Reviewed By: cipolleschi Differential Revision: D65434413 fbshipit-source-id: b9147482f98f7e222405cc8d9e6f3c17a5f4ed02
Summary: Fixes facebook#47352 This fixes a bug when the user is providing its own CMakeLists.txt file say because they want to compile more C++ code than we actually provide. Previously the `*.cpp` will evalute file in the current directory, meaning that the app's default `OnLoad.cpp` file would be ignored. ## Changelog: [ANDROID] [FIXED] - Use absolute path when compiling appmodules.so sources Pull Request resolved: facebook#47379 Test Plan: Tested against the reproducer provided in: - Use absolute path when compiling appmodules.so sources Reviewed By: cipolleschi Differential Revision: D65428676 Pulled By: cortinico fbshipit-source-id: 7f3e4d470da0fffc5191c1a2c7e8fec517fee496
Summary: Fixes facebook#47495 `JavaTimerManager` is being registered to receive headless tasks events in the [`TimingModule`](https://github.com/facebook/react-native/blob/0ee963ea65bcc88122044d51027511e611bde584/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/core/TimingModule.kt#L28-L29). This module is not used on bridgeless: [1](https://github.com/facebook/react-native/blob/0ee963ea65bcc88122044d51027511e611bde584/packages/react-native/Libraries/Core/setUpTimers.js#L44-L61), [2](https://github.com/facebook/react-native/blob/0ee963ea65bcc88122044d51027511e611bde584/packages/react-native/Libraries/Core/setUpTimers.js#L123-L132) and since it's loaded lazily, the event listener is never registered. This PR moves registration to the constructor of `JavaTimerManager` and deregistration to the `onInstanceDestroy` method. This way the event listener is always registered when an instance of the timer manager exists. ## Changelog: [ANDROID] [FIXED] - Fix timers in headless tasks on bridgeless mode Pull Request resolved: facebook#47496 Test Plan: See the reproducer from the issue Reviewed By: javache Differential Revision: D65615601 Pulled By: alanleedev fbshipit-source-id: 6e1d36f8783e813065f79730a928b99c3e385718
…acebook#47547) Summary: Pull Request resolved: facebook#47547 In [facebook#47176](facebook#47176) we disabled the generation of the component registration for app specific components as it was creating a circular dependency between the app and React Native. However, we made a couple of typos that make it not work as expected and users picked up those typos soon. This change fixes them for good. ## Changelog [iOS][Fixed] - Properly stop generating component registration for components defined in app. Reviewed By: blakef Differential Revision: D65750433 fbshipit-source-id: 1a879c5be014905558b9fd05e6f16ac36b784ed6
#publish-packages-to-npm&latest
Summary: Trivial typo - static analysis would have been a good thing. Changelog: [Internal] Pull Request resolved: facebook#47286 Test Plan: eyes Reviewed By: NickGerleman Differential Revision: D65145600 Pulled By: blakef fbshipit-source-id: 567ef3637441aa84651dce03f45b80068c5f4290
Summary: This pull request addresses a CMake configuration issue where an invalid escape character in file paths caused the build process to fail. Specifically, it resolves the issue in the React Native CMake configuration file where the path separator was incorrectly handled, leading to an error in the build system. the issue is in [This Issue](expo/expo#32955) and [This](expo/expo#32957) ## 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 --> [INTERNAL] [FIXED] - Corrected invalid escape character in CMake path handling Pull Request resolved: facebook#47641 Test Plan: To test the changes, I performed the following steps: 1. Cloned the repository and checked out the `fix-cmake-invalid-escape-character` branch. 2. Ran the CMake build on a Windows environment where the issue was previously occurring. 3. Verified that the build process completed successfully without the "invalid character escape" error. 4. Ensured that the path handling now works correctly in CMake on Windows platforms. Reviewed By: rshest Differential Revision: D66073896 Pulled By: cipolleschi fbshipit-source-id: bd2a71bb00ce5c5509ed403842c995c32f58f91d
Summary: Pull Request resolved: facebook#47702 Use `file(TO_CMAKE_PATH` to normalize paths, and normalizing `input_SRC` as it's already a CMake path. Changelog: [Internal] Reviewed By: rshest Differential Revision: D66101321 fbshipit-source-id: e81af40551d2777901f9c7cf9a4175f2bce76ec8
Summary: Pull Request resolved: facebook#47874 We should be searching for the .bat file on Windows to remain compatible with some user setups. Changelog: [Android][Fixed] look for sdkmanager.bat Reviewed By: cipolleschi Differential Revision: D66295240 fbshipit-source-id: 6b79a9aa40f77ed9c5b3d6ad92b1a62e78159223
Summary: CI is failing to build HermesC on windows due to a version mismatch of the CMake already installed ## Changelog: [Internal] - Fix Windows CI for HermesC Pull Request resolved: facebook#47867 Test Plan: GHA Reviewed By: robhogan Differential Revision: D66292617 Pulled By: cipolleschi fbshipit-source-id: 5e8f4f45e33fbdd9ff163b4e8a09cb98d4366dc7
#publish-packages-to-npm&latest
Changelog: [Internal]
amgleitman
approved these changes
Nov 25, 2024
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.
Summary:
Merge up to the 0.76.3 release upstream. Add a changefile to publish a new virtualized-lists release
Test Plan:
CI should pass.