Commit Graph

921 Commits

Author SHA1 Message Date
Janic Duplessis
4e215b20a3 Fix event ordering when combining coalescable and non-coalescable events (#24693)
Summary:
Fixes an issue introduced in https://github.com/facebook/react-native/pull/15894 that can cause events to be dispatched out of order.

Also reverted `viewTag` moved to optional property as it didn't actually work and makes code more complex.

[iOS] [Fixed] - Fix event ordering when combining coalescable and non-coalescable events
Pull Request resolved: https://github.com/facebook/react-native/pull/24693

Differential Revision: D15279513

Pulled By: shergin

fbshipit-source-id: 3c64aba6d644ea9564572e6de8330b59b51cf4a9
2019-05-09 10:36:56 -07:00
licd
3163479e0f add nonnull/nullable for swift (#24729)
Summary:
add nonnull/nullable for swift

[iOS] [Changed] - add nonnull/nullable for swift
Pull Request resolved: https://github.com/facebook/react-native/pull/24729

Differential Revision: D15273046

Pulled By: shergin

fbshipit-source-id: 5603007e5aa01d54064da349666a4e4d998b5471
2019-05-08 18:21:26 -07:00
zhongwuzw
3c125e867f Call designated initializer for SurfaceHostingProxyRootView (#24368)
Summary:
1. Call designated initializer for SurfaceHostingProxyRootView.
2. Make super class designated initializer `-initWithSurface:sizeMeasureMode:` `NS_UNAVAILABLE`.

cc. shergin

[iOS] [Fixed] - Call designated initializer for SurfaceHostingProxyRootView
Pull Request resolved: https://github.com/facebook/react-native/pull/24368

Differential Revision: D15144991

Pulled By: shergin

fbshipit-source-id: c999ac64c766242ceff157b0d9600190c1f3add8
2019-04-29 21:41:55 -07:00
Harry Nguyen
e7e1a93d8a Fix exception when attempt to load image from file system (#24457)
Summary:
When trying to load image using method "RCTImageFromLocalAssetURL", if URL's path in file system representation return null (in my case, imageUrl is base64 format returned from server so it's always null) then method "stringWithUTF8String" will raise an exception.

[iOS] [Fixed] - Fix exception when attempt to load image from file system
Pull Request resolved: https://github.com/facebook/react-native/pull/24457

Differential Revision: D15123680

Pulled By: shergin

fbshipit-source-id: bc34b3d7c79a8f9157729c4cf0486507c3c15ddf
2019-04-29 10:23:02 -07:00
zhongwuzw
ffa3b0d4d6 Add convert compatible of NSString for bridge message data (#24630)
Summary:
Fixes https://twitter.com/estevao_lucas/status/1117572702083190785?s=215 in #24626 . Now we try to convert any id to `NSString`, not throw error.

cc. cpojer .

[iOS] [Fixed] - Add convert compatible of NSString for bridge message data
Pull Request resolved: https://github.com/facebook/react-native/pull/24630

Differential Revision: D15120205

Pulled By: cpojer

fbshipit-source-id: 4849a8e941410b292f971608a9cdb38c11502445
2019-04-29 06:36:41 -07:00
zhongwuzw
e9adbb58ba Adding nonnull macro to component event (#24523)
Summary:
We don't accept any nil parameters, so let compiler check it~

cc. janicduplessis cpojer

[iOS] [Fixed] - Adding nonnull macro to component event
Pull Request resolved: https://github.com/facebook/react-native/pull/24523

Differential Revision: D15022399

Pulled By: shergin

fbshipit-source-id: 8165d49c62d69404aac7ea0ba4291e397d962617
2019-04-24 08:16:01 -07:00
Ram N
a6fb3d3a35 Check for image in local assets also in sync image calls
Reviewed By: PeteTheHeat

Differential Revision: D14660673

fbshipit-source-id: 7ed58331b5c61777dc084fa2cf9a460761d723c2
2019-04-23 19:09:06 -07:00
Héctor Ramos
335c39c895 Back out OSS-only change from D14811733 and fix regression in open source iOS tests
Summary: The changes made in D14811733 introduced a regression in React Native's open source iOS tests.

Reviewed By: fkgozali

Differential Revision: D14895096

fbshipit-source-id: 4307b094e3bc94e9f703b0271a95a9b318bcd49a
2019-04-11 12:57:15 -07:00
zhongwuzw
1da1e8c6f3 Enhance search of directories when load local asset image (#23857)
Summary:
Currently, `RCTLocalAssetImageLoader` only support directory of `Library` and `App bundle`, actually, we need to support other directories like `tmp` or `Documents`. Otherwise, the local image load in `tmp` or `Documents` would be handled by `NSURLSession`, we don't need that.

<img width="405" alt="image" src="https://user-images.githubusercontent.com/5061845/54188126-0ba66100-44ea-11e9-9a7b-0f721100e9be.png">

[iOS] [Fixed] - Enhance search of directories when load local asset image
Pull Request resolved: https://github.com/facebook/react-native/pull/23857

Differential Revision: D14894136

Pulled By: shergin

fbshipit-source-id: 26361cd952a423467be9af9a84a80100d868776b
2019-04-11 11:43:06 -07:00
Gunnar Kudrjavets
0c0aff295b Wipe out -DRCTLOG_ENABLED=0
Summary:
The root cause comes down to the fact that historically `-DFOO=0` and `-DFOO=1` patterns have been used when it comes to preprocessor definitions and various engineers use `#ifdef FOO` and `#if FOO` interchangeably. That results in an incorrect pattern because the real evaluation is following:

- `FOO` not defined:  `#if FOO` = `FALSE`, `#ifdef FOO` = `FALSE`.
- `-DFOO=0`: `#if FOO` = `FALSE`, `#ifdef FOO` = `TRUE`.
- `-DFOO=1`: `#if FOO` = `TRUE`, `#ifdef FOO` = `TRUE`.

Reviewed By: aditya7fb

Differential Revision: D14811733

fbshipit-source-id: a7fedc56ab0ce8eedcb48bda783739a4c1eecf81
2019-04-09 20:08:14 -07:00
Alfred Zien
ff66600224 Use constructor attribute instead of +load objc method (#24155)
Summary:
Xcode 10.2 forbids creating categories for swift class that uses `+load` method. In react-native categories like this are used to register swift classes as modules (macro `RCT_EXTERN_MODULE`) This PR changes it to use `__attribute__((constructor))` instead of objc `+load` method.

I introduced new macro for this purpose, `RCT_EXPORT_MODULE_NO_LOAD`, it expands in something like:
```
void RCTRegisterModule(Class);

+ (NSString *)moduleName {
  return @"jsNameFoo";
}

__attribute__((constructor)) static void initialize_ObjcClassFoo{
  RCTRegisterModule([ObjcClassFoo class]);
}
```

Functions marked with `__attribute__((constructor))` are run before main and after all `+load` methods, so it seems like correct thing to do.

Fixes https://github.com/facebook/react-native/issues/24139
Doc about loading order https://developer.apple.com/documentation/objectivec/nsobject/1418815-load?language=objc

[iOS] [Fixed] - Fix runtime crash in xcode 10.2 when using RCT_EXTERN_MODULE for swift classes.
Pull Request resolved: https://github.com/facebook/react-native/pull/24155

Reviewed By: javache

Differential Revision: D14668235

Pulled By: shergin

fbshipit-source-id: 0c19e69ce2a68327387809773848d4ecd36d7461
2019-03-31 22:40:59 -07:00
Janic Duplessis
41343f6a73 BREAKING - RCTEvent improvements, remove deprecated [sendInputEventWithName:body:] (#15894)
Summary:
This makes the RCTEvent protocol more generic to make it easier to use the event coalescing feature for type of events other than components. This does a few other improvements that will be useful in follow up PRs.

- Add `RCTComponentEvent` which is used instead of deprecated `[sendInputEventWithName:body:]` and remove that method completely (was only used at 2 places).
- Make `coalescingKey` optional for events that return NO from `canCoalesce`.
- Make `viewTag` optional for events that are not related to views.
- Fast path for events that return NO from `canCoalesce`.
- Add a missing test for event coalescing with different view tags.

Ended up making only one PR for all this since the changes are related and hard to separate.

**Migration**
Use a custom RCTEvent subclass with `[sendEvent:]` (preferred way to allow type safe events) or `RCTComponentEvent`.

**Test plan**
- Ran RCTEventDispatcher unit tests
- Tested manually in RNTester

Changelog:

[iOS] [Changed] - Remove deprecated RCTEvent method, sendInputEventWithName:body:
Pull Request resolved: https://github.com/facebook/react-native/pull/15894

Reviewed By: shergin

Differential Revision: D13726194

Pulled By: hramos

fbshipit-source-id: 11f63a99e08f46ec6b4f16f8d9949cdbf5c3fe13
2019-03-27 11:20:22 -07:00
Ramanpreet Nara
8618a5824f Add support for argument conversion via RCTConvert
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
2019-03-22 16:23:40 -07:00
Rafi Ciesielczuk
e2bf843d86 Introduce Module Setup Metric (#23859)
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
2019-03-22 10:43:00 -07:00
Valentin Shergin
95b05c0d82 Revert D14425373: [react-native][PR] [iOS] Remove explicitly add png file extension when load local image
Differential Revision:
D14425373

Original commit changeset: 3cc06c9a3d68

fbshipit-source-id: eef2ee9a459c35dcb30e0c023eb24854529149be
2019-03-20 14:46:53 -07:00
zhongwuzw
e40a76715a Remove explicitly add png file extension when load local image (#23864)
Summary:
We need to remove adding png file extension when path has not extension. Two reasons:
1. `imageWithContentsOfFile` or other `UIKit` methods can load png image correctly, even if path has not `png` file extension.
2. Sometimes, people may have file that actually not have file extension, it's the designated behavior for user. Like #23844 .

CC. sahrens cpojer .

[iOS] [Fixed] - Remove explicitly add png file extension when load local image
Pull Request resolved: https://github.com/facebook/react-native/pull/23864

Reviewed By: shergin

Differential Revision: D14425373

Pulled By: hramos

fbshipit-source-id: 3cc06c9a3d68cadf652c1de742f3cce26258c874
2019-03-18 23:03:32 -07:00
Pavlos Vinieratos
629708beda Crash reporting heaven (#23691)
Summary:
<!-- Explain the **motivation** for making this change. What existing problem does the pull request solve? -->
I have used RN for a long time, and for all this time, crash reporting has been less great than native development crash reporting. At some point, companies like sentry, bugsnag and a bunch of others started supporting sourcemaps for js crashes in RN, which helped a lot.
But native crashes were (and still are) much harder to diagnose.

..Until now :D

I have make a repo of a sample RN app, included this PR in it, and some code and screenshots to help.
The repo is [here](https://github.com/pvinis/react-native-project-with-crash-heaven-pr).

I was trying to get good crash reports from native crashes in iOS for a looong time. I spoke with people in sentry, in bugsnag and more, and I could not get this solved. There was no clear way to get the **native** crashed to display correctly.
I made two repos here, one for [sentry](https://github.com/pvinis/SentryBadStack) and one for [bugsnag](https://github.com/pvinis/BugsnagBadStack), demonstrating the correct js handling and the bad native handling.

After all this, and talks with their support, twitter etc, I investigated further, on **why** this was happening. I thought there must be some reason that native crashes look bad in all the tools, and in the same way. Maybe it's not their fault, or up to them to fix it, or maybe they didn't have the experience to fix it.

In a test project I created, I checked what's up with the `RCTFatalException`, and I found out that the React Native code is catching the `NSException`s that come from any native modules of a RN app and converting it into an string and sending it to `RCTFatal` that created an `NSError` out of that string. Then it checks if the app has set a fatal error handler and if not, goes ahead and throws that `NSError`.

The problem here is that `NSException` has a bunch more info that the resulting `NSError` is missing or is altering. Turning the callstack into a string renders crash reporting tools useless as they are missing the original place the exception was thrown, symbols, return addresses etc. In both repos above it can be seen that both tools were thinking that the error happened somewhere in the `RCTFatal` function, and it did, since we create it there, losing all the previous useful info of the original exception. That leaves us with just a very long name including a callstack, but very hard to actually map this to the code and dsym.

I added a fatal exception handler, that mirrors the fatal error handler, as the error handler is used around React Native internal code.

Then I stopped making a string out of the original `NSException` and calling `RCTFatal`, and I simply throw the exception. This way no info is lost!

Finally, I added some code examples of native and js crashes and added a part in the `RNTester` app, so people can see how a js and a native error look like while debugging, as well as try to compile the app in release mode and see how the crash report would look like if they connect it to bugsnag or sentry or their tool of choice.

I have attached some images at the bottom of this PR, and you can find some in the 3 repos I linked above.

[iOS] [Fixed] - Changed the way iOS native module exceptions get handled. Instead of making them into an `NSError` and lose the context and callstack, we keep them as `NSException`s and propagate them.
[General] [Added] - Example code for native crashes in iOS and Android, with buttons on RNTester, so developers can see how these look when debugging, as well as the crash reports in release mode.
Pull Request resolved: https://github.com/facebook/react-native/pull/23691

Reviewed By: fkgozali

Differential Revision: D14276366

Pulled By: cpojer

fbshipit-source-id: b308d5608e1432d7676447347ae77c0721094e62
2019-03-12 19:41:51 -07:00
Spencer Ahrens
3e40837a85 Fix animation delay
Summary:
We currently rely on the Paper UIManager calling `uiManagerWillPerformMounting` to flush the animated operations queue, which includes starting and stopping animations. This mostly works right now because Fabric always starts after Paper, but sometimes Paper doesn't fire `uiManagerWillPerformMounting` for a while, which can delay an animation starting.

To fix this, I force a flush of the queues on the UIThread whenever start or stop is called. This should be safe because the order of animation operations is still preserved, and start/stop are (almost?) always called in dedicated event handler loops, so any other updates like changing the way nodes are attached should already have been processed from a previous JS execution loop.

Reviewed By: JoshuaGross

Differential Revision: D14313502

fbshipit-source-id: 2a2b0c614fd1a591bd04b6b3fafcc09ff6c9d6e7
2019-03-07 17:39:00 -08:00
aleclarson
dc893756b8 Properly validate JS->native method calls (#23658)
Summary:
Between invalidating a bridge and suspending its JS thread, native modules may have their methods called.

Only warn when a native module has been invalidated, which happens right before its JS thread is suspended.

Avoid initializing a native module's instance if its bridge is invalidated.

/cc fkgozali f945212447 (commitcomment-32467567)

[iOS] [Fixed] - Properly validate JS->native method calls
Pull Request resolved: https://github.com/facebook/react-native/pull/23658

Differential Revision: D14287594

Pulled By: fkgozali

fbshipit-source-id: 89dd1906a0c55f3f48ba4ff220aac0cddf2eb822
2019-03-04 17:39:53 -08:00
zhongwuzw
17ae61e14c Fixed method invoke when both nullable and __unused exist (#23726)
Summary:
We have parse code to try to support exported method parameter annotated `__unused` and `nullable`, but actually, seems we don't support it entirely, for example, we parse `__unused` firstly, then we parse `nullable`, actually, this case not exist, because `nullable` macro must appear right after `(`, before `__unused`, so we can't parse success, leads to error signature and crash.

[iOS] [Fixed] - Fixed method invoke when both nullable and __unused exist
Pull Request resolved: https://github.com/facebook/react-native/pull/23726

Differential Revision: D14298563

Pulled By: cpojer

fbshipit-source-id: 934e1d384a5d67b7bbf117fd2dec51f50979c979
2019-03-03 20:45:06 -08:00
zhongwuzw
7025cce3f4 Fixed crash if event doesn't have body parameter (#23711)
Summary:
We need to check `body[@"target"]` wether equal to `nil`, if it is, return to prevent crash.

[iOS] [Fixed] - Fixed crash if event doesn't have body parameter
Pull Request resolved: https://github.com/facebook/react-native/pull/23711

Differential Revision: D14298543

Pulled By: cpojer

fbshipit-source-id: d5e8cd69438f323ae102e61618c0371a01bee347
2019-03-03 20:29:58 -08:00
zhongwuzw
d8d42c7310 Added support of __nullable and __nonnull for module exported method signature (#23731)
Summary:
For the `NullabilityPostfix`, we can use `_Nullable` or `__nullable`. Please see https://developer.apple.com/swift/blog/?id=25 , and we also use it in our code.

[iOS] [Added] - Added support of __nullable and __nonnull for module exported method signature
Pull Request resolved: https://github.com/facebook/react-native/pull/23731

Differential Revision: D14298483

Pulled By: cpojer

fbshipit-source-id: 4dd3188f51be3808ea2b97ce19aa7a85ea1f4a9e
2019-03-03 20:06:21 -08:00
zhongwuzw
06c6c7c673 Remove compatible system code for iOS8 and before (#23656)
Summary:
We already only support `iOS9+`, so we can remove all compatible codes now.

[iOS] [Fixed] - Remove compatible system code for iOS8 and before
Pull Request resolved: https://github.com/facebook/react-native/pull/23656

Differential Revision: D14224986

Pulled By: hramos

fbshipit-source-id: cac9ffe6788dd3eaf4f4f5f2b219f325ba78e85f
2019-02-26 07:58:52 -08:00
zhongwuzw
81860c59c3 Remove compiler warning (#23588)
Summary:
Fixed compiler warning, after this, seems we have no warning for framework in debug mode.
<img width="1280" alt="image" src="https://user-images.githubusercontent.com/5061845/53224564-2d14e980-36b0-11e9-85f4-46304513b18d.png">

[iOS] [Fixed] - Remove compiler warning
Pull Request resolved: https://github.com/facebook/react-native/pull/23588

Differential Revision: D14181748

Pulled By: cpojer

fbshipit-source-id: 8b633e7cdb7b3b8029f4145a1155e540ac516191
2019-02-22 01:40:09 -08:00
Albert Martin
bca85101cd fix: prevent exception when imageName is null (#20120)
Summary:
When an image source is parsed via `RCTImageFromLocalAssetURL` there seem to be certain situations in which `imageName` is empty from `RCTBundlePathForURL`. Any call to `UIImage imageNamed` with a an empty parameter will throw an exception:

```
CUICatalog: Invalid asset name supplied: '(null)'
```

In my case, the asset URL was pointing to an image in the application sandbox rather than the `NSBundle`. In this case `UIImage imageNamed` was throwing before the call to `NSData dataWithContentsOfURL` below could correctly resolve the image.

This change simply skips the call to `UIImage imageNamed` if no `imageName` value is set.
Pull Request resolved: https://github.com/facebook/react-native/pull/20120

Differential Revision: D14163101

Pulled By: cpojer

fbshipit-source-id: ceec95c02bf21b739962ef5618947a5726ba0473
2019-02-20 22:57:40 -08:00
Luis Rivera
fd96e2cb2c Feature: dev menu option to change packager location during runtime (#21970)
Summary:
Add an option to dev menu to change packager location on the fly on iOS (similar to Dev Settings in Android)
Pull Request resolved: https://github.com/facebook/react-native/pull/21970

Differential Revision: D14162776

Pulled By: cpojer

fbshipit-source-id: 3cca4f4fbd8c599bd5342ba1ae64905a03270d48
2019-02-20 22:07:11 -08:00
Eric Lewis
468ae234a6 Fix xcode warnings (#23565)
Summary:
As part of #22609, this fixes yet more warnings.
- Adding more __unused to params.
- Refactors `isPackagerRunning` to use NSURLSession.
- Turns off suspicious comma warnings

[iOS] [Fixed] - Xcode Warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/23565

Differential Revision: D14161151

Pulled By: cpojer

fbshipit-source-id: 339874711eca718fc6151e84737ccc975225d736
2019-02-20 18:46:23 -08:00
Eric Lewis
19866aee3d Fix 50 xcode warnings (#23553)
Summary:
This PR reduces the number of warnings in React from 68 to 18. Mostly by marking unused variables. RNTester's warnings are more than halved.

[iOS] [Fixed] - Xcode warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/23553

Differential Revision: D14151339

Pulled By: hramos

fbshipit-source-id: 8255330bf910a69a4c03051d91d7b0de3fadf2d1
2019-02-20 10:17:26 -08:00
Kevin Gozali
a9dd828c68 Don't attempt to load RCTDevLoadingView lazily
Summary:
There's a very old issue with reload logic: invalidation and resetting up of the bridge could be racing. In this case, we hit a redbox when:
* Chrome debugger is enabled in previous app run, then we launch the app again
* The bridge starts, then immediately reloads itself to connect to Chrome
* On the 2nd setup, the logic to update the green loading bar, with the % indicator for loading off metro, failed to find the DevLoadingView module instance because the bridge is in the middle of invalidating

See https://github.com/facebook/react-native/issues/23235

To test:
Using react-native init from github, do the steps in https://github.com/facebook/react-native/issues/23235, no more redbox. Note that the loading indicator % won't be proper still, but at least it doesn't crash/redbox.

Reviewed By: JoshuaGross

Differential Revision: D14110814

fbshipit-source-id: 835061e50acc6968bffbcc2ddfbe8da79a100df9
2019-02-15 13:31:51 -08:00
zhongwuzw
cbf65f2cf4 Enable edge antialiasing only for transforms with perspective (#19360)
Summary:
Enable edge antialiasing only for transforms with perspective

[iOS] [Added] - RCTViewManager: Enable edge antialiasing only for transforms with perspective.

Pull Request resolved: https://github.com/facebook/react-native/pull/19360

Reviewed By: cpojer

Differential Revision: D14088290

Pulled By: hramos

fbshipit-source-id: 2113c7a29efce5ca9990e2a79c69fc70bdf8a041
2019-02-15 10:09:07 -08:00
Mujtaba Alboori
65cfb9a47d iOS support keyboardType ASCIICapableNumberPad (#20597)
Summary:
React native is missing a keyboard type for iOS so I included support for it. So it can be used in react native app
https://developer.apple.com/documentation/uikit/uikeyboardtype/uikeyboardtypeasciicapablenumberpad

Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.

If this PR fixes an issue, type "Fixes #issueNumber" to automatically close the issue when the PR is merged.
Pull Request resolved: https://github.com/facebook/react-native/pull/20597

Differential Revision: D14103498

Pulled By: cpojer

fbshipit-source-id: b0aa17f3e527734c6e90eadc73c96db4dd002698
2019-02-15 08:44:03 -08:00
zhongwuzw
da023c5873 No force layout of superview and contentSize recalculation if intrinsicContentSize not changed
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/20664

Differential Revision: D14103326

Pulled By: cpojer

fbshipit-source-id: 9346f12827843960178fbeef646bdb4c2cbd71ce
2019-02-15 08:35:41 -08:00
zhongwuzw
e5fbd39450 Fix image wrong scale factor when load image from file system (#23446)
Summary:
Regression, fix image load from `~/Library` not respect scale factor.
Fixes #22383 , the bug comes from [Clean up some URL path handling](998197f444).

[iOS] [Fixed] - Fix image wrong scale factor when load image from file system
Pull Request resolved: https://github.com/facebook/react-native/pull/23446

Differential Revision: D14099614

Pulled By: cpojer

fbshipit-source-id: eb2267b195a05eb70cdc4671536a4c1d47fb03e2
2019-02-15 02:43:04 -08:00
Peter Argany
0bde29e197 Consider SSTs in Platform.isTesting
Summary:
A common util from RN to gate on testing code is `Platform.isTesting()`

Unfortunately, this util does not account for ServerSnapshotTests, since they don't use apple's XCTest infra.

Reviewed By: sahrens

Differential Revision: D13981728

fbshipit-source-id: bf902a04f5d7fcb98a06816f5c2c9b082e7d14b8
2019-02-07 11:07:17 -08:00
Ramanpreet Nara
f37093319b Start using getConstants
Summary:
TurboModules depend on a getConstants method. Existing ObjectiveC modules do not have this method. Therefore, I moved the contents of `constantsToExport` to `getConstants` and then had `constantsToExports` call `getConstants`.

facebook
Since all NativeModules will eventually need to be migrated to the TurboModule system, I didn't restrict this to just the NativeModules in Marketplace.

```
const fs = require('fs');

if (process.argv.length < 3) {
    throw new Error('Expected a file containing a list of native modules as the third param');
}

function read(filename) {
    return fs.readFileSync(filename, 'utf8');
}

const nativeModuleFilenames = read(process.argv[2]).split('\n').filter(Boolean);

nativeModuleFilenames.forEach((fileName) => {
    if (fileName.endsWith('.h')) {
        return;
    }

    const absPath = `${process.env.HOME}/${fileName}`;
    const fileSource = read(absPath);

    if (/(\n|^)-\s*\((.+)\)getConstants/.test(fileSource)) {
        return;
    }

    const constantsToExportRegex = /(\n|^)-\s*\((.+)\)constantsToExport/;
    const result = constantsToExportRegex.exec(fileSource);

    if (result == null) {
        throw new Error(`Didn't find a constantsToExport function inside NativeModule ${fileName}`);
    }

    const returnType = result[2];

    const newFileSource = fileSource.replace(
        constantsToExportRegex,
        '$1- ($2)constantsToExport\n' +
        '{\n' +
        `  return ${returnType.includes('ModuleConstants') ? '($2)' : ''}[self getConstants];\n` +
        '}\n' +
        '\n' +
        '- ($2)getConstants'
    );

    fs.writeFileSync(absPath, newFileSource);
});
```

```
> xbgs -l ')constantsToExport'
```

Reviewed By: fkgozali

Differential Revision: D13951197

fbshipit-source-id: 394a319d42aff466c56a3d748e17c335307a8f47
2019-02-04 17:46:56 -08:00
Ramanpreet Nara
0ceefb40d5 Enable module lookup in TurboModules
Summary:
NativeModules are instantiated by the bridge. If they choose, they can capture the bridge instance that instantiated them. From within the NativeModule, the bridge can then be used to lookup other NativeModules. TurboModules have no way to do such a lookup.

Both NativeModules and TurboModules need to be able to query for one another. Therefore, we have four cases:
1. NativeModule accesses NativeModule.
2. NativeModule accesses TurboModule.
3. TurboModule accesses NativeModule.
4. TurboModule accesses TurboModule.

In summary, this solution extends the bridge to support querying TurboModules. It also introduces a `RCTTurboModuleLookupDelegate` protocol, which, implemented by `RCTTurboModuleManager`, supports querying TurboModules:
```
protocol RCTTurboModuleLookupDelegate <NSObject>
- (id)moduleForName:(NSString *)moduleName;
- (id)moduleForName:(NSString *)moduleName warnOnLookupFailure:(BOOL)warnOnLookupFailure;
- (BOOL)moduleIsInitialized:(NSString *)moduleName
end
```

If TurboModules want to query other TurboModules, then they need to implement this protocol and synthesize `turboModuleLookupDelegate`:

```
protocol RCTTurboModuleWithLookupCapabilities
property (nonatomic, weak) id<RCTTurboModuleLookupDelegate> turboModuleLookupDelegate;
end
```

NativeModules will continue to use `RCTBridge` to access other NativeModules. Nothing needs to change.

When we attach the bridge to `RCTTurboModuleManager`, we also attach `RCTTurboModuleManager` to the bridge as a `RCTTurboModuleLookupDelegate`. This allows the bridge to query TurboModules, which enables our NativeModules to transparently (i.e: without any NativeModule code modification) query TurboModules.

In an ideal world, all modules would be TurboModules. Until then, we're going to require that TurboModules use the bridge to query for NativeModules or TurboModules.

`RCTTurboModuleManager` keeps a map of all TurboModules that we instantiated. We simply search in this map and return the TurboModule.

This setup allows us to switch NativeModules to TurboModules without compromising their ability to use the bridge to search for other NativeModules (and TurboModules). When we write new TurboModules, we can have them use `RCTTurboModuleLookupDelegate` to do access other TurboModules. Eventually, after we migrate all NativeModules to TurboModules, we can migrate all old callsites to use `RCTTurboModuleLookupDelegate`.

Reviewed By: fkgozali

Differential Revision: D13553186

fbshipit-source-id: 4d0488eef081332c8b70782e1337eccf10717dae
2019-01-31 11:35:05 -08:00
Kevin Gozali
8a50bc3ab3 iOS: Make each module implement getTurboModuleWithJsInvoker: instead of having centralized provider
Summary:
For better modularity, each module conforming to RCTTurboModule should provide a getter for the specific TurboModule instance for itself. This is a bit more extra work for devs, but simplify tooling and allow better modularity vs having a central function that provides the correct instance based on name.

Note: Android may or may not follow this new pattern -- TBD.

Reviewed By: RSNara

Differential Revision: D13882073

fbshipit-source-id: 6d5f82af67278c39c43c4f7970995690d4a82a98
2019-01-30 17:32:16 -08:00
Pavlos Vinieratos
61ca119650 Replace deprecated stringByReplacingPercentEscapesUsingEncoding: with stringByAddingPercentEncodingWithAllowedCharacters: (#19792)
Summary:
Replace some NSString deprecated methods.
motivation for these prs is less warnings reported on xcode everytime we compile a rn app.

N/A

[INTERNAL] [DEPRECATIONS] [NSString] - Replace NSString deprecation methods.
Pull Request resolved: https://github.com/facebook/react-native/pull/19792

Differential Revision: D8515136

Pulled By: cpojer

fbshipit-source-id: 4379ef4e229ef201685b87e54ac859ba3d30a833
2019-01-25 05:25:39 -08:00
Francisco Javier Trujillo Mata
a93db4915b Removing warning for Pointer is missing a nullability type specifier … (#17872)
Summary:
Xcode 9 has compiler settings that are more strict. This can occur if someone updates there project to use the default settings.

This patch declares the default type instead of allowing the compiler to determine it. Instead of () we now say (void) in a block call.

Motivation
It was just trying to get my project totally empty of warnings, and it has no side effects. If there are side effects, then we should fix the type and not go with empty to represent void.

Test Plan
Update project settings in Xcode. This code doesn't have any known side effects since the compiler assumes the type is void when not declared.

Release Notes
[DOCS] - Fixed potential compiler build issue on Xcode 9 after updating settings in project.
Pull Request resolved: https://github.com/facebook/react-native/pull/17872

Differential Revision: D6981435

Pulled By: hramos

fbshipit-source-id: 508ecea0f8874dc16a25f1dee6255481b309f8c2
2019-01-16 09:50:48 -08:00
Wojciech Tyczynski
3654b9edb2 Differentiate swipe and tap events (#22916)
Summary:
Motivation:
----------

As developers want to handle multiple actions on Siri Remote input when using TVEventHandler, it is crucial to differentiate 'swap' and 'tap' events.

Changelog:
----------

[tvOS] [Changed] - 'up', 'down', 'left' and 'right' events are now connected with tapping on edges of remote. New events 'swipeUp', 'swipeDown', 'swipeLeft' and 'swipeRight' added to detect swipes.
Pull Request resolved: https://github.com/facebook/react-native/pull/22916

Differential Revision: D13682705

Pulled By: hramos

fbshipit-source-id: 233ad1cecc04ca4ced75cd00e7fcb65d224ed3ca
2019-01-15 16:29:38 -08:00
Alexander Nikiforov
19d04a312b iOS: Clear Linking.getInitialURL during bridge reload (#22659)
Summary:
On iOS platform, RN retains launchOptions dictionary after bridge reload which can lead to unexpected consequences to a developer. The app will receive the same value for `Linking.getInitialURL` during initial launch and during bridge reload. Here's an example from our application. We use deeplinks via custom URL scheme so a user can open the app via link. Also, we reload the bridge when a user signs out. So if a user opens the app via URL, logs out, and a second user logs into the app, the app will behave as though the second user launched the app via the same deeplink. Because reload destroys the JS engine, there's nowhere for our app to remember that it already handled the deeplink activation.

On iOS Linking.getInitialURL() gets URL from the _launchOptions dictionary, so by setting it to nil we prevent retention of initialURL after reload.

This change makes iOS's behavior consistent with Android's. On Android, the launch URL is stored on the `Intent` and reloading the app involves creating a new `Intent`. Consequently, the launch URL is dropped as desired during the reload process.
Pull Request resolved: https://github.com/facebook/react-native/pull/22659

Differential Revision: D13564251

Pulled By: cpojer

fbshipit-source-id: 4c6d81f1775eb3c41b100582436f1c0f1ee6dc36
2019-01-15 02:10:08 -08:00
Spencer Ahrens
a3b348eacb clean up surface register / start
Summary:
`RCTSurfaceHostingProxyRootView` surfaces are still automatically started right after the initialization to match `RCTRootView` interface, but `RCTSurfaceHostingView` must be started explicitly now. Also fixed some internal stuff so start and register are clear and distinct.

Background / initial motivation:

One tricky bit - we render the template as part of init`ing the rootView, so we don't know what the surfaceId will be before hand to register the UITemplate. Two possible solutions:

1) Require start be called explicitly after initializing the rootView, and setup the context in between.
2) Do something like "setUITemplateConfigForNextSurface" before creating the rootView, and have some hook when the surfaceId is assigned that associates the surfaceId with that "next" UITemplate stuff before.

(1) seems a lot cleaner, but it requires ever user of rootView to explicitly call start on it - how do you feel about that? Seems like we could also use that start call to decide if the initial render should be synchronous or not? start vs. startSync?

Reviewed By: mdvacca

Differential Revision: D13372914

fbshipit-source-id: 6db297870610e6c231f8a78c0dd74d584cb64910
2018-12-21 18:00:35 -08:00
Alex Dvornikov
97eb53d14f Update RCTFormatError to support segment ids
Reviewed By: PeteTheHeat

Differential Revision: D13507444

fbshipit-source-id: ed55ce4cfa26f54db87a753867b6cf710936ba5a
2018-12-18 16:23:39 -08:00
Valentin Shergin
1bd66d9aa9 RCTSurface: Calling start is now required to start the Surface
Summary:
So, it does not start itself automatically right after instantiation.
(Classic RCTSurface still kinda start itself automatically but only because start/stop concept is not implemented for this yet.)

Reviewed By: sahrens

Differential Revision: D13461294

fbshipit-source-id: 05430688f69a0d9bf75d03e6d25f02ccd5d3176a
2018-12-18 12:57:38 -08:00
Kevin Gozali
54a6e1ad38 Surface: fixed up surface stage value check
Summary: Some logic to check for surface stage should've done bitwise `&` operation instead of equality check, because we do bitwise `|` whenever we "set stage".

Reviewed By: shergin

Differential Revision: D13459156

fbshipit-source-id: 94e2f5279fb1a31060beb7d6195953b25ce603c9
2018-12-13 16:52:30 -08:00
Birkir Rafn Guðjónsson
ba9c208cdf Support additional UIBarStyle's in RCTConvert (#20102)
Summary:
Adds two additional UIBarStyles to RCTConvert

- [x] UIBarStyleBlackOpaque
- [x] UIBarStyleBlackTranslucent

Does not affect any tests or current usage of this conversion.
Pull Request resolved: https://github.com/facebook/react-native/pull/20102

Differential Revision: D13421942

Pulled By: hramos

fbshipit-source-id: 1e609eca0fdea2b56b9f6ac87e759c661bdee12b
2018-12-11 17:43:24 -08:00
Zack Sheppard
2831d9ef61 Extend reason message for RCTFatalException (#22532)
Summary:
Fixes #22530

As described in the issue, the previous behavior for the `RCTFatal` macro was to truncate the `reason` on the resulting `NSException` to 75 characters. This would ensure the reason would fit on a single line, but resulted in issues debugging errors that occurred in the wild, as many crash logging tools (like Sentry) discard the `name` value of the exception and use the `reason` as their primary identifier. At 75 characters, useful information like the location of the error would usually be truncated.

- [x] This extends the truncation threshold to 175 characters, which should be short enough to prevent full-screen-takeover length errors, but long enough to provide useful context to the error.
- [x] This adds a `userInfo` value to the resulting `NSException`. It copies over the `userInfo` from the `NSError` passed to the macro, and adds an "untruncated message" value that contains the untruncated version of the `NSException`'s reason.

[iOS] [Changed] - RCTFatalExceptions now include more information in their reason and a userInfo.

<!--

  CATEGORY may be:

  - [General]
  - [iOS]
  - [Android]

  TYPE may be:

  - [Added] for new features.
  - [Changed] for changes in existing functionality.
  - [Deprecated] for soon-to-be removed features.
  - [Removed] for now removed features.
  - [Fixed] for any bug fixes.
  - [Security] in case of vulnerabilities.

  For more detail, see https://keepachangelog.com/en/1.0.0/#how

  MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

  EXAMPLES:

  [General] [Added] - Add snapToOffsets prop to ScrollView component
  [General] [Fixed] - Fix various issues in snapToInterval on ScrollView component
  [iOS] [Fixed] - Fix crash in RCTImagePicker

-->
Pull Request resolved: https://github.com/facebook/react-native/pull/22532

Differential Revision: D13373469

Pulled By: cpojer

fbshipit-source-id: ac140d14ce76e1664869437c2c178bdd65ab6e0e
2018-12-06 20:22:25 -08:00
Peter Argany
18f3de9dce Allow init of Native Module before bridge is initialized [2/N]
Summary:
This may be controversial.

Right now, RelayPrefetcher is initialized [here](https://fburl.com/p01iunr1), after bridge is initialized. I want to create a FBRelayPrefetcherModule instance eagerly (diff 3 in stack), and then pass that into the bridge module registry. This way, native side gets to use RelayPrefetcher before bridge is init, and JS still accesses the same instance of FBRelayPrefetcherModule.

The only other option is drastically change bridge init, to allow passing in some eagerly initialized instances.

Reviewed By: shergin

Differential Revision: D13164277

fbshipit-source-id: b45111cd68d78820e61e4fca7e54a7e8df32a3f0
2018-11-26 17:28:11 -08:00
Kevin Gozali
aad83cc238 iOS TM: RCTEnableJSINativeModule => RCTEnableTurboModule
Summary: Leftover name change from previous codemod.

Reviewed By: shergin

Differential Revision: D13117615

fbshipit-source-id: 2584a2a90d3db6ed9a9e9cb8727c51da34f0927c
2018-11-20 01:20:59 -08:00
Karan Thakkar
1fa56a0ac4 Rename requiresMainThreadSetup -> requiresMainQueueSetup in code comment (#22328)
Summary:
Update reference to property in code comment in `RCTBridgeModule`. There is no such thing as `requiresMainThreadSetup` in the codebase. Its called `requiresMainQueueSetup` now.
Pull Request resolved: https://github.com/facebook/react-native/pull/22328

Differential Revision: D13115352

Pulled By: shergin

fbshipit-source-id: 511d627388b51029821c4b1ddf4caac30e573040
2018-11-17 21:43:07 -08:00