Summary: Simple macro to do check if TurboModule is enabled and the particular object is RCTTurboModule compliant.
Reviewed By: PeteTheHeat
Differential Revision: D13978368
fbshipit-source-id: 660c7cab7bb074d80d57abead951dad19306ae73
Summary: Recycling and dealloc were not implemented at all before for Slider, so I've taken a first stab at it. It's a little more complex than I initially thought, due to things I don't 100% understand about UISlider as well as Fabric, so I've left a TODO note to fix this at some point. We should be aware that view recycling doesn't appear to be working the way I would expect currently though.
Reviewed By: shergin
Differential Revision: D13965475
fbshipit-source-id: fd18a219cead770b63b514fdc868e23214e735b7
Summary: Don't use shared_ptr in this case, it's not needed.
Reviewed By: shergin
Differential Revision: D13965413
fbshipit-source-id: ec98c13f53c7d558a0cb68cea0f97568dd202cd8
Summary:
Otherwise, the bot will nag issues that should be whitelisted. This is a trivial change,
Pull Request resolved: https://github.com/facebook/react-native/pull/23319
Differential Revision: D13974621
Pulled By: hramos
fbshipit-source-id: e3b0f98f0332c75563b2031474fecf4317525cad
Summary:
Go back to simpler issue template titles, with emoji. The goal is to make it easier, at a glance, to pick the right template.
We also add a new "regression" template that will automatically label the issue as a regression.
Other changes:
- Added Christoph as a CODEOWNER for package.json and markdown file changes. Usually, any PR that touches package.json has to be imported manually by a Facebook employee.
[General] [Changed] - Updated GitHub issue templates.
Pull Request resolved: https://github.com/facebook/react-native/pull/23296
Differential Revision: D13974378
Pulled By: hramos
fbshipit-source-id: d8962a84397c16e7556ac167a3505f7c7d725915
Summary:
add back buildToolsVersion to build.gradle, because many third-party modules depend on it. The lack of this info causing issues in 0.58.
[Android] [Changed] - add back buildToolsVersion to template
Pull Request resolved: https://github.com/facebook/react-native/pull/23316
Differential Revision: D13974264
Pulled By: hramos
fbshipit-source-id: 058fc5617196d6481d1b69d2420c8b2be9cf4f81
Summary: Flow doesn't guarantee that AnimatedMock and AnimatedImplementation won't diverge. Christoph suggested a quick jest test in D13811035
Reviewed By: cpojer
Differential Revision: D13953915
fbshipit-source-id: ba5aeafded429113cc60a6250b5b29d2f8c8ab28
Summary:
In the path to TurboModules, this was the only package that was using LazyReactPackage. Moving this to use TurboReactPackage now ensures that all the packages now have a similar interface, and more importantly, implement `NativeModule getModule(String name)` method.
Note that in OSS, we still do not run the annotation processor. As a result, we manually create the ReactModuleInfoProvider map, so that this works in OSS also.
Reviewed By: fkgozali
Differential Revision: D9419573
fbshipit-source-id: f5b15713aff0c1a221767f1c23d9b76b04434570
Summary:
This is a re-submit of D13895627 which got landed but didn't include a fix to Instagram's code. The sheriffs were unsure how it got landed without running the build.
Currently, before we store the image to cache, we only respect `Cache-Control`, actually, we also may need to check `Expires`、`Last-Modified`, refer to [MDN docs](https://developer.mozilla.org/en-US/docs/Web/HTTP/Caching#Freshness), and [okhttp](568a91c44a/okhttp/src/main/java/okhttp3/internal/cache/CacheStrategy.java (L268)) respect the `MDN`, so in iOS, we can also respect this.
[iOS] [Fixed] - Respect `MDN` cache strategy before cache the image.
Reviewed By: shergin
Differential Revision: D13896822
Pulled By: cpojer
fbshipit-source-id: 8c1714f4a17ad40496146806cff3e188a60be93c
Summary:
RN supports API 16 and above, but we have redundant historical artifacts where we check and target APIs 16 and below. This PR removes redundant artifacts.
[Android] [Changed] - remove redundant targetApi and version checks
Pull Request resolved: https://github.com/facebook/react-native/pull/23302
Differential Revision: D13970434
Pulled By: mdvacca
fbshipit-source-id: 096b5ee6c8f076b0365e7dda0e77940290077ea2
Summary: The biggest change is that (1) the image proxy/observer code from the Image component has been generalized, (2) the four image props for the Slider component are fully supported, (3) a handful of props that were ignored or buggy on iOS now perform as expected.
Reviewed By: shergin
Differential Revision: D13954892
fbshipit-source-id: bec8ad3407c39a1cb186d9541a73b509dccc92ce
Summary:
Follow up to address Spencer's feedback on D13811035
With the spread of AnimatedImplementation, it's impossible to tell whether AnimatedMock gets out of date.
This change coupled with D13953915 forces anyone adding to AnimatedImplementation to consider the mock.
Reviewed By: sahrens
Differential Revision: D13965249
fbshipit-source-id: e324364a75abd42d89d6222151453021618bcd5d
Summary:
Make Text prop types exact to catch tons of errors, including typos like in https://fb.workplace.com/groups/rn.support/permalink/2306953619353240/.
I tried to fix things when it was totally obvious what the intent was, but otherwise tried to keep the existing behavior the same, even if it meant that usage of some props was getting ignored, like `hitSlop`.
Reviewed By: TheSavior
Differential Revision: D13892999
fbshipit-source-id: 5003508a648287e4eca8055fb59da5f03bd066cc
Summary:
This removes the accidental double include of the header `unistd.h` from `JSBigString.h`. One was added by me as part of #22330, and one by matthargett as part of #21764
[General] [Fixed] - `JSBigString.h`: Removed accidental double include of header `unistd.h`
Pull Request resolved: https://github.com/facebook/react-native/pull/23297
Differential Revision: D13961223
Pulled By: hramos
fbshipit-source-id: c0dba8a475b3c09356d34cb65b989c286793fa67
Summary:
Location, NetInfo and Vibration modules require its own permissions to work properly, and Android Studio and lint shows warning in the modules because permissions are not found in AndroidManifest.xml of ReactAndroid, due to that not all apps require these functionalities/permissions. Therefore, developers have to add required permissions if they want to use before mentioned functionalities.
This PR suppresses missing permission warnings.
[Android] [Changed] - Suppress missing permission warnings
Pull Request resolved: https://github.com/facebook/react-native/pull/23294
Differential Revision: D13959120
Pulled By: cpojer
fbshipit-source-id: 10526f1218989b805528a5415e03371d5865be63
Summary:
Running *lint* on RN found that there are some Java 8 features used without specifying Java 8 compatibility in projects. This PR adds Java 8 compatibility and fixes errors caused by Java 8 feature use. I suspend that it may be cause of many failures on older Androids, but also found that many modules/packages switched to and require Java 8.
```java
../../src/main/java/com/facebook/react/devsupport/BundleDownloader.java:167: Try-with-resources requires API level 19 (current min is 16)
../../src/main/java/com/facebook/react/devsupport/DevServerHelper.java:658: Try-with-resources requires API level 19 (current min is 16)
```
For more information https://developer.android.com/studio/write/java8-support
[Android] [Changed] - Enable Java 8
Pull Request resolved: https://github.com/facebook/react-native/pull/23295
Differential Revision: D13959096
Pulled By: cpojer
fbshipit-source-id: 0bfd0565b61a132906cf35ee55b4afcf5450f7cb
Summary:
This change is aiming to reduce some of the forking changes we have internally in order to use CxxReact for some additional out of tree platforms.
Some of the fixes allow more of the code to compile when using Microsoft Visual Studio Compiler. In particular the change around the default value of RN_EXPORT and some changes around how to enable the packing attribute.
Another change moves more of the code for JSBigFileString into the cpp file, so that people can share the header but replace the implementation as appropriate for other platforms.
And finally the removal of an unused header include.
This is unlikely to be the extent of the changes required for MSVC, but at least gets one of our complication blocks to work against an unforked RN.
Pull Request resolved: https://github.com/facebook/react-native/pull/22182
Differential Revision: D12967758
Pulled By: cpojer
fbshipit-source-id: a2cc018aedaa9916cd644bfbd9e3a55330cd4c52
Summary:
React Native's minSdkVersion is 16, or we support Android versions 16 (Jelly Bean) and above. But in the code we have many checks if Android is Jelly Bean or newer, which are unnecessary. This PR removes unnecessary Android version checks, also uses Android version names instead of numbers.
[Android] [Changes] - remove unnecessary Android version checks
Pull Request resolved: https://github.com/facebook/react-native/pull/23277
Differential Revision: D13955909
Pulled By: cpojer
fbshipit-source-id: 6b1caa5ef4fe42273d3c69a6617fff140a697b5c
Summary:
This pull request adds a function called `warnOnce` that prints a warning message to the console once per session.
It uses a unique key per callsite to help ensure that the message is not printed multiple times.
[General] [Added] - Added new `warnOnce` function for printing a message to the developer console once per session
[General] [Changed] - Changed the warnings in `react-native-implementation` to use `warnOnce`
Pull Request resolved: https://github.com/facebook/react-native/pull/22109
Differential Revision: D13955887
Pulled By: cpojer
fbshipit-source-id: aa51ac427a80cc0554a6bcc915715821d0bd5439
Summary:
Adding support for `intent://` URIs.
More details about this type of URIs: https://developer.chrome.com/multidevice/android/intents
Reviewed By: mdvacca
Differential Revision: D13817496
fbshipit-source-id: e1eeb8bd4e293e2c24ce0910286724108b3e10de
Summary:
Animated views can cause flakiness in snapshot tests. This mock replaces all provided Animated transforms with a blank animation.
This could potentially break some tests which animate in elements and then verify their existence. I can deal with that fallout in follow up diffs. One option is making all animations take 0 seconds when testing.
Reviewed By: cpojer
Differential Revision: D13811035
fbshipit-source-id: cc6b13c7d6bad29b125d35ef759a269bb0372e67
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
Summary: Now that Flow v0.92.1 is deployed, we can backout {D13942380}, which was a temporary fix.
Reviewed By: avikchaudhuri
Differential Revision: D13952515
fbshipit-source-id: d423379a03fff09316aa499e1152b410a1bb4178
Summary:
This diff adds performance loggers for Fabric in Android to be able to compare current version or RN with Fabric
This is the summary of Points and Annotations:
- **UIManager_CommitStart**: time that React starts the commit (react tree is ready to start rendering in native)
- **UIManager_LayoutTime**: this is the time it takes to calculate layout in yoga
- **UIManager_FabricFinishTransactionTime**: Time it takes transform "C++ mutationInstructions" into "Java MountItems" and cross boundaries from C++ to Java (including serialization of data) (THIS IS ONLY FABRIC)
- **UIManager_DispatchViewUpdates**: time right before RN moves the mount operations to the Queue that is going to be processed in the next tick UI thread
- **UIManager_BatchRunStart**: time right before the mountItems are going to be process in the UI Thread
- **UIManager_BatchedExecutionTime**: time it took to run batched mountItems (usually layout and prop updates on views)
- **UIManager_NonBatchedExecutionTime**: time it took to run non-batched mountItems (usually creation of views)
Reviewed By: fkgozali
Differential Revision: D13838337
fbshipit-source-id: 0a707619829e7d95ce94d9305ff434d1224afc46
Summary:
ViewHelper has only setBackground method which works identical to ViewCompat.setBackground from Android Support Library. This PR removes ViewHelper, and uses ViewCompat instead.
[Android] [Changed] - remove ViewHelper, use ViewCompat instead
Pull Request resolved: https://github.com/facebook/react-native/pull/23280
Differential Revision: D13950510
Pulled By: mdvacca
fbshipit-source-id: 10b5122affac17d4b66fb995339c6715c0871ed0
Summary:
Follow-up to https://github.com/facebook/react-native/pull/19809
This fix generalizes the `setAttributedString:` fix to single-line text fields.
Fixes#19339
_Pull requests that expand test coverage are more likely to get reviewed. Add a test case whenever possible!_
Pull Request resolved: https://github.com/facebook/react-native/pull/22546
Differential Revision: D13948951
Pulled By: shergin
fbshipit-source-id: 67992c02b32f33f6d61fac4554e4f46b973262c1
Summary: Just tightened up the lookup logic to guard against nullptr.
Reviewed By: RSNara
Differential Revision: D13948001
fbshipit-source-id: 55bf46619712e60e195ea12f2b8eb799f80a3bbe
Summary:
It's okay for TurboModules to not exist. Therefore, `__turboModuleProvider(moduleName)` should return null if `moduleName` doesn't refer to an actual TurboModule. The current implementation assumes that the TurboModule must exist, and therefore, it crashes the app when it doesn't (assertion error).
This change is important because it helps us land D13887962. When we switch `I18nResources` to `TurboModuleRegistry` in D13887962, on Wilde, it won't be found in `NativeModules`. Therefore, we'll try to lookup this module in `__turboModuleProvider`, and this will crash the app. It seems like `I18nResources` is a Java-only module?
Reviewed By: fkgozali
Differential Revision: D13924589
fbshipit-source-id: 7ac7b1873e06852e5aafcaaef5c24cbc548ee444
Summary: `__turboModuleProxy` doesn't exist if you're not in the TurboModules QE. If such is the case, then we should just return null when `TurboModuleRegistry.get` is called.
Reviewed By: fkgozali
Differential Revision: D13937143
fbshipit-source-id: d3f11c52b7cbecaefba675d714f0d67236071389
Summary:
@public
Makes it work nicely with gtest.
Also allows rhs *and* lhs to offer `operator(YGStyle)()`.
Reviewed By: SidharthGuglani
Differential Revision: D13942573
fbshipit-source-id: ff8b3a9aa6f05ca1b0572eb97d0ad23b09d77871
Summary:
@public
Adds `_pt` and `_percent` user defined literals to create `YGValue` instances.
This allows to create `YGValue`s in the following form:
```
use namespace facebook::yoga::literals;
auto a = 123_pt; // == YGValue{123.0f, YGUnitPoint}
auto b = -12.5_percent; // == YGValue{-12.5f, YGUnitPercent}
```
Reviewed By: SidharthGuglani
Differential Revision: D13942100
fbshipit-source-id: ce1e2f9431c3e2a99c6e11896a712539cc535e0d
Summary: Flow v0.92 is crashing sometimes after a save when run through Nuclide. This fixes the issue temporarily until we can push a Flow v0.92.1
Reviewed By: samwgoldman
Differential Revision: D13942380
fbshipit-source-id: f7bf2aef0aab90980bbb786d89710f399c9427d5
Summary:
When using react-native inside a monorepo with yarn workspaces the react-native installation can get hoisted and the assumption that the project is located in the folder where node_modules is is no longer true.
To handle this case I added an option to pass the path of the project root.
For example when using yarn workspace we can use something like this in the xcode scripts:
Assumes the following folder structure
/packages/myapp
/node_modules/react-native
```sh
export NODE_BINARY=node
export PROJECT_ROOT=$PWD/..
../../../node_modules/react-native/scripts/react-native-xcode.sh
```
It could be possible to change the default to `$PWD/..` since pwd is where the xcode project is located but then it would break if the native project structure is not the one we assume. To avoid the breaking change I decided to just keep the existing behaviour and allow changing the path with `PROJECT_ROOT`.
[ios] [added] - Allow changing the project path in react-native-xcode.sh
Pull Request resolved: https://github.com/facebook/react-native/pull/23273
Differential Revision: D13941793
Pulled By: cpojer
fbshipit-source-id: f394641b1c9d01f32bc4169097e39fc14df8191f
Summary:
When using building React Native using the `.xcodeproj` (either via linked projects in Xcode, or precompiling React Native, which is what we do), you'll get a duplicate symbol error:
```
duplicate symbol __ZN8facebook5react10IInspectorD0Ev ...
```
And a few more.
This is because the `InspectorInterfaces.cpp` file is included in _both_ the `React` target _and_ the `jsinspector` target, and since the `jsinspector` target gets linked into the `React` target, this means the symbols from `InspectorInterfaces.cpp` end up in `libReact.a` twice.
<img width="187" alt="screen shot 2019-02-04 at 11 43 39 am" src="https://user-images.githubusercontent.com/819705/52189088-93190880-288a-11e9-8411-b44b59e8e461.png">
This PR removes `InspectorInterfaces.cpp` from the `React` target, as I believe was the original intent.
Since this bug is in the `xcodeproj` it only affects builds that use that, so CocoaPods and Buck users are unaffected.
[iOS][Fixed] - Fix potential linker issues when using xcode project
Pull Request resolved: https://github.com/facebook/react-native/pull/23284
Differential Revision: D13941777
Pulled By: cpojer
fbshipit-source-id: 8a3ffb4fc916ff6570bbff8794b4515b48055667
Summary:
- Improve type inference of the `event.easing` keyboard event property with the `KeyboardEventEasing` type.
- Exporting `KeyboardEventName` and `KeyboardEventEasing` for others to use.
- Removing unnecessary optional property flag (ie.`?`) for all properties in `KeyboardEvent`. (I personally haven't seen any of the properties not being returned for all keyboard events).
[iOS] [Changed] - Refine Keyboard API Event typings
Pull Request resolved: https://github.com/facebook/react-native/pull/23272
Differential Revision: D13941758
Pulled By: cpojer
fbshipit-source-id: 4891898c29cf6963069dbe6b4d439694761fd9e7
Summary:
Commit 06c13b3e06 pretend to update jest
to new version, but also adds jest and jest-junit to runtime dependencies
list (in addition to dev dependencies).
This commit fixes this, by removing jest and jest-junit from runtime
dependeincies.
As for motivation... story is short:
1. Updating RN 0.58.1 -> 0.58.3 I found that whole babel@6 infrastructure was returned to my `node_modules` (it is dangerous, because many tools like metro can found it and use it instead of babel@7).
2. It was because of this commit: 9d19ab0c0c that have 130 random files changed (and provides no meaning description).
But it points to: b864e7e63e (which also provides no meaning description, why it was reverted?)
And finally points to: 696bd89013, which was a "sync with master" commit. Ok. So it was from master.
3. Then I checked that jest still in runtime dependencies on master and fixed that. Also I hope this will be cherry picked into 0.58-stable.
[General] [Fixed] - Fix jest and jest-junit to be only development dependencies
Pull Request resolved: https://github.com/facebook/react-native/pull/23276
Differential Revision: D13941275
Pulled By: cpojer
fbshipit-source-id: a6f3377e670554b21f3ebd2f12d33e29d2969df1
Summary: This is a temporary change to measure production data
Reviewed By: fkgozali
Differential Revision: D13906807
fbshipit-source-id: 2a2f71aa379c4aca63c7bb4a9644704f713cb088
Summary: Simple diff that adds QPL marker to track time it takes to load .so file used by RN Fabric
Reviewed By: fkgozali
Differential Revision: D13934149
fbshipit-source-id: a5cb0fc255394cf6c936b9018c753dcc774c347f
Summary: The goal is to be able to use MobileConfig params inside of core React Native C++ code. This works on Catalyst iOS now and Wilde; need to add support for FB4A and Catalyst Android.
Reviewed By: fkgozali
Differential Revision: D13883007
fbshipit-source-id: 115fe6cc884d2a0b9ca26dadf867a5f0ae99f262
Summary: For now, do `require('NativeModules')` instead of `import {NativeModules} from 'react-native'`.
Reviewed By: mdvacca
Differential Revision: D13934066
fbshipit-source-id: 5188b11428a4dca8cecd1934e593d89a6e3fde2e
Summary:
Given two apps loaded side-by-side and when a `Keyboard` event is triggered, there is no way to ascertain which app triggered the keyboard event. This ambiguity can arise in slide over/split view scenarios.
This pull request exposes the `isLocalUserInfoKey` property of the native `UIKeyboard` iOS events to the `Keyboard` event listener; this property will return `true` for the app that triggered the keyboard event.
(Also, I threw in a couple of Keyboard.js tests just for fun 😅)
[iOS][Added] - Expose isLocalUserInfoKey to keyboard event notifications
1. Load two apps side-by-side, with the app on the left side subscribing to the keyboard events (and logging out the events as they happen)
1. Trigger a keyboard to appear with the left app. The logged keyboard event will contain the `isEventFromThisApp` property which will be true.
1. Dismiss the keyboard
1. Trigger a keyboard to appear with the right app. The left app will still log the keyboard event, but the event's `isEventFromThisApp` property will be false (because the left app didn't trigger the keyboard event)
Pull Request resolved: https://github.com/facebook/react-native/pull/23245
Differential Revision: D13928612
Pulled By: hramos
fbshipit-source-id: 6d74d2565e2af62328485fd9da86f15f9e2ccfab
Kevin Gozali
Dulmandakh
Joshua Gross
Héctor Ramos
Peter Argany
David Vacca
Ram N
zhongwuzw
Spencer Ahrens
empyrical
REDMOND\acoates
Angel Kjoseski
Ramanpreet Nara
Gabe Levi
Igor Mandrigin
David Aurelio
Danilo Bürger
Vojtech Novak
Janic Duplessis
Tyrone Trevorrow
nossbigg
Alex M
Sam Linnett
Vladimir Timofeev
