Summary:
This is a quite fragile and important part of the render engine. We need to dirty Yoga node only in cases where a change affects layout. In the case of over-dirtying, we can kill performance. In the case of under-dirtying, we can produce an incorrect layout.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596279
fbshipit-source-id: 9f2ac67c44cb35c8ba44be1025b94b7921b74e17
Summary:
This is aligned with all simular changes that we do for `*Props` types (see D19583582 and D19390813).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596281
fbshipit-source-id: 283b89c65504e20a461b12c2d1218325c6621dd8
Summary:
Reasons:
* The name of the method now better represent what it's doing;
* It exposes information about "dirty" state of the node without opening actual `LayoutableShadowNode` protected APIs;
* It's a tiny bit faster now because it checks the flag before calling Yoga.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596282
fbshipit-source-id: 3d87d9d5ba20bb8e360683f149b5ebf90beecd65
Summary:
After D19565499, the `LongLivedObjectCollection` will be cleared on the JS thread when the jsi::Runtime is deleted. This diff makes it so that we never hold strong references to `CallbackWrapper`s in our Android TurboModules infra. Therefore, we can leverage the changes in D19565499 to ensure that our `jsi::Function`s are deleted before the `jsi::Runtime`.
## Caveat
If you delete a TurboModule by itself, it's jsi::Functions that haven't been invoked won't be released. This is also the case for iOS. I plan to fix this for both iOS and Android at a later point in time.
Changelog:
[Android][Fixed] - Refactor jsi::Function cleanup in TurboModules
Reviewed By: mdvacca
Differential Revision: D19589151
fbshipit-source-id: efa3cc6c83634014159ac7500dcf6bef9c925762
Summary:
It makes perfect sense because `Builder` builds totally new shadow trees, so those are not sealed or used by anyone yet.
Assigning it to const shared pointer will do logical sealing (and it does not requires const-cast).
Fewer const-casts in the code, fewer bugs.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596284
fbshipit-source-id: 75d1c706034958ba7e4bc80a68af75a57c46eb6f
Summary:
Before this change `Element<X>` cannot have children of `Element<Y>` which was wrong because we don't have such limitation in Fabric.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19596283
fbshipit-source-id: 9002f5dd42b3d05e7cf492499499399c97b58152
Summary:
Changelog: [Internal]
# Analysis
Measure returns following values for `frame.y` when tapping item in bottom sheet.
Fabric 412.33331298828125.
Paper 49.
In Paper, the frame.y returned is the position of tapped item in bottom sheet relative to the bottom sheet itself, which is correct.
This can happen in both BottomSheet and Modal.
# Why it happens?
In [UIManager.getRelativeLayoutMetrics](https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/fabric/uimanager/UIManager.cpp?commit=a372cf516ba1245ad9462e68376ee759c118a884&lines=172-181) if `ancestorShadowNode` is nullptr we populate `ancestorShadowNode` with `rootShadowNode` for the surface.
Problem is that BottomSheet that is presented, is not a separate surface. This means that we climb up the shadow node hierarchy all the way to root shadow node and keep adding offsets. Even though we should stop when we hit shadow node representing BottomSheet.
# How could be this fixed?
I think we should add a new shadow node trait that would mark node as root node. As we are traversing the shadow node tree upwards and adding offset of that node, once we hit a node that is "anchor", we would immediately stop the traversal.
This solution is inspired by Paper where the node representing BottomSheet has a special flag which marks it as "root" node.
accepttoship
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D19640454
fbshipit-source-id: bde623b1f41a9745a41f0aada7221bf924fad453
Summary:
## Description
You're not supposed to hold on to JSI objects (ex: `jsi::Function`) past the point where their `jsi::Runtime` is deleted. Otherwise, we get a dangling pointer crash, like this: T60262810! Historically, this cleanup problem has always been really tricky to get right. With this diff, I hope to fix that problem once and for all by deleting all `jsi::Function`s when we delete the global `__turboModuleProxy` function.
## Current Setup
- The TurboModules infra uses weak references to `CallbackWrapper`s to hold on to the `jsi::Function`s passed from JS to ObjC.
- The LongLivedObjectCollection holds on to strong references to `CallbackWrapper`s. This ensures that the `jsi::Function`s aren't deleted prematurely. This also means that we can use `LongLivedObjectCollection` to delete all `CallbackWrappers`.
- `TurboModuleBinding` is the abstraction we use to install the global `__turboModuleProxy` function. It is owned by `TurboModuleManager`, and `TurboModuleManager` uses it to clear all references to `jsi::Function`s, when we delete all NativeModules.
## Solution
1. Transfer ownership of `TurboModuleBinding` from `TurboModuleManager` to the `__turboModuleProxy` function.
2. Clear the `LongLivedObjectCollection` when `TurboModuleBinding` is deleted.
Changelog:
[iOS][Fixed] - Clear all held jsi::Functions when jsi::Runtime is deleted
Reviewed By: JoshuaGross
Differential Revision: D19565499
fbshipit-source-id: e3510ea04e72f6bda363a8fc3ee2be60303b70a6
Summary:
Changelog: [Internal]
Exposes `synchronouslyWaitForStage` to `RCTFabricSurface`.
This is a first step towards having screenshot tests rendered with Fabric.
Reviewed By: shergin
Differential Revision: D19603837
fbshipit-source-id: 26c14cf3bbd67fea96319ff08d3321557ddcdd9c
Summary:
We are deprecating SharedShadowNode in favor of ShadowNode::Shared, the previous diff already removed most of the usages of SharedShadowNode
changelog: [internal]
Reviewed By: shergin
Differential Revision: D19217718
fbshipit-source-id: 1b7cd1af0ecdc6112befdc45c1dd0fca5012ab21
Summary:
ShadowNode::Shared and SharedShadowNode represent the exact same type. Nowadays we use ShadowNode::Shared instead of SharedShadowNode.
This diff replaces usages of SharedShadowNode for ShadowNode::Shared in the fabric folder.
Changelog: [internal]
Reviewed By: shergin
Differential Revision: D19217717
fbshipit-source-id: 112331b22251aa3a3d5e183395c54f2ca0f56e47
Summary:
Here is a mutation test for the Diffing algorithm that we use to diff ShadowNode trees and flatten them. As a side-effect, it also "tests" that Concurrent Yoga does not crash and produces decent cloning requests.
The test works this way:
1. We create a random ShadowNode tree;
2. We create a View tree from that that we continue to maintain;
3. We apply random mutation on this;
4. We layout the tree;
5. We generate the mutation instruction comparing a previous tree with a new one;
6. We apply mutations on the first tree;
7. We generate a new tree from scratch;
8. We compare the new tree with the tree updated with mutations and expect equivalence;
9. Repeat a million times.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D19357714
fbshipit-source-id: 04765ede87d91180952ae650ff0d505dfac2ed8e
Summary:
In my app I have a case where I need to pass a very large string (45MB) between JS and native. This is obviously suboptimal, but… this is where I'm at.
The main bottleneck to doing this turned out to be `jsi`'s `JSStringToSTLString()`, which was extremely slow. In my case, 4.7s to execute. After this change, 204ms.
I don't really know C++, so I'm not sure this code is 100% correct and safe, and I bet it could be done even better by avoiding the extra memory allocation (would shave off another 70ms).
## Changelog
[General] [Changed] - Make JSStringToSTLString 23x faster
Pull Request resolved: https://github.com/facebook/react-native/pull/26955
Reviewed By: shergin
Differential Revision: D19578728
Pulled By: motiz88
fbshipit-source-id: 2fbce83166953ce928f0a6aa36eed710bfe05383
Summary:
Changelog: [internal]
1. Creates `ShadowNodeFamily` inside `ComponentDescriptor`.
2. As a side effect of this, we no longer need `ComponentDescriptor::createEventEmitter` so it is removed.
This is a step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates.
Reviewed By: shergin
Differential Revision: D19514906
fbshipit-source-id: 04ad3c621886be56925acd76f9b35a09d8c5e15a
Summary:
Changelog: [internal]
1. Use `ShadowNode::Shared` instead of `SharedShadowNode`.
2. Initialise `ShadowNodeFamily` before `ShadowNode`.
Why?
This is a step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates.
Reviewed By: shergin
Differential Revision: D19471399
fbshipit-source-id: 2f67901c901349d238c711f9eeaadb19fe7c1110
Summary:
Changelog: [internal]
1. Moves `ShadowNode::getAncestors` to `ShadowNodeFamily`.
2. Exposes shadowNode's family through `ShadowNode::getFamily()`.
# Why?
This is a first step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates.
Reviewed By: shergin
Differential Revision: D19465188
fbshipit-source-id: b5a3625aa21c040301259de02beedbf97e11f20e
Summary:
`SnapToInterval` is a Float in JavaScript, if we pass it and try to convert it to Int, it crashes in C++.
exception
> libc++abi.dylib: terminating with uncaught exception of type folly::ConversionError: Loss of precision during arithmetic conversion: (long long) 1.15
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19580509
fbshipit-source-id: f705f92953195a9e034f6ce3fe7a077007d5212d
Summary:
Collecting Telemetry is a crucial part of building a performant UI framework; we do that but we need to improve it to make the data more reliable, actionable and trustful.
Now we collect time points as the number of milliseconds from the start of the CLOCK_MONOTONIC epoch. That's fine but it also has problems:
Sometimes a millisecond is an eternity. We have only 16 (or fewer) of them on each frame. What if some operation takes 1ms (according to telemetry) but we have to run it a dozen times? Does it mean that it's 12 ms in total? So, we lack precision.
This is not type-safe. Do you know how many milliseconds in a microsecond? I don't. We multiply that on magical constants hoping that we copied that from some other place right.
The current implementation is not cross-platform. We have ifdefs for iOS and Android and Unix and Windows (which is now implemented).
So, this diff replaces that with using `std::chrono` which is part of the standard library that designed to fix all those concerns. We also define our type-aliases on top of that to express our concrete constrains:
We use `std::chrono::steady_clock` as the base clock which is according to the standard using `clock_gettime(CLOCK_MONOTONIC, ... )` if available. So, it's fast and compatible (the same under the hood) with Android infra.
We use nanoseconds when we store time durations (TelemetryDuration type).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D19184569
fbshipit-source-id: 7a44688f4bb3bfc6e3009874f0075c531c8569a1
Summary:
Some tests which will be useful for the next diff.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D19482255
fbshipit-source-id: 3384662477f750620a37acc4a277a55c2dbd8d0e
Summary:
This is a part of migration staterted in D19390813.
There is no need to have those as `const`. The whole `*Props` object is usually `const` (and when it's not, props should not be too).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D19583582
fbshipit-source-id: 9c680268f944cdf08669fce7e997b05f23a02667
Summary:
Fixes https://github.com/facebook/react-native/issues/27137
This PR fixes an issue on iOS where RCTTextView height is not calculated as it should for some fonts where font `leading` attributed is not equal to zero, which results in wrong baseline alignment behaviour.
The fix for this is by setting `usesFontLeading` property of `NSLayoutManager` to `NO`, which results is a layout behavior that is similar to `UILabel`
Probably the documentation for `usesFontLeading` describes why UILabel has a different (correct) layout behavior in that case
> // By default, a layout manager will use leading as specified by the font. However, this is not appropriate for most UI text, for which a fixed leading is usually specified by UI layout guidelines. These methods allow the use of the font's leading to be turned off.
## Changelog
[iOS] [Fixed] - Fix RCTTextView layout issue that happens on some font with `leading` attribute not equal to zero, which causes wrong base-alignment layout
Pull Request resolved: https://github.com/facebook/react-native/pull/27195
Test Plan:
Below are the test results before and after the change, and comparing that to native UILabel behavior.
The test is done with using system font and custom font (`GothamNarrow-Medium`) and font size 50
[GothamNarrow-Medium.otf.zip](https://github.com/facebook/react-native/files/3832143/GothamNarrow-Medium.otf.zip)
```js
const App: () => React$Node = () => {
return (
<View style={{flex: 1, margin: 40, flexDirection: 'row', justifyContent: 'center', alignItems: 'baseline'}}>
<View style={{width: 30, height: 30, backgroundColor: 'lightgray'}} />
<Text style={{fontSize: 50, backgroundColor: 'green', fontFamily: 'GothamNarrow-Medium'}}>{'Settings'}</Text>
</View>
);
};
```
-------
### Before the fix
<img width="962" alt="Screenshot 2019-11-11 at 16 53 26" src="https://user-images.githubusercontent.com/5355138/68601049-dd778780-04a3-11ea-879e-cc7b4eb2af95.png">
-----
### After the fix
<img width="944" alt="Screenshot 2019-11-11 at 16 55 11" src="https://user-images.githubusercontent.com/5355138/68601180-1d3e6f00-04a4-11ea-87bc-61c6fa2cdb18.png">
-----
### Using `UILabel`
<img width="805" alt="Screenshot 2019-11-11 at 16 59 28" src="https://user-images.githubusercontent.com/5355138/68601487-b2d9fe80-04a4-11ea-9a0f-c025c7753c24.png">
Differential Revision: D19576556
Pulled By: shergin
fbshipit-source-id: 4eaafdab963c3f53c461884c581e205e6426718a
Summary:
This change will keep project consistency.
This change repeat the previous commit 582738bdc8.
That was created by sammy-SC and reviewed by shergin
That changed
* `ASSERT_TRUE` -> `EXPECT_TRUE`
* `ASSERT_NEAR` -> `EXPECT_NEAR`
* `ASSERT_EQ` -> `EXPECT_EQ`
That said
> 1. Replace ASSERT_* with EXPECT_*. Assert is a fatal assertion. Expect is non-fatal assertion. So if assert fails, tests do not continue and therefore provide less information.
>
> 2. Rename tests in `RawPropsTest.cpp` from `ShadowNodeTest` to `RawPropsTest`.
>
> Source: https://github.com/google/googletest/blob/master/googletest/docs/primer.md#basic-assertions
## Changelog
[CATEGORY] [TYPE] - Message
Pull Request resolved: https://github.com/facebook/react-native/pull/27850
Differential Revision: D19568014
Pulled By: shergin
fbshipit-source-id: 7c22cd1c7ec919675e834a060bd5e681d43a8baf
Summary:
The reason for this change is that it is the primary root that we want people to be using and the naming should reflect that.
#nocancel
build-break
overriding_review_checks_triggers_an_audit_and_retroactive_review
Changelog: [Internal]
Oncall Short Name: fbobjc_sheriff
Differential Revision: D19431128
fbshipit-source-id: c7208e20ed0f5f5eb6c2849428c09a6d4af9b6f3
Summary:
Fix initial width measurement of AndroidTextInput.
The lifecycle here between measure and layout is a little wacky. I put this in comments too, but:
1. Measure is called first. It's marked as const so it can't call updateStateIfNeeded.
2. Layout is called immediately after. It's not const so it calls updateStateIfNeeded.
3. The state is updated, but it's not part of a commit, it just mutates the node in-place.
4. If the node isn't dirtied again, measure won't be called again.
For completeness: I did try calling `dirtyLayout` in the `layout` method. That does not work.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19549803
fbshipit-source-id: f3798e10dca2edacb364cc5b53f58f091de5e4d0
Summary:
Since we're using `tryCommit`, there could be races between two updates to state attached to the same node, especially if a ComponentDescriptor's `createState` implementation uses previous state data to create the new state object. By creating the State object from the "old" node within the tryCommit lambda, we prevent races.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19542908
fbshipit-source-id: 787e4aefc573adf1fa308a6fa859123976a823e7
Summary:
Changelog: [Internal]
The inspector API doesn't really need a `HermesRuntime`, all it needs is a `jsi::Runtime` and a `Debugger &`.
Change the return type of `RuntimeAdapter::getRuntime` to be `jsi::Runtime`.
This will allow the inspector to use the tracing runtime instead of the direct hermes runtime.
Reviewed By: willholen
Differential Revision: D18973867
fbshipit-source-id: 6809e52452a35e62be9ca8143aeaba8964c98eaa
Summary:
Previously, State revision number was implemented manually as part of the StateData. Now we have it as a built-in concept in State, so we can rely on that.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19467161
fbshipit-source-id: cac907265090730cdb89207aad2b52141cda5dc6
Summary:
This diff introduces a revision concept to a State object. Every newly created State object gets a revision number which equals a source state object revision number plus one.
This functionality is useful to detect which state object is newer.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19467162
fbshipit-source-id: fc5e165193009c9818bf124bd4df7021288b2389
Summary:
Replace with `:container_evicting_cache_map`
Changelog: [Internal]
Rename `//xplat/folly:evicting_cache_map` to `//xplat/folly:container_eviciting_cache_map` to follow the new target name conventions
Reviewed By: JoshuaGross, nlutsenko
Differential Revision: D19374914
fbshipit-source-id: fadcaeac20fe61af79e1eecb1be5642f2ef96285
Summary:
`Element` is an abstraction layer that allows describing component hierarchy in a declarative way. Creating `Element`s themself does not create a component tree (aka `ShadowNode` tree) but describes some hierarchical structure that might be used to build an actual component tree (similar to XML Elements).
`Element<>` provides some basic type-safety guarantees: all modifications of element objects require using objects (such as Props or State) of compatible type.
For now, the only useful application of that is building tests.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19512392
fbshipit-source-id: eb0711c2a537865fa5454dbede53412a135058cf
Summary:
# Changes
1. Fixes a bug when calling `LayoutableShadowNode::getRelativeLayoutMetrics` with `this` being the same node as ancestor.
2. Refactors logic that increments `layoutMetric.frame.origin` by iterating through ancestors.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19468690
fbshipit-source-id: 5b9e187adc26a206da035e4387bb5f528aabdbb2
Summary:
This is an incomplete effort to migrate from libfb to libfbjni. This is needed to restore the compatibility with Flipper and other FB Android projects that make use of FBJNI. Effectively, the outcome is that `fbjni` no longer has a checked-in copy here, but instead relies on the public artifacts published at github.com/facebookincubator/fbjni that can be deduplicated at build-time.
**A non-exhaustive list of tasks:**
* [X] Gradle builds the SDK and RNTester for Android.
* [X] Buck build for rntester works in OSS.
* [ ] Move from `java-only` release to full `fbjni` release. This requires finding a solution for stripping out `.so` files that the old `Android.mk` insists on including in the final artifacts and will clash with the full distribution.
* [ ] Import this and fix potential internal build issues.
* [ ] Verify that the changes made to the Hermes integration don't have any unintended consequences.
## Changelog
[Android] [Changed] - Migrated from libfb to libfbjni for JNI calls
Pull Request resolved: https://github.com/facebook/react-native/pull/27729
Test Plan:
- CI is already passing again for Gradle and Buck in OSS.
- After applying the following patch, RNTester builds and works with the latest Flipper SDK:
```
diff --git a/RNTester/android/app/build.gradle b/RNTester/android/app/build.gradle
index b8a6437d7..eac942104 100644
--- a/RNTester/android/app/build.gradle
+++ b/RNTester/android/app/build.gradle
@@ -170,10 +170,19 @@ dependencies {
debugImplementation files(hermesPath + "hermes-debug.aar")
releaseImplementation files(hermesPath + "hermes-release.aar")
- debugImplementation("com.facebook.flipper:flipper:0.23.4") {
+ debugImplementation("com.facebook.flipper🐬+") {
exclude group:'com.facebook.yoga'
- exclude group:'com.facebook.flipper', module: 'fbjni'
- exclude group:'com.facebook.litho', module: 'litho-annotations'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-network-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
+ }
+
+ debugImplementation("com.facebook.flipper:flipper-fresco-plugin:+") {
+ exclude group:'com.facebook.yoga'
+ exclude group:'com.facebook.fbjni'
}
if (useIntlJsc) {
```
Reviewed By: mdvacca
Differential Revision: D19345270
Pulled By: passy
fbshipit-source-id: 33811e7f97f44f2ec5999e1c35339909dc4fd3b1
Summary:
LocalData isn't used by any components. We've moved towards State.
Changelog: [internal]
Reviewed By: shergin
Differential Revision: D19250738
fbshipit-source-id: 38ab19f63e4a249748555e9181141340729b4510
Summary:
LocalData isn't used by any components. We've moved towards State.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D19250737
fbshipit-source-id: 10bf0b62ffad01ad10b07d029e84df4f312780a1
Summary:
LocalData isn't used by any components. We've moved towards State.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D19250732
fbshipit-source-id: dd23a723c5392632165798f6365ea107b37b54cd
Summary:
LocalData isn't used by any components. We've moved towards State.
Changelog: [internal]
Reviewed By: mdvacca
Differential Revision: D19250603
fbshipit-source-id: f95da375117dcb8f6540a6c75d3a80bba630672c
Summary:
Adds a bare minimum test that verifies correct behaviour of `getRelativeLayoutMetrics`.
Changelog: [internal]
Reviewed By: shergin
Differential Revision: D19449128
fbshipit-source-id: afde997a770921d580575eb0cdd04fce6252cb5a
Summary:
# Logs
```
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 0 tag: 1432 debugValue: r1
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r2
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r2
[tid:com.facebook.react.JavaScript][UIManager.cpp:197] width: 0 tag: 1432 debugValue: r1/sealed <----------------- FIRST TAP
[tid:com.facebook.react.JavaScript][UIManager.cpp:197] width: 0 tag: 1432 debugValue: r1/sealed <----------------- FIRST TAP
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r4
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r5
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r6
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r7
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r8
[tid:com.facebook.react.JavaScript][UIManager.cpp:197] width: 375 tag: 1432 debugValue: r7/sealed <----------------- SECOND TAP
[tid:com.facebook.react.JavaScript][UIManager.cpp:197] width: 375 tag: 1432 debugValue: r7/sealed <----------------- SECOND TAP
```
# What’s happening here?
Opening a *BottomSheet* and tapping the first item. As you can see before the item is tapped, it has *width* set to 375 in revision *r2*.
When the tap happens, JavaScript is requesting an old revision of ShadowNode, which still has width 0.
# My assumption.
1. Native creates node with *width* 0 and returns handle to *JS*.
2. Native *clones* the node, sets its *width* to 375, doesn’t tell *JS* about it. This update is due to state change.
3. *JS* tries to get the size of the node, but asking for first revision of the component it receives width 0.
# Other observations
1. Manually setting width to 375 in UIManager::getRelativeLayoutMetrics fixes the problem.
2. This happens only on device, I wasn’t able to reproduce this on simulator.
# Fix
Find the newest revision of ShadowNodeFamily, and return its layoutMetrics.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19433873
fbshipit-source-id: 4558cf6e704051e9b3968e83821d8d25b3dadcda
Summary:
# Problem
The issue is in implementation of `LayoutableShadowNode::getRelativeLayoutMetrics`.
If you have view tree (A has a child B), and you call `B.getRelativeLayoutMetrics(A)`, the expected result is B's origin within A. The implementation wasn't reflecting that, it was reflecting B's origin within A + A's origin within its parent.
# Fix
When iterating over ancestors of ShadowNode, the last ancestor should be skipped.
AncestorList is a list that starts with provided ancestor and ends with parent of `this`. To skip provided ancestor we iterate to `rend() - 1`.
# Why does it work in some cases?
This function is triggered from `UIManager.getRelativeLayoutMetrics` without `ancestorShadowNode` provided, we find the RootShadowNode, which has origin `{0, 0}`.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19447900
fbshipit-source-id: 4a9606dc1fab3fecfb85d337b014188d80e5b355
Summary:
Changelog: [Internal]
Refactor the setup code for tests to a single place.
Now there is fixed set of nodes where we test different methods of ShadowNode.
Reviewed By: shergin
Differential Revision: D19464966
fbshipit-source-id: 749e9f56ac2e5489647885b2ddcb1309eb20909a
Valentin Shergin
Ramanpreet Nara
Joshua Gross
Samuel Susla
Moti Zilberman
Pasquale Anatriello
David Vacca
Mike Vitousek
Radek Pietruszewski
Kevin Gozali
Ahmed Ibrahim
Yuki Ueda
Josh Leibsly
Riley Dulin
Michael Lee (Engineering)
Kudo Chien
Pascal Hartig
