Summary:
See commentary at top of stack.
Changelog: [Added][Fabric] New API for efficient props construction
Reviewed By: javache
Differential Revision: D37051020
fbshipit-source-id: 643e433c0d0590cfcd17bc7a43d105bed6ff12ef
Summary:
See commentary at top of stack.
Changelog: [Added][Fabric] New API for efficient props construction
Reviewed By: javache
Differential Revision: D37050961
fbshipit-source-id: 170a09c08d7406b6aac51d7e78cf295a72fdcf91
Summary:
See commentary at top of stack.
Changelog: [Added][Fabric] New API for efficient props construction
Reviewed By: javache
Differential Revision: D37050376
fbshipit-source-id: 2bea35a6d604704cf430bd3b2914988227d1abf8
Summary:
Perf numbers for this stack are given in terms of before-stack and after-stack, but the changes are split up for ease of review, and also to show that this migration CAN happen per-component and is 100% opt-in. Most existing C++ components do not /need/ to change at all.
# Problem Statement
During certain renders (select critical scenarios in specific products), UIManagerBinding::createNode time takes over 50% of JS thread CPU time. This could be higher or lower depending on the specific product and interaction, but overall createNode takes a lot of CPU time. The question is: can we improve this? What is the minimal overhead needed?
The vast, vast majority of time is taken up by prop parsing (specifically, converting JS values across the JSI into concrete values on the C++ props structs). Other methods like appendChild, etc, do not take up a significant amount of time; so we conclude that createNode is special, and the JSI itself, or calling into C++, is not the problem. Props parsing is the perf problem.
Can we improve it? (Spoiler: yes)
# How does props parsing work today?
Today, props parsing works as follows:
1. The ConcreteComponentDescriptor will construct a RawPropsParser (one per component /type/, per application: so one for View, one for Image, one for Text... etc)
2. Once per component type per application, ConcreteComponentDescriptor will call "prepare" on the RawPropsParser with an empty, default-constructed ConcreteProps struct. This ConcreteProps struct will cause RawProps.at(field) for every single field.
3. Based on the RawProps::at calls in part 2, RawPropsParser constructs a Map from props string names (width, height, position, etc) to a position within a "value index" array.
4. The above is what happens before any actual props are parsed; and the RawPropsParser is now ready to parse actual Props.
5. When props are actually being parsed from a JSI dictionary, we now have two phases:
1. The RawPropsParser `preparse`s the RawProps, by iterating over the JSI map and filling in two additional data structures: a linear list of RawValues, and a mapping from the ValueIndex array (`keyIndexToValueIndex_`; see step 3) to a value's position in the values list (`value_` in RawPropsParser/RawProps);
2. The ConcretePropT constructor is called, which is the same as in step 2/3, which calls `fieldValue = rawProps.at("fieldName")` repeatedly.
3. For each `at` call, the RawProps will look up a prop name in the Map constructed in step 3, and either return an empty value, or map the key name to the `keyIndexToValueIndex_` array, which maps to a value in `values_`, which is then returned and further parsed.
So, a few things that become clear with the current architecture:
1. Complexity is a property of the number of /possible/ props that /can/ be parsed, not what is actually used in product code. This violates the "only pay for what you use" principal. If you have `<View opacity={0.5} />`, the ViewProps constructor will request ~170 properties, not 1!
2. There's a lot of pre-parsing which isn't free
3. The levels of indirection aren't free, and make cache misses more likely and pipelining is more challenging
4. The levels of indirection also require more memory - minor, but not free
# How can we improve it?
The goal is to improve props parsing with minimal or zero impact on backwards-compability. We should be able to migrate over components when it's clear there's a performance issue, without requiring everything gets migrated over at once. This both (1) helps us prove out the new architecture, (2) derisks the project, (3) gives us time, internally and externally, to perfect the APIs and gradually migrate everything over before deleting the old infrastructure code entirely.
Thus, the goal is to do something that introduces a zero-cost abstraction. This isn't entirely possible in practice, and in fact this method slightly regresses components that do not use the new architecture /at all/, while dramatically improving migrated components and causing the impact of the /old/ architecture to be minimal.
# Solution
1. We still keep the existing system in place entirely.
2. After Props are constructed (see ConcreteComponentDescriptor changes) we iterate over all the /values/ set from JS, and call PropsT::setProp. Incidentally, this allows us to easily reuse the same prop for multiple values for "free", which was expensive in the old system.
3. It's worth noting that this makes a Props struct "less immutable" than it was before, and essentially now we have a "builder pattern" for Props. (If we really wanted to, we could still require a single constructor for Props, and then actually use an intermediate PropsBuilder to accumulate values - but I don't think this overhead would be worth for the conceptual "immutability" win, and instead a "Construct/Set/Seal" model works fine, and we still have all the same guarantees of immutability after the parsing phase)
# Implementation Details
# How to properly construct a single Prop value
Minor detail: parsing a single prop is a 3-step process. We imagine two scenarios: (1) Creating a new ShadowNode/Props A from nothing/void, so the previous Props value is just the default constructor. (2) Cloning a ShadowNode A->B and therefore Props A must be copied to Props B before parsing.
We will denote this as a clone from A->B, where A may or may not be a previous node or a default-constructed Props node; and imagine in particular that we're setting the "opacity" value for PropsB.
We must first (1) copy a value over from the previous version of the Props struct, so B.opacity = A.opacity; (2) Determine if opacity has been set from JS. If so, and there is a value, B.opacity = parse(JSValue). (3) If JS has passed in a value for the prop, BUT the value is `null`, it means that JS is resetting or deleting the prop, so we must set it BACK to the default. In this case we set PropsB.opacity = DefaultConstructedProps.opacity.
We must take care in general to ensure that the correct behavior is achieved here, which should help to explain some of the code below.
## String comparisons vs hash comparisons
In the previous system, a RawPropsKey is three `const char*` strings, concatenated together repeatedly /at runtime/. In practice, the ONLY reason we have the prefix, name, suffix Key structure is for the templated prop parsing in ViewProps and YogaStyableProps - that's it. It's not used anywhere else. Further, the key {"margin", "Left", "Width"} is identical to the key {"marginLeftWidth", null, null} and we don't do anything fancy with matching prefixes before comparing the whole string, or similar. Before comparison, keys are concatenated into a single string and then we use `strcmp`. The performance of this isn't terrible, but it's nonzero overhead.
I think we can do better and it's sufficient to compare hashed string values; even better, we can construct most of these /at compile time/ using constexpr, and using `switch` statements guarantee no hash collisions within a single Props struct (it's possible there's a collision between Props.cpp and ViewProps.cpp, for example, since they're different switch statements). We may eventually want to be more robust against has collisions; I personally don't find the risk to be too great, hash collisions with these keys are exceedingly unlikely (or maybe I just like to live dangerously). Thus, at runtime, each setProp requires computing a single hash for the value coming from JS, and then int comparisons with a bunch of pre-compiled values.
If we want to be really paranoid, we could be robust to hash collisions by doing `switch COMPILED_HASH("opacity"): if (strcmp(strFromJs, "opacity") == 0)`. I'm happy to do this if there's enough concern.
## Macros
Yuck! I'm using lots of C preprocessor macros. In general I found this way, way easier in reducing code and (essentially) doing codegen for me vs templated code for the switch cases and hashing prop names at compile-time. Maybe there's a better way.
Changelog: [Added][Fabric] New API for efficient props construction
Reviewed By: javache
Differential Revision: D37050215
fbshipit-source-id: d2dcd351a93b9715cfeb5197eb0d6f9194ec6eb9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/34015
I noticed this was a very slow part of my build because each instance of the binary would run its own yarn install. Instead use the yarn_workspace approach to share a single yarn setup.
Changelog: [Internal]
Reviewed By: IanChilds
Differential Revision: D37178005
fbshipit-source-id: cba51e168c5a2f2ee6468acb8c144db4ad352d95
Summary:
A huge set of props use YGValue directly, say something really basic like `margin`/`position`/`padding`/`border`.
All of these according to CSS spec actually support `number | "em" | "px" | %` units, but we are going to throw and hard crash on `em` and `px`, which are unsupported in React Native.
Using `tryTo` instead of `to` (noexcept vs throwing method) for conversion, and treating things like `margin: 50px` same way as we would treat `margin: false` which is not really supported.
Changelog:
[General][Fixed] - Fixed a crash on deserialization of props when using 'px'/'em' units.
Reviewed By: bvanderhoof
Differential Revision: D37163250
fbshipit-source-id: 59cbe65a821052f6c7e9588b6d4a0ac14e344684
Summary: Some files relying on -include_pch and therefore they miss Foundation.h and UIKit.h includes. This diff is fixing missing imports
Reviewed By: rmaz
Differential Revision: D37140239
fbshipit-source-id: bc57921e0c8365e0e9a5a571d607ba40ff1b31f3
Summary:
Changelog: [Internal][Changed]
The diff I'm backing out accidentally made Hover events in Felios apps that use RN under the hood stopped working in headsets (Oculus, Arcata). So we can't test our apps properly without these events.
We, with the diff author Luna tried to fix that but it turned out to be not easy so we decided to revert the commit in order to unblock experiences teams.
Original commit changeset: d6b5c32ae50b
Original Phabricator Diff: D36601638 (https://github.com/facebook/react-native/commit/40769f2212dcd85cba2e54973179c7de445499a4)
(Note: this ignores all push blocking failures!)
Reviewed By: arhelmus
Differential Revision: D37135208
fbshipit-source-id: 4f7d5f168b795690e951ce7063ae3feec3338772
Summary:
Adds support for handling animations in response to events on the platform side, without needing a JS round trip. With this TM, NativeViewEvents will no longer affected by JS thread - hover events will not be delayed when JS thread is busy. This makes a significant difference for VR panel apps - see test plan for an example for the before and after in Store.
Changelog:
[Internal] - Make NativeAnimatedNodesManager public
Reviewed By: JoshuaGross
Differential Revision: D37082069
fbshipit-source-id: 330acd78c547587de5545b61895e0d821fb99552
Summary:
We currently wrap colors in an object to make it look similar to a `PlatformColor` object, but since this is a hot codepath, let's just optimize it to a simple array of strings. The next step is to apply a layer of caching here, but this should be a simple improvement.
Changelog: [internal]
Reviewed By: JoshuaGross
Differential Revision: D31057046
fbshipit-source-id: f68e17167ddd5bba3b545d039600c7c9b40808f5
Summary:
In https://github.com/facebook/react-native/pull/32695, the `Performance.now()` implementation changed to use unix epoch timestamps instead of a monotonic clock.
This is problematic, because it means that performance measurements get skewed if the device clock changes between two measurements.
With this change, the clock is now monotonic (and the implementation stays consistent between platforms).
More details and repro steps can be found in [this issue](https://github.com/facebook/react-native/issues/33977)
Closes https://github.com/facebook/react-native/issues/33977
## Changelog
[General] [Fixed] - Use monotonic clock for performance.now()
Pull Request resolved: https://github.com/facebook/react-native/pull/33983
Test Plan:
Run on iOS and Android:
```
const now = global.performance.now()
console.log(`${Platform.OS}: ${now}`)
```
Reviewed By: JoshuaGross, cipolleschi
Differential Revision: D37066999
Pulled By: dmitryrykun
fbshipit-source-id: 298547bf39faea1b025c17ff2d2e1a03f929865b
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33982
This Diff moves another part of the utilities from the `react_native_pods` file to a specific `utils.rb` file.
It adds tests for these utils and improve our test mocks.
The goal is to simplify the `react_native_pods.rb` so it's easier to work with it.
I decided to split this diff in 2 because it was becoming quite big.
## Changelog
[iOS][Changed] - Refactoring part of the react_native_pods.rb script
Reviewed By: cortinico
Differential Revision: D37006265
fbshipit-source-id: ffaac3270cb098fa30b73c97ce7cd350dfb8d7d6
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33978
This Diff moves part of the utilities from the `react_native_pods` file to a specific `utils.rb` file.
It adds tests for these utils and improve our test mocks.
The goal is to simplify the `react_native_pods.rb` so it's easier to work with it.
I decided to split this diff in 2 because it was becoming quite big.
## Changelog
[iOS][Changed] - Refactoring part of the react_native_pods.rb script
Reviewed By: cortinico
Differential Revision: D37004347
fbshipit-source-id: a5156f7c199d082d5d895a58af80948556c51c2a
Summary:
Reverts https://github.com/facebook/react-native/pull/29012
It is not really possible to properly validate if ENTRY_FILE exists since it is resolved by metro, and the server is not always running from the app root, like in a monorepo with multiple RN apps running on the same metro server.
In my case I run metro on the repo root and entry file will be something like `apps/app/index.js`. `-f "$ENTRY_FILE"` will fail since the script is run from the project folder (`PROJECT_ROOT`, in my case the `apps/app` folder, so it tries to resolve `apps/app/apps/app/index.js`).
I don't think this check is actually useful since metro will report the error if the entry file is invalid (fixed in https://github.com/facebook/react-native/pull/30150). The error is not as user friendly, but I think it is still fine. Maybe it could be improved in metro.
## Changelog
[iOS] [Fixed] - Don't validate ENTRY_FILE in react-native-xcode.sh
Pull Request resolved: https://github.com/facebook/react-native/pull/32762
Test Plan:
Before
<img width="854" alt="image" src="https://user-images.githubusercontent.com/2677334/146100834-39310c9f-1767-496a-8698-1026726a1120.png">
After if file is actually missing
<img width="987" alt="image" src="https://user-images.githubusercontent.com/2677334/146100893-d01e2247-c787-4174-ac60-2f308c338c8f.png">
After if file exists, builds successfully.
Reviewed By: cortinico
Differential Revision: D37066073
Pulled By: cipolleschi
fbshipit-source-id: 8f6b149099a39d9179996bb965daa6cd9e06feac
Summary:
Please see this issue https://github.com/facebook/react-native/issues/33034 for details on the problem solved by this PR.
## 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
-->
[CATEGORY] [TYPE] - Message
[ios] [changed] - HTTP Response headers added to the error object passed to JS code.
Pull Request resolved: https://github.com/facebook/react-native/pull/33880
Test Plan:
Tested:
(All tests done on images in rn-tester app, which is a part of this repo)
1. onError method in case image has an HTTP Error
2. onError method in case of non http error (DNS error)
3. Successful image load
Reviewed By: cortinico
Differential Revision: D37066159
Pulled By: cipolleschi
fbshipit-source-id: 546f7678fa0321fe6c6fbef55288715cb6ddc2fd
Summary:
The idea behind this is to encapsulate as much build logic as possible inside a `.cmake` file which is contained inside React Native.
This reduces the API surface for the users, once we apply this change to the `template` project, and makes easier for us to evolve native library dependencies on Android, without having to worry about asking users to replicate those changes.
Currently the change is only on RN Tester, will replicate to the template afterwards
## Changelog
[Internal] [Changed] - Encapsulate all the CMake build logic inside a `ReactNative-application.cmake` file for RN Tester
Pull Request resolved: https://github.com/facebook/react-native/pull/33985
Test Plan: Circle CI
Reviewed By: cipolleschi
Differential Revision: D37039658
Pulled By: cortinico
fbshipit-source-id: 536593e3b7227158acba3f0fb6561efaaa9720a5
Summary:
Changelog:
[Internal][Fixed] - https://github.com/facebook/react-native/pull/33973 breaks the internal CI, as it depends on the outdated offline mirror for `Pods/Target Support Files`.
Reviewed By: cortinico
Differential Revision: D37038213
fbshipit-source-id: 1d27c9c32f2c3ddecd15a83935c520d1e1524b21
Summary:
It turned out the previous attempt to rely on the Event's UIManagerType wasn't sufficient, as not all Fabric touch event had a surfaceId set on them, e.g. Modal etc.
This brings back the UIManagerType detection based on reactTag, but do it only for non-rootView to keep handling touch via the right dispatcher for rootView as well.
Changelog: [Fixed][Android] Bring back non-rootview touch handling based on reactTag
Reviewed By: JoshuaGross, sshic
Differential Revision: D37063335
fbshipit-source-id: 76e2d7ae5f00006c5ecaf50c86920ea6e85155b7
Summary:
TurboCxxModule doesn't use the the default JSI getter that TurboModule offers, and has custom behaviour for `getConstants` which broke the I18nAssets module in the binding experiment.
Changelog: [Internal]
Reviewed By: ryancat
Differential Revision: D37030917
fbshipit-source-id: 187f159abc76f792ad2c7045dc2852d216ea978d
Summary:
[A recent fix](https://github.com/facebook/react-native/pull/33076) to Android to set focusable to true when accessible is true, and this caused several components not to work correctly.
This JS change essentially reverts the default back to Text not being focusable unless it is explicitly set. Android's "auto" behavior is better than setting `accessible=true`, and it's also the behavior React Native has had since accessibility on Android was implemented.
# Wall of Text Explanation
Explanation From Brett's comment [here](https://www.internalfb.com/diff/D35908559?dst_version_fbid=700876567897063&transaction_fbid=477905564133412)
blavalla
Generally speaking, "accessible" in react native maps to "focusable" in Android views, and the default value for "focusabe" for a TextView (and actually all views) is "auto" not "false". The difference here is that "false" is telling the system to explicitly disallow focus on this element, where as "auto" is telling the system that it's up to whatever service is trying to focus to determine if it should or not.
In the case of text, Talkback generally does default to focusing on Text when it's set to "auto", though it also does try to combine this text together with other not-explicitly focusable siblings and roll the focus up to some common ancestor element.
In the case of TetraButton here, I would expect the default behavior would be that the text is "auto" focusable, so Talkback would combine the text here with the parent <TetraPressable> (which is explicitly focusable via accessible="true").
...
[This diff](https://github.com/facebook/react-native/pull/33076) was to fix the issue with "disabled" not properly announcing on text views, which was commonly occuring due to the description-combining feature described above. Basically, when Talkback decides to combine not-explicitly-focusable elements together, it ignores properties like "disabled", "selected", etc. so when combined only the text is transferred.
The "fix" here was to make sure that if disabled was set, that an element was always explicitly focusable so that it wouldn't be eligible to be combined with others. I think that as a general concept makes sense, but the fix actually surfaced an issue that is likely a much older bug.
This line in <Text>
```
accessible={accessible !== false}
```
Is basically always setting accessible="true" unless it's explicitly set to false, and has been in there for years. It was likely added to force text to be accessible by default for iOS. But until [this diff](https://github.com/facebook/react-native/pull/33076) this line was basically a no-op for Android, since setting accessible="true" on text would do nothing at all.
[This diff](https://github.com/facebook/react-native/pull/33076) changed this so that setting accessible="true" worked how you'd expect, by making the view explicitly focusable, which was necessary for the disabled behavior to work properly. But that means that now by default all text views are explicitly focusable on both iOS and Android, and this there is likely many components that were built that don't expect this to be the case.
It doesn't seem like the right fix here is to revert this behavior to its previous state, as it wasn't working how anyone would expect it to if they looked at the code, and it seems like we were relying on some fairly undocumented behavior of Talkback to get it to work how we wanted. If we truly only wanted accessible="true" to be set on all TextViews for iOS, we should be explicit about it and do a platform check before setting that property. If we didn't want this to be iOS-specific, then everything is now actually working as originally intended.
For reference, this is the diff that introduced the default-accessible text - https://www.internalfb.com/diff/D1561326, and the description makes it clear that this was only tested on iOS, and the behavior was explicitly trying to map to iOS norms such as not allowing nested accessible elements.
Changelog:
[Android][Fixed] Make Text not focusable by default
Reviewed By: ryancat
Differential Revision: D36991394
fbshipit-source-id: c45d2ada72bb2d6ffeee6947d676a07fb8899449
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/33885
When building Hermes for React Native, point to the React Native JSI location to ensure both React Native and Hermes use the exact same version of JSI.
Changelog:
[iOS] [Changed] - When Hermes is enabled, it will use the same copy of JSI as React Native
Reviewed By: cortinico, cipolleschi
Differential Revision: D36567471
fbshipit-source-id: 97d954ef34007bd31f008fab451512194060d670
Summary:
Allow an arbitrary path to hermes-runtime-darwin-vX.Y.Z.tgz to be specified. This can be used in CI or in local e2e tests to test with Hermes enabled without having a matching GitHub release.
Usage:
```
HERMES_ENGINE_TARBALL_PATH=~/Downloads/hermes- runtime-darwin-v0.69.0.tar.gz \
USE_HERMES=1 \
pod install
```
Changelog: [Internal]
Reviewed By: cortinico
Differential Revision: D36985477
fbshipit-source-id: 853829c89e6f0ac3f63781c7f290cf3994b8e0cd
Summary:
Changelog: [iOS][Fixed] - `_scrollViewComponentView` is set to `RCTPullToRefreshViewComponentView's` superview:
```
_scrollViewComponentView = [RCTScrollViewComponentView findScrollViewComponentViewForView:self];
```
It should be safe to make it weak.
Reviewed By: javache
Differential Revision: D36998626
fbshipit-source-id: 2130b743d181e15986cb68636d60507a986968e1
Summary:
This fixes an issue in the C++ TurboModule codegen where optional return types would result in a compiler error because they were not being defaulted to `null` when converting to `jsi::Value`.
Changelog:
Internal
Reviewed By: javache
Differential Revision: D36989312
fbshipit-source-id: 525f9ce7a5638ba5a655fa69ba9647978030ab0b
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:
`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: https://github.com/facebook/react-native/pull/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:
D36784563 (https://github.com/facebook/react-native/commit/ec307e0167deca7f17640cd3c5a60f6be5f47b62) 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
Summary:
D36902220 (https://github.com/facebook/react-native/commit/a04195167bbd8f27c6141c0239a61a345cac5a88) 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:
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:
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 https://github.com/facebook/react-native/pull/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: https://github.com/facebook/yoga/pull/1150
Reviewed By: dmitryrykun
Differential Revision: D36966687
Pulled By: cortinico
fbshipit-source-id: a34a54d56df43ab4934715070bab8e790b9abd39
Summary:
https://github.com/facebook/react-native/commit/3337add547c60b84816ef5dad82f4ead2e8742ef 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: https://github.com/facebook/react-native/pull/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:
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
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
Joshua Gross
Dustin Shahidehpour
Rick Hanlon
Nikita Lutsenko
Ruslan Latypov
Artem Goncharov
Bartosz Kaszubowski
Alex Hunt
CodemodService FBSourceClangFormatLinterBot
Genki Kondo
Pieter De Baets
Pieter Vanderwerff
Xin Chen
Olivier Payen
Riccardo Cipolleschi
Janic Duplessis
Supreet Singh
Nicola Corti
Kudo Chien
Dmitry Rykun
Kevin Gozali
Vincent Riemer
Kacie Bawiec
Héctor Ramos
Scott Kyle
luoxuhai
Charles Dudley
Christian Ruink
Carmi Grushko
Zolboobayar Gantumur
