Summary:
Adds `TextKind` trait to Paragraph and TextInput shadow nodes.
Later on it could cause confusion why they do not have this trait.
Changelog: [Internal]
Reviewed By: shergin, mdvacca
Differential Revision: D20254008
fbshipit-source-id: 56fb264fa4fe38a5f00f080558cdc7992fd51fea
Summary:
Instead of comparing the entire AttributedString, compare just the strings and the TextAttributes of Fragments.
Concretely what I'm seeing is that the Frame of the associated parent ShadowViews are changing very frequently, making it impossible to actually modify the TextInput in some cases. However, we shouldn't forcibly reset the content of the TextInput if the frame of the parent is changing and not the actual child contents.
Changelog: [Internal] Fabric TextInput bug fix
Reviewed By: shergin
Differential Revision: D20319359
fbshipit-source-id: 2f51f521ad76fff9da6f6c8b5e795f03c33e496f
Summary:
It's not clear why exactly but seems in some cases, for some views the hierarchical relationship between views is required (when it should not be conceptually). Turning this feature off for Android for now.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D20292833
fbshipit-source-id: 1aab8468cedeb5c1440a95944be7eca3216e0db0
Summary:
Of course if props aren't changing, rawProps won't have paddings anymore and so padding gets reset.
Also, make sure we pay attention to paddingVertical and paddingHorizontal styles.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20269583
fbshipit-source-id: 2903fd0f7e6178f506109467d9a8f67b6d013a25
Summary: Now, following the previous diff, we remove `LayoutableShadowNode::isLayoutOnly()` and change the view flattening algorithm to rely on two new traits. See the previous diff to learn more about how it works.
Reviewed By: sammy-SC
Differential Revision: D20212252
fbshipit-source-id: 87a07e8bb17b2e66e5703f107dc35ca7a8e49634
Summary:
This diff introduces two new `ShadowNodeTrait`s that we will use in the future in the new (slightly tweaked) view-flattening algorithm. (Note: this diff does not enable the new flattening, it's just preparation.)
The idea is that we split the notion of `isLayoutOnlyView` into two traits:
* `FormsView`: `ShadowNode`s with this trait must be represented as `ShadowView`s. Normal "visible" ShadowNodes will have this trait, but "layout only views" in old nomenclature will not.
* `FormsStackingContext`: `ShadowNode`s with this thread not only must be represented as `ShadowView`s but also have to form a "stacking context" which means that their children must be mounted as `ShadowView`'s children.
Our implementation does not exactly follow W3C spec in terms of sets of props that create the stacking context (because of historical reasons and mobile specifics) but ideologically it's the same. We start from the very conservative implementation where only views with background-color and borders do not form stacking context and then we probably extend that to more props.
Most importantly for us now, we will enforce the absence of ``FormsStackingContext` for `ParagraphShadowNode` and the presence of `FormsView` for `TextShadowNode` on Android where it's essential for mounting layer.
Read more about stacking context here: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Positioning/Understanding_z_index/The_stacking_context
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20212253
fbshipit-source-id: 0fbaee214ce2c5886cb0232843a2a3c7bb20655d
Summary:
Passing `layoutContext` to yoga crashes fb4a-64. Until I have a solution let's back this out.
Original commit changeset: 9df7da4bef8c
Changelog: [Internal]
Reviewed By: makovkastar
Differential Revision: D20246917
fbshipit-source-id: 220744fde9f74e8157fc0714c63639b01152e4ab
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: sammy-SC
Differential Revision: D20212255
fbshipit-source-id: 87f3d8e7a94c6626bd9b0fc304f75e915dd73d4c
Summary:
Changelog: [Internal]
Use LayoutContext to pass `fontSizeMultiplier` down to ParagrapShadowNode.
Reviewed By: shergin
Differential Revision: D20184596
fbshipit-source-id: 3965a127069a21328ed19cb3f9732f0a2d1c4d58
Summary:
In order to build dynamic text sizing, `LayoutableShadowNode::measure` needs to accept `LayoutContext`
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D20184598
fbshipit-source-id: 8928b59d51948caf3654f40049212a89a91dceb6
Summary:
In order to build dynamic text sizing, we need to pass layoutContext to `yogaNodeMeasureCallbackConnector`
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D20184597
fbshipit-source-id: 9df7da4bef8cbad3bc87d63ed0c5aa5f420dbc11
Summary:
Clearing `props` and `state` (which we don't use) allows avoiding retain cycles.
The memory leak was caused by a retain cycle:
AttributedString -> Fragment -> ShadowView -> PharagraphState -> TextLayoutManager -> Cache -> AttributedString.
We don't use `props` and `state` parts of `ShadowView` inside `AttributedString`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20182791
fbshipit-source-id: 2ddc5d53a1cb594e3d1cc39933e8958eb6425389
Summary:
Changelog: [internal]
# Problem
We have node A with N children.
Calling cloning constructor on `YogaLayoutableShadowNode` causes new `yogaNode_` to be created.
However if `fragment.children` is nil in cloning constructor, which basically says children were not changed, then the existing children's `owner` becomes invalid.
# Solution
In the clone constructor, always call `updateYogaChildren`. This updates yoga children with the new `owner`.
Reviewed By: JoshuaGross
Differential Revision: D20139582
fbshipit-source-id: 3932694d4381b601df07dd8a57887ce7c09f1582
Summary:
Cloning subtrees is not something specific to a RootNode, so it makes sense to have it in ShadowNode. Soon we will use that to clone subtrees inside Paragraph component to implement Inline Views.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20090666
fbshipit-source-id: 0a64ef9bda438cd55d5fd21d3ad83b36221fa89e
Summary:
All logic that is performed on the root node only was moved from `layout` to a separate methods `layoutTree`. That makes the code simpler and allows reusing `layoutTree` in InlineViews feature.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20052025
fbshipit-source-id: 3070a1cca4e322c6d077ede751ea80359c96a600
Summary:
Similar to a previous diff but for `setChildren`.
`YogaLayoutableShadowNode::setChildren()` was renamed to `YogaLayoutableShadowNode::updateYogaChildren()`. Now we don't need to pass an argument to this function because the object is already initialized. The new name also disambiguates this method with `getChildren()` from `ShadowNode` (which does something completely different). The rest of the changes is just type adjustments.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20052031
fbshipit-source-id: 6157cad9b55d4cdd97ce04e1278ac1369bfb96bc
Summary:
`YogaLayoutableShadowNode::setProps()` was renamed to `YogaLayoutableShadowNode::updateYogaProps()`. Now we don't need to pass an argument to this function because the object is already initialized. The new name also disambiguates this method with `getProp()` from `ShadowNode` (which does something completely different).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052018
fbshipit-source-id: 4d9651bb4452408708fa1b36bb5c5c8639f7d1bb
Summary:
How we can finally make `ViewProps` inherit `YogaStylableProps` and remove `dynamic_cast`. This overall change is aligned with D19963353.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052026
fbshipit-source-id: 1066f9a259b50d4672111c9c03a71c2590d59aca
Summary:
Here we delete several constructors that were needed only because `YogaLayoutableShadowNode::yogaStyle` was private (which is not true anymore).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC, mdvacca
Differential Revision: D20052033
fbshipit-source-id: c2afcaf2fdebb65cdcde6c214c8b78aa35d45e91
Summary:
Here we refine the `ShadowNode::BaseTraits` static method. Before this change, ConcreteViewShadowNode<> enforces Layoutable and YogaLayoutable traits. This change moves that responsibility to LayoutableShadowNode and YogaLayoutableShadowNode which makes overall logic more coherent.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052027
fbshipit-source-id: fd25264204b0232b1dbbff6f9dfefd9fbcb146c4
Summary:
Now we can move custom to `YogaLayoutableShadowNode` code from `ConcreteViewShadowNode<>` template to `YogaLayoutableShadowNode` itself reducing the amount of templated code and reduce overall complexity.
Note, here we have to do `dynamic_cast` for now, we will address that in coming diffs.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20052021
fbshipit-source-id: dac5969a97b75e54c7728a1ca8161922bd2245ca
Summary:
The fact that `LayoutableShadowNode` now inherits `ShadowNode` allows us to de-virtualize `getLayoutableChildNodes` and move that to `LayoutableShadowNode`. Less code, less virtual dispatch, less complexity.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052032
fbshipit-source-id: 580e86b5a746028e470788e00027f247bf77126c
Summary:
D19963353 mentioned the infrastructure that re-routes methods calls related to adding and cloning children between YogaLayoutableShadowNode and ShadowNode. `cloneAndReplaceChild` is exactly this. It was implemented as a virtual method that is called from `ConcreteViewShadowNode`. The whole process requires building a list of children of some class and passing that as a list of pointers. Now we don't need it all that because we can call directly and statically. That change will allow us to simplify that infra even more in the future diffs.
With all previous changes, now we can implement `getYogaLayoutableChildren` inside `YogaLayoutableShadowNode` and call that statically. Eventually, that will allow us to remove templated `getChildrenSlice`. Previously the call for that method must be in `ConcreteViewShadowNode`, now it's not true anymore which we will use later to even better goodness.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D20052020
fbshipit-source-id: e5c819a4d21b2dbcd08f3439e1783e3a9cba5ef4
Summary:
D19963353 mentioned the infrastructure that re-routes methods calls related to adding and cloning children between YogaLayoutableShadowNode and ShadowNode. `cloneAndReplaceChild` is exactly this. It was implemented as a virtual method that is called from `ConcreteViewShadowNode`. The whole process requires building a list of children of some class and passing that as a list of pointers. Now we don't need it all that because we can call directly and statically. That change will allow us to simplify that infra even more in the future diffs.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052022
fbshipit-source-id: ddf341c112edd8a2f79eaf74465a9a360a168541
Summary:
`traitCast` is a special form of static_cast that checks additional requirements (similar to `dynamic_cast`) before performing the cast. We will use that in many places in the coming diffs.
Restructuring the class hierarchy in the previous diff finally allows us to do static casts among ShadowNode and LayoutableShadowNode and other classes (previously it wasn't allowed because of lack of common base class).
Why we don't want to use `dynamic_cast`:
* It's expensive (and we need to do the cast on the hottest fragments of the framework). (See: (1) http://www.stroustrup.com/fast_dynamic_casting.pdf and (2) https://www.youtube.com/watch?v=ARYP83yNAWk Herb Sutter & proposal of `down_cast`).
* It's code-size inefficient, whereas `static_cast` has zero runtime and code-size overhead.
* Removing `dynamic_cast` will allow us finally to opt-out `RTTI` (additional code size and perm wins).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20052024
fbshipit-source-id: d293c1cf80deb7817d333d5306d6b32bf3abdb27
Summary:
This is a very crucial change, everything else in this stack depends on it.
Here is the set of constraints that we have for ShadowNode-based class hierarchy:
* `ShadowNode` is a base class that defines basic operations on the nodes. It does not have virtual methods by design (virtual dispatch hurts performance; we want to limit and centralize that in `ComponentDescriptor`s).
* `ConcreteShadowNode<>` template *statically* wires particular `ShadowNode` and particular `Props` establishing a type-safe relationship between them ensured on compile time.
* Not all `ShadowNode`s are "layoutable", not all "layoutable" `ShadowNode`s use Yoga to do layout.
* Layotability (and YogaLayotability) feature is implemented as two classes `LayoutableShadowNode` and `YogaLayoutableShadowNode`. These classes essentially need to know something about the ShadowNode nature of the object (get the list of children or replace some children).
Before the change, `LayoutableShadowNode` and `YogaLayoutableShadowNode`classes did not inherit `ShadowNode` because we don't want to use *virtual inheritance* for `ShadowNode`: `ConcreteShadowNode<>` already inherits `ShadowNode`, so if we make `LayoutableShadowNode` inherits `ShadowNode`, we will have to somehow flatten this inheritance hierarchy (and the virtual inheritance is an answer to that). (Yes, C++ supports multiple inheritance and virtual inheritance.)
Before this change, we solved this dilemma this way: We have a subclass-template `ConcreteViewShadowNode<>` that inherits `ConcreteShadowNode<>` and `YogaLayoutableShadowNode`. Then, we had a bunch of methods that implement the rerouting of some functionality from `YogaLayoutableShadowNode` to `ShadowNode` and vise-versa. (See the diagram "Before".)
That worked fine, except the caveats:
* That wiring is nasty, complex, hard to reason about and overall limiting.
* There is no way to statically cast `LayoutableShadowNode` to `ShadowNode` (because there is no common base class). That forces us to use dynamic_cast on some perf critical paths (including layout, diffing and so on).
* Adding features that rely on interop between `LayoutableShadowNode` and `ShadowNode` is a nightmare.
It should be a better way to deal with this dilemma, and this diff implements a different approach: We can have a base class of `ConcreteShadowNode<>` as a template parameter. With this approach, we can make `LayoutableShadowNode` inherit `ShadowNode` and when we need to instantiate a `ConcreteShadowNode<>` that needs to be layoutable, we can just specify `YogaLayoutableShadowNode` as a base class. (See the diagram "After".)
This simple change will allow us to simplify a lot of things. The rest of the stack is about getting rid of unnecessary moving parts. Which will finally allow us to build "Inline Views" feature.
```
╭──────────────────────╮
│ ◎ ○ ○ ░░░░░░░░░░░░░░░│
├──────────────────────┤
│ │
│ │
│ Before │
│ │ ┌────────────────────────────┐ ┌────────────────────────────┐
│ │ │ │ │ │
│ │ │ ShadowNode │ │ LayoutableShadowNode │
└──────────────────────┘ │ │ │ │
└────────────────────────────┘ └────────────────────────────┘
▲ ▲
│ │
╔════════════════════════════╗ ┌────────────────────────────┐
║ ║ │ │
┌────────�║ ConcreteShadowNode<> ║ │ YogaLayoutableShadowNode │
│ ║ ║ │ │
│ ╚════════════════════════════╝ └────────────────────────────┘
│ ▲ ▲
│ │ │
│ │ │
│ │ │
│ │ ╔════════════════════════════╗ │
│ │ ║ ║ │
│ └─────║ ConcreteViewShadowNode<> ║─┘
│ ║ ║
│ ╚════════════════════════════╝
│ ▲
│ │
│ ┌──────────────┴───────────────┐
│ │ │
│ │ │
│ │ │
┌────────────────────────────┐ ┌────────────────────────────┐ ┌────────────────────────────┐
│ │ │ │ │ │
│ TextShadowNode │ │ ViewShadowNode │ │ ParagraphShadowNode │
│ │ │ │ │ │
└────────────────────────────┘ └────────────────────────────┘ └────────────────────────────┘
╭──────────────────────╮
│ ◎ ○ ○ ░░░░░░░░░░░░░░░│
├──────────────────────┤
│ │ ┌────────────────────────────┐
│ │ │ │
│ After │ │ ShadowNode │
│ │ │ │
│ │ └────────────────────────────┘
│ │ ▲
└──────────────────────┘ ┌────────────────────────────────────┤
│ │
│ ╔════════════════════════════╗
┌────────────────────────────┐ ║ ConcreteShadowNode ║
│ │ ║ <Base: ShadowNode> ║
│ LayoutableShadowNode │ ║ ║
│ │ ╚════════════════════════════╝
└────────────────────────────┘ ▲
▲ │
│ ┌────────────────────────────┐
┌────────────────────────────┐ │ │
│ │ │ TextShadowNode │
│ YogaLayoutableShadowNode │ │ │
│ │ └────────────────────────────┘
└────────────────────────────┘
▲
│
╔════════════════════════════╗
║ ConcreteShadowNode ║
║ <Base: ║
║ YogaLayoutableShadowNode> ║
╚════════════════════════════╝
▲
│
╔════════════════════════════╗
║ ║
║ ConcreteViewShadowNode<> ║
║ ║
╚════════════════════════════╝
▲
├─────────────────────────────────────┐
│ │
┌────────────────────────────┐ ┌────────────────────────────┐
│ │ │ │
│ ParagraphShadowNode │ │ ViewShadowNode │
│ │ │ │
└────────────────────────────┘ └────────────────────────────┘
```
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19963353
fbshipit-source-id: b65c8a5064bdb54ab64f08a8e546aa9e2b5a486b
Summary:
For backwards-compatibility with Paper, we're implementing a feature in Fabric that will allow TextInputs to use the default padding from the theme in Android.
Note that this uses some pretty ugly hacks that probably shouldn't be used inside of components at all: looking directly at rawProps, overriding props/Yoga styles in the component descriptor, etc. I would (personally) really like to kill this feature entirely unless and until we can find a more elegant solution.
Changelog: [Internal]
TextInputs are still not pixel-perfect with Paper, but they're much closer, and the underline visual glitchiness is no longer an issue.
Reviewed By: mdvacca
Differential Revision: D20109605
fbshipit-source-id: 543282843e0a9f03a504d72d7a014431099bd64c
Summary:
We are moving towards 100%-prettified files. That's the first step when we apply Clang Format for `ReactCommon`.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D20110895
fbshipit-source-id: 0a0ce4997cf1c3721b0b07ef78c1a57ce87d20f9
Summary:
Having the overridden function that returns a different type is apparently not a good idea (and might cause bugs and unexpected behavior), so it was renamed. The function also got a new return type (`const &` instead of `std::shared_ptr`) for simplicity, better performance, and smaller code size.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19837694
fbshipit-source-id: b7a96424bd040409371724907b3fb3931cd8a2e8
Summary:
Before this change the Element<> API didn't allow to specify state objects and crashed during instantiation of non-null states.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19816358
fbshipit-source-id: 95ba5e03ea98c0260593462146e8523c95245e2b
Summary:
This pull request fixes a few `#include`s in the Fabric source:
* Changes `<Glog/logging.h>` to `<glog/logging.h>` in two files, which was an issue for case-sensitive file systems
* In `ParagraphProps.cpp`, changes the include of `attributedstring/primitives.h` from a relative style to a more absolute style.
## Changelog
[Internal] [Fixed] - Fabric: Fix various includes
Pull Request resolved: https://github.com/facebook/react-native/pull/27947
Test Plan: Fabric compiles perfectly, and the Fabric test suite passes.
Reviewed By: shergin
Differential Revision: D19788148
Pulled By: sammy-SC
fbshipit-source-id: 30a297e8aea50c7c7fd5f48745edf04811a6bdbd
Summary:
Systraces were disabled a year ago in D14019272 because we suspected they can negatively affect perf. We don't think this is the case anymore.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19786284
fbshipit-source-id: 185ed45b134fdcadf131cfddfcf8faf37537a684
Summary:
These props are applied to `View`, therefore they most not be a part of base text attributes, so we reset them.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D19764144
fbshipit-source-id: a0d2900e3161c47b83114360c843fa85a4389f8a
Summary:
Of course, compare_exchange_strong didn't actually do what I wanted.
Using a mutex is simpler and actually has the semantics we want: atomically get the current value, compare, and bail if the value is the same, or swap and continue.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19754451
fbshipit-source-id: 6b0aef217b235959af683ec5e31b07a0dd7bb040
Summary:
Simple hack to prevent duplicate onLayout events from being emitted to JS.
Because of the addition of State Reconciliation to the Fabric lifecycle (see previous diff), in certain circumstances entire subtrees can be relayed-out many, many times even though they aren't changing. For JS product code that responds to every onLayout and forces a ReactJS tree commit (see: some usages of VirtualizedList) this can cause an async infinite loop of commits and layouts even though the tree and the LayoutMetrics aren't actually changing. Even though nothing is changing, it can still cause serious performance regressions and even some bugs since the internals of various state machines may assume onLayout won't be called many times.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19715280
fbshipit-source-id: d879e24f1c7b1f710ad430b7473aa9293d093dea
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:
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:
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:
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:
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:
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:
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:
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🐬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:
interop layer doesn't deal with `nil` being sent as event, example https://fburl.com/diffusion/t5zx6xm0.
This later triggers following assertion https://fburl.com/diffusion/dr5xzgic.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19309356
fbshipit-source-id: 82b3841c337f949ce7ca0b875c98f608edddb599
Summary:
This is C++ part of the implementation of Fabric-compatible <TextInput> component on iOS.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19169987
fbshipit-source-id: e2eac9beac6d1e124b5176b4a23676b8e05490c3
Summary:
ImageState if created with default constructor is created with `imageRequest_` being nullptr.
Calling `getObserverCoordinator()` on it was causing a crash.
We create initial state data with `imageRequest` populated.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19332711
fbshipit-source-id: 0266222551dbfb10b3f86e72a43d5306650fd09b
Summary:
Changelog: [Internal]
We are moving away from LocalData in favour of State.
Reviewed By: shergin
Differential Revision: D19250592
fbshipit-source-id: 6d2eef9a0c0e53e0146da609ba0c24fa09766433
Summary:
Changelog: [Internal]
We are moving away from LocalData in favour of State.
Reviewed By: shergin
Differential Revision: D19247031
fbshipit-source-id: 8884133b13dd111e8d9e2cd4936bf90bfc5b4932
Summary:
This diff changes the signature of ComponentDescriptor constructor to make it simpler and easier to support: now all arguments are passed via struct that contains all these arguments as fields.
Now the ComponentDescriptor constructor accepts three arguments one of which is optional. This causes some confusion and the possibility of bugs in all subclasses that needs to implement a custom constructor. Mostly because in every case we need to ensure that the constructor:
* Accepts and pass down all parameters/arguments;
* Accepts the right types of those parameters (shared vs weak pointers, references vs values).
* Accepts all thee arguments and pass them (including flavor!). We failed this point several times.
Overal that makes the code simpler and allows changing the set of parameters relatively easy. (There is no plan for it!)
Look at the LOC balance: less code!
Changelog: [INTERNAL]
Reviewed By: sammy-SC
Differential Revision: D18548173
fbshipit-source-id: 5d038b135e004f6c054026b3235ed57db99c086d
Summary:
As part of the plan is splitting ShadowNodeFragment into two parts. ShadowNodeFamilyFragment is already in place. This diff removes use of `ShadowNodeFragment::eventEmitter` and goes over all call sites to change it to `ShadowNodeFamilyFragment::surfaceId`.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19146697
fbshipit-source-id: 22cae5404b0f3098feb86c0437a4aa256d5b773e
Summary:
As part of the plan is splitting `ShadowNodeFragment` into two parts. `ShadowNodeFamilyFragment` is already in place. This diff removes use of `ShadowNodeFragment::surfaceId` and goes over all call sites to change it to `ShadowNodeFamilyFragment::surfaceId`.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19115785
fbshipit-source-id: 5972332c3360b88ca935581ed36070f26e678b22
Summary:
As part of the plan is splitting `ShadowNodeFragment` into two parts. `ShadowNodeFamilyFragment` is already in place. This diff removes use of `ShadowNodeFragment::tag` and goes over all call sites to change it to `ShadowNodeFamilyFragment::tag`.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19115781
fbshipit-source-id: 6ab3464a063c220d0924bf5a69b75449ca178650
Summary:
Having automatic defaults/an optional arg for `convertRawProp` has caused way more problems than it is worth. Remove the default argument.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19151594
fbshipit-source-id: 839ec8d138b2c3c083f221a2871582454004648c
Summary:
This implements propagation of ScrollView's contentOffset value on Android. That allows `LayoutableShadowNode::getRelativeLayoutMetrics` (and some measure functions) return values that take that info into account.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D19027433
fbshipit-source-id: 023ff9642d023971b3d24d5cc5f7c2f4b443031e
Summary:
Motivation: in Marketplace, there's a TextInput in JS that sends { lineHeight: null } to C++. In Paper this was correctly handled as a default value, which just causes that property to not be set. In C++ in Fabric, it was being parsed incorrectly as 0 (because of incorrect defaults being passed into `convertRawProp`), causing the text to not be visible at all. The solution is to make sure that its default value is NaN, which is handled correctly, and causes the Text to render again.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19128049
fbshipit-source-id: fe985428f3ed8b90d56cfa387fbc2d1476d19d36
Summary:
The coefficients in the code were incorrect: the default value of the scale should be 1, not 0.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sahrens
Differential Revision: D19101221
fbshipit-source-id: 3b4c3afc692ddb6bf32bf2344a77ec5ca34a06e4
Summary:
Setting `padding` for Yoga node dirties it.
In the previous implementation, we did it for all newly cloned nodes. Now we do it only if State actually changes.
This is important because usually, SafeAreaView is a container node; that means that if some descendant of SafeAreaView changes its state, the recloning of all nodes down to root node will cause dirtying SafeAreaView and cause relayout if the whole subtree.
E.g., if we put ScrollView inside SafeAreaView without the fix, onScroll events will cause Yoga relayout (because ScrollView updates own state on debounced onScroll event).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18987158
fbshipit-source-id: a3130c607c37e54ce813113222cd4a3872c58b6a
Summary:
iOS/UIKit prop `safeAreaInsets` sometimes produces values (and calls `safeAreaInsetsDidChange`) that practically the same but differ just enough to make operator `==` return `false`. That causes an indefinite state update loop and dizzying of the interface.
We do the same in Paper component as well.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18964089
fbshipit-source-id: 4c714d2554fdee0f6e999921f69b95676080b8fa
Summary:
When we designed the Fabric's event priorities we followed the model "let's assign all priorities to all events as they should theoretically be and create a flag that disables sync execution (because we were unsure that it's stable enough". After some experiments, it's clear that this model is not feasible. We realized that we were often not sure about the exact event priority that should be assigned to a particular event. We also realized that the priorities in web work differently compare to RN. And we are not sure how we should ensure ordering among events in different queues with different priorities.
At the same time, we want to use (or experiment with) sync events for in some cases when we sure about desired behavior and/or where async events make no sense.
This diff deletes the macro that disables sync priorities and explicitly assigns async priorities to events that previously had sync ones.
The actual behavior should not change.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18950431
fbshipit-source-id: 4033ef63d4e736075b525a693cd514d7b92d5bb0
Summary:
Having those arguments optional does not really make anything easier to use; on another side that makes it hard finding problems caused by missing that parameter.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18797337
fbshipit-source-id: c119b8f6a03994072edb45e39337e33b0f8b602f
Summary:
`accessibilityTraits` was missing, this diff adds it.
Also there is a name mis match, in javascript it is called `accessibilityRole`.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D18857668
fbshipit-source-id: 10656e8fb4e8c1d771a72c7f354b845e41cfc313
Summary:
Here the deal with the clamp thing:
A layout engine asks nodes to measure their content providing layout constraints (min and max size) and that would be kind from nodes to return a result that in space of those constraints. Sometimes, satisfying that is an additional separate step of that operation, e.g. if we know that the intrinsic size of some node is `{100, 100}`, we need to clamp that to satisfy constraint `[min: {200, 200}, max: {inf, inf}]`.
In case when we need to cache measure information, it becomes tricky enough when we need to make the cache work most efficiently. For the case of measuring Text, we have an insight: the available maximum width is important, the rest three metrics (maximum height and minimum size) are not.
That means that any changes in those three metrics don't affect the actual measurement result but will affect the final value because the actual value will be clamped by constraints that didn't influence measuring in the first place.
That's why moving clamping outside of a cache is the first step into making it more efficient.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18848584
fbshipit-source-id: 8feeb3f1a9d0e9691abac8be43639487a626fec3
Summary:
Before this change, C++ couldn't propagate changes that updated TextInputs that were completely empty. In C++ the AttributedString cannot contain Fragments with empty text, so a completely empty TextInput would have no Fragments; this breaks the C++ state value updating infra since it relies on copying over existing fragments.
Instead, now we propagate default TextAttributes and ShadowView through State, so that State updates can use them to construct new Fragments.
Changelog: [Internal]
Reviewed By: shergin, mdvacca
Differential Revision: D18835048
fbshipit-source-id: 58ac94c5454c8610c6287b096b62199045e5879b
Summary:
Support for modifying AndroidTextInputs with multiple Fragments was added on the Java side in a previous diff.
This diff adds support on the C++ side for the following scenario:
A <TextInput> is initially given contents via children <Text> notes, which represents multiple Fragments (that could have different TextAttributes like color, font size, backgroundcolor, etc). The Android EditText view must get this initial data, and then all updates after that on the Android side must flow to C++ so that the C++ ShadowNode can perform layout and measurement with up-to-date data.
At the same time, the <TextInput> node could be updated from the JS side. All else equal, this would cause the native Android EditText to be replaced with the old, original contents of the <TextInput> that may not have been updated at all from the JS side.
To mitigate this, we keep track of two AttributedStrings with Fragments on the C++ side: the AttributedString representing the values coming from <TextInput> children, from JS (`treeAttributedString`); and the AttributedString representing the current value the user is interacting with (`attributedString`). If the children from JS don't change, we don't update Android/Java with that AttributedString. If the children from JS do change, we overwrite any user input with the tree from JS.
Changelog: [Internal]
Reviewed By: shergin, mdvacca
Differential Revision: D18785976
fbshipit-source-id: a1f3a935e02379cabca8ab62a39cb3c0cf3fbca5
Summary:
Currently the TextInput background is same as the background of the `text` value's attributes, because of the way we're parsing props and using the TextInput's props both for the background of the TextInput, and for the AttributedString/background color of the `text` prop node's attributes. If the background color has opacity, the `text` prop node will not have the correct background because it will be applied twice. Fix it.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18760384
fbshipit-source-id: 0cdcc8dd8839dd47e8fe0f593b4696bc16a62333
Summary:
Fix and simplify `AndroidTextInputShadowNode::getAttributedString` so that it (1) works, and (2) is aligned with the equivalents in the old Java code.
The issue is that we weren't picking up `text` attributes since we're only traversing children and not the TextInput node itself. If a `text` attribute is present it needs to be treated as its own Text node.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18739830
fbshipit-source-id: 4b3bc81dbe8c241c2e06fe5be1f9b50e49132890
Summary:
This is necessary for allowing TextInput updates from Java.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18739831
fbshipit-source-id: 0ba2d7eac96cac7471c5e46cc1e03a4d065229f5
Summary:
The diff implements a new class called `AttributedStringBox` that represents an object storing a shared `AttributedString` *or* a shared pointer to some opaque platform-specific object that can be used as an attributed string. The class serves two main purposes:
- Represent type-erased attributed string entity (which can be platform-specific or platform-independent);
- Represent a container that can be copied with constant complexity.
Why? Several reasons:
- Sometimes it makes sense to keep an attributed string as a shared resource. This way we don't need to pay for expensive copying and we also implement a copy-on-write semantics on top of that if needed.
- We need to extend a TextLayoutMeasure API to support measuring some platform-specific attributed string implementation to remove the necessity of converting a string back and forth between representations. That's especially important for TextInput because we will need to measure that very efficiently (and the source of measuring, in this case, is a platform attributed string).
In other words, we need something to store inside TextInputState to measure and update very efficiently. The source of this data might be a native TextInput control or a data from React, to represent that kinda object we need this data structure (and interfaces that deal with it).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18670793
fbshipit-source-id: bc0164f801f28642f7c6da340af12acf33b85d24
Summary:
In AndroidTextInput, support codegen'd ViewCommands in native and add three commands that will eventually replace usage of setNativeProps on Android.
TextInput will use these commands in a future diff.
Changelog: [Internal]
Reviewed By: TheSavior
Differential Revision: D18612150
fbshipit-source-id: 5d427040686e8c5ab504dd845bc8ef863f558c35
Summary:
In C++ we return default/placeholder text instead of text in `getAttributedString` so that measurement is correct. This is fine but we shouldn't actually set the attributedString on the ReactEditText view.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18672369
fbshipit-source-id: 1bb5cddda3cf78f2cff6f805e67c8994ab32ee7c
Summary:
iOS and other platforms have direct access to C++ StateT structs, whereas Java only has access to a Java map equivalent - state updates from Java can't update complex types, or must incur significant cost to reconstruct large objects from their folly::dynamic representation (not to mention the complexity of implementing the Java-to-C++ struct converters). Thus it's hard for Java to update StateT's with complex types on the C++ side.
This diff makes a minor change to Android's updateState which uses both the folly::dynamic data from Java as well as the previous State, so each StateT can have fields that are read-only from the Java perspective.
Motivation: For AndroidTextInput we need to set params from Java, without being able to send all of the State params from Java.
In this diff, we introduce a new State constructor that takes the previous State value and a folly::dynamic. It is up to each State implementation how the additional parameter will be used.
We migrate every existing component except for AndroidTextInput in this diff.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18672365
fbshipit-source-id: 4469e0a3c7658c204089c6fed39394979883f124
Summary:
Use a similar setup as Paragraph, and support in Fabric:
- Correct measuring of AndroidTextInput
- Correct display of AndroidTextInput attributed strings
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18669957
fbshipit-source-id: 84e0ad8021c9edf8219e0c673c781276ca29787d
Summary:
Before this change `AttributedString::Fragment` had two ShadowView objects (`shadowView` and `parentShadowView`). This diff unifies those two things into one. That allows us to save some CPU and memory and makes things a bit simpler. Besides that, now the length of NSAttributedString and AttributedString is now always the same (it's one Unicode character for an attachment for both).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18607658
fbshipit-source-id: 502ae244e98a52694adc0d646650f8ea0d7922ae
Summary:
We need to call all custom base constructors as part of a custom constuctor of the superclass otherwise C++ will use the default one (and that's not what we want here).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18607654
fbshipit-source-id: 6e6154ce4b546c0717a8a67e3c497dfcd696ec41
Summary:
In AndroidTextInput ShadowNode, return AttributedString for placeholder text if text is empty. This ensures that empty TextInputs will have non-zero height.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18623063
fbshipit-source-id: 2eec2b794b526e8ff5435352e20ccb659358fc84
Summary:
Changelog: [Internal]
In paper implementation:
`accessibilityActivate` returns NO in case `onAccessibilityTap` is nil.
In Fabric we have no option to detect whether `onAccessibilityTap` is nil or isn't but we don't want to prevent VoiceOver from tapping an element. This could potentially trigger action associated with element twice.
Let's say you have `onPress` and `onAccessibilityTap`, it will trigger both if you trigger action through VoiceOver.
Reviewed By: shergin
Differential Revision: D18572432
fbshipit-source-id: c5ac002317c798a10045b6f05738299d0ae27456
Summary:
We will have several consumers for the measure infra soon (TextInput will use the same TextLayoutManager). It makes sense to move the cache there.
In the future, iOS and Android implementations will probably use a bit different (platform-specific) cache implementations because we will implement the ability to measure "opaque"/platform-specific text containers alongside with normal AttributeStrings.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18445855
fbshipit-source-id: 7b7a65152ac13c74525da695612ae034904e82bf
Summary:
Changelog: [Internal]
The main use-case here is to get the rootTag off RCTImageView, for image loading instrumentation. The fact is, each RCTView subclass already has `reactTag` attached today. We already have the `rootTag` when the view is created by the UIManager, so why not just attach it like reactTag? If we don't, looking up the rootTag from the native component is non-trivial and extremely inefficient (have to jump to shadow queue, back to main queue, etc).
Reviewed By: sammy-SC
Differential Revision: D18497002
fbshipit-source-id: 8409e3a1c95e09accedd959592cbf178fab0b2c3
Summary:
Changelog: [Internal]
For attribution purpose, pass in the surfaceId and let the app-specific image loader handle it.
Reviewed By: shergin
Differential Revision: D18494106
fbshipit-source-id: e22ca339a2dd12c5bd619b596c7db9c49dc111d0
Summary:
As soon as we have traits infra, it's pretty straight-forward to implement this kinda flag. Without the traits, it's challenging to build that in a performant and elegant way. The challenges of the dynamic_cast-involved approach are:
* How to check for that feature in YogaLayoutableShadowNode?
* Even if we use `dynamic_cast`, to which class to cast?
* We also need to do that in constructor... and dynamic_cast-like checks for `this` don't work in constructors (C++ design limitation).
* How to scale that if we have more classes like Paragraph (and we will have it for sure)?
* Performance.
* Relying on RTTI.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18525347
fbshipit-source-id: b1915f43ff3fe4364ab6345fb9d1becc591b5a35
Samuel Susla
Joshua Gross
Valentin Shergin
generatedunixname89002005287564
David Vacca
Kevin Gozali
Janette Cheng
Josh Leibsly
empyrical
Michael Lee (Engineering)
Pascal Hartig