Summary:
This diff fixes a bug in the calculation of layout for text components
The rootcause of the bug is that fabric is not taking into consideration height constraints as part of the cache for text measurments.
The title text was being measured with a specific height constraint (22px) at the begining of the render, later there was a re-measure for the same Text component with a different height constraint, but fabric was reusing the result of the first calculation instead of re-measuring the text.
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D26676716
fbshipit-source-id: 3e769e0ca35b3e363b96d3a6d1626a091eaad908
Summary:
I thought we'd need LayoutDirection on the platform at some point, but it turns out we never use it, and don't seem to need it since components just query via `I18nUtil` one time,
and it's expected that apps restart if language changes and we need to switch between LTR and RTL. So, it seems like we'll not have any need for this on the platform at any point.
And we can save a byte per layout instruction now. Huzzah!
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26660908
fbshipit-source-id: 54c7d132f5fa260a93fc7f09f7cf63059d52ed1f
Summary:
We recently fixed RTL scrolling in Fabric on iOS: D26608231 (https://github.com/facebook/react-native/commit/e5921f7f384af45df4f355fa3fa1b58a20a269d3)
Turns out, the mechanism for RTL scrolling on Android is completely different. It requires that content be wrapped in a "directional content view", which is `View` in LTR and `AndroidHorizontalScrollContentView` in RTL, backed by `ReactHorizontalScrollContainerView.java`.
iOS doesn't require that and just uses View and some custom logic in ScrollView itself.
In the future it would be great to align the platforms, but for now, for backwards-compat with non-Fabric and so we don't have to tear apart ScrollView.js, we codegen the AndroidHorizontalScrollContentView so it exists in C++, register the component, and stop mapping it to View explicitly in C++.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26659686
fbshipit-source-id: 3b9c646dbdb7fe9527d24d42bdc6acb1aca00945
Summary:
Changelog:
[General][Added] - Added `debugName` parameter to `renderApplication` to use as the display name for the React root tree
Reviewed By: rickhanlonii
Differential Revision: D26637787
fbshipit-source-id: 3ddc037573f4434101a9d3dcb5592a127193481c
Summary:
"The instance should not stick around after unmount..." - Tim Yung, 2021
I have a hypothesis that, if a component instance of an animated component sticks around after unmount, it could cause memory leaks due to references to Fabric ShadowNodes across the JSI (this would not impact non-Fabric... in theory).
Wild guess. If OOMs disappear then maybe this hypothesis is correct, but it's a long shot. I figure there's ~no harm in doing this cleanup here anyway.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D26650348
fbshipit-source-id: 90633db650b65755cacfb52344e7b53e46c9b125
Summary:
There's logic in Animated JS that prevents flattening of animated views in Fabric. However, we cannot actually detect Fabric vs non-Fabric in the first render pass; in the past we defaulted to assuming non-Fabric. Now we assume Fabric for View flattening purposes.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D26647393
fbshipit-source-id: c91b51aeeb4f352cc502bc018f086e36fd1ffd85
Summary:
Changelog: [Internal]
Adds react_debug dependency in Android.mk where it was missing
Reviewed By: mdvacca
Differential Revision: D26617400
fbshipit-source-id: 5ac799269b106eadd881d30490ac34bd2134a9b7
Summary:
allow-large-files
RN Tester is using an old version of Flipper. This will help testing regressions in the latest version (which is installed when starting a new project). This also fixes an issue where libevent is incompatible between the one in flipper and when using hermes on iOS. To fix it I changed to use the version published on cocoapods instead of using a local podspec (see https://github.com/facebook/flipper/issues/1916).
## Changelog
[General] [Changed] - Update flipper
Pull Request resolved: https://github.com/facebook/react-native/pull/31010
Test Plan:
- Tested that RN tester builds and flipper works with hermes enabled / disabled and fabric on iOS
- Tested that RN tester builds and flipper works on Android
Reviewed By: fkgozali
Differential Revision: D26592317
Pulled By: PeteTheHeat
fbshipit-source-id: 2cd278c7a51b1859dab0465846b061221f07d3f6
Summary:
## Issue:
Sometimes ReactTextView are vertically displayed as one column with bridgeless.
## Root cause:
After debugging, I found out this is caused by a workaround in 2016 to fix a crash caused by mLayout occasionally being non-null and triggers relayout during setText.
https://github.com/facebook/react-native/pull/7011
## Fix
Revert previous hack, if the crash happens again I'll try to fix it.
Changelog:
[Android][Fixed] - Fix text in ReactTextView sometimes being vertically displayed
Reviewed By: mdvacca
Differential Revision: D26581756
fbshipit-source-id: a373d84dc1ab3d787bda7ec82f2d0865a354cf60
Summary:
(This is the second version of the diff that includes additional checkes and syncronization around `start` and `stop` methods).
This diff migrates iOS renderer to using SurfaceHandler directly instead of calling Scheduler methods.
Major changes:
* RCTFabricSurface only stores a SurfaceHandler and platform-specific parts (such as a UIView instance and TouchHandler); all other pieces of state are stored in SurfaceHandler object.
* All the state changes initiated from RCTFabricSurface are performed via calling corresponding methods on SurfaceHandler (bypassing RCTSurfacePresenter, RCTScheduler, Scheduler, and UIManager).
* Every RCTSurfaceHandler is responsible for observing `UIContentSizeCategoryDidChangeNotification` and communicating the change down to SurfaceHandler. (Besides code simplifications it's more correct because on iOS the DPI actually depends on a particular UIScreen/UIWindow associated with UIView and *not* global for the whole app.)
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: ShikaSD
Differential Revision: D26614577
fbshipit-source-id: 428f4c4cc1ca46cd97be0413e9279e29a54807f6
Summary:
Changelog: [internal]
Check for null before passing it to `EventTarget` constructor.
Reviewed By: JoshuaGross
Differential Revision: D26605779
fbshipit-source-id: a2773c8123d83c25736bccefe656d1def8794091
Summary:
Changelog: [internal]
`shadowNodeFromValue` can return nullptr. Let's make sure it returns valid value before dispatching command.
Reviewed By: JoshuaGross
Differential Revision: D26605350
fbshipit-source-id: eb9a0347c95ba07fd7e9b7ddeca7e6d6011f50ad
Summary:
Since iOS 14 refresh control is sometimes visible when it shouldn't. It seems to happen when it is removed and added back to the window. This repros easily when using react-native-screens with react-navigation tabs. Inactive tabs are detached from the window to save resources.
Calling endRefreshing when refresh control is added to the window fixes the layout. It will also be called on first mount where it is not necessary, but should be a no-op and didn't cause any issues. I also decided to call it for all ios versions, although it is only needed on iOS 14+ to avoid forking behavior more.
## Changelog
[iOS] [Fixed] - Fix RefreshControl layout when removed from window
Pull Request resolved: https://github.com/facebook/react-native/pull/31024
Test Plan:
Before:
https://user-images.githubusercontent.com/2677334/108666197-93ea5a80-74a4-11eb-839b-8a4916967bf8.mov
After:
https://user-images.githubusercontent.com/2677334/108666223-9ea4ef80-74a4-11eb-8489-4e5d257299c8.mov
Reviewed By: shergin
Differential Revision: D26590759
Pulled By: PeteTheHeat
fbshipit-source-id: b8c06068a24446b261cbeb88ff166289724031f1
Summary:
Changelog: [Internal]
Debuggable flag defines multiple things for flavours including `NDEBUG` flag in native builds. We need to explicitly state this from build.gradle to use it.
Reviewed By: JoshuaGross
Differential Revision: D26610482
fbshipit-source-id: e0c8095e239241c57a119e561b125cab16bf299f
Summary:
Changelog: [internal]
Prevent crash when casting of state to `ImageShadowNode::ConcreteState` fails. This doesn't fix root cause of the problem but stops the app from crashing.
Reviewed By: JoshuaGross
Differential Revision: D26604807
fbshipit-source-id: 17a2ead56ac68e560070ed4defd364a9d1dfd1e8
Summary:
Changelog:
[General][Added] - Added an example showcasing how separator callbacks work in SectionList for RNTester
Reviewed By: nadiia
Differential Revision: D26575122
fbshipit-source-id: 46710e2647c84bdf083265ce04ba330bd70eb2a7
Summary:
Changelog: [internal]
### Why does the crash happen?
The crash can happen if runtime is destroyed before background executor lambda is run. Destroying a shadow node after runtime leads to a crash through the chain of ownerships.
Chain of ownership:
`ShadowNode -> ShadowNodeFamily -> EventEmitter -> EventTarget -> Pointer`
Pointer tries to call `invalidate` method on raw pointer to the runtime which is gone.
https://www.internalfb.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/jsi/jsi/jsi.h?commit=2ee3ae0c6a64&lines=335-339
To work around this, weak pointers are passed to lambda. This way the lambda is less likely to be the last owner of shadow nodes. Possibility of race still exists but it less likely to happen.
## Other solution
Alternatively, we could make sure native Runnable queue in Java is emptied as part of tear down process. We can even implement both solutions as they are semantically correct.
Reviewed By: shergin
Differential Revision: D26582554
fbshipit-source-id: b1b8a92237902bc4c40376176f575caa24a41a05
Summary:
Surprisingly, it's not that trivial to pass `LayoutContrants` to `YGNodeCalculateLayout` in a way that always works. The problem is that `YGNodeCalculateLayout` does not allow expressing the constraints explicitly, so we need to pass them as `YGStyle` properties of a root node. With this approach, we unconditionally apply them as `YGStyle`s as actual values or `Undefined` value (which overrides some other values that can be previously set by calling this function or other code). We also intentionally preserve `height` and `width` values because it's a common use-case when a component explicitly specifies its size.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D26583550
fbshipit-source-id: 2cd506fbdc9e6a1a8f119d09ccfd34f876a13625
Summary:
The purpose of this change is to make TTRC markers work similarly for bridge loading and bridgeless loading so we could compare performance between them.
There are mainly four cases involved:
```REACT_BRIDGE_LOADING_START,
REACT_BRIDGE_LOADING_END,
REACT_BRIDGELESS_LOADING_START,
REACT_BRIDGELESS_LOADING_END
```
First 2 are for beginning/ending of bridge loading which includes creating fragment, loading JS bundle, running JS bundle and creating react instance. Last 2 are for similar purpose with bridgeless.
Changelog: [Internal]
Reviewed By: lunaleaps
Differential Revision: D26514499
fbshipit-source-id: 65d6f3cc7de9e07a7a3a802dd77138e74c23aa5b
Summary:
Changelog:
[Internal] - Use lazy task APIs in the Gradle new plugin
Gradle now migrates to [lazy task APIs](https://docs.gradle.org/current/userguide/lazy_configuration.html) to help with project configuration time. It helps even in the rn-tester case, e.g. `./gradlew help` executes in 2-3s instead of 20s.
The migration is quite simple - use `Provider` and `tasks.register` instead of `task.create` everywhere.
Reviewed By: mdvacca
Differential Revision: D25946748
fbshipit-source-id: 2ccd2f881afe7601e506dc7adcc8a658c7267328
Summary:
Changelog: [Internal]
Extracts task definition for bundling and hermes binary into separate tasks in the new plugin.
Reviewed By: mdvacca
Differential Revision: D25915057
fbshipit-source-id: b1d8a4b5e8789c3b7832efea13274435c9391ccb
Summary:
Changelog:
[Android][Added] - Basic definition for react gradle plugin
Adds plugin and build configuration + copies config from react.gradle to extension.
Copies internals of react.gradle to the plugin. Will be refactored in the next commits.
Reviewed By: mdvacca
Differential Revision: D25693008
fbshipit-source-id: b0feaa02cee8a1ee94d032426d19c501ff3b2534
Summary:
Changelog: [internal]
shergin found that folly's merge_patch implementation doesn't propagate `null` correctly (details in D26435620 (https://github.com/facebook/react-native/commit/1e9f63fe277c42d812ef007ced7eff1688602b62)). This is a requirement and needs to be adjusted in props forwarding on Android.
As far as we know this isn't causing any bugs but it is an error that should be fixed.
Reviewed By: shergin
Differential Revision: D26545821
fbshipit-source-id: 9edd24aecfcde17f5d9c1197f65db0e0f3f9e364
Summary:
We have a custom STUB_VIEW_ASSERT that helps debugging stub view issues on platforms (like iOS) where flushing logs around assert-time isn't 100% guaranteed.
Move that logic into react_native_assert since it's generally useful.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26567218
fbshipit-source-id: 79f5ae66fc65a0af48dbcf4c7204ac8245911cb0
Summary:
See react_native_assert.{h,cpp}. Because of the BUCK+Android issue where NDEBUG is always defined, we use react_native_assert instead of assert to enable xplat asserts in debug/dev mode.
This migrates most of the codebase, but probably not 100%. The goal is to increase assertion coverage on Android, not to get to 100% (yet).
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26562866
fbshipit-source-id: a7bf2055b973e1d3650ed8d68a6d02d556604af9
Summary:
These assertions will already run by default on iOS, macOS, Windows, etc, all OSes except Android because of a longstanding Buck+Android NDEBUG issue.
Align platforms by using REACT_NATIVE_DEBUG flag.
Only impacts debug/dev builds.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D26561977
fbshipit-source-id: 324875c48b2a138e8ab55630c3e2ec43c2f768c3
Summary:
This PR fixes few suppressed Flow issues in `VirtualizedList` component.
I have also removed few no longer valid Flow suppressions in `ActivityIndicator` and `ReactNativeTestTools`.
## Changelog
[General] [Fixed] - Flow: fix few issues in `VirtualizedList`,
[General] [Removed] - Flow: remove few no longer valid suppressions in `ReactNativeTestTools`
Pull Request resolved: https://github.com/facebook/react-native/pull/30128
Test Plan: Successful run of `yarn flow` check.
Reviewed By: kacieb
Differential Revision: D26489398
Pulled By: lunaleaps
fbshipit-source-id: db71d8bfeb3b6dc5a2738a73ac6644aef4a0311b
Summary:
Changelog: [Internal]
This moves enabling/disabling fast refresh off of `bridge.enqueueJSCall` in bridgeless mode.
Reviewed By: sammy-SC
Differential Revision: D26290378
fbshipit-source-id: ed8a3389b9812cedf7181971656dacd98ff7ecfd
Summary:
Changelog: [Internal]
This diff allows `RCTDevSettings` to access bundleURL directly instead of relying on the bridge.
Reviewed By: RSNara
Differential Revision: D26289592
fbshipit-source-id: e22e583e51323bfe66f6e424d9f47edf71b1ed9f
Summary:
Changelog: [Internal]
Problem:
The Bridge holds and exposes the bundle URL for native modules to inspect via `bridge.bundleURL`.
Solution:
This follows the same pattern we've been using for attaching objects to native modules: declare a protocol with the required object, attach that object during module init.
Differential Revision: D26289581
fbshipit-source-id: 35ae83912e392be84f55c8d056c8da15ba75060f
Summary:
Changelog: [Internal]
I'm trying to get Fast refresh working in bridgeless mode. I need the ` require('./setUpReactRefresh');` line to be executed to do so.
I'm not sure why this was commented out in the first place, it seems to be working fine in FBiOS/FB4A.
Reviewed By: p-sun
Differential Revision: D26289573
fbshipit-source-id: 6151f781c31e3aadaebfeb759d3eb776e8b525cc
Summary:
Use REACT_NATIVE_DEBUG for consistent branding and to prevent potential collisions with other codebases.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26517424
fbshipit-source-id: c85740d4e5320cc14023eb6f521bb1a242ae56fe
Summary:
Use react_native_assert in LayoutAnimations to enable asserts to fire on Android.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26517096
fbshipit-source-id: f000c4848f29c8779170625d357f547f2e9e6365
Summary:
This is a backout of D24290778 (https://github.com/facebook/react-native/commit/33d6091cac5ed2f8cef06d4740c8b42fcd107271).
The original diff causes T85097896 and probably T85069775. For now I am going to unland this, and then I will re-land this after the cut with a fix.
Original commit changeset: 62b600c3f1b2
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D26515027
fbshipit-source-id: 057663b092e6b1d74db83c40d656b2ae142bd622
Summary:
This prop is iOS only.
Changelog:
[iOS][Added] Add an example showing the prop contentInset in SectionList
Reviewed By: lunaleaps
Differential Revision: D26491787
fbshipit-source-id: 31398388081a22ccd17cc256aa174ffe7764e7a9
Summary:
This will allow these asserts to crash on Android debug builds.
We will migrate more sites as we confirm this is stable through testing.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26409354
fbshipit-source-id: fb35cd8de29890f7c2b761435eaa02de377bdd1e
Summary:
BUCK always defines NDEBUG on Android builds. This is a longstanding issue and it's tricky to work around.
Previous attempts to fix this within React Native were difficult because disabling NDEBUG caused lots of issues that were difficult to track down.
Instead, I am (1) introducing a new RN_DEBUG flag that can be used cross-platform, (2) whenever NDEBUG is *not* enabled, RN_DEBUG will automatically be defined, (3) enables debug-only code to be compiled on Android, (4) enables us to selectively, slowly migrate `assert` to `rn_assert` in a way that doesn't impact non-Android platforms, but allows us to maintain stability of Android debug builds.
Actually enabling the RN_DEBUG flag in debug builds is done in FB-internal code. I assume the NDEBUG issue is not a problem when compiling in open-source without BUCK.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26409355
fbshipit-source-id: 285b8073bba3756834925727bfa28d3c6bc06335
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/30988
We have a bunch of flags scattered throughout the codebase with poor hygiene and commenting. Consolidate.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26392518
fbshipit-source-id: 2823de123a5009d6b8c358e8a3f451b9fa0e05b7
David Vacca
Stiopa Koltsov
Joshua Gross
Rubén Norte
Andrei Shikov
Janic Duplessis
Lulu Wu
Valentin Shergin
Samuel Susla
Nadiia D
Luna Wei
simek
Peter Argany
Pieter Vanderwerff
Paige Sun
Kacie Bawiec