Summary:
~~⚠️ Depends on https://github.com/facebook/flipper/pull/1086 and a new Flipper release.~~
Fixes 17f025bc26 (commitcomment-38831234)
Currently user’s are being told to add a definition of the `FB_SONARKIT_ENABLED` macro and examples, including those in stock React Native templates, set this for the user by making use of a `post_install` hook in the user’s `Podfile`. This leads to confusion, fragile code [when a user’s project dir structure deviates from vanilla], and is ultimately not necessary as CocoaPods already has dedicated mechanisms to:
* specify build settings (through the `xcconfig` property);
* and selectively include certain pods only in certain build configurations (e.g. debug).
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[iOS] [Changed] - Entirely control Flipper being enabled through inclusion in Podfile and optionally limiting to certain build configurations using the `:configuration` directive.
Pull Request resolved: https://github.com/facebook/react-native/pull/28796
Test Plan: Tested using the changes of https://github.com/facebook/flipper/pull/1086 in a new app that uses RN `master`.
Reviewed By: priteshrnandgaonkar
Differential Revision: D21449754
Pulled By: passy
fbshipit-source-id: 9ff7c7f4ffc32b364b1edd82b94e0b80c3997625
Summary:
Pass a default empty object to `AppContainer` if no `initialProps` were passed to `renderApplication`.
This prevents issues on Android, where we do not pass a default empty `initialProps` from native, as we do on iOS.
Changelog: [General] [Fixed] - Handle nullish `initialProps` correctly in `renderApplication`
Reviewed By: motiz88
Differential Revision: D21448692
fbshipit-source-id: 9630bdc2414532999abf3bf9da25047f0482fcab
Summary:
We consume Hermes through multiple .so's, which means we have multiple (weak) typeinfo definitions of facebook::jsi::JSError. Previously we were using gnustl, which would strcmp typeinfo to decide whether a certain exception handler applies, which meant this didn't cause any major issues. However since this is deprecated, we recently switched to libc++, which does not have this by behaviour (or it does, but behind a flag I'm not sure how to enable). This causes any JS exceptions to fall through from our exception handlers and fatal the app.
This problem is actually documented in the common Android NDK problems page: https://android.googlesource.com/platform/ndk/+/master/docs/user/common_problems.md#rtti_exceptions-not-working-across-library-boundaries
The suggested solution is to ensure that any exception types have a key function defined (a non-pure, out-of-line virtual function). The simplest one to add is a virtual destructor. This makes the object file that holds the implementation of the destructor export a non-weak typeinfo definition which will at load time override the other weak versions.
I'm not sure why we're the first to hit this. RN's JSIExecutor doesn't explicitly reference JSError which probably helps (https://github.com/facebook/react-native/blob/master/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp#L256-L258) and they also don't use unguarded callbacks like we do.
Changelog: [Internal]
Reviewed By: mhorowitz
Differential Revision: D21426524
fbshipit-source-id: 474284ada1ca2810045dc4402c420879447f9308
Summary:
Move and create an empty rule that redirects as well, to handle //arvr rules
Need to do this way, since ovrsource sync rules are in different repo.
allow_many_files
allow-large-files
Steps:
- [X] Move glog from xplat/third-party to /third-party
- [ ] Update references in ovrsource to translate to //third-party instead of //xplat/third-party
- [ ] Get rid of temporary rule
- [ ] Update fbsource/third-party/glog to 0.3.5 (what we have in ovrsource)
Changelog: [Internal] Update reference for glog from xplat/third-party to /third-party.
Reviewed By: yfeldblum
Differential Revision: D21363584
fbshipit-source-id: c1ffe2dd615077170b03d98dcfb77121537793c9
Summary:
`SynchronousEventBeat` and `AsynchronousEventBeat` are a cross-platform re-implementation of run loop related parts of `MainRunLoopEventBeat` and `RuntimeEventBeat` (iOS specific classes for now). In the future, they will replace iOS- and Android-specifc event beat classes.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21341996
fbshipit-source-id: 8eda9a5df537cd666b7728e32212a8bb5ddb3ab7
Summary:
`RunLoopObserver` is one of the core interfaces that bridge intrinsically platform-specific functionality to cross-platform React Native core. `RunLoopObserver` allows subscribing for notifications about changes in a run loop life cycle. Primarily it supposed to be used for observing UI (aka main) and JavaScript execution thread/run-loop.
Having a `RunLoopObserver` implemented in a platform-specific manner allows building these components in a cross-platform manner:
* Sync and async UI event delivery pipeline;
* Timing for some animation engine;
* Timers (probably additional features are required).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21341997
fbshipit-source-id: 7ef61fb51f550dd0f2e89c64af657e0f0de029aa
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/28833
The checked-in RNTester podfile was generated using Cocoapods version 1.9.1. This doesn't match the version currently used in CircleCI, which is 1.8.4. In order to update the offline mirrors and land them without breaking CircleCI, it seems we need to switch back to 1.8.4. This diff updates the podfile back to 1.8.4 and updates the offline mirrors.
Reviewed By: fkgozali
Differential Revision: D21392989
fbshipit-source-id: b14aa6e2798175534e9416410ba9d6877fb718c0
Summary:
This diff fixes a NullPointerException thrown when calling measureLayout function on a virtual node.
changelog: [Android] Fix measureLayout function for VirtualTexts
Reviewed By: JoshuaGross
Differential Revision: D21435030
fbshipit-source-id: aba6d81f333464e49d2d769b111842e7ae8ce769
Summary:
Quick diff to log of content of UpdateState mount item. This is useful for debugging. Note this will ONLY be logged when the constant FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT is set to true
changelog: [Internal][Android] internal log for fabric android
Reviewed By: JoshuaGross
Differential Revision: D21428345
fbshipit-source-id: d000eb6dbdd39d15935fa2102072790e17372682
Summary:
Only breaking changes appear to be dropped Node 6 support: https://github.com/yargs/yargs/blob/master/CHANGELOG.md. Deduplicates quite a few copies of Yargs, yay!
Changelog: [Internal]
(Note: this ignores all push blocking failures!)
Reviewed By: motiz88
Differential Revision: D21426137
fbshipit-source-id: b091e29ac2d9464d6ce9a716a99f7ae156a91a01
Summary:
Before this change, in case of incorrect measurements, Fabric's implementation of `measure` and `measureInWindow` incorrectly returned negative height and width. Now it returns zeros (as classic React Native does).
Fabric:
This does not fix `measureLayout` called for virtual nodes. This is not so trivially to fix and it will be done separately.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross, yungsters, mdvacca
Differential Revision: D21433239
fbshipit-source-id: fbaf5ee35c690506822c634daac4426542c2cdcf
Summary:
Workaround for a bug with Fabric when pressing on virtual text.
Changelog:
[Internal]
(Note: this ignores all push blocking failures!)
Reviewed By: JoshuaGross
Differential Revision: D21432793
fbshipit-source-id: fe20eeadd5365707fb71edae7a76d374e26b4c86
Summary:
For some reason the specs were internal, but the native impl is still in github. So let's move these to github for consistency.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D21419934
fbshipit-source-id: f2c4486edca43c4348f3a3c6ce98f76a322bab0b
Summary:
Based on feedback we're updating these titles to be more clear for their source.
Changelog: [Internal]
Reviewed By: cpojer
Differential Revision: D21413486
fbshipit-source-id: c144e7f759a4ff263b7ec80fa643eeb8ffac741b
Summary:
This diff fixes an off-by-one error probably caused by my font ligatures where when exactly two exceptions are thrown at the same time we would show the second exception instead of the first. If three or more were thrown, we would show the second.
I also fixed some tests that had the wrong descriptions and wrong behavior enforced.
Changelog: [Internal]
Reviewed By: cpojer
Differential Revision: D21413186
fbshipit-source-id: 8e2940c89251dc042b10c6a2a2186089b6e7b53d
Summary:
In iOS 11, [CADisplayLink](https://developer.apple.com/documentation/quartzcore/cadisplaylink)'s frameInterval was deprecated in favor of preferredFramesPerSecond, but these two properties have different underlying assumptions.
- set frameInterval to 2 for 30fps
- set preferredFramesPerSecond to 30 for 30fps. When you use preferredFramesPerSecond, assume frameInterval is 1.
This fix ensures gifs in <Image> component will animate at same speed regardless of framerate.
Reviewed By: shergin
Differential Revision: D21414014
fbshipit-source-id: 40ab23bab1990cf65d2802830b6835f350999537
Summary:
Android ScrollView/HorizontalScrollView `smoothScrollTo` contains some logic that, if called multiple times in a short amount of
time, will treat all calls as part of the same animation and will not lengthen the duration
of the animation. This means that, for example, if the user is scrolling rapidly, multiple
pages could be considered part of one animation, causing some page animations to be animated
very rapidly - looking like they're not animated at all.
We use a custom animation to perform `smoothScrollTo` to improve the UX.
This resolves a longstanding issue in non-Fabric RN, as well as Fabric, since this code is shared between the platforms.
Changelog: [Update] Android ScrollView/HorizontalScrollView scrolls using custom animations instead of default Android `smoothScrollTo` implementation, leading to smoother scrolls for paginated ScrollViews
Reviewed By: mdvacca
Differential Revision: D21416520
fbshipit-source-id: 6ebe63cb054a98336b6e81253d35623fe5522f89
Summary:
Import folly to handle optionals (`folly::Optional<__type__>`)
Sort modules and indent generated code to match output from the old codegen. While not strictly necessary as these are generated files that should not be edited by hand, I found that matching the old codegen in this regard made it less of a chore when it came to comparing the output of both codebases.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D21395231
fbshipit-source-id: 289d617d7a2d93724456c80afea57a49c108cb9b
Summary:
Fixes https://github.com/facebook/react-native/issues/28726
When importing TouchableOpacity, it is treated as any by Flow. Replacing ComponentType with AbstractComponent works.
The [Flow documentation](https://flow.org/en/docs/react/types/#toc-react-componenttype) says the following about ComponentType:
> Note: In 0.89.0+, React.ComponentType is an alias for React.AbstractComponent<Config, any>, which represents a component with config type Config and any instance type.
So I'm thinking that since the instance type is treated as any with ComponentType, Flow treats TouchableOpacity as any as well.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[General] [Fixed] - Fix Touchable{Opacity,Bounce,Highlight} being exported as `any` (Flow)
Pull Request resolved: https://github.com/facebook/react-native/pull/28737
Test Plan: I have done the same changes to react-native in my project's node_modules and seen that the components TouchableOpacity went from any to AbstractComponent with some props. Now I have a bunch of errors because I'm sending in wrong props to some touchables, which is good!
Reviewed By: cpojer
Differential Revision: D21362601
Pulled By: TheSavior
fbshipit-source-id: 5b98cc79eaef034eccdb7f47242f9f44be2ef2b8
Summary:
We are currently very strict about breakpoint location matching. This
diff allows some fuzz in the column, but not in the line.
Changelog: [Internal] Setting Hermes breakpoints no longer requires exact column match
Reviewed By: avp
Differential Revision: D21343198
fbshipit-source-id: a59786a9d63f9fe1ed576835ed660ba3343affe1
Summary:
`renderApplication` receives the root component that we need to render and an optional wrapper component. There are cases where we want to use the initial props passed to the root component in the wrapper component as well (e.g.: to provide a specific context to the root component), so this adds modifies `AppContainer` to accept the initial props and inject them into the wrapper component.
Changelog: [General] [Added] - Modified `renderApplication` to forward `initialProps` to `WrapperComponent`
Reviewed By: fkgozali
Differential Revision: D21347486
fbshipit-source-id: 1c4f702a3875077630de1a44d3ac9ef2c80bc10c
Summary:
Since support for contentOffset was added to horizontal ScrollView on android (30cc158a87) I'm seeing a crash in my app because of a library. What happens is that it passes a partial object for contentOffset so something like `{x: 1}` which causes a crash on Android.
According to the flow types the object should always contain both x and y but I think we should preserve the runtime behaviour and just use 0 like iOS does.
## Changelog
[Android] [Fixed] - Allow passing partial contentOffset to ScrollView on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/28817
Test Plan: Tested that passing partial object for contentOffset does not crash.
Reviewed By: JoshuaGross
Differential Revision: D21396319
Pulled By: shergin
fbshipit-source-id: 4b52c868e3bfe183ff7f68a76ac34d1abd5e1069
Summary:
The core issue solved in D21389371 was that erased elements of a TinyMap were being iterated over, because TinyMap has somewhat-complicated logic around cleaning out the underlying vector. In some very marginal cases, vectors were not being cleaned and an iterator pointing at erased elements was being returned.
The diff prevents some possible crashes in `begin()` and `find()` while making it much less likely to iterate over erased elements.
We also add a unit test to catch the case fixed in D21389371, in particular.
We also are keeping the code added in D21389371 (for now) since it's a cheap check, and will be a safeguard until we have rigorous testing around TinyMap. To be clear that logic should noop currently, but will prevent crashes in case guarantees around TinyMap change in the future.
Currently there is only one line of code that actually uses the TinyMap iterator, so this should be safe.
Reviewed By: shergin
Differential Revision: D21392762
fbshipit-source-id: 36dc998958c230fad01af93338974f8889cbcf55
Summary:
Before, compilation fails with P130281113. After fixing BUCK target, fails with P130281201.
After all changes, the tests succeed.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21390996
fbshipit-source-id: c85aa43b3ef7fc9642d226ae706c937b2a5a408d
Summary:
While #import is common in Objective C, it's a vendor specific extension in C++, only supported by GCC/Clang, and only when -pedantic is off. Its use causes build breaks with MSVC. Replace it with the standard #include.
## Changelog
[Internal] [Fixed] - Don't use #import in C++ Code
Pull Request resolved: https://github.com/facebook/react-native/pull/28825
Test Plan: We've ben running this change within react-native-windows for some time.
Differential Revision: D21391233
Pulled By: shergin
fbshipit-source-id: c0f94f314c46d6ac24067bbdcd5aaaeec9da283f
Summary:
Add a `console.warn()` call when calling `_scrollRef.scrollTo`, because `scrollTo` is not guaranteed to exist on `_scrollRef`.
Context:
`VirtualizedList` holds `_scrollRef`, which is usually a reference to a `ScrollView` component. However, there are several cases where it holds a `View` or other type of component instead.
A custom component can be passed in to `renderScrollComponent`, and then `_scrollRef` will point to that custom component. Additionally, if two VirtualizedLists are nested with the same orientation, `_defaultRenderScrollComponent` will return a View instead of a ScrollView.
Due to these possibilities, `_scrollRef` is not guaranteed to have a `scrollTo` method.
Changelog: [General] [Added] - Add warning when scrollRef does not have a scrollTo method
Reviewed By: JoshuaGross, TheSavior
Differential Revision: D21386842
fbshipit-source-id: 01e167e0ae0edea8f29853e8b242ce88a5103b49
Summary:
When we call `.erase` on the TinyMap, it sets the key of the element to 0. When we call `.begin()` later, TinyMap will sometimes, but not always, clean up the underlying Vector. This means that *most* of the time, underlying erased elements will be removed from the Vector; but sometimes erased elements will still be there when iterating over it.
This was causing us to generate extra "Create" mutations.
To fix this, for now we just check for zeroed-out elements when iterating over the vector.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21389371
fbshipit-source-id: 1e641050987d40a3f3e31499dcb373cfb28ae6f8
Summary:
The default application requires Kotlin version that is not supported by the Gradle plugin (should be at least `1.3.10`). However, instead of upgrading, we should remove it entirely. Here's why.
This commit 29d3dfbd19 introduced Detox for RNTester Android application.
Since the commit doesn't mention Detox for the default application and there are no Detox tests present by default in the default application, I believe that this addition was performed by a mistake.
The best way is to remove Kotlin from the default template. This step is described in the Detox documentation and all users that are integrating Detox will be asked to perform it anyway. No need to do it for them.
## Changelog
[ANDROID] [INTERNAL] - remove Kotlin from the default template
Pull Request resolved: https://github.com/facebook/react-native/pull/28626
Test Plan: Building a brand new project with `master` should work
Differential Revision: D21388961
Pulled By: shergin
fbshipit-source-id: 92666aa67f92b29f4e7f9c036b332bd058cdd49e
Summary:
Changelog: [internal]
As part of recycle, we should delete state. This is a common pattern used in other components as well.
Reviewed By: shergin
Differential Revision: D21348782
fbshipit-source-id: a5dee2f4ccee9b19498db31dab1983d8879dca71
Summary:
`babel/preset-env` pulls in a number of unused deps, like `caniuse-lite` (3 MiB) that knows about which browsers support certain features. We do not ship to browsers and always know which version of node we are using, so we don't need to pull this in.
I changed `jscodeshift` to optionally depend on `babel/preset-env` instead of always pulling it in.
This reduces node_modules by 7 MiB.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D21374475
fbshipit-source-id: 6f55e96e990ec0ca12f17bb3657bfa5429796b93
Summary:
Removes `PropTypes` as a dependency of `VirtualizedList` by no longer validating the return value of `getItemLayout`.
Changelog: [Internal]
Reviewed By: TheSavior, cpojer
Differential Revision: D21370890
fbshipit-source-id: 966db3557b714987aa91179c7654a5ebf27818ad
Summary:
Our parsers accept these but they are invalid. This fixes them.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D21373812
fbshipit-source-id: 6084757b9f842644fe629ae5e6d85baec611588d
Summary:
There's a corner case where:
* The bridge gets invalidated, and native modules are cleaning up themselves (but not done yet)
* Something asks for a NativeModule instance - ideally it should just get nil at this point
* If TM Manager is invalidating, we get nil correctly, but we continue thru the old NativeModule lookup logic
* The latter will fail anyway, and would throw a redbox (RCTLogError).
So, if the bridge is invalidated, if TM Manager returns nil, we should just return nil for old NativeModule lookup.
The module of interest is RCTImageLoader, which was requested by RCTImageView on deallocation. The problem is RCTImageView got dealloc'ed **after** the bridge has been invalidated, so the lookup would always fail...
Bonus: RCTImageView should just keep a weak ref to the RCTImageLoader, so that:
* if the imageLoader is still alive on image dealloc, it can still access them (last minute "meaningless" cleanup)
* if the imageLoader is gone, then the image deallocation doesn't do anything
Changelog: [iOS] [Fixed] - Fix module lookup race condition on bridge invalidation.
Reviewed By: p-sun, sammy-SC
Differential Revision: D21371845
fbshipit-source-id: 862dc07de18ddbfb90e87e24b8dbd001147ddce4
Summary:
I removed 1 MiB from `jscodeshift` as it was shipping with docs: 5885662920
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D21368889
fbshipit-source-id: 452fd4cedcc749d972adbb69df5c95117dd55b15
Summary:
This diff refactors the ImageLoaderModule class in order to prevent early execution of Fresco.getImagePipeline method when prefetching MC is disabled (see stack D21362266)
changelog: [Internal][Android]
Reviewed By: shergin
Differential Revision: D21368516
fbshipit-source-id: 53f99cd3c3f4848364182cb954a8d34821cb6d9e
Summary:
This diff extends the logging of image prefetching to include react_native ID in the ContextChain of the Fresco logger.
This is necesary to properly assign pre-fetching of RN Android images to the react_native ID
changelog: [Internal][Android]
Reviewed By: fkgozali
Differential Revision: D21362266
fbshipit-source-id: ff64f0ebd12f61b713996558eb2d962f96ad8d94
Summary:
This diff injects Fresco.ImagePipeline into ImageLoaderModule in order to ensure prefetching uses the same ImagePipeline than ReactImageView
changelog: [internal][Android]
Reviewed By: fkgozali
Differential Revision: D21362267
fbshipit-source-id: d6f55cffb6be6a012c2afb4ad5db2072bc7100ec
Summary:
This diff refactors the ReactCallerContextFactory class to recive surfaceID as a parameter instead of ThemedReactContext
This is necessary for the next diff of the stack
changelog: [Internal][Android]
Reviewed By: fkgozali
Differential Revision: D21362265
fbshipit-source-id: d0079788049fe86d2873eb6aa4bf1115b33457af
Summary:
In D20551411 we (I) replaced `std::mutex` with `std::recursive_mutex` in `executeSynchronouslyOnSameThread_CAN_DEADLOCK` in order to support synchronous `RuntimeExecutor`. It turned out that the idea was cool but totally incorrect. Quite obviously, double locking recursive mutex on the same thread does nothing, so the whole setup does not work at all.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D21328072
fbshipit-source-id: 7f1f81de61c36adb167fe624ada3bd71913bdf7c
Summary: Changelog: [Changed][General] Update react-native-community/eslint-config to 1.1.0, adding the new color rule
Reviewed By: rickhanlonii
Differential Revision: D21342153
fbshipit-source-id: ac1367353d4d3e69b6df29dc16f9fcb60cde3519
Summary:
Updating pods and fixing an issue in the RN podfile.
Changelog: [iOS][Fixed] Update podfile for RNTester
Reviewed By: PeteTheHeat
Differential Revision: D21336180
fbshipit-source-id: 4d51f53466ec2d432cf7a5775cb4d69d47423077