Summary:
`Better` is a trivial collection of basic tools borrowed from other low-level general purpose libraries (like Folly, Abseil or Boost). The main goals of Better:
- Make the codebase more portable;
- Make the dependency list explicit (by decoupling it as a dependency list of Better);
- Make relying on modern C++ patterns and tools in code simple and easy.
- Make executing experiments with different dependencies easier.
As a first example usage, this diff replaces std::unordered_map with an efficient one from folly on the one of the hottest paths.
Reviewed By: JoshuaGross
Differential Revision: D13944565
fbshipit-source-id: 5fa2c4abe6c17f7361eddcc25f968b6440d5d9db
Summary:
Kotlin is getting traction and more developers write RN native modules in it. This PR adds nullable annotations to help with Kotlin null inference and improve developer experience. Also it'll help checking code quality using lint.
I skimmed through JavaOnlyMap.java, JavaOnlyArray.java, ReadableNativeArray.java, ReadableNativeMap.java, WritableNativeArray.java and WritableNativeMap.java to infer nullability.
This is breaking change to Kotlin code.
[Android] [Changed] - Add nullable annotations to ReadableMap, WritableMap, ReadableArray, Writable.
Pull Request resolved: https://github.com/facebook/react-native/pull/23329
Differential Revision: D14002571
Pulled By: cpojer
fbshipit-source-id: 899d8b3b0a5dad43e8300e6c4ea4208cca0f01a9
Summary:
Changed Danger's config so that it provides advice whenever it finds an issue with the pull request template, instead of posting a warning.
Updated Danger several major versions, from 2 to 7. I worked through any breaking changes, which were minimal (change `yarn danger` to `yarn danger ci`).
Added a flag to have Danger post these messages as GitHub Checks instead of as a comment. This slightly buries Danger's output, as it's no longer posted as a comment, but I believe it integrates more nicely into the GitHub interface.
[GENERAL] [Changed] - GitHub-only change: updated Danger config to be nicer to PRs
Pull Request resolved: https://github.com/facebook/react-native/pull/23334
Differential Revision: D14002313
Pulled By: cpojer
fbshipit-source-id: b97ca7b7bd164646b249b7c64b1134306e0f38a8
Summary: Turn on xplat warnings individually rather than all at once. This makes it easier for the team to add new lints that we don't necessarily want to enable globally.
Reviewed By: panagosg7
Differential Revision: D13993571
fbshipit-source-id: 019f301f584be268fdcbf5b0540f63ce6fcd54e3
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
Summary: This tool doesn't work and is a frequent source of pain for people. Nobody maintains it. Let's kill it and stop recommending it to people (PR on the website repo here: https://github.com/facebook/react-native-website/pull/761 ).
Reviewed By: TheSavior
Differential Revision: D13987587
fbshipit-source-id: d04d31d3ecba4b9f9387ecb86355b5a5d08485c4
Summary:
Moves `.clang-format` to repo root to allow for easier discoverability, and integration with auto-format plugins for editors and IDEs.
Pull Request resolved: https://github.com/facebook/react-native/pull/23328
Differential Revision: D13986715
Pulled By: davidaurelio
fbshipit-source-id: fcfda59842da10cd4bc02e4550b74782fbb59e0d
Summary: This is the first step in organizing React Native slightly differently. This doesn't set up a "monorepo" structure for the GitHub repo yet, it merely moves a few files around and I slightly updated the package.json file for the codegen project.
Reviewed By: rickhanlonii, TheSavior
Differential Revision: D13974180
fbshipit-source-id: f53375f3b6618ef12658064cb1fc690ef1f95299
Summary:
Fixes#21243.
Fixes#20908.
Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: https://github.com/facebook/react-native/pull/21951
Differential Revision: D13980799
Pulled By: cpojer
fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
Summary:
Our long-term plan is to completely illuminate `jsi::Value`-to-`folly::dynamic` serialization step in prop parsing process improving performance and memory pressure. At the same time, we don't want to introduce a hard dependency in application code to JSI because it exposes direct access to VM and prevents parsing some data that come *NOT* from JSVM.
RawValue is an extremely light-weight (hopefully fully optimized-out) abstraction that provides limited JSON-like and C++-idiomatic interface.
The current particular implementation is still using `folly::dynamic` inside, but I have fully JSI-powered one which will replace the current one right after we figure out how to deal with folly::dynamic-specific callsites. Or we can implement RawValue in a hybrid manner if a code-size implication of that will be minimal.
Reviewed By: JoshuaGross, mdvacca
Differential Revision: D13962466
fbshipit-source-id: e848522fd242f21e9e771773f2103f1c1d9d7f21
Summary: Nothing really changed; the change is only to better express an original intent.
Reviewed By: JoshuaGross
Differential Revision: D13962464
fbshipit-source-id: f385db8ba8662f2150181e47fc6a2a981f809e96
Summary: At times, the lookup logic may find a class that's not compliant with RCTTurboModule. If so, it shouldn't be instantiated, and we assume the module doesn't exist.
Reviewed By: JoshuaGross, RSNara
Differential Revision: D13979004
fbshipit-source-id: ac1fa9cc456715cddd101fff13f5a41f9528a74e
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
David Vacca
Valentin Shergin
Christoph Nakazawa
Dulmandakh
Héctor Ramos
Daniel Sainati
Peter Argany
David Aurelio
dchersey
Kevin Gozali
Joshua Gross
Ram N
zhongwuzw
Spencer Ahrens
empyrical
REDMOND\acoates
Angel Kjoseski
Ramanpreet Nara
Gabe Levi
Igor Mandrigin