Do not virtualize items adjacent to the last focused item #2
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
NickGerleman
force-pushed
the
realize-last-focused
branch
from
a4d2c3a
to
94c7285
Compare
NickGerleman
force-pushed
the
realize-last-focused
branch
2 times, most recently
from
21a1ae7
to
3ab1de1
Compare
NickGerleman
pushed a commit
that referenced
this pull request
Dec 17, 2021
Summary: In D30104853 (facebook@e35a963), we added fix to the issue where touches on the child view that is outside of its parent view's boundary are not registered. The only exception to that is if the parent view has overflow style `overflow: hidden`. In that case, touches happen outside of the parent view should be ignored. {F686521911} That fix works, but it increases the complexity for the DFS algorithm used to find the touch target in two ways: 1. Before we only traverse views that contain the touch event point. If the touch event point is outside of the view, we won't step in and traverse that part of the tree. This is actually what caused the initial problem. The fix removed that boundary check (where `isTransformedTouchPointInView` used to do) and push it later. This increases the number of tree traversal a lot. 2. The check for `overflow: hidden` is happened after we find a potential target view, which means we've spent time to find the target view before we decide if the target view is even a candidate. This diff aims to update for the #2 item above. Since we are checking the style of the parent view, not the target view, it's not necessary to check that after we find the target view. We could check the parent view and if it has `overflow: hidden` on it, any pointer event that are not in the boundary can be ignored already. Changelog: [Internal][Android] Reviewed By: mdvacca, ShikaSD Differential Revision: D33079157 fbshipit-source-id: c79c2b38b8affb9ea0fd25b5e880b22466ab7ed9
…tarball. Summary: I'm backporting PR facebook#33945 to main as it was merged on the release branch to unblock 0.69. Those changes are necessary as Hermes was not being donwloaded at all during `pod install` on .69 and the app was failing to build with a missing `hermes/hermes.h` import. Changelog: [iOS] [Fixed] - Fix Hermes not being properly downloaded during pod install Reviewed By: hramos Differential Revision: D36810787 fbshipit-source-id: f898e61b6536dfcfc81feeff740703fbd697b000
Summary: Creates a base TurboModule for Catalyst. Reviewed By: ann-ss Differential Revision: D36707915 fbshipit-source-id: 9a246f238efdcdde45e0332f7b868478deadf50e
Differential Revision: D36707915 (facebook@528414c) Original commit changeset: 9a246f238efd Original Phabricator Diff: D36707915 (facebook@528414c) fbshipit-source-id: a581b745abd781a97d5ffde303db69ba3e23c51c
Summary: Without getting into the weeds too much, RawPropParser "requires" that props be accessed in the same order every time a Props struct is parsed in order to most optimally fetch the values in a linear-ish fashion, basically ensuring that each rawProps.at() call is an O(1) operation, and overall getting all props for a particular component is O(n) in the number of props for a given struct. If props are called out of order, this breaks and worst-case we can end up with an O(n^2) operation. Unfortunately, calling .at(x) twice with the same prop name triggers the deoptimized behavior. So as much as possible, always fetch exactly once and in the same order every time. In this case, we move initialization of two fields into the constructor body so that we can call .at() a single time instead of twice. In the debug props of ViewProps I'm also reordering the fields to fetch them in the same order the constructor fetches them in, which will make this (debug-only) method slightly faster. What's the impact of this? If you dig into the Tracery samples, the average/median RawPropsParser::at takes 1us or less. However, in /every single/ call to createNode for View components, there is at least one RawPropsParser::at call that takes 250+us. This was a huge red flag when analyzing traces, after which it was trivial (for View) to find the offending out-of-order calls. Since this is happening for every View and every type of component that derives from View, that's 1ms lost per every 4 View-type ShadowNodes created by ReactJS. After just 100 views created, that's 25ms. Etc. There are other out-of-order calls lurking in the codebase that can be addressed separately. Impact scales with the size of the screen, the number of Views they render, etc. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D36889794 fbshipit-source-id: 91e0a7ca39ed10778e60a0f0339a4b4dc8b14436
Summary: See also D36889794. This is a very similar idea, except the core problem is that BaseTextProps is accessing the same props as ViewProps; and for ParagraphProps parsing, it first defers to ViewProps' parser and then BaseTextProps. RawPropsParser is optimized to access the same props in the same order, *exactly once*, so if we access a prop out-of-order, or for a second time, that access and the next access are deoptimized. Paragraph/Text, in particular, were quite bad because we did this several times, and each out-of-order access requires scanning /all/ props. This fixes the issue, at least partially, by (1) pulling all the duplicate accesses to the beginning of BaseTextProps, and (2) accessing them all in the same order as ViewProps, relatively (some props are skipped, but that matters less). Practically what this means is that now, all of Props' accesses have a cost of O(1) for lookup, or a total of O(n) for all of them; each access is at the n+1 position in the internal RawPropsParser array, so each access is cheap. BaseTextProps' duplicate accesses, even though there are only 4 of them: (1) the first one scans the entire array until we reach the prop in question; (2) the next accesses require scans, but not whole-array scans, since they're in order. (3) The BaseTextProps accesses /after/ the duplicate accesses are all O(1). tl;dr is that before we had something like O(n*6) cost for BaseTextProps parsing and now it is O(n*2). Similar to my summary in the last diff: we may want to revisit the RawPropsParser API... but I want to tread gently there, and this gets us a large improvement without major, risky changes. Empirically, based on a couple of systraces, average time for a single UIManager::createNode called from JS thread, before this stack: 17us. After: 667ns (3% as long). On average, for every 60 createNode calls, we will save 1ms on the UI thread. The savings will be greater for certain screens that use many Views or Text nodes, and lesser for screens that use fewer of these components. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D36890072 fbshipit-source-id: 5d24b986c391d7bb158ed2f43d130a71960837d1
Summary: In the constructor we should get the default gravity params (as we did previously) and then never change these; thus, we can also make these fields final. These are used in `initView` during construction and upon recycling to reset vertical and horizontal alignment to their defaults. Changelog: [Internal] Reviewed By: genkikondo Differential Revision: D36885646 fbshipit-source-id: 2f4d0b125b8645a380a08965e08db3ba1f12cae3
Summary: D36612758 (facebook@40f4c66) attempted to remove the check that the animated node was native when fetching animated prop values for render, in order to fix an issue that arises when a node is converted to native before the initial call to render to mount the component, where the initial value is not applied. However, this causes issues for cases where we call setValue on an animated node soon after render, such as on componentDidUpdate. setValue first updates the animated node value on JS side, then calls the native API setAnimatedNodeValue. The problem is that the next render will then use these updated values in the style props (because we removed the check for native in D36612758 (facebook@40f4c66)), resulting in a diff in props. Since Animated and Fabric aren't synchronized, there's a race condition between SurfaceMountingManager.updateProps and NativeAnimatedNodesManager.runUpdates To allow proper rendering of initial values of native animated nodes, and mitigate the issue when calling setValue on an animated node soon after render, during initial render we use the initial values of both native and non-native driven nodes, and on subsequent renders we only use the initial values of non-native driven nodes. An alternative considered was to add internal state to the nodes themselves (AnimatedProps, AnimatedStyle), but keeping it in sync with the component state is not easy/clean as AnimatedProps can be recreated and reattached at any time for a mounted component. Changelog: [Internal][Fixed] - Only use value of natively driven nodes on initial render Reviewed By: JoshuaGross Differential Revision: D36902220 fbshipit-source-id: c20f711aa97d18a2ed549b5f90c6296bf19bb327
Summary: The current implementation of `throwJSError` places it in jsi.cpp, but does not actually export it. This means that when JSI is being provided by a dynamic library, `detail::throwJSError` will not be available. To fix this, move the definition of `throwJSError` into jsi-inl.h, similar to all of the other functions in the `detail` namespace. This uses a roundabout implementation of `throwJSError` in order to avoid directly using `throw`, which would fail to compile when exceptions are turned off. Changelog: [Internal] Reviewed By: jpporto Differential Revision: D36873154 fbshipit-source-id: bbea48e0d4d5fd65d67a029ba12e183128b96322
…as been deallocated Summary: Changelog: [iOS][Fixed][Internal] - Protect handlers in RCTNetworking with mutex even if RCTNetworking has been deallocated # The Logview We get this error in LogView: `Unhandled JS Exception: Error: Exception in HostFunction: mutex lock failed: Invalid argument`, which is an C++ std::error. "This typically happens when .lock() is called on a mutex that is not yet constructed, or has already been destructed." # Hypothesis of issue The LogView says the line that throws this softerror is [RCTNetworking.ios.js](https://github.com/facebook/react-native/blob/8bd3edec88148d0ab1f225d2119435681fbbba33/Libraries/Network/RCTNetworking.ios.js#L87). Inside RCTNetworking, there's only [one mutex and only one line where is is being used](https://github.com/facebook/react-native/blob/8bd3edec88148d0ab1f225d2119435681fbbba33/Libraries/Network/RCTNetworking.mm#L207-L215 ), to protect the _handlers array. My guess is that RCTNetworking was deallocated, which made `_handlersLock` nil, so it resulted in this error when we tried to lock it. # This diff * Add `std::lock_guard<std::mutex> lock(_handlersLock);` to `invalidate()` * Move `_handlersLock` logic to its own method instead of using a lambda. If `self` is being deallocated, I would guess that this method (`[self prioritizedHandlers]`) would return nil. Referencing this for correct ways to use lock_guard with mutex: https://devblogs.microsoft.com/oldnewthing/20210709-00/?p=105425 Reviewed By: fkgozali Differential Revision: D36886233 fbshipit-source-id: 60246f4d9bbc1d834497e4fb8a61d9c0e9623510
Summary: This diff fixes the rendering of transparent borders in RN Android views The fix clips the view ONLY when there is a border color that's non transparent This change unifies the rendering of transparent and semitransparent borders for Views between RN Android, iOS and Web Changelog: [Android][Fix] Fix rendering of transparent borders in RN Android Reviewed By: JoshuaGross Differential Revision: D36890856 fbshipit-source-id: 38fc2ae215f136160a73ca470e03fada8cb81ced
Summary: Pull Request resolved: facebook#33937 This moves the build of RNTester from Unix Make to CMake This will serve as a blueprint for users that are looking into using CMake end-to-end in their buildls. In order to make this possible I had to: * Add an `Android-prebuilt.cmake` file that works similar to the `Android-prebuilt.mk` for feeding prebuilt .so files to the consumer build. * Update the codegen to use `JSI_EXPORT` on several objects/classes as CMake has stricter visibility rules than Make * Update the sample native module in `nativemodule/samples/platform/android/` to use CMake instead of Make Changelog: [Internal] [Changed] - Build RN Tester with CMake Reviewed By: cipolleschi Differential Revision: D36760309 fbshipit-source-id: b99449a4b824b6c0064e833d4bcd5969b141df70
Summary: Problem - Error when trying to publish to Apple Store in debug scheme Error thread - facebook#31507 ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [General][Removed] - The diffs renames the required variable which was causing conflicts in names with Apple core SDK's Pull Request resolved: facebook#33153 Reviewed By: lunaleaps Differential Revision: D34392529 Pulled By: sshic fbshipit-source-id: 78387999f94e0db71f5d3dafff51e58d7d0c1847
Summary: When tapping on ReactRootView and having Fabric enabled, the touch logic mistakenly dispatch the event to JS via the legacy renderer. This is because the destination was computed based on reactTag (odd = legacy, even = Fabric), but root view tag happens to be always odd (always ends with 1). This change uses the right destination based on what the Event itself tells us, instead of deriving from the reactTag. Changelog: [Android][Fixed] Fix Fabric touch event dispatching for root views Reviewed By: JoshuaGross, sshic Differential Revision: D36917300 fbshipit-source-id: 838b4e135d7df07c37040bd47d71370ff10df067
Summary: This sync includes the following changes: - **[dd4950c90](facebook/react@dd4950c90 )**: [Flight] Implement useId hook ([facebook#24172](facebook/react#24172)) //<Josh Story>// - **[26a5b3c7f](facebook/react@26a5b3c7f )**: Explicitly set `highWaterMark` to 0 for `ReadableStream` ([facebook#24641](facebook/react#24641)) //<Josh Larson>// - **[aec575914](facebook/react@aec575914 )**: [Fizz] Send errors down to client ([facebook#24551](facebook/react#24551)) //<Josh Story>// - **[a2766387e](facebook/react@a2766387e )**: [Fizz] Improve text separator byte efficiency ([facebook#24630](facebook/react#24630)) //<Josh Story>// - **[f7860538a](facebook/react@f7860538a )**: Fix typo in useSyncExternalStore main entry point error ([facebook#24631](facebook/react#24631)) //<François Chalifour>// - **[1bed20731](facebook/react@1bed20731 )**: Add a module map option to the Webpack Flight Client ([facebook#24629](facebook/react#24629)) //<Sebastian Markbåge>// - **[b2763d3ea](facebook/react@b2763d3ea )**: Move hydration code out of normal Suspense path ([facebook#24532](facebook/react#24532)) //<Andrew Clark>// - **[357a61324](facebook/react@357a61324 )**: [DevTools][Transition Tracing] Added support for Suspense Boundaries ([facebook#23365](facebook/react#23365)) //<Luna Ruan>// - **[2c8a1452b](facebook/react@2c8a1452b )**: Fix ignored setState in Safari when iframe is touched ([facebook#24459](facebook/react#24459)) //<dan>// - **[62662633d](facebook/react@62662633d )**: Remove enableFlipOffscreenUnhideOrder ([facebook#24545](facebook/react#24545)) //<Ricky>// - **[34da5aa69](facebook/react@34da5aa69 )**: Only treat updates to lazy as a new mount in legacy mode ([facebook#24530](facebook/react#24530)) //<Ricky>// - **[46a6d77e3](facebook/react@46a6d77e3 )**: Unify JSResourceReference Interfaces ([facebook#24507](facebook/react#24507)) //<Timothy Yung>// - **[6cbf0f7fa](facebook/react@6cbf0f7fa )**: Fork ReactSymbols ([facebook#24484](facebook/react#24484)) //<Ricky>// - **[a10a9a6b5](facebook/react@a10a9a6b5 )**: Add test for hiding children after layout destroy ([facebook#24483](facebook/react#24483)) //<Ricky>// - **[b4eb0ad71](facebook/react@b4eb0ad71 )**: Do not replay erroring beginWork with invokeGuardedCallback when suspended or previously errored ([facebook#24480](facebook/react#24480)) //<Josh Story>// - **[99eef9e2d](facebook/react@99eef9e2d )**: Hide children of Offscreen after destroy effects ([facebook#24446](facebook/react#24446)) //<Ricky>// - **[ce1386028](facebook/react@ce1386028 )**: Remove enablePersistentOffscreenHostContainer flag ([facebook#24460](facebook/react#24460)) //<Andrew Clark>// - **[72b7462fe](facebook/react@72b7462fe )**: Bump local package.json versions for 18.1 release ([facebook#24447](facebook/react#24447)) //<Andrew Clark>// - **[22edb9f77](facebook/react@22edb9f77 )**: React `version` field should match package.json ([facebook#24445](facebook/react#24445)) //<Andrew Clark>// - **[6bf3deef5](facebook/react@6bf3deef5 )**: Upgrade react-shallow-renderer to support react 18 ([facebook#24442](facebook/react#24442)) //<Michael サイトー 中村 Bashurov>// Changelog: [General][Changed] - React Native sync for revisions bd4784c...d300ceb jest_e2e[run_all_tests] Reviewed By: cortinico, kacieb Differential Revision: D36874368 fbshipit-source-id: c0ee015f4ef2fa56e57f7a1f6bc37dd05c949877
Summary: This PR adds `READ_VOICEMAIL` and `WRITE_VOICEMAIL` permissions to the PermissionsAndroid library. Resolves facebook#33922. https://developer.android.com/reference/android/Manifest.permission#READ_VOICEMAIL https://developer.android.com/reference/android/Manifest.permission#WRITE_VOICEMAIL ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Added] - Add READ_VOICEMAIL and WRITE_VOICEMAIL permissions to PermisionsAndroid library. Pull Request resolved: facebook#33965 Test Plan: ``` PermissionsAndroid.READ_VOICEMAIL === 'com.android.voicemail.permission.READ_VOICEMAIL' PermissionsAndroid.WRITE_VOICEMAIL === 'com.android.voicemail.permission.WRITE_VOICEMAIL' ``` Reviewed By: kacieb Differential Revision: D36933524 Pulled By: cortinico fbshipit-source-id: f5283d526aeb68c2724654e22ae16c8c3f69f740
…face Summary: This diff address an edge case when the pending events are enqueued when the surface is stopped. In this case we will reset map that holds view state to null, which will cause NPE. Changelog: [Android][Fixed] - Fix edge case when we enqueue a pending event to views on stopped surface Reviewed By: javache, gorodscy Differential Revision: D36912786 fbshipit-source-id: 3ae5a4b08a0a6bf55538d69ac80a101c2c3d899a
Summary: Seems like an obvious typo! Whoops! Not causing any known issues, but... this should be fixed. Changelog: [Internal] Reviewed By: genkikondo Differential Revision: D36940476 fbshipit-source-id: d534ca3763b1f91e41c56953bf3d665e86db9e2b
Differential Revision: D36926167 fbshipit-source-id: e5c262e269b3ccb2afe04b45398ec66fc5c48df2
Summary: facebook@3337add made some changes to method signature but the template wasn't updated. This adds the missing changes. ## Changelog [Internal] [Fixed] - Pass string by ref in TurboModule template Pull Request resolved: facebook#33970 Test Plan: Didn't test the template directly, but the change is trivial. Reviewed By: cortinico Differential Revision: D36964481 Pulled By: dmitryrykun fbshipit-source-id: 561e32f218baf398b8d4d8c77381a2642e22ef42
Summary: This change is mostly needed to support the new react-native architecture with Swift. Some private yoga headers end up being included in the swift build and result in compilation failure since swift cannot compile c++ modules. See facebook#33381. The most reliable fix is to include all headers as public headers, and add `#ifdef __cplusplus` to those that include c++. This is already what we do for other headers, this applies this to all headers. Tested in the YogaKitSample, and also in a react-native app. Changelog: [iOS] [Changed] - Make all Yoga headers public and add #ifdef __cplusplus X-link: facebook/yoga#1150 Reviewed By: dmitryrykun Differential Revision: D36966687 Pulled By: cortinico fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
Summary: Changelog: [iOS][Internal] - Add dispatching of pointerover/pointerout events Reviewed By: lunaleaps Differential Revision: D36718156 fbshipit-source-id: c2fee2332ac52e617db938e321e3b38fd5c35ad3
…n Android. Summary: Changelog: [Android][Fix] - When `onEndReachedThreshold` is set to 0 `onEndReached` function on `VirtualizedList` properly fires once the user scrolls to the bottom of the list. Reviewed By: genkikondo Differential Revision: D36488189 fbshipit-source-id: b95f3291f2957273280696d8840c1e000d669380
Summary: D36902220 (facebook@a041951) changed Animated to only use value of natively driven nodes on initial render. However, there remained a case where we could end up with a race condition between Fabric prop update (via SurfaceMountingManager.updateProps) and Animated (via NativeAnimatedNodesManager.runUpdates), when an animation on a node that was created natively is triggered close to render (such as in componentDidUpdate). This happens as Animated and Fabric aren't synchronized, and at the platform level, they do not know each other's state. Say we have two items, where opacity is used to indicate whether the item is selected. On initial render, A's opacity is set to 1, and animated sets opacity to 1; B's opacity is set to 0, and animated sets opacity to 0. When B is selected (and causes A and B to rerender), A's opacity is now set to null, and animated sets opacity to 0; B's opacity is also now set to null, and animated sets opacity to 1. A's props have changed, and thus the default opacity value of 1 is applied via Fabric, but Animated also sets the opacity to 0 - either may end up being the value visible to the user due to the nondeterministic order of Fabric update props and Animated. This is what is causing T122469354. This diff addresses this edge case by using the initial prop values for native animated nodes, for subsequent renders, to ensure that values of native animated nodes do not impact rerenders. This diff also fixes a bug in OCAnimation where translateX/Y values of 0 in transition will result in render props containing translateX/Y instead of transform, resulting in potentially incorrect pressability bounds. Changelog: [Internal][Fixed] - Only use initial value of natively driven nodes on render Reviewed By: JoshuaGross, javache Differential Revision: D36958882 fbshipit-source-id: 10be2ad91b645fa4b8a4a12808e9299da33aaf7d
Summary: D36784563 (facebook@ec307e0) caused some issues with TextInputs with numeric keyboard types not respecting the secureTextEntry prop Changelog [Android] [Fixed] - Revert PR 33924 because of issues with TextInputs with numeric keyboard types not respecting the secureTextEntry prop Reviewed By: makovkastar Differential Revision: D36994688 fbshipit-source-id: 89cd556ca1cf8fd89560aeb9ead129607b4c13c6
…book#33972) Summary: `OTHER_CFLAGS` doesn't seem to work when the file is imported from Objc++. This causes flipper to not be included. ## Changelog [iOS] [Fixed] - Use `GCC_PREPROCESSOR_DEFINITIONS` to set `FB_SONARKIT_ENABLED` Pull Request resolved: facebook#33972 Test Plan: Tested the change in an app. Used a breakpoint to see that flipper code is not executed before this change, and is after. Also made sure other `GCC_PREPROCESSOR_DEFINITIONS` set by cocoapods are still properly inherited. Reviewed By: cipolleschi Differential Revision: D37001624 Pulled By: dmitryrykun fbshipit-source-id: 920f3fe368a9fbe2cde9aa1e6d5b3b883c42119d
Summary: This diff adds an assertion to make sure the pending events are enqueued only when the event emitter is null. This is to avoid unexpected workflow when we queue events but we should just dispatch them. Changelog: [Android][Internal] - Make sure we only queue events when event emitter is null Reviewed By: javache Differential Revision: D36916482 fbshipit-source-id: fff305615b302ece26bc2482c826b74de4f70266
Summary: Changelog: [internal] - Upgrade ESLint version to the latest - Upgrade ESLint plugin versions to the latest to ensure compatibility - Switch from eslint-plugin-flowtype to eslint-plugin-ft-flow - Updates lint suppressions - all `flowtype/` rules to `ft-flow/` ### `flowtype` vs `ft-flow` `eslint-plugin-flowtype` is well out of date and no-longer maintained. It's been abandoned by its owner. This means it crashes on ESLint v8. `eslint-plugin-ft-flow` is a fork of the above and is maintained by the same people that own the `flow-typed` project. Reviewed By: jacdebug Differential Revision: D37727177 fbshipit-source-id: 516be42f947fec9c886526c182a608b3311c0b50
Summary: Changelog: [iOS][Internal] - Add key modifier properties to the PointerEvent interface This diff adds implementations of the `ctrlKey`, `shiftKey`, `altKey`, and `metaKey` properties on the PointerEvent interface for iOS. Reviewed By: lunaleaps Differential Revision: D37869377 fbshipit-source-id: b187bae93fbfc97b6ca1d8d9786ad85343484b3d
Summary: Changelog: [RNTester][Internal] - Add the ability for platform tests to be marked as skipped There are some properties/tests in the web platform tests that we don't want to especially priorities (especially if they aren't especially relevant to the specification and just legacy browser compat) so this adds the ability for us to mark these tests as skipped. This is so that we can ensure that our platform tests are "complete" while decreasing the noise and distraction from failing tests that don't really apply to RN. Reviewed By: lunaleaps Differential Revision: D37889470 fbshipit-source-id: 6516ac90c242d662518a95cb9ba8ce643f6bb09c
…ndAccessibilityEvent Summary: Changelog: [Internal] Rename AccessibilityInfo.sendAccessibilityEvent_unstable to sendAccessibilityEvent In Fabric, we want people to use `AccessibilityInfo.sendAccessibilityEvent` instead of `UIManager.sendAccessibilityEvent` for Android. The API is not unstable. There is a test in [AccessibilityExample.js](https://github.com/facebook/react-native/blob/c940eb0c49518b82a3999dcac3027aa70018c763/packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js#L959) in RNTester to confirm that it works. A search for [`AccessibilityInfo.sendAccessibilityEvent_unstable` in Github](https://github.com/search?q=AccessibilityInfo.sendAccessibilityEvent_unstable&type=Code) shows that it's not being used yet, which makes sense because it's an Fabric API. Therefore it's safe to rename it. Reviewed By: sammy-SC Differential Revision: D37901006 fbshipit-source-id: 73f35b09ca8f9337f4d66a431f0a3f815da38249
Summary: Add API for setting/getting native state. When present, the internal NativeState property of an object always stores a NativeState with a pointer to a heap-allocated shared_ptr + a finalizer that simply `delete`s it. Changelog: [Internal][Added] - JSI API for setting/getting native state on a JS object Reviewed By: jpporto Differential Revision: D36499239 fbshipit-source-id: a1ff1905811db1aac99ece3f928b81d0abfb342b
Summary: Log whether bridgeless mode is enabled or not when calling into a JavaScript module method fails. We are seeing a surge of these crashes in MessageQueue.js bridgeless mode. This temporary log will help us drill down into the issue. Changelog: [Internal] Created from CodeHub with https://fburl.com/edit-in-codehub This should not mess with the sitevar overrides. These are the two diffs where we mapped the venice and bridge errors to the same mid: D36649249, D36649249. Reviewed By: fkgozali Differential Revision: D37898744 fbshipit-source-id: 0ab337c8c0d73bd70c4756a16b8ece8ba8730c47
Summary: D37801394 (facebook@51f49ca) attempted to fix an issue of TextInput values being dropped when an uncontrolled component is restyled, and a defaultValue is present. I had missed quite a bit of functionality, where TextInput may have child Text elements, which the native side flattens into a single AttributedString. `lastNativeValue` includes a lossy version of the flattened string produced from the child fragments, so sending it along with the children led to duplicating of the current input on each edit, and things blow up. With some experimentation, I found that the text-loss behavior only happens on Fabric, and is triggered by a state update rather than my original assumption of the view manager command in the `useLayoutEffect` hook. `AndroidTextInputShadowNode` will compare the current and previous flattened strings, to intentionally allow the native value to drift from the React tree if the React tree hasn't changed. This `AttributedString` comparison includes layout metrics as of D20151505 (facebook@061f54e) meaning a restyle may cause a state update, and clear the text. I do not have full understanding of the flow of state updates to layout, or the underlying issue that led to the equality check including layout information (since TextMeasurementCache seems to explicitly compare LayoutMetrics). D18894538 (facebook@254ebab) used a solution of sending a no-op state update to avoid updating text for placeholders, when the Attributed strings are equal (though as of now this code is never reached, since we return earlier on AttributedString equality). I co-opted this mechanism, to avoid sending text updates if the text content and attributes of the AttributedString has not changed, disregarding any layout information. This is how the comparison worked at the time of the diff. I also updated the fragment hashing function to include layout metrics, since it was added to be part of the equality check, and is itself hashable. Changelog: [Android][Fixed] - Fix `AttributedString` comparison logic for TextInput state updates Reviewed By: sammy-SC Differential Revision: D37902643 fbshipit-source-id: c0f8e3112feb19bd0ee62b37bdadeb237a9f725e
…book#34176) Summary: Pull Request resolved: facebook#34176 It extracts the code related to the codegen from the main `react_native_pods` script to a dedicated file, adding also tests. ## Changelog [iOS][Changed] - Move codegen in separate files Reviewed By: cortinico Differential Revision: D37755818 fbshipit-source-id: 99760d1def26ddbf065fdd234e0d183c2795513c
Summary: Pull Request resolved: facebook#34177 This Diff destructures the parameters of the use_react_native! function. It does that in a backward compatible way, so we there should be no disruptions. It also adds documentation to the various public method we want to export to our users. ## Changelog [iOS][Changed] - Destruct use_reactnative parameters and ad ddocumentation Reviewed By: cortinico Differential Revision: D37787365 fbshipit-source-id: 27f9030db2e8c6c66b9548b4c1287eb8165ae5fc
…ook#34206) Summary: I just opened another PR and noticed the changelog page in the wiki redirected to the react native documentation. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Internal] [Changed] - Update link to changelog documentation in PULL_REQUEST_TEMPLATE Pull Request resolved: facebook#34206 Reviewed By: NickGerleman Differential Revision: D37920424 Pulled By: cortinico fbshipit-source-id: f4e47172a13fe5b42c29e320d34816b490a14b6c
…correctly (facebook#34221) Summary: This fix is necessarly to ensure that when working on the codebase in the `0.XX-stable` branches (ex. when you are working on a release) the Hermes podfile is correctly set against the right commit for that branch, and not latest commit from main branch of Hermes repo. I didn't add a check to verify that the file `.hermesversion` exists because I think it's safe to assume that the file and the tag correctly exists when this step (doing a pod install on the `0.XX-stable` branch). Once this is merged, we need to cherry pick it on both the 0.69 and 0.70 branches ## Changelog [iOS] [Fixed] - Hermes pod: change logic to use the hermes tag to set the pod source correctly Pull Request resolved: facebook#34221 Test Plan: * git clone the repo * checkout 0.69-stable branch * follow https://reactnative.dev/contributing/release-testing * without this commit, when testing RNTester + iOS + Hermes the app will insta-crash on opening * with it, the app gets build successfully Reviewed By: cortinico Differential Revision: D37957660 Pulled By: cipolleschi fbshipit-source-id: 4e50099ed712b1ad8e6439822e3f530142982c1b
…facebook#34215) Summary: This resolves issues where the node_modules structure is not hoisted (like with pnpm). Since the template does not directly depend on the cli, it doesn't exist in the pnpm node_modules root. Moving it to the rn scripts makes sure that the relative require starts in the correct directory for both hoisted and pnpm structures. ## Changelog [iOS] [Fixed] - Fix cocoapods cli native_modules require for pnpm node_modules Pull Request resolved: facebook#34215 Test Plan: 1. react-native init 2. rm -rf node_modules 3. pnpm i 4. bundle install 5. bundle exec pod install --project-directory=ios This should succeed. Without the patch, it will fail with ``` [!] Invalid `Podfile` file: cannot load such file -- /.../node_modules/react-native-community/cli-platform-ios/native_modules. # from /.../ios/Podfile:2 # ------------------------------------------- # require_relative '../node_modules/react-native/scripts/react_native_pods' > require_relative '../node_modules/react-native-community/cli-platform-ios/native_modules' # # ------------------------------------------- ``` Reviewed By: cortinico Differential Revision: D37959152 Pulled By: cipolleschi fbshipit-source-id: 7fa9af4a8c153cfd38360f57eca415a8c252dbd5
Summary: soloader 0.10.3 will apparently cause crashes in instrumented tests, 0.10.4 appears to fix these crashes Related: facebook/SoLoader#94 Related: reactwg/react-native-releases#26 (comment) ## Changelog [Android] [Changed] - Bump Soloader to 0.10.4 Pull Request resolved: facebook#34213 Test Plan: This is hard to test since it's in the AAR etc., I'm hoping CI is sufficient as the previous soloader bump PR went through similarly Reviewed By: cipolleschi Differential Revision: D37960320 Pulled By: cortinico fbshipit-source-id: ce1611d7b30df737c8525a70839b5491a6585c75
Summary: There are many files across fbobjc relying on -include_pch and therefore they miss Foundation.h and UIKit.h includes. This diff was generated by a codemod and fixes these missing includes. More details on the missing imports https://fb.workplace.com/groups/929548250966094/permalink/981237982463787/ Changelog: [Internal] Reviewed By: yannickl Differential Revision: D37282740 fbshipit-source-id: 0f419025b3cf2f811e96ff464cb19e8e5a25aa09
Builds upon the `CellRenderMask` data structure added with facebook#31420, and VirtualizedList coverage added with facebook#31401. VirtualizedList currently keeps a [first, last] range as state, tracking the region of cells to render. The render functions uses this as an input, along with a few special cases to render more (sticky headers, initial render region.) This change moves to instead keep state which describes discontiguous render regions. This mask is continually updated as the viewport changes, batch renders expand the region, etc. Special cases are baked into the render mask, with a relatively simple tranformation from the mask to render function. This representation makes it much easier to support keyboarding scenarios, which require keeping distinct regions (e.g. for last focused) realized while out of viewport. MS/FB folks have a video discussion about VirtualizedList here: https://msit.microsoftstream.com/video/fe01a1ff-0400-94b1-d4f1-f1eb924b1809 facebook#31401 added quite a few snapshot tests, centering around the logic this change is touching. I manually validated RNTester FlatList examples (and their should be some upstream UI testing for them).
This change also includes the contents of facebook#32638 This change makes VirtualizedList track the last focused cell, through the capture phase of `onFocus`. It will keep the last focus cell, and its neighbors rendered. This allows for some basic keyboard interactions, like tab/up/down when on an item out of viewport. We keep the last focused rendered even if blurred for the scenario of tabbing in and and out of the VirtualizedList. Validated via UT.
NickGerleman
force-pushed
the
realize-last-focused
branch
from
1bd3342
to
8437121
Compare
NickGerleman
added a commit
that referenced
this pull request
Sep 27, 2022
Summary: VirtualizedList would more gracefully handle out of range cells than VirtualizedList_EXPERIMENTAL, which treats it as an invariant violation. D39244112 (facebook@7aa203b) attempted to fix an issue where recalculation of cells around viewport can include out of range cells, but it is still showing up later. This change adds a bounds check to the remaining branch we control, and an assertion that `computeWindowedRenderLimits` is not returing something out of range to discover if that is the cause. Reviewed By: yungsters Differential Revision: D39267445 fbshipit-source-id: 64c99da28b5b01ef61784079b586e355f73764a1
NickGerleman
pushed a commit
that referenced
this pull request
Nov 22, 2023
Summary: Pull Request resolved: facebook#41466 ## Context In open source, all apps use the same turbomodulemanager delegate (i.e: the default delegate). This diff introduces the buck infra that makes the oss default delegate work for meta apps. Concretely, we are going to make React Native use the same delegate for **all** Meta apps. Each Meta app will: 1. At build time, generate a unique TMProvider map 2. At app init time, initialize the default delegate with the TMProvider map. ## Implementation **Step #1:** At build time, generate a unique TMProvider map **Insight:** Buck genrules can accept, as input, the output of a buck query. So, here's how we get this done: 1. Buck query (i.e: input to Genrule): Given the app's deps, query all the schemas in the app. 2. Genrule: Read the schemas to generate the TMProvider map. The TMProvider map will also contain **all** the app's C++ module codegen. Concretely: 1. This diff introduces a macro: rn_codegen_appmodules(deps). 2. rn_codegen_appmodules(deps) generates appmodules.so, which contains the TMProvider map. **Step #2:** At app init time, initialize the default delegate with the TMProvider map. This is how we'll initialize the DefaultTurboModuleManagerDelegate: 1. DefaultTurboModuleManagerDelegate will load appmodules.so during init. 2. When loaded, appmodules.so will assign the code-generated TMProvider map to DefaultTurboModuleManagerDelegate. ## Impact This should allow us to: 1. Get one step closer to getting rid of the `js1 build turbomodule-manager-delegates --target <app>` script 3. Remove the TurboModuleManagerDelegate from React Native's public API. (Because we use one delegate for all React Native apps in Meta and OSS) Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D50988397 fbshipit-source-id: 0ca5dec14e2dae89ec97f5d39a182c7937c5c7bf
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.
This change also includes the contents of facebook#32638
This change makes VirtualizedList track the last focused cell, through the capture phase of
onFocus. It will keep the last focus cell, and its neighbors rendered. This allows for some basic keyboard interactions, like tab/up/down when on an item out of viewport. We keep the last focused rendered even if blurred for the scenario of tabbing in and and out of the VirtualizedList.Validated via UT.