Issue 42164 #42181
Closed
Issue 42164 #42181
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: Pull Request resolved: facebook#41701 I did a hotfix for this logic in D51618512. This does a small refactor to improve the code (moving more shared code to the hook and avoiding creating a closure unnecessarily in every call to it). Changelog: [internal] Reviewed By: javache Differential Revision: D51660288 fbshipit-source-id: 472836840b19958402bd0de3e2c09c7cec004156
Summary: Pull Request resolved: facebook#41700 The type definition of `useMergeRefs` is incorrect, which forces all callsites to use `$FlowFixMe`. This fixes the definition and removes all the `$FlowFixMe`s caused by it. Changelog: [internal] Reviewed By: javache Differential Revision: D51660716 fbshipit-source-id: 4d4d3a72bdca8c409fd1dda59cc2c94113b024bb
…of Fabric initialization Differential Revision: D51224854 Original commit changeset: 2af802140436 Original Phabricator Diff: D51224854 fbshipit-source-id: 039337be7057c9625d4a6e53520a18cd5071813e
Differential Revision: D50926218 Original commit changeset: f311affb0f82 Original Phabricator Diff: D50926218 fbshipit-source-id: 313fd5aff1314860994487d1f4d17d2a2d5fe8c1
Summary: Pull Request resolved: facebook#41709 We shipped these new create()/reload()/destroy() methods to the Facebook app: - Part 1: D50802718 - Part 2: D50803283 This diff just enables them everywhere, by default. Created from CodeHub with https://fburl.com/edit-in-codehub Changelog: [Internal] Reviewed By: cortinico Differential Revision: D51590843 fbshipit-source-id: 02abeea78b7b7b844552989ad58d0a2f048424ad
Summary: Pull Request resolved: facebook#41711 We want the default position to be relative for a number of reasons. This should be fine for the most part but putting a killswitch around this change just in case things blow up. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D51643446 fbshipit-source-id: 4f7d1e498eb663801ef6d88ba9cd9b64c781d66b
Summary: New implementation: This PR adds cast from interface Binding to BindingImpl class. Previous implementation: The changes made in this PR make the `mBinding` field of `FabricUIManager` visible for JNI. Without these changes, calling the method `JFabricUIManager::getBinding()` would result in an error. <img width="400" alt="Screenshot 2023-11-27 at 13 55 44" src="https://github.com/facebook/react-native/assets/36106620/04418291-8ce8-4bae-b16c-29a5c9f2ee52"> In the `react-native-reanimated` library, we utilize `JFabricUIManager::getBinding()`, and we have noticed this issue since version 0.73. This isn't perfect solution, but I'm not certain which change in RN or FBJNI is the source of the problem. If there are any alternative solutions worth considering, I am open to discussing them. Usage of `getBinding()` in Reanimated: https://github.com/software-mansion/react-native-reanimated/blob/main/android/src/main/cpp/NativeProxy.cpp#L57 ## Changelog: [ANDROID] [FIXED] - Fix type for unrecognisable field mBinding Pull Request resolved: facebook#41657 Test Plan: Just call `JFabricUIManager::getBinding` method (https://github.com/facebook/react-native/blob/v0.73.0-rc.5/packages/react-native/ReactAndroid/src/main/jni/react/fabric/JFabricUIManager.cpp#L14) or run app with repro: https://github.com/piaskowyk/missing-mBinding-repro after the app lunch you will receive error from above screenshot. Co-author: tomekzaw Reviewed By: NickGerleman Differential Revision: D51661873 Pulled By: javache fbshipit-source-id: 1891c36bf25c503ebc9b0501211df03be6f74115
Summary: Pull Request resolved: facebook#41704 `mOnViewAttachItems` was set to be be concurrent, but this would be unexpected, as all mount item operations occur solely on the main thread. Simplify this to be just a LinkedList and annotate the methods as being UI thread only. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D51662154 fbshipit-source-id: 9fe5784bce8a38d1339b5e3675791414676b6f4d
Summary: Pull Request resolved: facebook#41719 We leak ReactInstanceManager into a static singleton in `ReactCxxErrorHandler.setHandleErrorFunc`. Clean it up in `destroy()`. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D51706624 fbshipit-source-id: 642825ba14ff0a9710b4435f5fb6026b3a81b711
Summary: Pull Request resolved: facebook#41436 Add a JSI API for associating some native memory with a JS object. This is intended to provide a mechanism to trigger more frequent garbage collection when JS retains large external memory allocations, in order to avoid memory buildup. This diff just adds the JSI method, without any implementations. Changelog: [General][Added] - Added JSI method for reporting native memory to the GC. Reviewed By: tmikov Differential Revision: D50524912 fbshipit-source-id: c8df0e18b0415d9523e0a00f6d0ed2faa648ac68
…book#41689) Summary: Pull Request resolved: facebook#41689 Bump hermes-parser and related packages to 0.18.0. Changelog: [internal] Reviewed By: SamChou19815 Differential Revision: D51642821 fbshipit-source-id: b7abde7d3e0de195c18a5cb18d4cdd0a1d435127
Summary: Pull Request resolved: facebook#41725 Reviewed By: SamChou19815 Differential Revision: D51713020 fbshipit-source-id: 84dc8c882f7603d642cb4ccb735fad0fb7e25d4d
Summary: Pull Request resolved: facebook#41712 I fixed the const correctness of YGConfigGetErrata a while back when fixing up other YGConfig accessors. Changelog: [Internal] Reviewed By: christophpurrer Differential Revision: D51689323 fbshipit-source-id: 1af3deb44ec03a8a65643fa1496c534ac8f6d057
Summary: Pull Request resolved: facebook#41732 Was reading the code in this file and noticed that this comment is no longer true after D51068417 (facebook/yoga#1460). Updated the comment to reflect the current state of things Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D51730986 fbshipit-source-id: beaa5de9576d86e56def35f6e970376c7be8f7ee
…k#41736) Summary: Pull Request resolved: facebook#41736 Changelog: [Internal] Reviewed By: gkz Differential Revision: D51735062 fbshipit-source-id: 942264cdc9f71e4aaa6f730d68f5a2a6e2fc7493
Summary: Pull Request resolved: facebook#41728 Adding APIs for `getFabricUIManager()` to ReactContext and it's subclasses. This will replace the `getJSIModule()` post JSI module deletion. Reviewed By: philIip Differential Revision: D51718430 fbshipit-source-id: c897ab0ee9e755e3fdb3d1e5629177818870f293
Summary: This PR fixes typo in CircleCI config ## 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] - Typo in circleci config Pull Request resolved: facebook#41727 Test Plan: CI Green Reviewed By: cipolleschi Differential Revision: D51748329 Pulled By: cortinico fbshipit-source-id: 99f54c5b9ec4113205642076c010b748ab6229f6
…41720) Summary: Pull Request resolved: facebook#41720 We currently go via the UI thread, so we can use AsyncTask to schedule the final bit of async ReactContext destruction. This is a requirement for the AsyncTask API, which is also deprecated. We should figure out a better way to schedule and re-use threads across React Native Android, but until then, we can just create a new Thread here, which is also what we do for instance creation. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D51706689 fbshipit-source-id: cf17e20e91b195b956b1701e6d91d563fdba4d15
…#41715) Summary: Currently React Native defines `NDEBUG` flag for all pods in Fabric only. This is useful for other libraries, like Reanimated, because they have no easy way of defining their compilation flags (at least none that I know of). Therefore defining `NDEBUG` for both architectures would be beneficial. ## Changelog: Pick one each for the category and type tags: [IOS] [CHANGED] - Add `NDEBUG` flag for Release builds for both architectures Pull Request resolved: facebook#41715 Test Plan: Run ruby test suite. ## Notes For the time being I just copied `prepare_pod_target_installation_results_mock` and `def prepare_installer_for_cpp_flags` to `utils-test.rb` since I wasn't sure how to handle the installer mock. Reviewed By: cortinico Differential Revision: D51708382 Pulled By: cipolleschi fbshipit-source-id: ff206f8fc151934dbae89aacd1bc69c57b4f28ee
Summary: Pull Request resolved: facebook#41748 Bumping to the latest stable of Gradle Changelog: [Internal] [Changed] - Gradle to 8.5 Reviewed By: cipolleschi Differential Revision: D51749139 fbshipit-source-id: 2ee0f9a6c910dd5221f7f63c0c599d4ab181e10a
Summary: Pull Request resolved: facebook#41747 Let's update AGP to the latest minor Changelog: [Internal] [Changed] - AGP to 8.2.0 Reviewed By: cipolleschi Differential Revision: D51749138 fbshipit-source-id: fe473b6d1613b73e60e65848c20098e3f77d3a61
…book#41738) Summary: Pull Request resolved: facebook#41738 Replacing the callsite to `context.getFabricUIManager()` in UIManagerHelper instead of `getJSIModule()` Fixing the crash by directly making `getFabricUIManager()` of `ReactContext` independent of the assertion. Reviewed By: philIip Differential Revision: D51719040 fbshipit-source-id: f9118b16614724a1d6dabe59d5c4d25dd4bdbc73
Summary: iOS?ios?android?Android? Always making typos when using the local testing script with the platform argument... No more! ## Changelog: [INTERNAL][ADDED] - Improved E2E local testing script to be more flexible Pull Request resolved: facebook#41751 Reviewed By: cortinico Differential Revision: D51758529 Pulled By: huntie fbshipit-source-id: d9e633567a59fcfac1057cf1f21714ccef27ebb2
Summary: Pull Request resolved: facebook#41600 Changelog: [Internal] Reviewed By: cipolleschi Differential Revision: D51515743 fbshipit-source-id: 156d9119d84d82f31b62b89b1916365547ce0afe
…zation (facebook#41739) Summary: Pull Request resolved: facebook#41739 Refactoring `DefaultReactNativeHost` to use the new way of Fabric initialization through `FabricUIManagerProviderImpl` Reviewed By: philIip Differential Revision: D51719555 fbshipit-source-id: bad471a8a273accecb0641ccaa77223534cd45fd
Summary: Changelog: [Internal] Reviewed By: pieterv Differential Revision: D51768863 fbshipit-source-id: 716add02d82cdfa56f2d3fc4d4560fcb26e8f426
…acebook#41733) Summary: Pull Request resolved: facebook#41733 I am currently implementing position: static in Yoga. I have a huge stack of changes that is ready to ship but we are waiting on the default position type to be relative before shipping. The reason being, my changes will affect a whole ton of styles where there is no position set so if we can make static no longer the default we can safely ship this new code. However, this will take a while and keeping up with this stack of diffs though merge conflicts, flakey tests, and general slowness for my IDE is getting annoying. So a solution here is to ship that stack and make it so that no one gets this functionality by changing the strict layout conformance to include the errata that is gating my changes. The end result being that the code can be shipped but will have no affect at the time being. Right now, because that code is in a different branch and not on prod, this change will do nothing. Changelog: [Internal] Reviewed By: javache Differential Revision: D51731778 fbshipit-source-id: f0b7fd8559adb19e1658b3ac64fcfc4c5f8ecdf7
Summary: Forward-fixing the crash in Bridgeless mode caused by UIManagerProvider changes to replace JSI Module bypass-github-export-checks Reviewed By: philIip Differential Revision: D51778494 fbshipit-source-id: f1837d7b164051e326e1227c432e493bad16cd51
Summary: Was stepping with debugger through the code & noticed few typos. ## 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] [FIXED] - Typos in `ReactCompoundViewGroup` comments Pull Request resolved: facebook#41729 Test Plan: Typos in docs Reviewed By: cortinico Differential Revision: D51753447 Pulled By: arushikesarwani94 fbshipit-source-id: b373d67ca8b6c9f22d80ea1ccee98ecc5151b325
Summary: Pull Request resolved: facebook#41766 Changelog: [Internal] Adds a simple example showing a recursive node, stored inside a collection in a Cxx TM. Currently we can't auto-generate [the necessary C++ Types](https://reactnative.dev/docs/next/the-new-architecture/cxx-custom-types#struct-generator) - but we can add it later if this scenarios becomes really common. Reviewed By: rshest Differential Revision: D51783974 fbshipit-source-id: 7352db1a354cd7da32febc650f7cc5e10dd16d2d
Summary: Pull Request resolved: facebook#42062 We have had relative as the default for a few big frameworks/apps right now (Fabric FB iOS, Fabric FB Android, OSS, Litho, CK) and have not run into issues. Seems it is safe to pull the trigger here and put everything on relative 🎉 This also fixes a test that relied on this default, changes the layout metrics default, and removes the gating plumbing that was in place earlier. Lastly, a few animation tests start failing after this change. Seems that there is an animation bug with relative trees that would have existed already, so this is merely discovering that that bug exists, not causing any extra issues. Since that test is a set of random trees with random props it is very hard to debug and I am just adding skips to the failing ones. Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52137858 fbshipit-source-id: 6856bc608b8211c868c9ee81fc92e005ec3d2faa
Summary: Pull Request resolved: facebook#42063 Static is no longer the default with the previous diff. We can undo this change. See D51731778 (facebook#41733) for context Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52381124 fbshipit-source-id: 1e4336be6db5aa5d8786fb4f0a211558a7f66365
…n React 18 (facebook#42133) Summary: Pull Request resolved: facebook#42133 ## Changelog: [General][Fixed] - TouchableBounce, TouchableHighlight and TouchableNativeFeedback dropping touches with React 18. TouchableBounce, TouchableHighlight and TouchableNativeFeedback do not trigger onPress when used with React 18. This is because it resets its pressability configuration in `componentWillUnmount`. This is fine, we want to stop deliver events and restart all timers when component is unmounted. ``` componentWillUnmount(): void { this.state.pressability.reset(); } ``` But TouchableBounce, TouchableHighlight and TouchableNativeFeedback were not restarting the pressability configuration when component was mounted again. It was restarting the configuration in `componentDidUpdate`, which is not called when component is unmounted and mounted again. Reviewed By: fkgozali Differential Revision: D52514643 fbshipit-source-id: 0d6ae4bb7c2a797cc443181459c5614da0ecfc7a
Differential Revision: D52529945 fbshipit-source-id: b43fc3e4cf207a63232b41b3e7b790ab9ef66880
…42136) Summary: Pull Request resolved: facebook#42136 `Wpedantic` flags usage of variadic macros with zero arguments. This is widely supported by different compilers (including MSVC), but was previously forbidden by the standard. C++ 20 explicitly allows them, so, theoretically Clang should know not to warn about these now. Let's try that. Changelog: [Internal] Reviewed By: cortinico Differential Revision: D52534129 fbshipit-source-id: e27a75081fac6b4196c6dbb5242812877b0bd679
Summary: Pull Request resolved: facebook#42125 This diff changes how numeric types are generated for Objective-C native modules. Before this diff: |Codegen Type|Objective-C Type| | -- | -- | |number|double| |Float|double| |Double|double| |Int32|double| After this diff: |Codegen Type|Objective-C Type| | -- | -- | |number|double| |Float|**float**| |Double|double| |Int32|**NSInteger**| Changelog: [iOS][Breaking] - Codegen: mapping for numeric types is changed for Objective-C native modules. `Float` -> `float`; `Int32` -> `NSInteger`. Reviewed By: cipolleschi Differential Revision: D52479442 fbshipit-source-id: 1b2e101a9593a75c7c19b0da3a01a0e592a35ba5
…ook#42144) Summary: Pull Request resolved: facebook#42144 D51895785 changed several CMake libraries from shared to static, including `jsinspector`. This happens to be semantically incorrect in the case of `jsinspector`, as the library contains singletons which can be inadvertently duplicated due to static linking. As a result, different parts of the code can end up accessing different instances of a supposed singleton, leading to bugs. Here we revert the change to `jsinspector` (only) and add an explanatory comment to signpost this for future readers. ## More context & general principle While nothing is broken today, allowing static libraries to contain global state is brittle and breaks in surprising ways: * The upcoming diff D52231237 introduces a new dependency on `jsinspector` which builds cleanly, but causes debugging to stop working because of the duplicated singleton. * The only reason debugging currently works in the CMake build of Bridgeless is by a happy accident: the shared library `hermesinstancejni` depends on `reactnativejni` through a chain of three other libraries unrelated to debugging, and as a result, can access `reactnativejni`'s copy of `jsinspector` (see graph). {F1237835169} It seems that the safest rule of thumb, given the way React Native is currently structured, is that **singletons should live in their own shared libraries** so no call site can cause them to be duplicated through static linking. (It's reasonable to revisit this guidance if we manage to consolidate React Native into one monolithic shared library, eliminating the footgun at the source.) Changelog: [Internal] [Changed] - Change jsinspector back to a shared library in the CMake build. Reviewed By: cortinico, NickGerleman Differential Revision: D52541488 fbshipit-source-id: 502210add0b734a9bbc470bdf38fb70a41e149a9
Summary: Pull Request resolved: facebook#42126 This diff changes how numeric types are generated for Android native modules. Before this diff: |Codegen Type|Java Type| | -- | -- | |number|double| |Float|double| |Double|double| |Int32|double| After this diff: |Codegen Type|Java Type| | -- | -- | |number|double| |Float|**float**| |Double|double| |Int32|**int**| Changelog: [Android][Breaking] - Codegen: mapping for numeric types is changed for Android native modules. `Float` -> `float`; `Int32` -> `int`. Reviewed By: cipolleschi Differential Revision: D52420921 fbshipit-source-id: 32b3bbdf5fd24db8d7ac12c262bab5fde4e1f2bc
Summary: Pull Request resolved: facebook#42139 Bump to the latest Metro release. Metro release notes: https://github.com/facebook/metro/releases/tag/v0.80.3 Changelog: [General][Changed] - Bump Metro to ^v0.80.3 Reviewed By: GijsWeterings Differential Revision: D52520542 fbshipit-source-id: 3c089e3f033a4d0597e9adb50d3377f1ad822743
Summary: Bump folly version to 2024.01.01.00. Actually we need a version newer than v2023.08.14.00 with the facebook/folly@c52d449 fix. That will fix build error on Android: ``` In file included from /Users/kudo/expo/expo/node_modules/react-native-reanimated/android/src/main/cpp/NativeProxy.cpp:3: In file included from /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/jsi/include/jsi/JSIDynamic.h:10: In file included from /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/dynamic.h:1310: In file included from /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/dynamic-inl.h:22: In file included from /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/Conv.h:124: In file included from /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/Demangle.h:19: /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/FBString.h:1721:19: error: no member named 'strong_ordering' in namespace 'std' return std::strong_ordering::equal; ~~~~~^ /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/FBString.h:1723:19: error: no member named 'strong_ordering' in namespace 'std' return std::strong_ordering::less; ~~~~~^ /Users/kudo/.gradle/caches/transforms-3/dd158a7d05d059a173ae31ca6d78ac49/transformed/jetified-react-android-0.74.0-nightly-20240103-0e533f308-SNAPSHOT-debug/prefab/modules/folly_runtime/include/folly/FBString.h:1725:19: error: no member named 'strong_ordering' in namespace 'std' return std::strong_ordering::greater; ~~~~~^ 3 errors generated. ``` ## Changelog: [GENERAL] [CHANGED] - Bump folly version to 2024.01.01.00 Pull Request resolved: facebook#42145 Test Plan: ci passed Reviewed By: cortinico, cipolleschi Differential Revision: D52546945 Pulled By: NickGerleman fbshipit-source-id: 64aacb1d310062dddf987c7b95f10a477e293693
Summary:
This PR resolves the potential problem of misconfiguration of components after being recycled. Some of them have custom, sometimes native (e.g. connected to VCs) logic that messes up with the concept of recycling.
bypass-github-export-checks
## Changelog
Added `shouldBeRecycled` field checking to `RCTComponentViewClassDescriptor `, a check for it in `_enqueueComponentViewWithComponentHandle:(ComponentHandle)componentHandle
componentViewDescriptor:(RCTComponentViewDescriptor)componentViewDescriptor` method, and a default implementation in `RCTComponentViewDescriptor` returning `YES` in order not to change the default behavior.
[iOS] [Added] - Add `shouldBeRecycled` method on `iOS`.
Pull Request resolved: facebook#35378
Test Plan: Override this method in your custom `componentView` and see that the component is not recycled.
Reviewed By: javache
Differential Revision: D41381683
Pulled By: cipolleschi
fbshipit-source-id: 10fd1e88f99b3608767c0b57fad462837924f02a
…window apps (facebook#42036) Summary: This PRs refactors `RCTKeyWindow()` to be more resilient and work in multi-window apps. After my recent PR got merged (facebook#41935) it significantly reduced the number of calls to `RCTKeyWindow()` and now it's called only when necessary. So this PR makes this function a bit more resource intensive but it guarantees that we will find current scene's key window. This would also fix some brownfield scenarios where React Native is working in multi-window mode and in the future allow us to more easily adopt `UIWindowSceneDelegate` bypass-github-export-checks ## Changelog: [IOS] [CHANGED] - refactor `RCTKeyWindow` to be more resilient and work in multi-window apps Pull Request resolved: facebook#42036 Test Plan: Checkout RNTester example for Alerts and LoadingView. https://github.com/facebook/react-native/assets/52801365/8cf4d698-db6d-4a12-8d8d-7a5acf34858b Reviewed By: huntie Differential Revision: D52431720 Pulled By: cipolleschi fbshipit-source-id: 0d6ef1d46b2428c30c9f64dae66b95dbc69f0a3b
…#42148) Summary: Pull Request resolved: facebook#42148 facebook#41625 cleaned up some usages, but there are some remaining ones. Changelog: [iOS][Fixed] Further cleaned up RCT_USE_HERMES Reviewed By: cipolleschi Differential Revision: D52555309 fbshipit-source-id: e19d10de339636eca2f4762a78cebb282d0427d9
Summary: By default SectionList overrides `stickyHeaderIndices` with generated array based on `sections` provided via props. With this changes list header and footer keeps mounted when sections prop is changing from empty list to filled list and vice versa. ## Changelog: [General] [Fixed] - Fix remount of header and footer in `SectionList` while transiting between empty and filled state <!-- 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#42080 Test Plan: **Before:** https://github.com/facebook/react-native/assets/16048381/18d31bc2-817e-4a8d-88a8-0ad19fc71816 **After:** https://github.com/facebook/react-native/assets/16048381/e205faad-7d55-4f96-a866-56e5eca976b6 **Playground:** https://snack.expo.dev/Ypb-SSHVz?platform=android ## Knowledge base https://www.smashingmagazine.com/2021/08/react-children-iteration-methods/ Reviewed By: NickGerleman Differential Revision: D52508916 Pulled By: cipolleschi fbshipit-source-id: 430463261887e9551f10c5c2dae352e0060ad6c4
…42064) Summary: Pull Request resolved: facebook#42064 # Context Dealing with how we should stack Views in a world with static is actually a bit complicated, despite the fact that static cannot get zIndex. The outline of everything can be found in this spec: https://www.w3.org/TR/CSS21/zindex.html. That is a bit dense, but mainly because there is much more functionality to consider in the browser than we need to in RN (like block layout, inline, floats, etc). Basically it comes down to this order, with larger numbers stacked on top of smaller ones: 1) Root 2) Negative zIndex positioned nodes (ties in document order) 3) Non positioned nodes in document order 4) Positioned nodes with no zIndex (or 0) in document order* 5) Positioned nodes with positive zIndex (ties in document order) Document order is a pre-order traversal of the tree. The asterisk on 4 is where it gets a bit complicated. Even though there is no zIndex set, you should still treat these nodes as if they form a stacking context - with their descendants stacked relative to the parent stacking context like normal if they have zIndex set. Essentially what this means in our world is that a static child will not come before (under) a positioned parent if they share the same stacking context. Without this, you would need to form a stacking context on all positioned nodes with static children if you wanted them to appear at all since static comes before (under) positioned nodes. # Implementation Implementing this was a bit tricky. We had to go in pre-order traversal of the tree, but if we see a relative node it needs to form a "pseudo-stacking-context" to allow static to show over it, but if any of those descendants have a zIndex set, that should be relative to the parent stacking context like normal, not this "pseudo-stacking-context". Without static we take care of this just fine because it just ends up being a pre-order traversal of the tree, then sort by zIndex. My approach was to ignore zIndex's at first and gather all nodes that share the same stacking context in an array as if none of them had any zIndex set. After that was gathered, then sort by zIndex to get the final order for said stacking context. The second part was already implemented. For the first part I just did a pre-order traversal of the tree (stopping at nodes that form stacking contexts) and: * If a node was non static, add to the end of the list * If a node was static, insert into the list right after the previously inserted static node that shares the same "psuedo-stacking-context", or the parent if there were none. * Since inserting into arrays is slow, I opted to use `std::list` In effect this was a pre-order traversal where static nodes are "sorted" to come first in the list of children, which is what we want since these nodes should come under non-positioned nodes that have the same "pseudo-stacking-context". My implementation is a bit slower. The previous one just visits all nodes once with a pre-order traversal. This will also do that and thanks to `std::list`, we have no non-constant time operations while coming in that order. After the fact, however, we need to convert back to `std::vector` which is the type used across the mounting layer, so we have to iterate over this list again. I think this is the fastest we can do this and it is optimized for a world where most nodes are relative (since it will be the new default). Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52103057 fbshipit-source-id: 0b3fb28530cfc8ef248dbdc564b5c4b4a82045a4
Summary: Pull Request resolved: facebook#42021 # Context A while ago D48905010 was committed that set the CALayer's zPosition equal to the zIndex passed in via props. This was to avoid an issue like: https://pxl.cl/40F72 where the rotating view would clip into the background. There are a few issues here. * The current zIndex code is designed to be cross platform on the C++ layer and not the native layer. This diverges from the Android behavior and adds a special case for this platform when we have the ability to share all of this logic * Static nodes will apply a zIndex when they shouldn't. This code pre-empts the static check [here](https://www.internalfb.com/code/fbsource/[cf8ad268b4cf]/xplat/js/react-native-github/packages/react-native/ReactCommon/react/renderer/components/view/ConcreteViewShadowNode.h?lines=98) that zeroes out the "order index" for static nodes * As a result of the above, static nodes can eclipse their children which should never happen because static nodes ignore zIndex values # Reason for the clipping The reason this clipping is happening is because the red/blue views share the same stacking context as the white background (as indicated by the vertical black line). {F1175070418} This in combination with the fact that our zIndex implementation will NOT set iOS's zPosition means that these three views (red view, blue view, white background view) all have the same zPosition (0) and will be laid out in the order described by the orderIndex mentioned earlier. This index essentially just changes the document order that is used for tiebreakers when zPosition is tied. So, all the views are on the same stacking context and they all have no zPosition set. Add the rotation that the colored views are doing and you get this clipping. Apple will change the "zPosition" in a sense for the parts of the view that should be perceived as "further away" due to the rotation. So, we clip into our background which has a lower order index but the same zPosition. # This change The fix here just makes it so that the rotating views are not on the same stacking context as the background so the changing zPosition from the rotation does not matter. This can be achieved by setting the zIndex of the container to any number (among other things). Note that this is only the case because the default position type is relative in this stack. Otherwise you would also need to set the position type as well. Now the stacking context looks like: {F1175083828} and the problem is solved! Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52181701 fbshipit-source-id: 580f860273b9c8470181d92d7ad542546664ed77
Summary: Pull Request resolved: facebook#42022 Some tests to ensure that the order of nodes is correct as defiend by order index that is derived from zIndex + positioning. These tests do not ensure that the native platform then goes and lays them out correctly. That will be done later with e2e tests on catalyst Changelog: [Internal] Reviewed By: NickGerleman Differential Revision: D52295063 fbshipit-source-id: 8ae29fc50ad65db8e5c0ab28a132546d8489dffe
Summary: facebook@f7219ec deprecated some of the methods in `RCTBundleURLProvider, and is part of React Native version 0.73. Let's remove the deprecated options for React Native 0.74+ bypass-github-export-checks ## Changelog: [IOS] [REMOVED] - Remove deprecated RCTBundleURLProvider options Pull Request resolved: facebook#42114 Test Plan: CI should pass. Reviewed By: huntie Differential Revision: D52537732 Pulled By: cipolleschi fbshipit-source-id: ea5d17c7c66a60bceb2a12f2e17e39be4c56d422
…ook#42156) Summary: Pull Request resolved: facebook#42156 changelog: [internal] Add a missing nullptr check to prevent crash if called after component was reused Reviewed By: fkgozali Differential Revision: D52572807 fbshipit-source-id: 1b5b26996e562abbcb986865299e02df20b58043
Summary: Pull Request resolved: facebook#42118 JFrog CDN periodically gives us headaches when downloading boost. this change moves from JFrog to the official CDN by boost. ## Changelog [Internal] - Download boost directly from boost archives Reviewed By: cortinico Differential Revision: D52479171 fbshipit-source-id: a53a9cb2ea6dfdf2b82b3c8e69c697b24cc40cf2
facebook-github-bot
added
the
CLA Signed
Base commit: 8c14997 |
…ok#42158) Summary: Pull Request resolved: facebook#42158 Changelog: [Internal] * Ports an existing Java/ObjC InspectorPackagerConnection behaviour to the C++ implementation: socket errors should trigger a reconnection. This was a simple omission in D52134592. * Clarifies the relationship between the `didFailWithError` and `didClose` methods on `IWebSocketDelegate`: calling either one will terminate the connection (and trigger a reconnection), and it's legal to call `didClose` after `didFailWithError`. * I'm also adding logic to ensure we don't double-schedule reconnections if both methods are called. * Cleans up the scaffolding comments from D52134592 Reviewed By: huntie Differential Revision: D52576727 fbshipit-source-id: 07e5a5c36222dc7bede8bcb17a1f3ced2788736b
|
birdofpreyru
force-pushed
the
issue-42164
branch
from
04e6da3 to
2e99f4c
Compare
|
Closed in favor of PR #42184 into main branch. |
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:
Closes #42164
Changelog:
[IOS] [FIXED] - Fixes
with-environment.shscript for the case when Node can't be found prior to loading.xcode.envTest Plan:
This is a trivial update.