Summary:
We want completely remove usages of Bitfield as it uses templates which bloats binary size
This stack will reduce 2.2 Kb in binary size bringing it down to 61.3KB on armv7
In this diff we are removing usage of template while updating styles.
## Changelog:
[Internal][Yoga] : Not using templates for updating styles
Reviewed By: astreet
Differential Revision: D18519570
fbshipit-source-id: 2324088b8c63154f818b1da1edf24c0533e10082
Summary:
The margin and padding are resolved incorrectly for leaf nodes (nodes with measure function set) if margin and padding are used in percentages.
Here we were using node's width instead of container width to calculate the margin and padding.
Fixed this to use container's width.
## Changelog:
[General][Yoga] : Fixed an issue where margin and padding were resolved incorrectly for leaf nodes (nodes with measure function set) if margin and padding are used in percentages.
Reviewed By: alickbass
Differential Revision: D17130520
fbshipit-source-id: ac904d432f121973e7739debd9136909b5ca1427
Summary:
Why crash in production if we can make potential bug not compile?
Changelog: [INTERNAL]
Reviewed By: sammy-SC
Differential Revision: D18499813
fbshipit-source-id: dc2d7ac12b8a2ae27d1e6b832fccc1555cab3b75
Summary:
The original implementation had an incorrect order of fields in the struct wich causes bugs with border drowing.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18444733
fbshipit-source-id: e4611e6e90eee85e7cdebcee3c5bb6886417aa26
Summary:
We will use it soon for `EdgeInsets`
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18444732
fbshipit-source-id: f03aa2a8889d426095a954d88f9cb6172c3e376e
Summary:
In Yoga, margin is not calculated correctly when defined in terms of percentage at one place.
According to CSS docs , margin percentage should be calculated according to width of container's block in case of horizontal writing mode. (https://fburl.com/5imus0it)
We were using height of container causing some issues in both android and iOS.
## Changelog:
[Yoga] [Fixed] - margin if defined in percentage should use container's width in horizontal writing mode
Reviewed By: alickbass
Differential Revision: D18395285
fbshipit-source-id: 87ebd013e3cba36da45f6548e4dff1bce69cce9b
Summary:
setBreakpoint differs from the existing setBreakpointByUrl in that you
can specify a scriptId directly. This was not really possible before
now, since scriptIds were mapped via filenames rather than to
RuntimeModules.
When (re)loading files with the same name, this call can be used to set
breakpoints in a specific one, rather than just the latest version.
Changelog: [Internal]
Reviewed By: bestander
Differential Revision: D18362054
fbshipit-source-id: 6ff1e736c79a4e75dc424316675d426ce9902f5f
Summary:
We were calling `sendErrorToClient` on error, but this function only
returns a closure that we threw away. Change to actually send an error
response.
Changelog: [Internal]
Reviewed By: bestander
Differential Revision: D18362055
fbshipit-source-id: 2ff1750ffd2c32a9291d3ed74fa0aaecd76c64c6
Summary:
When a filename (potentially with changes) is loaded multiple times, we keep
multiple instances around but fail to alert the attached debugger about this.
By coupling ScriptIDs to RuntimeModules rather than filenames, we can track
multiple instances of the same name instead.
Changelog: [Internal]
Reviewed By: bestander
Differential Revision: D18362052
fbshipit-source-id: 2147af32bc48c5fd2a2b080ea09dfa4133de7cf5
Summary:
There's some incompatibilities which need to be fixed for real,
but this is a temporary fix to make the build work again.
Changelog: [Internal]
Reviewed By: willholen
Differential Revision: D18364011
fbshipit-source-id: c168e6496a504f9b00da0b9d758c50e2c6f314a4
Summary:
In this diff we integrate the Picker component on Android in Fabric. Since both AndroidDropdownPickerNativeComponent and AndroidDialogPickerNativeComponent use `style?: ?TextStyleProp`, which is not supported by the JS codegen, I had to handwrite the C++ files and check them in.
The component is not fully functional yet because `setNativeProps` is not supported in Fabric. I will fix this in the next diff.
Changelog:
[Android] [Added] - Integrate Picker into Fabric on Android
Reviewed By: mdvacca
Differential Revision: D17954435
fbshipit-source-id: 6b7b029ab0c84c27a48c7dddd66878c9dea324bf
Summary:
Changelog: [internal]
if you called `ContextContainer.insert("key", <value>)` and then again `ContextContainer.insert("key", <new_value>)`, the old value will remain in `ContextContainer`. This seems to be by design however it causes crash when reloading surface that uses `ComponentDescriptorRegistry_DO_NOT_USE_PRETTY_PLEASE`.
We fix it by erasing `ComponentDescriptorRegistry_DO_NOT_USE_PRETTY_PLEASE` in Scheduler before inserting a new one.
Reviewed By: shergin
Differential Revision: D18421153
fbshipit-source-id: a01e5f6ac864fd641674d84e97d447f8cb9114be
Summary:
ScrollView isn't using codegen, therefore we need to manually write commands interface. I also typed commands so it uses `Double` instead of `number`.
Changelog: [Internal]
Reviewed By: TheSavior
Differential Revision: D18371887
fbshipit-source-id: 3bd11b9969b80ce6d2302e2f0da28884e9221b7e
Summary:
With this diff (and maybe other coming in the future), we are enforcing a model where Fabric Core actively/architecturally prevents `jsi::Object` to overlive `UIManager`. That never was a promise that but I think it's achievable; it should help to eliminate the whole class of possible crashes (application layer still must ensure that Scheduler is deallocated before `jsi::Runtime`).
Clearing all pointers to `ShadowNode`s in `MountingCoordinator` prevents from extending the lifetime of ShadowNodes stored inside revoked `MountingCoordinator` (the lifetime of `MountingCoordinator` is up to application layer).
That should help prevent crashes (not all of them!) caused by `jsi::Object`s overliving `jsi::Runtime`.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D18333708
fbshipit-source-id: 4391998b8f805563cfaff834fc56e85b98bc4d68
Summary:
`mem` package vulnerability found.
> In nodejs-mem before version 4.0.0 there is a memory leak due to old results not being removed from the cache despite reaching maxAge. Exploitation of this can lead to exhaustion of memory and subsequent denial of service.
ref: https://github.com/sindresorhus/mem/commit/da4e4398cb27b602de3bd55f746efa9b4a31702b
`relay-compiler` depends on that.
```
relay-compiler@6.0.0
└─┬ yargs@9.0.1
└─┬ os-locale@2.1.0
└── mem@1.1.0
```
Pull Request resolved: https://github.com/facebook/relay/pull/2934
Test Plan:
- Tested the Relay compiler briefly on the open source examples project, seems like yargs doesn't error
- `js1 build relay --project facebook`
- `js1 jest`
Reviewed By: alunyov
Differential Revision: D18397131
Pulled By: kassens
fbshipit-source-id: 8b7fc5e237c7b4ce14ff3809cdd59d243e7c4523
Summary:
PushNotificationiOS wasn't used by anything in ReactInternal, so I just removed it from the target.
## Codemod
Everywhere we required `ReactInternal`, we now also require `RCTPushNotification`:
```
> xbgr -f 'BUCK$' 'ReactInternal"' -l | xargs -I {} sed -i '' $'s|ReactInternal",|ReactInternal",\"fbsource//xplat/js:RCTPushNotification",|g' $HOME/{}
> xbgr -f 'BUCK$' 'ReactInternalApple"' -l | xargs -I {} sed -i '' $'s|ReactInternalApple",|ReactInternalApple",\"fbsource//xplat/js:RCTPushNotificationApple",|g' $HOME/{}
> arc f
```
Changelog:
[Internal] - Separate RCTPushNotification from ReactInternal
Reviewed By: PeteTheHeat
Differential Revision: D18363643
fbshipit-source-id: b8d123f40741c6d200dc9e736e64e885c2572e15
Summary:
None of the code inside `ReactInternal` depended on the `RCTLinkingManager` NativeModule. So I extracted `RCTLinking` into its own BUCK target. This will make it easier to make `RCTLinkingManager` TurboModule-compatible.
## Codemod
Everywhere we required `ReactInternal`, we now also require `RCTLinking`:
```
> xbgr -f 'BUCK$' 'ReactInternal"' -l | xargs -I {} sed -i '' $'s|ReactInternal",|ReactInternal",\"fbsource//xplat/js:RCTLinking",|g' $HOME/{}
> xbgr -f 'BUCK$' 'ReactInternalApple"' -l | xargs -I {} sed -i '' $'s|ReactInternalApple",|ReactInternalApple",\"fbsource//xplat/js:RCTLinkingApple",|g' $HOME/{}
> arc f
```
Changelog:
[Internal] - Separate RCTLinking from ReactInternal
Reviewed By: fkgozali
Differential Revision: D18314747
fbshipit-source-id: d9b5f536a6e93a0aca8721801a2ee5d446e0d4a6
Summary:
The documentation for the method says that it must be called from a subclass, but the methods wasn't marked as protected.
Calling `beat` directly will bypass custom logic implemented in subclasses (because it's not a virtual function (by-design)). That's practically what happened before and what the previous diffs in the stack fix.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D18339594
fbshipit-source-id: 3d8c53b69e1b72d447714dd91c9392bdfb05b9ae
Summary:
Interop layer can now forward commands to paper components.
Changelog: [internal]
Reviewed By: shergin
Differential Revision: D18285766
fbshipit-source-id: 33fe071c3000569d52fedcbcdeccc354dfe277d9
Summary:
This implement new <UnimplementedView> (only for iOS for now) that relies on the new "reactive component registration" functionality.
The `UnimplementedView` component is the perfect example of that. It's simple and uniquely required some constraints that we want to implement: the same component class registered several times with different handles and names.
This change serves two needs:
1. Providing an example of how that functionality can be used in more complex cases.
2. That will allow removing some `UnimplementedView`-specific code from the core (and very hot pathes of the system) and make them `noexcept`. That will eventually allow removing some public APIs from RawValue (constructing from folly::dynamic) that currently impose some implementation details and probably prevent us from making it slightly faster. There are only two consumers of this API, this is one of them.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D17211913
fbshipit-source-id: df1a1ac1a36289ef79904d509d38ee8b3f5588fb
Summary:
It's a logical error on the application layer if a Scheduler is being deallocated before all Surfaces were stopped. Besides that this requirement must be satisfied to make concurrent deallocation of Scheduler and calling on it from `UIManager` side thread-safe.
Changelog: [Internal]
Reviewed By: JoshuaGross, sammy-SC
Differential Revision: D18333709
fbshipit-source-id: 4af0ba488bb4306eb82807c2f0ee5b7c598dcfed
Summary:
There is no need for that functionality to be delegated to Scheduler, that's a leftover of the previous iteration of the design.
Nothing really changed.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18285501
fbshipit-source-id: 4b60445b663a2431fbdc7cc1120f46d56819c430
Summary:
Now `UIManager` serves as `ShadowTree`'s delegate instead of `Scheduler`.
There is no good reason for Scheduler to be a ShadowTree delegate anymore, now UIManager owns all ShadowTree instances (via the registry). The existing configuration causes a crash because `ShadowTree` can overlive the `Scheduler`. Having a `UIManager` as a delegate of `ShadowTree` solves the problem because `UIManager` checks the own delegate for (which is actually a Scheduler) nullability before calling something on it (and `Scheduler` nulls that delegate during own deallocation process).
(No actual ownership relations were changed in the diff.)
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18285500
fbshipit-source-id: 35ac4f37b6abdf4857a23c4869785a323e433317
Summary:
This diff removes `ComponentDescriptorProviderRegistry::remove()` and two derivative interfaces.
First, we don't use that and there is no concrete idea why we would need to use that. Those were originally built only for symmetry with limited knowledge about what exactly we need.
Second, those methods are actually dangerous and probably must not be supported by design. Removing a ComponentDescriptorProvider destroys already registered `ComponentDescriptor`s, and at the same time we might have ShadowNodes referring to that (which will cause a crash), and there is no reasonable way to check for the existence of those nodes.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18285497
fbshipit-source-id: b461e38b923c217a256e1155689311397a994feb
Summary:
Use double for YGRoundValueToPixelGrid calculations as we were losing some precision in float operations
#Changelog:
[Internal][Yoga] Use double for YGRoundValueToPixelGrid calculations
Reviewed By: astreet
Differential Revision: D18225999
fbshipit-source-id: 69c05f56a0e0f3433bf0bd958aa07d26dd83fe02
Summary:
Now we also collect mounting time inside MountingTelemetry.
We will use it soon.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18285826
fbshipit-source-id: 512fc62c210a111614b0defb0d76cbd6228fe89f
Summary:
Now the function supports iOS.
We will use it soon.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18285828
fbshipit-source-id: b9c16433e9c75ab4c071e4bd55074757372f6c0a
Summary:
A very common pattern I've seen in RN codebase:
- (instancetype) init {
[[NSNotificationCenter defaultCenter] addObserver:self ...]
}
- (void) dealloc {
[[NSNotificationCenter defaultCenter] removeObserver:self ...]
}
From Apple:
https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc
> If your app targets iOS 9.0 and later or macOS 10.11 and later, you don't need to unregister an observer in its dealloc method.
RN targets iOS9+
Changelog: [Internal][Cleanup] Remove unneeded NSNotification center removeObserver
Reviewed By: shergin
Differential Revision: D18264235
fbshipit-source-id: 684e5f5555cec96b055b13cd83daaeb393f4fac9
Summary:
Before this change, all methods `ShadowNode::getComponentHandle()` and `ShadowNode::getComponentHandle()` returned the values directly from a `ComponentDescriptor` (via `ShadowNodeFamily`). Now, we store/cache those values in ShadowNodeFamily object (and ShadowNode's methods use that).
We need that to ensure that calling those methods doesn't access the `ComponentDescriptor` object because it's difficult to guarantee that this object overlives all `ShadowNode`s. This is especially important when we build `ShadowView`s from `ShadowNode`s: this might happen way after the code is already deallocated and the only living objects are MountingTransactionCoordinator and essentially two pointers to root nodes of before and after trees (that still needs to be mounted).
This diff introduces no actual changes in the ownership model.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18290024
fbshipit-source-id: 4135a277515a84b053fe0d0dde48fba59bf2aae3
Summary:
Original commit changeset: 4b18a931a2e4
The original PR wherein these changes were introduced was problematic. When you expose headers that use C++ from the Yoga podspec, they get automatically imported in the Yoga umbrella file. This causes compilation errors, most likely because it's possible for non-c++ files to import these headers (i.e: RCTConvert.m).
I didn't dig into this too much, but since Fabric still doesn't fully compile in OSS, I think it's reasonable to revert this PR for now. cc Kevin Gozali.
Changelog:
[iOS][Fixed] - Undo Fabric-related podspec change
Reviewed By: fkgozali
Differential Revision: D18284536
fbshipit-source-id: a90454b945af0235424dc56408400cd35efd4e7a
Summary:
Using compiler flag -fvisibility=hidden and explicitly setting visibility to default to public methods
#Changelog:
[Internal] [Yoga] Use compiler flag -fvisibility=hidden for reducing yoga binary size
Reviewed By: astreet
Differential Revision: D18029030
fbshipit-source-id: 545e73f9c25f3108fc9d9bb7f08c157dbc8da005
Summary:
We suspect that the re-registration of `ComponentDescriptorProvider`s during Bridge reloading might cause crashes in Fabric core. That happens because the re-registration process replaces already existing and being used ComponentDescriptors in the managed registries with the exact same new ones, which forces old ones to be deallocated and all pointers to them invalid. (On of the fundamental Fabric design decision is that `ShadowNode`s don't own/retain ComponentDescriptors.)
It seems was already indirectly addressed in application code on iOS but still fixing that in the core is valuable.
Android implementation does not use reactive component registration, so it was already fine.
As the follow-up diff, we plan to remove "removing" capabilities from ComponentDescriptorRegistry and ComponentDescriptorProviderRegistry to make it even more future-proof.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18273683
fbshipit-source-id: 7615627842855f078a3fdf3049f5511f59700972
Summary:
This diff finally uses all facilities from the previous diffs to build an implementation of `RCTMountingTransactionObserving` protocol which does *not* require using expensive Objective-C runtime features.
In the coming diffs, we will see how it can/should be used.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D18217101
fbshipit-source-id: 34f411dcb527dc81570c2f2833ce13b40e1450db
Summary:
See the comment in RCTMountingTransactionObserving first.
I think we have to add this to the iOS mounting layer to be able reasonably easy implement things like:
* MovableNavigationBar: seems, currently we don't handle the situation when the container was mounted first and the nested scroll view second.
* TTI component: It does not have access to telemetry.
The protocol is meant to replace `RCTSurfacePresenterObserver`.
Changelog: [Internal] Fabric-specific change.
Reviewed By: mdvacca
Differential Revision: D16270107
fbshipit-source-id: 2d4bdb7d0092cc214cc433fc633e41e58f6677df
Summary:
Use padding instead of setting size of SafeAreaView, this should make it more consistent with Paper component.
changelog: [internal]
Reviewed By: shergin
Differential Revision: D18225793
fbshipit-source-id: 08dccbdae0e4f7a7847501a06e17d4c26473462a
Summary:
This parameter used to be useful for a custom format hermes was developing,
but since Hermes now outputs the Chrome format it isn't useful.
Chrome actually disallows prettified JSON, and requires a special version that is faster to parse.
Therefore, `compact` was the only supported mode.
Changelog: [Internal] Remove compact parameter from `createSnapshotToFile`
Reviewed By: willholen
Differential Revision: D17726742
fbshipit-source-id: 6f39af9046dff2f3b4fba822312a9a89c939ed89
Summary:
Use `reactTag` instead of address of `UIView` to map events from paper components to Fabric.
changelog: [internal]
Reviewed By: shergin
Differential Revision: D17954974
fbshipit-source-id: 0d8bf748e58f4cb6769e107bc7fd0e66b93d8f12
Summary:
This diff changes how arbitrary-node-replacing (aka `RootShadowNode::clone`) algorithm works.
Original implementation worked this way: We specified a node that needs to be replaced and a new replacement node. Then the algorithm finds a node with *the same family* as given the to-be-replaced node and replaced that with a given replacement.
The problem with this approach is that we are replacing a node that we have very little info about (we only know that it shares the same family as specified one). At the same time, we build the replacement based on the exact node that we have. Then imagine the case in which the "target" node can progress/change between a moment where we get a reference to it and a moment where we are trying to clone the tree. In this case, we will replace a "progressed" node with a modified version of the obsolete node.
Practically speaking, it was possible that during a state update we were replacing a node that just got new children with a bit older version of that with old children but with a new state.
How to deal with it? This diff introduces a new interface for this method that allows separating the target node and the actual act of cloning the corresponding node. Instead of specifying actual replacement, we now specify a function that performs the cloning/transformation on-demand on the very exact node that was in the tree at the moment of cloning.
This change does not change/affect any ownership-related relationships between trees and/or nodes.
Ideally, probably, the interface should accept ShadowNodeFamily instance instead of a ShadowNode instance to make the behavior very clear but that requires a bunch of low-level changes that it out of the scope of this fix.
```
The old approach.
┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐
│ │ │ │ │ │
│ A(r0) │ │ A(r1) │ │ A(r2) │
│ │ │ │ │ │
└─────────────────┘ └─────────────────┘ └─────────────────┘
│ │ │
│ Let's update the │ Meanwhile the node B │
▼ state of this ▼ gets new children. ▼
┌─────────────────┐ node to s1. ┌─────────────────┐ ┌─────────────────┐
│ │ │ │ │ │ Created
│ B(r0, s0) │ ───────▶ │ B(r1, s0) │ ───────▶ │ B(r2, s1) │ from
│ │ │ │ │ │ B(r0).
└─────────────────┘ └─────────────────┘ └─────────────────┘
│ │ │
┌──────┴──────┐ ┌─────────────┼─────────────┐ ┌──────┴──────┐
│ │ │ │ │ │ │ What just
▼ ▼ ▼ ▼ ▼ ▼ ▼ happe..?
┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐
│ │ │ │ │ │ │ │ │ │ │ │ │ │
│ C(r0) │ │ D(r0) │ │ C(r1) │ │ D(r1) │ │ X(r0) │ │ C(r0) │ │ D(r0) │
│ │ │ │ │ │ │ │ │ │ │ │ │ │
└───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘
The new approach.
┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐
│ │ │ │ │ │
│ A(r0) │ │ A(r1) │ │ A(r2) │
│ │ │ │ │ │
└─────────────────┘ └─────────────────┘ └─────────────────┘
│ │ │
│ Let's update the │ │
▼ state of this ▼ ▼
┌─────────────────┐ node to s1. ┌─────────────────┐ ┌─────────────────┐
│ │ │ │ │ │ Created
│ B(r0, s0) │ ───────▶ │ B(r1, s0) │ ───────▶ │ B(r2, s1) │ from
│ │ │ │ │ │ B(r1).
└─────────────────┘ └─────────────────┘ └─────────────────┘
│ │ │
┌──────┴──────┐ ┌─────────────┼─────────────┐ ┌─────────────┼─────────────┐
│ │ │ │ │ │ │ │
▼ ▼ ▼ ▼ ▼ ▼ ▼ ▼
┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐ ┌───────────┐
│ │ │ │ │ │ │ │ │ │ │ │ │ │ │ │
│ C(r0) │ │ D(r0) │ │ C(r1) │ │ D(r1) │ │ X(r0) │ │ C(r1) │ │ D(r1) │ │ X(r0) │
│ │ │ │ │ │ │ │ │ │ │ │ │ │ │ │
└───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘ └───────────┘
```
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D18229704
fbshipit-source-id: face6d0e5c240224ce49e93e783cff3172b60529
Summary:
We used to use `getTag` to check that some two nodes are clones of each other, not we have a dedicated method for that that exactly ensures that without reling on a sideeffect (wich tag equality is).
That is just much less error-prone.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D18231006
fbshipit-source-id: 6b247ed0eaded1fed8fd7fa820e80cd58602110c
Sidharth Guglani
Valentin Shergin
Will Holen
Marc Horowitz
Andres Suarez
Oleksandr Melnykov
Samuel Susla
Shingo Sato
Ramanpreet Nara
Peter Argany
Dmytro Kasianchuk
Riley Dulin