Summary:
This is a small micro-optimization in Diffing algorithm.
Seems we don't need to store full ShadowView objects in `insertedPairs` map, we can store only pointers to them. That can save memory and CPU cycles because we will not need to store full objects and copy shared pointers (which is somewhat expensive).
Reviewed By: mdvacca
Differential Revision: D15200498
fbshipit-source-id: 2a268c3ee80755555bff3317e10e679be1cf9830
Summary: Diffing is already pretty fast, but using move semantic should make it even faster. ShadowViews have shared pointers, so moving them can save us atomic counter bumps.
Reviewed By: mdvacca
Differential Revision: D15200496
fbshipit-source-id: 6fb0eb79e07cd6ae9b3100713497c634f306bc18
Summary: Convert FabricUIManager.measure params to floats. Currently we convert parameters to ints across the JNI boundary, and then back to floats several times in Java. This is unnecessary and actually makes measurements trickier. The new implementation uses floats across the JNI boundary and uses Float.POSITIVE_INFINITY to represent unconstrained values, which is consistent with Fabric C++ as well.
Reviewed By: shergin, mdvacca
Differential Revision: D15176108
fbshipit-source-id: cf849b3773007637f059279460163872f300a4aa
Summary: Workaround for bugs with originalConsole.assert firing when it shouldn't.
Reviewed By: Hypuk
Differential Revision: D15201459
fbshipit-source-id: d4cf648725cf42754561468b23ea8edd7c1b84b2
Summary:
On iOS, `RCTRedBox` will not update the displayed stack trace if the message string sent with the update differs from the original error message. As JS errors are shown in two stages - before and after symbolication - there was previously a case where the message would differ between the two `updateExceptionMessage` calls, blocking the update and leaving only the unsymbolicated trace visible. This diff fixes that.
Longer term, we should also change `RCTRedBox`'s logic to rely on the JS-provided `exceptionID` instead of the message string - similar to what we do on Android.
Changelog:
[iOS] [Fixed] - Fix redbox JS symbolication when adding JS engine tag to the message
Reviewed By: sahrens
Differential Revision: D15202524
fbshipit-source-id: 237fc090e88b0c609865e0aed842d6a609c1239a
Summary: Looks like FBJNI exports a C Macro that does exactly what `throwIfJNIReportsPendingException` does. Therefore, I'm replacing `throwIfJNIReportsPendingException` with calls to `FACEBOOK_JNI_THROW_PENDING_EXCEPTION()`.
Reviewed By: mdvacca
Differential Revision: D15174820
fbshipit-source-id: 9dfb519352cbd5f37527675323cbabad05e31d4a
Summary:
`jclass` in `JNI` is just a regular local reference. Therefore, it's unsafe to keep a static reference to it. Link: http://journals.ecs.soton.ac.uk/java/tutorial/native1.1/implementing/refs.html.
This bug made it so that when you clicked on `getConstants` twice in the TurboModule playground, the app would crash.
Reviewed By: mdvacca
Differential Revision: D15174821
fbshipit-source-id: 13b2b8726473acc9b07306558044d26bed0db92d
Summary: Previously, we'd override the `TurboModule::get` method inside the `JavaTurboModule` class to return a special `jsi::Function` in the case that the property being accessed was "getConstants". We really don't need to do this because we can simply special-case the invocation of the `getConstants` method inside the `JavaTurboModule::invokeJavaMethod` method.
Reviewed By: mdvacca
Differential Revision: D15174822
fbshipit-source-id: 0ee705be841757d3870c908da911c3872b977a9f
Summary: We conducted an experiment with different measure cache sizes. This has now been deallocatedi (D15183473). Remove the necessary APIs.
Reviewed By: SidharthGuglani
Differential Revision: D15183486
fbshipit-source-id: a38fa5a3ab0321c2521265f7d1cd6b495efd76cf
Summary:
@public
`YGConfig::YGConfig(YGConfig*)` was not initializing the same fields as the default constructors.
Here, we make the default constructor delegate to the more specialized one to remove duplication.
Reviewed By: SidharthGuglani
Differential Revision: D15164599
fbshipit-source-id: 27247709091b7664386057d09ac67d481877871f
Summary:
In fabric, the measureLayout method expects 'node ref' instead of 'node handle'.
Node refs are supported by the current production version of RN and for Fabric, no changes should be expected in the current production version of RN
Reviewed By: TheSavior
Differential Revision: D15103116
fbshipit-source-id: cde94f61eaf6aa52ae4bd6f89082d18141d0da28
Summary:
* invokeMethod() ends up not useful because each platform has its own way of invoking the platform methods
* invalidate() is not necessary because there's already the destructor of each C++ class
Reviewed By: mdvacca
Differential Revision: D15187833
fbshipit-source-id: 9478ed1e6288da30c67179e03a7bc7da6043280b
Summary: This new target provides dependencies for using folly futures
Reviewed By: willholen
Differential Revision: D15018282
fbshipit-source-id: c38ad4775102b9f0c10b3a52c5a18f00aa398322
Summary: This make iteration work better without needing to clean as much
Reviewed By: willholen
Differential Revision: D15018285
fbshipit-source-id: 034f5529e2e51711aeaa75360ad10bb1f85c7fb8
Summary:
The path to copy log_severity.h could refer to the
destination, which would result in an empty file being copied on some
rebuilds.
Reviewed By: willholen
Differential Revision: D15018283
fbshipit-source-id: 0081526a9686de8c74753738c165753de6dda18d
Summary:
@public
We want to enable tooling, instrumentation, and statistics within Yoga without coupling these functionalities to our core code.
This commit introduces the foundations of a simple, global event system.
For the time being, we will only support a single subscriber. Should we require more than one, we can add support for it later.
Reviewed By: SidharthGuglani
Differential Revision: D15153678
fbshipit-source-id: 7d96f4c8def646a6a1b3908f946e7f81a6dba9c3
Summary:
Previously we computed the list of nodes that need to be notified about layout changes using a list of mutation instructions. That was fine, but that's not really compatible with some other changes that I plan to make, so I decided to change it (make it better).
Besides the better design (debatable; fewer dependencies to unrelated moving pieces), here is why I believe the new way is more performant:
* The new approach has no `dynamic_casts`, whereas the previous has tons of them (two per a mutation). If a `dynamic_cast` takes 10 ns, for 500 nodes it can take up to 5ms only for casts. (Non-scientific assumption.)
* After removing dependency to mutation instruction, we can enable flattening for views which have `onLayout` event.
Reviewed By: mdvacca
Differential Revision: D15110725
fbshipit-source-id: 31a657ccfd02441734ad1d71a833653223163289
Summary:
It's easy to accidentally trigger this invariant when adding / moving around a component that relies on a FlatList.
There might be some unexpected behavior when this occurs, i.e. messed up virtualization / viewability logging. But to me, that is a better outcome than crashing the JS context.
Reviewed By: sahrens
Differential Revision: D14975295
fbshipit-source-id: 18015a780a153aae995723b120440be0e55d8e8b
Summary: Instrumentation tests are expensive and flaky. Luckly this one does not need to be instrumentation one.
Reviewed By: mdvacca
Differential Revision: D15158985
fbshipit-source-id: 3c88e5a0d82db2cd00f5866c3f9956409cc8fc7f
Summary:
Currently, every time a touchable is pressed on Android, a system sound is played. It was added in the PR #17183. There is no way to disable it, except disabling touch on sound on the system level. I am pretty sure there are cases when touches should be silent and there should be an option to disable it.
Related PRs - #17183, #11136
[Android][added] - Added a touchSoundDisabled prop to Touchable. If true, doesn't system sound on touch.
Pull Request resolved: https://github.com/facebook/react-native/pull/24666
Differential Revision: D15166582
Pulled By: cpojer
fbshipit-source-id: 48bfe88f03f791e3b9c7cbd0e2eed80a2cfba8ee
Summary:
@public
Makes bitfield getters/setters part of the bitfield ref template.
Since we introduced the tracking bit as template parameter in D14933022, every bitfield ref is an individual class anyway, and having function pointers doesn’t potentially lead to less code generation anyway.
Furthermore, this change can (in the absence of tracking bits) avoid less specialized templates dealing with refs, and to dynamic dispatch as a consequence.
Reviewed By: SidharthGuglani
Differential Revision: D15085495
fbshipit-source-id: 0dd70fa05e9d43a29e38a619cddb642c9ca3f7ab
Summary:
@public
In order to optimise property storage, we have to know how style properties are used in our apps.
Here, we add a bitmask that allows us to track which properties are set explicitely, and use that for our analysis.
Reviewed By: SidharthGuglani
Differential Revision: D14933022
fbshipit-source-id: 1ab8af562b14baba1d02057e527aa36d5c9a7823
Summary:
@public
The extra overload of `updateStyle` introduced in D15078961 can also handle `BitfieldRef`.
That means that we can remove the more specific implementation previously introduced for `BitfieldRef`
Reviewed By: SidharthGuglani
Differential Revision: D15081069
fbshipit-source-id: 98f1f3478627974c5273c85d268ca07350f303d7
Summary:
@public
Change style property accessors to return `Ref` instances instead of references to `CompactValue`.
This will allow to track assignments to properties later on, e.g. for instrumentation or dynamic property storage.
Reviewed By: SidharthGuglani
Differential Revision: D15078961
fbshipit-source-id: 259f05f7d30f093c04bf333c5bd4fb3601b8e933
Summary:
When acquiring the `PARTIAL_WAKE_LOCK`, Android requires a tag to identify the source, normally the class name. This tag will show on dumpsys call and Google Play developer console.
`getSimpleName` will work fine as long as not enable ProGuard, in my case, it transformed the class name to just `"c"`, and I take my half day to find where the `c` comes from.
`getCanonicalName` will add the package path, which is more friendly for developers.
Later we can even let the developer choose the tag name, but this will require API break changes.
[Android] [Changed] - Use class canonical name for PARTIAL_WAKE_LOCK tag
Pull Request resolved: https://github.com/facebook/react-native/pull/24673
Differential Revision: D15164306
Pulled By: cpojer
fbshipit-source-id: fd65f9e5250c180b0053940b17877fe36af5d48b
Summary:
Start a `HeadlessJsTaskService` on Android without registered is dangerous on apps because `HeadlessJsTaskService` will acquire a [`PARTIAL_WAKE_LOCK`](https://developer.android.com/topic/performance/vitals/wakelock), without calling `onHeadlessJsTaskFinish` this lock won't release until timeout(if exist). This lock will prevent the android device from sleeping.
Although on JS will throw an error if no headless tasks registered, but it's hard to notice while app in the background. No visual information is displayed.
This PR will log a warning instead of Error, and just mark the task to finished on native if nothing registered in order to release the wake lock.
[Android] [Fixed] - Fix unexpected PARTIAL_WAKE_LOCK when no headless tasks registered.
Pull Request resolved: https://github.com/facebook/react-native/pull/24671
Differential Revision: D15164310
Pulled By: cpojer
fbshipit-source-id: 05b62017ba094d0faabc2848dc8bb6c26101321b
Summary: This diff exposes the Legacy method UIManager.measureInWindow as part of Fabric
Reviewed By: shergin
Differential Revision: D15110795
fbshipit-source-id: 2b4bf47452f7272fd3edc4e580e65ae7ec2f2622
Summary:
Different frameworks use different kinds of floats, optional floats, and floats with assigned unit names. All those approaches use different ways to represent undefined and empty values. To deal with it we need to have some helper functions.
So, this diff changes some ways that we convert some corner values (like NaN and empty value). That change is motivated by recent personal discoveries in this field that shifted my vision on that. E.g. ComponentKit does not use `CGFloatMax` value as `Infinite` value. UIKit is also (surprisingly to me) okay with using `Infitite` instead of `CGFloatMax`. And, in general, seems using really conceptually appropriate values (instead of UIKit-inspired ones) it's the right thing to do.
Reviewed By: mdvacca
Differential Revision: D15155189
fbshipit-source-id: 33e15141f1ca3efb400a7160811224335de34ba1
Summary: `kFloatUndefined` means "no value here", but in this particular case, we have to have `Infinity` value that represents maximum available space.
Reviewed By: mdvacca
Differential Revision: D15155190
fbshipit-source-id: d2de20681ad04da7444331eff44b93d2bd0200e3
Summary:
We don't need to have those constants because this functionality is available in STL via `std::numeric_limits<YourParticularFloatType>::infinity()` (or `::min()` and `::max()`).
At the same time usage of `kFloatMax` was replaced with `Infinity` (which is a different value). Using `max` instead of `Infinity` was an attempt to mimic iOS/UIKit model where `Infinity` and `NaN` values usually are not being used. However, now this does not seem like a good idea. This concept is not used anywhere else (even in CK which is totally incompatible with it) and de-facto in RN we use it only in few places. So, let's use Infinity in places where it's logically appropriate.
Reviewed By: mdvacca
Differential Revision: D15155191
fbshipit-source-id: 4d24350c7540cec074a8b040d7c13f257aa812e7
Summary: This diff exposes the Legacy method UIManager.measureLayout as part of Fabric
Reviewed By: shergin
Differential Revision: D15103117
fbshipit-source-id: 4cf7ab3776f6a541cf0d6a00789420a0bb008fae
Summary: This diff migrates VirtualizedList to use ref.measureLayout instead of UIManager.measureLayout, this is a pre-req to make measureLayout to work in Fabric
Reviewed By: JoshuaGross, TheSavior
Differential Revision: D14865762
fbshipit-source-id: 45dd3374813370188c914acfc7e631075508f74a
Summary: The map of sComponentNames ONLY contains the names of components that are different between JS and Android. This diff adds a method to unify the way we use this map.
Reviewed By: shergin
Differential Revision: D15076549
fbshipit-source-id: 9df750dca305e55cb44037bc63f3ebb6476c8b81
Summary:
This diff changes the style of the Modal container style as a pre-requisite to implement Modal on Fabric.
In the current version of React Native the size of the View container is determined at runtime using the size of the screen. The size of this view is set overriding the Widht and Height of this Container View (using the screen size), in Fabric we can not update the size of that view, instead we set the size of the RCTModalHostView to take the size of the whole screen.
Since the RCTModalHostView has an absolute position and it has the size of the screen, making this change should be enough to keep backward compatibility and be able to implement Modal on Fabric
Reviewed By: JoshuaGross
Differential Revision: D15067616
fbshipit-source-id: d7302ef3afc503adfee10e12fb6d0ebd371fb7ed
Summary: This adds support for specifying the exact selector name for each exposed ObjC method. This allows us to avoid dynamic method lookup every time a method is called from JS.
Reviewed By: RSNara
Differential Revision: D15141611
fbshipit-source-id: ed2820782ab013369e4e1f22dbce31d9838a17bb
Summary:
RNTester currently has four third party dependencies that are fetched from the network at build time:
- glog
- double-conversion
- boost
- folly
These dependencies are cached to ~/.rncache by default. This location may not be ideal for use in CI, therefore the cache location is now made configurable via a RN_CACHE_DIR envvar.
Reviewed By: cpojer
Differential Revision: D15141391
fbshipit-source-id: b51d749412c49500a657bd18e4c9520ddb30ea2c
Summary:
The `testBundleURL` test was disabled in open source recently due to issues running it successfully in Facebook's internal CI. We're now skipping `RCTBundleURLProviderTests` tests internally, so it's safe to re-enable now and ensure it runs in Circle CI.
Changelog:
[iOS] [Changed] - Re-enable testBundleURL unit test.
Reviewed By: cpojer
Differential Revision: D15140192
fbshipit-source-id: 5f6a91f3ce8cea245be31dff3ffb86768deab0be
Summary:
`OnValueChange` function of `Picker` is called when Picker is initialized.
[Android][fixed] - `OnValueChange` will be called only when the `selectedValue` changes.
Pull Request resolved: https://github.com/facebook/react-native/pull/24653
Differential Revision: D15146483
Pulled By: cpojer
fbshipit-source-id: e26f5aa4caa673015c50c853f00b99572e803755
Summary:
Bump the `react-native-community/eslint-config` version so we can release it.
[General] [Changed] - Bumped react-native-community/eslint-config version
Pull Request resolved: https://github.com/facebook/react-native/pull/24650
Differential Revision: D15146456
Pulled By: cpojer
fbshipit-source-id: 9a6767f502f1b5dcd49cf55b4e35c0d851f33222
Summary: Trivial diff that adds extra logging information on Exceptions that are thrown by the FabricViewTest
Reviewed By: shergin
Differential Revision: D14817899
fbshipit-source-id: 32e1d1fcd1292715dfcf2750d3f14c668927c8b8
Summary:
This diff fixes a bug that is reproducible when a view is reordered in a different level of hierarchy in the react tree.
Even if this is not supported by react, this can still happen because of viewFlattening.
Reviewed By: shergin
Differential Revision: D14817452
fbshipit-source-id: 13425b0e6a280affe681e80b4a6daa17ee56251a
Summary: Apparently we can/should not have in RCTConversions because it creates unnecessary dependency to core iOS module.
Reviewed By: mdvacca
Differential Revision: D15055325
fbshipit-source-id: 507f5a40c03b5c261967de4504297d31ecd02783