Summary:
In D14571128, we made it so that when a JS object's property was `undefined`, we wouldn't insert that property into the corresponding NSDictionary. Here are two important observations about that diff:
1. ALL JS `null`s were now being converted to `NSNull`, and JS `undefined`s were now being converted to `nil`.
2. If a JS object's property was explicitly `null`, then we'd insert `NSNull` into the corresponding dictionary.
Considering that when a property doesn't exist in a `NSDictionary`, property access returns `nil`, I've made it so that if a JS object's property is either `null` or `undefined`, then we simply do not insert it in the corresponding `NSDictionary`. Also, I've reverted #1 and made it so that `undefined` and `null` always map to the ObjC `nil`.
This shouldn't unfix the problem that D14571128 was trying to fix.
Here's my understanding of the problem that D14571128 was trying to fix (to make sure I'm not breaking something by this diff).
This method was invoked from JS.
```
RCT_EXPORT_METHOD(logEvents:(NSDictionary *)events)
{
RCTAssert(events, @"You should provide events for logger");
[[NSNotificationCenter defaultCenter] postNotificationName:@"FBReactPerfLoggerDidReceiveEventsNotification"
object:nil
userInfo:@{@"FBReactPerfLoggerUserInfoPerfEventsKey" : [events copy]}];
}
```
The above dispatch calls into this method, which appends `events` into `_pendingJSPerfEvents`.
```
- (void)reactPerfLoggerDidReceiveEvents:(NSNotification *)notification
{
NSDictionary *events = notification.userInfo[@"FBReactPerfLoggerUserInfoPerfEventsKey"];
if (events) {
dispatch_async(_eventQueue, ^{
if (self->_sessionData.perfLoggerFlagId != nil) {
if ([self processJSPerfEvents:events]) {
[self reportMetricsIfFinished];
}
} else {
[self->_pendingJSPerfEvents addObject:events];
}
});
}
}
```
Then, in `_processJSPerfEvents`, we do the following (link: https://fburl.com/tr4wr2a7):
```
NSNumber *actionId = events[@"actionId"];
if (actionId) {
self->_sessionData.actionId = actionId;
}
```
So, if `undefined` or `null` was passed in as the `actionId` property of the `events` JS object in `FBReactPerformanceLogger logEvents:`, then we'd default the `NSDictionary` to have `NSNull` in the corresponding property. This is bad because we had this line in FBReactWildePerfLogger (link: https://fburl.com/2nsywl2n): `actionId ? [actionId shortValue] : PerfLoggerActions.SUCCESS`. Essentially, this is the same problem that my diff is trying to fix.
Reviewed By: fkgozali
Differential Revision: D14625287
fbshipit-source-id: c701d4b6172484cee62494256175e8b205b23c73
Summary:
We want the ability to use Linkify on android text elements. This only adds this property to Text and not TextInput since there are some functional differences with how the types could be used between iOS and android - iOS allows one or many types while Linkify restricted us to providing only one option (using the masks).
Performance is affected ONLY FOR TEXT ELEMENTS USING THIS FEATURE since Linkify is searching for patterns.
Pull Request resolved: https://github.com/facebook/react-native/pull/19216
Differential Revision: D14621883
Pulled By: cpojer
fbshipit-source-id: cb692021d314140b9a92b29e23384afd7fd1b09e
Summary:
This PR is a follow-up to #21211 by request of hramos to incorporate some additional crash fixes / synchronization edge cases found in our production testing. What follows is largely a copy of the original PR description from #21211 - see that PR for original discussion thread as well as context on why this replacement PR was needed.
This PR is a minimalistic change to RCTTiming that causes it to switch exclusively to NSTimer (i.e., the 'sleep timer') in order to continue triggering timers when the app has moved to the background.
Many people have expressed a desire for background timer support on iOS. (See #1282, #167, and #16493). In our app — a podcast/audio player — we use background timers to ensure that we never lose track of the user's playback position, should the app crash or be terminated by the OS.
The RCTTimer module uses a RN-managed CADisplayLink if the next requested timer is less than a second away; otherwise, it switches to an NSTimer (which is refers to as a 'sleep timer' in source). The RN-managed CADisplayLink is always disabled when the app goes to the background (and thus cannot be used); however, the NSTimer will still issue its callbacks in the background.
This PR adds a flag to track whether the app is in the background, and if so, all timers are routed through NSTimer until the app returns to the foreground. vishnevskiy at Discord opened a similar PR (#16493) that implements a drop-in for CADisplayLink which falls back to NSTimer, but I decided to incorporate the background-NSTimer logic directly into RCTTimer, since NSTimer is already in use.
It's worth noting that the background NSTimer may not fire as often as requested — it may give the appearance of lagging depending upon your app's priority in the background. For our audio app, NSTimer fires exactly on schedule if there's an open AVAudioSession and audio is playing; if audio is not playing, it fires about half as often as requested, which is still adequate for networking polling and other tasks.
It's worth noting that background timers only function as long as an app is actually running in the background. Apple offers a variety of Background Modes (which can be toggled in the Capabilities section of the target inspector in Xcode), and the app will need to be legitimately using one of these modes in order for this change to provide any value — otherwise it will be terminated within a couple of seconds of moving to the background.
The good thing about this change is that for apps that do perform essential computation in support of their Background Mode, they can now use `setTimeout` and `setInterval` without problem — whereas in the past, neither would ever trigger their callback until the app returns to the foreground.
Pull Request resolved: https://github.com/facebook/react-native/pull/23674
Differential Revision: D14621326
Pulled By: shergin
fbshipit-source-id: c76e060ad2c662c140d7d2f4fb5aaa7094032515
Summary: This diff fixes a NoSuchElementException that was being thrown at ReactChoreographerDispatcher.doFrame(). The root cause was a lack of syncronization in removeFrameCallback().
Reviewed By: shergin
Differential Revision: D14619386
fbshipit-source-id: 80bc9e44866218d2a8703b3186f6958c145f260b
Summary:
[Android] [Fixed] - LayoutAnimations cause invalid view operations
The dating team has found a consistent repro where following an order of steps will get you the following exception:
https://lookaside.facebook.com/intern/pixelcloudnew/asset/?id=2113362972287761
The exception is due to the fact that the following operation
`delete child tag 17 from parent tag 481 which is located at index 2`
cannot be performed because parent tag 481 only has 2 children.. and they also happen to not have the tag 17 as a child. Somehow the operations and the tree they act upon are out of sync.
RN receives operations from React via the native module `UIManagerModule`. The operations use tags (an identifier for a view) and indices to determine what view is updated. These operations (grouped together as a batch) are then passed to the UI thread where we perform them on the platform views.
LayoutAnimations are implemented per batch in RN. When LayoutAnimations are on, qualified view updates are animated. Because the delete operation is animated, RN doesn't remove the view from the platform view hierarchy immediately but asynchronously -- after the animation is complete. This is problematic for other view operations that rely on an index on where to insert or delete a view because during the creation of those operations, it was assumed all prior operations were performed synchronously.
This is what was occurring in the repro and there were silent view errors occurring before the exception was thrown.
This diff proposes a solution to track all pending delete operations across operations.
We introduce a map that is keyed on view tags and has a value of a sparse array that represents child index -> count of views that are pending deletion.
`{11: [0=1], 481: [2=1]}` where this would be interpreted as for index 0 under view tag 11, there is one async view deletion that has not completed and under tag 481 there is one async view deletion at index 2.
We use the map to adjust indices on add / delete operations. We also update the count when the deletion animation is complete and remove the tag from the map when it is deleted.
Regarding worst case sizing of the map => O(# of platform views rendered)
Reviewed By: mdvacca
Differential Revision: D14529038
fbshipit-source-id: 86d8982845e25a2b23d6d89ce27852fd77dbb060
Summary:
Allow interpolation of strings with useNativeDriver. This allows animating much more of react-native-svg. This PR adds support for native animation of lengths with units, path data, colors etc. Plus, fixing the redundantly created nodes and (and thus, previously incorrect) connection of native animated nodes, improving performance.
Docs will need to change, specifying that string interpolation works with the native driver as well.
[GENERAL] [Added] Add support for native driven string interpolation in Animated
[GENERAL] Fix exception: Expected node to be marked as "native"
[GENERAL] Fix connection of AnimatedNodes and creation of redundant AnimatedNodes
Pull Request resolved: https://github.com/facebook/react-native/pull/18187
Differential Revision: D14597147
Pulled By: cpojer
fbshipit-source-id: 82a948a95419236be7931a8cc4ff72f41e477e9c
Summary: This is the first diff part of moving WebView internally to Facebook. It will be available under the same name `WebView` but won't be required from `react-native` any longer.
Reviewed By: TheSavior
Differential Revision: D14598043
fbshipit-source-id: f870d3f58e1d304071405344de09598dd22cdcc2
Summary:
Moved layout outputs transfer logic from YogaNodeJNIBase to YogaNodeJNI.
This change set is for adding experiment for layout outputs batching using a float array
Reviewed By: davidaurelio
Differential Revision: D14368107
fbshipit-source-id: 75ca330c1e7f07adc0ab8e7a5927d93977088918
Summary:
@public
I would like to get rid of implicit conversions between `YGValue` and `CompactValue`, because they don’t come for free.
That’s why I am adding `CompactValue` specific overrides for `YGResolveValue` and `YGValueEqual`, that do explicit casts. Up the commit stack, we will be able mark both `CompactValue(const YGValue&)` and `CompactValue::operator YGValue()` as `explicit`.
Reviewed By: SidharthGuglani
Differential Revision: D14598447
fbshipit-source-id: 75dc15cefb2dddcf8def891c5fb37893cacd9d46
Summary:
This adds TypeScript support to the community ESLint config. Our babel preset supports TypeScript by default, so it's nice to have TypeScript support pre-configured in the ESLint config too.
Note: Users need to install `typescript` in their project for linting to work for TypeScript files.
[General] [Added] - Add TypeScript support to the ESLint config
Pull Request resolved: https://github.com/facebook/react-native/pull/24100
Differential Revision: D14597127
Pulled By: cpojer
fbshipit-source-id: dfbf1b97061ed6e8c46aa49adb21630f5acdb5d1
Summary:
setNativeProps ends up calling UIManager.updateView which fails to find the view tag in the Paper UIManager and crashes.
This diff simply checks if the tag is managed by fabric, and calls `fabricUIManager.synchronouslyUpdateViewOnUIThread` if it is.
Not the ideal fix, but it generally works (js-driven animations work as well as in Paper) and it's better than crashing or not working at all.
Reviewed By: JoshuaGross
Differential Revision: D14414523
fbshipit-source-id: 0acd404f55094f8ce8eda39cb87ab58c727fb068
Summary: Removes useArrayNativeAccessor and everything needed to support it, similar to D14486283
Reviewed By: mdvacca
Differential Revision: D14487244
fbshipit-source-id: 7cfa91f7cf322c648c82be5951f3622cd6468961
Summary:
This was quite a rabit hole of remove deps -> delete dead code -> repeat.
Waaay simpler now with less duplicate lookups, redundant type verification, and extra function calls.
Reviewed By: mdvacca
Differential Revision: D14486283
fbshipit-source-id: 035db30181755d046a1ae99760468b954b2449df
Summary:
Changes our property access pattern to iterate through props once and pass the Object value directly rather than looking the value up in the map with the key.
Note some ViewManagers methods (especially yoga related ones on shadow nodes) expect a `Dyanamic`, so this diff also creates Dynamic's only when needed by the hand-written code, and introduces a new `DynamicWithObject` to create them that simply wraps the underlying object (as opposed to `DynamicWithMap` which wraps the map and does a lookup any time the `Dynamic` is accessed.
Reviewed By: mdvacca
Differential Revision: D14453300
fbshipit-source-id: df98567b6eff1e6b7c611f179eb11e413fb94e5d
Summary:
We will use it inside `core` module, so we have to decouple it from `view`.
As part of this, I added some comments, changed `const Float &` to just `Float` and put the implementation into `.cpp` file.
Reviewed By: mdvacca
Differential Revision: D14593952
fbshipit-source-id: 80f7746f4fc5b95febc8df9f5a9c0386a6425c88
Summary: This diff fixes an IllegalStateException that is thrown when the user click is on a edit text and tap 'Next' on the keyboard to focus on the next view, but the next view is hidden.
Reviewed By: lunaleaps, mmmulani
Differential Revision: D14598410
fbshipit-source-id: 2999cc468ed24bedff163eedcfaec50f6ee005d6
Summary: This is based on the work done in D8686586. Removed the logger instance from JSIExecutor constructor and installed it into the runtimeInstaller at all call sites.
Reviewed By: mhorowitz
Differential Revision: D14444120
fbshipit-source-id: 0476fda4230c467573ea04102a12101bcdf36c53
Summary:
`launchPackager.bat` starts metro server but does not pass projectRoot to it. So metro server starts in the wrong directory, It is because `startServerInNewWindow` pass `react-native` directory instead of `projectRoot` in the third argument of `spawn()` in `runAndroid.js`
Its working for people
See https://github.com/facebook/react-native/issues/23908#issuecomment-475889443
[Android] [Fixed] - projectRoot in launchPackager.bat
Pull Request resolved: https://github.com/facebook/react-native/pull/24115
Differential Revision: D14597101
Pulled By: cpojer
fbshipit-source-id: fb4155b72e35062cfb41fe1b3ecca0e2b4e849ce
Summary: This diff replaces usage of abort() with LOG(FATAL) when a prop-value is not found during parsing of prop values
Reviewed By: fkgozali
Differential Revision: D14591210
fbshipit-source-id: 4a8484ea6bdfec5534122ded43cc24ef80c13c1d
Summary: This diff replaces usage of abort() with LOG(FATAL) when a prop-value is not found during parsing of prop values
Reviewed By: sahrens
Differential Revision: D14570713
fbshipit-source-id: 57b0f993ba264a4949baf4022d807c55cdfe03b1
Summary: This diff removes BatchedExecutionTime from the metrics that are measured in Fabric. This does not add much value and it is not easy to compare against current version of RN
Reviewed By: shergin
Differential Revision: D14541976
fbshipit-source-id: c488e0951815306a978dd7a4246ec8a16d663218
Summary:
With our current infra, we support automatic conversion of method arguments using `RCTConvert`.
```
RCT_EXPORT_METHOD(foo:(RCTSound*) sound)
{
//...
}
```
```
interface RCTConvert (RCTSound)
+ (RCTSound *) RCTSound: (NSDictionary *) dict;
end
implementation RCTConvert (RCTSound)
+ (RCTSound *) RCTSound: (NSDictionary *) dict
{
//...
}
end
```
```
export interface Spec extends TurboModule {
+foo: (dict: Object) => void,
}
```
With this setup, when we call the foo method on the TurboModule in JS, we'd first convert `dict` from a JS Object to an `NSDictionary`. Then, because the `foo` method has an argument of type`RCTSound*`, and because `RCTConvert` has a method called `RCTSound`, before we invoke the `foo` NativeModule native method, we first convert the `NSDictionary` to `RCTSound` using `[RCTConvert RCTSound:obj]`. Essentially, if an argument type of a TurboModule method is neither a primitive type nor a struct (i.e: is an identifier), and it corresponds to a selector on `RCTConvert`, we call `[RCTConvert argumentType:obj]` to convert `obj` to the type `argumentType` before passing in `obj` as an argument to the NativeModule method call.
**Note:** I originally planned on using `NSMethodSignature` to get the argument types. Unfortunately, while the Objective C Runtime lets us know that the type is an identifier, it doesn't inform us which identifier it is. In other words, at runtime, we can't determine whether identifier represents `RCTSound *` or some other Objective C class. I figure this also the reason why the old code relies on the `RCT_EXPORT_METHOD` macros to implement this very same feature: https://git.io/fjJsC. It uses `NSMethodSignature` to switch on the argument type, and then uses the `RCTMethodInfo` struct to parse the argument type name, from which it constructs the RCTConvert selector.
One caveat of the current solution is that it won't work work unless we decorate our TurboModule methods with `RCT_EXPORT_METHOD`.
Reviewed By: fkgozali
Differential Revision: D14582661
fbshipit-source-id: 3c7dfb2059f031dba7495f12cbdf406b14f0b5b4
Summary:
<!--
Required: Write your motivation here.
If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
-->
Fixes#13754
Pull Request resolved: https://github.com/facebook/react-native/pull/18889
Differential Revision: D14486115
Pulled By: PeteTheHeat
fbshipit-source-id: 7b8b4fa9d2c99fc5d6145fed4681afdc4bb14fb8
Summary:
The `RCTBridge` contains numerous definitions of notification names, which we can observe in order to get insights into the React Native performance.
The Android implementation is a little different, such that you can listen for any of the [following](https://github.com/facebook/react-native/blob/master/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactMarkerConstants.java) marker constants, simply by including the following code:
```java
ReactMarker.addListener(new ReactMarker.MarkerListener() {
Override
public void logMarker(ReactMarkerConstants name, Nullable String tag, int instanceKey) {
Log.d("ReactNativeEvent", "name: "+ name.name() + " tag: " + tag);
}
});
```
This will allow you to perform the necessary processing, calculations as required.
---
This PR enables observing for the module setup event (`RCTDidSetupModuleNotification`) by including the respective module's name & setup time in milliseconds.
[iOS] [Added] - Gain insights on the module setup times by observing `RCTDidSetupModuleNotification`. The `userInfo` dictionary will contain the module name and setup time in milliseconds. These values can be extracted via `RCTDidSetupModuleNotificationModuleNameKey ` and `RCTDidSetupModuleNotificationSetupTimeKey`.
Pull Request resolved: https://github.com/facebook/react-native/pull/23859
Differential Revision: D14579066
Pulled By: PeteTheHeat
fbshipit-source-id: 52645127c3fc6aa5bd73e3bd471fccd79cb05c14
Summary:
@public
After changing native methods to return `long` rather than `YogaValue`, we switch them to JNI fast calls, as there is no more interaction with the Java GC.
Reviewed By: pasqualeanatriello
Differential Revision: D14576815
fbshipit-source-id: b5a33caef7343ba1de3d9634a50dc82ab3148cc7
Summary:
@public
Passing primitive data via JNI is more efficient than passing objects.
Here, we avoid creating `YogaValue` (Java) instances via JNI, and rather pass a `long` back to Java. The instance is then created by extracting the necessary bytes on the Java side.
Reviewed By: foghina
Differential Revision: D14576755
fbshipit-source-id: 22d09ad50c3ac6c49b0a797a0dad639ea4829df9
Summary:
@public
Moving all native methods in a single class provides the benefit of not having to load native bindings eagerly when just creating config objects in the startup paths, or setting Java-only values on them.
Loading native bindings triggers additional class loads (`YogaConfig` / `YogaNode`), and can lead to problems in multi-dex scenarions.
Reviewed By: pasqualeanatriello
Differential Revision: D14560658
fbshipit-source-id: 14e31e3c3b560675b5a752a38ae75ab80a565ea1
Summary:
Bug comes from #23545, if `allowedLength < 0`, it would crash if `text.length > 1`.
cc. cpojer
[iOS] [Fixed] - Fixed crash when textinput's default value exceeds maxLength
Pull Request resolved: https://github.com/facebook/react-native/pull/24084
Differential Revision: D14562719
Pulled By: cpojer
fbshipit-source-id: 87ed930e35b8fa889d8b04829795fa46b7787b07
Summary:
CocoaPods makes it easier to add new iOS dependencies to a project without having to manually edit Xcode projects. Editing Xcode projects is time consuming and merging changes to them difficult. Automating the changes to Xcode project `react-native link` is error prone. CocoaPods is a de-facto standard way to manage iOS dependencies and a central part of unimodules and upcoming improvements to `react-native link`.
This PR adds a `Podfile` to the default project template of React Native. To use a project with CocoaPods, after creating it, run `cd ios; pod install` and use the created `<projectname>.xcworkspace` file instead of the `.xcodeproj` file. (We could make this a part of `react-native init` so you only need to run one command when creating a project.)
[iOS] [Added] - Add CocoaPods Podfile to the project template
Pull Request resolved: https://github.com/facebook/react-native/pull/23563
Differential Revision: D14576505
Pulled By: cpojer
fbshipit-source-id: f6c9e93d61a52ad445d2931ccc4933d559a6ec1a
Summary:
Before invoking TurboModule ObjC methods, we loop through all the arguments and transform them from `jsi::Value` to ObjC data structures. `jsi::Value`s that represent JS Objects get converted to `NSDictionary`s in ObjC. This isn't good enough because `NSDictionary` isn't typed. What we really need is a C/C++ struct that represents the type of the JS Object.
Therefore, for every argument of a TurboModule method that is a JS Object, this diff allows you to specify a method on `RCTCxxConvert` (which you have to declare and implement) that transforms that type of JS Object into something else. Basically, with the changes in this diff, we'll be able to transform JS Objects into C++ struct instances, which gives us type safety for JS Object method argumetns to TurboModule functions.
I modified the codegen to also create a mapping from NativeModule method name => argument num => `RCTCxxModule` conversion selector. This way, the FB codegen that generates the `RCTCxxConversion` function also informs our TurboModule system which conversion function to use before we call the method requiring complex argument. I just had to extend the `ObjCTurboModule` class to accomplish this.
The old system relies on the additional method generated by `RCT_EXPORT_METHOD` macro. It takes the written code, does string processing on it to parse the type of the struct arguments, and then replaces all instances of `::` with `_`. Super hacky. I didn't take this approach because it seemed unnecessarily hacky brittle.
The other approach I considered was to try to use reflection to infer the type of the struct at runtime. We'd have to do a bit of string processing and concat the TurboModule class name with the struct type. The solution would be simpler because it'd only modify the objective C and it wouldn't touch the Codegen system. **Edit:** I implemented it here: D14513078. The one downside of this design is that the name of the conversion function is individually constructed in two different locations in the code (i.e: no source of truth for this name). The other downside is that we have to rely on `RCTBridgeModuleNameForClass`, which we want to eliminate. This also computationally more expensive since it requires string processing/regex parsing and additional reflection. Therefore, we decided to move on with the current solution.
Reviewed By: fkgozali
Differential Revision: D14513429
fbshipit-source-id: 3d1b87e02ee908a19305686ff82b2ed624d8ac67
Summary: [iOS] [Fixed] - The existing logic defaults `undefined` & `null` in JS to be `[NSNull null]` when converting JS object to `NSDictionary`. Let's not insert the prop to the dictionary if it's `undefined`.
Reviewed By: blairvanderhoof
Differential Revision: D14571128
fbshipit-source-id: e03c713b055672b0a001d3305d694912ee36ab36
Alexander Vasyuk
Christoph Nakazawa
Ramanpreet Nara
Kyle Pinkham
cojo
Ilja Daderko
David Vacca
Luna Wei
zhongwuzw
Mikael Sand
Sidharth Guglani
David Aurelio
satyajit.happy
Valentin Shergin
Spencer Ahrens
Dhaval Kapil
Mr. Anonymous
Yuan Du
nicholaslee119
Achille Urbain
Rafi Ciesielczuk
Ville Immonen
Kevin Gozali