fix: Bundle assets in monorepo #26940

Esemesek · 2019-10-21T10:55:23Z

Summary

In monorepo environment, metro isn't able to resolve react-native because the path to it is hardcoded.

I've also added packagingOptions to RNTester to make Android builds work for me. Let me know if this is something that is only specific to my setup, and shouldn't be added.

Changelog

[Android] [Fixed] - Fix bundleReleaseJsAndAssets in monorepo env

Test Plan

- Works in monorepo setup on MacOS
- Works with RNTester app

facebook-github-bot · 2019-10-21T10:55:33Z

Hi Esemesek! Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

facebook-github-bot · 2019-10-21T10:59:56Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

thymikee · 2019-10-21T10:59:44Z

RNTester/android/app/build.gradle

@@ -109,6 +108,15 @@ android {
        targetCompatibility JavaVersion.VERSION_1_8
    }

+    packagingOptions {
+        pickFirst 'lib/x86/libc++_shared.so'


cc @dulmandakh, ideas why @Esemesek needed that for the build to happen? Is this his local env issue, or something reproducible?

@Kudo will have more details here. We have discussed this and it's known issue in RNTester (I had to set it myself a few times too). It has something to do with the way packages are bundled and distributed, but I don't have enough context into that side of the build system.

The pickFirst concern is coming from 3rd party libraries built by different NDK and may depend on different libc++_shared.so. pickFirst cannot be a reliable way to choose which libc++_shared.so will be used.
Some context at: facebook/hermes#74
I was proposing to remove pickFirst especially from RN template.
For RNTester it is somehow acceptable but it would be great if we could not depend on it.

Both JSC and Hermes excluded libc++_shared.so right now.
The libc++_shared.so should come from Flipper.
@willholen did have a PR for that and it seems be fixed from Flipper side.
Maybe upgrade Flipper will fix the conflict issue.

Thanks @Kudo an explanation. I will remove this section.

react.gradle

thymikee

cc @grabbou

AshishKapoor

Looks good!

janicduplessis · 2019-10-28T04:14:18Z

Could we use

react-native/ReactAndroid/build.gradle

Line 223 in c591b08

def findNodeModulePath(baseDir, packageName) {

instead to find where the cli is installed? It was added recently to make resolution of node modules in build scripts better.

cpojer

See @janicduplessis's comment.

grabbou · 2019-10-30T10:37:04Z

@janicduplessis

In CLI, we have structured and developed code towards not relying on paths and locations at all. That way, we ensure smooth support for all projects, no matter their structure, as well as different resolves, such as Yarn PnP in the future.

Thanks to running the CLI with yarn or npx (that is required anyway with the new react-native init, so using it here doesn't have any trade-off), always finds and executes the react-native CLI from the bin folder.

The CLI then, starts looking for the react-native package and root of the project (where the closest node_modules folder is located). We do that with a similar function here.

See the PR for details: react-native-community/cli#768.

With that in mind, my suggestion would be to leave npx calls here instead.

For the future, it could make sense to decide on one strategy and use it everywhere. The benefit of relying on the CLI is that we unify the paths/resolutions with native_modules.gradle (auto-linking) and don't depend on paths. We also reduce code debt by not having the same logic implemented twice.

The disadvantage is that ReactAndroid now depends on the CLI, but in reality, these two are tightly coupled to each other and Gradle already relies on the CLI (as a part of auto-linking).

Let me know what you think

CC: @ide as the author of the PR

cpojer

Nice 👍

facebook-github-bot

@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

react-native-bot · 2019-11-05T11:10:05Z

This pull request was successfully merged by @Esemesek in a3b0804.

^{When will my fix make it into a release? | Upcoming Releases}

Summary: In monorepo environment, `metro` isn't able to resolve `react-native` because the path to it is hardcoded. I've also added `packagingOptions` to RNTester to make Android builds work for me. Let me know if this is something that is only specific to my setup, and shouldn't be added. ## Changelog [Android] [Fixed] - Fix `bundleReleaseJsAndAssets` in monorepo env Pull Request resolved: #26940 Test Plan: - [x] - Works in monorepo setup on MacOS - [x] - Works with RNTester app Differential Revision: D18323703 Pulled By: cpojer fbshipit-source-id: b8eb15dfd8a32ae11fd862fc725af9cffea2cf96

Summary: In monorepo environment, `metro` isn't able to resolve `react-native` because the path to it is hardcoded. I've also added `packagingOptions` to RNTester to make Android builds work for me. Let me know if this is something that is only specific to my setup, and shouldn't be added. ## Changelog [Android] [Fixed] - Fix `bundleReleaseJsAndAssets` in monorepo env Pull Request resolved: facebook/react-native#26940 Test Plan: - [x] - Works in monorepo setup on MacOS - [x] - Works with RNTester app Differential Revision: D18323703 Pulled By: cpojer fbshipit-source-id: b8eb15dfd8a32ae11fd862fc725af9cffea2cf96

Summary: This fix aims to address the issue when bundling an Android app for release and getting the error exhibited in #28002 which I also encountered myself. The config was changed sometime in November 2019 (as part of #26940, commit a3b0804) to be very opinionated when it comes to the use of `npx` which Gradle itself cannot find anyway (I have `npx` installed globally and it didn't pick it up). Another issue that the use of `npx` creates is that Gradle should only ever use the currently installed react-native cli rather than a (possibly) higher version which may not always have backward compatibility. The proposed change simply throws a more descriptive error rather than defaulting to a tool which may or may not exist on the machine, be it CI or a development environment. I've also modified the RNTester app to reflect the correct config implementation relative to the RNTester app itself. In real projects, the config inside `android/app/build.gradle` should look similar to the following snippet: ``` project.ext.react = [ cliPath: "$rootDir/../node_modules/react-native/cli.js", entryFile: "index.js" ]; ``` ## Changelog [Android] [Fixed] - Gradle release config Pull Request resolved: #28415 Test Plan: - [x] Successfully bundled an Android release build with correct config - [x] Works with RNTester app Reviewed By: mdvacca Differential Revision: D20714372 Pulled By: hramos fbshipit-source-id: 4d66139249c6f840582a71a48c64e6a6595f7af0

* Upgrade to Metro 0.59 Summary: Upgrades RN to Metro 0.59. Changelog: [Internal] Metro Upgrade Reviewed By: motiz88 Differential Revision: D20533864 fbshipit-source-id: 3c5fb8e37d2363edf0b9a1a8cfbdefba00763415 * Fix mock for TextInput (#28332) Summary: This PR adds the `isFocused` method to the mock of the TextInput component. My understanding some of the latest changes on the TextInput to make it use a forwardRef change the way this method is mock giving an error when trying to use in on a mock. The change suggested here fixes the issue. ## Changelog [JavaScript] [Fixed] - Fix the mock for TextInput to support the `isFocused` method Pull Request resolved: https://github.com/facebook/react-native/pull/28332 Reviewed By: cpojer Differential Revision: D20538044 Pulled By: TheSavior fbshipit-source-id: be734af105ab62ffdf9ed4017bd70845e207f8cd * Properly handle LogBox errors during tests Summary: This diff fixes an issue where errors in LogBox during tests would cause the tests to crash. The crash is due to the NativeExceptionsManager module not being mocked (as all native module need to be in tests). The fix is to properly mock the NativeExceptionManger. This fix exposed an infinite loop issue where failures in LogBox will be logged to the ExceptionManager, which logs to the console, which logs to LogBox, creating a loop. This diff also fixes that look by moving the LogBox internal error check to the top of the monkey patched console methods. Changelog: [Internal] Differential Revision: D20428590 fbshipit-source-id: 7289a480c99ba8dee67772178b7629afb40b330a * Back out "Track animations and flush them" Summary: Original commit changeset: b594d0e6e9b6 D20319824 introduced a problem in LayoutAnimations, which makes surfaced as the problem in T63911344. This diff reverts D20319824. Changelog: [Internal] Reviewed By: lunaleaps Differential Revision: D20541918 fbshipit-source-id: ff72b839f57d39051122920a38b2632cbb5ec362 * Consolidate "dispatchMountItems" reentrancy prevention code, and retry code, in one function Summary: Simplifying the dispatchMountItems reentrance and retry logic. Motivation: cleanup so I can work on dispatching ViewCommands before anything else. Importantly, this gives us the properties that: 1) Only one function is responsible for calling dispatchMountItems 2) Only one function is responsible for deciding if we shouldn't call dispatchMountItems due to reentrance 3) Only one function is responsible for all cleanup 4) Only one function maintains all of the relevant flags (except dispatchPreMountItems... two total now, instead of 4 before) Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D20437035 fbshipit-source-id: 5370366790eb25f653bee6c1950e747458374a61 * Only retry ViewCommand mount items if exception is marked as "Retryable" Summary: Instead of just blindly retrying all ViewCommands if they fail - which could be dangerous, since it's arbitrary imperative commands we'd be executing twice, potentially with bad app state - we only retry if the ViewCommand throws a "RetryableMountingLayerException". Changelog: [Internal] Optimization to ViewCommands Reviewed By: mdvacca Differential Revision: D20529985 fbshipit-source-id: 0217b43f4bf92442bcc7ca48c8ae2b9a9e543dc9 * Introduce early dispatch of ViewCommands in FabricUIManager Summary: Earlier this week I introduced a change in the old, non-Fabric renderer (D20378633 D20427803) that (gated behind a feature-flag) executes ViewCommands before all other types of commands, as a perf optimization and (I think) a potential fix for a category of race conditions. I've added more details in comments here. The Fabric renderer uses the same feature-flag that I introduced for the non-Fabric renderer. Changelog: [Internal] Fabric Reviewed By: mdvacca Differential Revision: D20449186 fbshipit-source-id: bb3649f565f32c417a6247369902333989a043aa * Change nightly build from hourly to nightly (daily at 00:00) (#28346) Summary: We initially added the nightly build test to run every hour, in order to more quickly validate it. Now that it has been validated we can run it every night as it is intended to do. cc hramos ## Changelog [General] [Changed] - Change nightly build from hourly to nightly Pull Request resolved: https://github.com/facebook/react-native/pull/28346 Reviewed By: cpojer Differential Revision: D20550143 Pulled By: hramos fbshipit-source-id: 9487c6785684ad6ea7e877290d50a33118090a7f * Add diffing to app bundle size reports (#28284) Summary: Add diffing to app bundle size reports. ## Changelog [Internal] [Changed] - Add diffing to app bundle size reports Pull Request resolved: https://github.com/facebook/react-native/pull/28284 Test Plan: - App bundle size reports should now display a diff where available - Right now, the database contains only one entry for the last known good iOS build - Triggering a new build should not create additional comments Reviewed By: cpojer Differential Revision: D20450158 Pulled By: hramos fbshipit-source-id: 720772275f24d3ff0a49705f4dada2efe2e99bd3 * feat: improve monorepo support by removing redundant PROJECT_ROOT (#28354) Summary: Historically, React Native didn't support a lot of custom project structures apart from the standard flat directory with `ios` and `android` folders. The CLI had to be explicitly started from the project root, otherwise Metro didn't work right. In order to resolve the project root in the most accurate way, React Native assumed that project root is always `../../` from its location in `node_modules` - this is not true when the installation gets hoisted (e.g. in a monorepo). To address that, janicduplessis brought support for custom [`PROJECT_ROOT`](https://github.com/facebook/react-native/commit/9ccde378b6e6379df61f9d968be6346ca6be7ead) that allowed overriding the `../../` in case it wasn't true. Today, CLI is able to automatically resolve the project root, no matter where it's started. It will traverse the tree of the directories upwards and stop as soon as it meets `package.json`. As a result, it doesn't really matter from where we start the CLI anymore as a part of `react-native-xcode.sh`. By replacing the default value of `$REACT_NATIVE_DIR/../../` with `$PWD, that is default for all Xcode scripts, we can make the setup for monorepo easier - nobody will need to set `$PROJECT_ROOT` in order to override the incorrect defaults. By default, all scripts defined in Xcode run from `$PWD` directory, which is the location of the iOS project. In the future, we will be able to remove `cd` entirely. To better understand this PR, let's look a few hypothetical structures as an example: #### Monorepo: > tl;dr works out of the box, no need to mess around with paths ``` - package.json - packages/ - my-app/ - index.js - package.json - ios/ - MyApp.xcodeproj ``` **Before this PR**, the `react-native-xcode.sh` will start the CLI like this: ```bash cd $REACT_NATIVE_DIR/../../ node <absolute_path_to_cli.js> bundle --entry-point index.js ``` - Because we change the directory to the root of monorepo, CLI throws an error. All in all, there's no `react-native` dependency at the workspace root. - Some users turn `no hoist` in an act of troubleshooting the errors, which resolves the problem - `react-native` is moved under `my-app/node_modules` which makes this mechanism resolve properly. - Some users find out about `PROJECT_ROOT` and set it to overwrite the default value. For example, setting `export PROJECT_ROOT = "$PWD/../` will set the directory to `my-app`, which has a dependency on `react-native` in a `package.json` and makes the CLI happy. **After this PR**, the `react-native-xcode.sh` will start the CLI like this: ```bash cd $PWD node <absolute_path_to_cli.js> bundle --entry-point index.js ``` - The `$PWD` is `packages/my-app/ios/` because that's where the Xcode project is located. CLI will automatically set the root to `../` because that's where it finds `package.json` with `react-native` dependency. It will pass that root to Metro, unless users have set a different one themselves. Thanks to that, all paths to JavaScript files remain working and unaffected. - No need to set `PROJECT_ROOT` anymore. - We don't rely on the location of `node_modules`, which is cleaner and future proof. #### Standard: > tl;dr no changes ``` - ios/ - MyApp.xcodeproj - index.js - package.json ``` **Before this PR**, the `react-native-xcode.sh` will start the CLI like this: ```bash cd $REACT_NATIVE_DIR/../../ node <absolute_path_to_cli.js> bundle --entry-point index.js ``` - Everything works fine. Path from `react-native` inside `node_modules` is correct - the project root is set right to `/` **After this PR**, the `react-native-xcode.sh` will start the CLI like this: ```bash cd $PWD node <absolute_path_to_cli.js> bundle --entry-point index.js ``` - The root will be set to where Xcode project is located, which is `/ios`. This is the PWD for all Xcode scripts. CLI will look for the `package.json` going upwards from `ios` folder. Will stop at `/`, find out it has `react-native` dependency, load it and its commands and proceed further. ## Changelog [iOS] [Feature] - Better monorepo support when building release apk Pull Request resolved: https://github.com/facebook/react-native/pull/28354 Test Plan: - All projects (standard/monorepo) run without issues. - PROJECT_ROOT is not needed. CC: Titozzz (who wrote monorepo guide), alloy, bartolkaruza Reviewed By: cpojer Differential Revision: D20558005 Pulled By: hramos fbshipit-source-id: 2551120beadcfd4c2f1393ce8a2c2fa6b93c9290 * Fix `test_android`: Remove references to fbsource cell (#28363) Summary: Fixes https://github.com/facebook/react-native/issues/28361. ## Changelog [Internal] [CI] - Fix test_android Pull Request resolved: https://github.com/facebook/react-native/pull/28363 Test Plan: Prior to fix: ``` react-native $ ./scripts/circleci/buck_fetch.sh Guessing 168a69309928ba16065cdb33b1775a4af9f924a6 as the last one used version. Using additional configuration options from /Users/hramos/.buckconfig.d/experiments, /etc/buckconfig.d/fb_chef.ini, /etc/buckconfig.d/fb_chef_override.ini Invalidating internal cached state: Watchman failed to start. This may cause slower builds. Parsing buck files: finished in 1.5 sec Buck wasn't able to parse /Users/hramos/git/react-native/ReactAndroid/src/main/java/com/facebook/fbreact/specs/BUCK: IOError: [Errno 2] No such file or directory: '/Users/hramos/git/react-native/tools/build_defs/platform_defs.bzl' Call stack: File "/Users/hramos/git/react-native/.buckd/resources/168a69309928ba16065cdb33b1775a4af9f924a6/buck_server/buck_parser/profiler.py", line 507, in wrapped return func(*args, **kwargs) File "/Users/hramos/git/react-native/ReactAndroid/src/main/java/com/facebook/fbreact/specs/BUCK", line 1 load("//tools/build_defs:platform_defs.bzl", "ANDROID") File "/Users/hramos/git/react-native/.buckd/resources/168a69309928ba16065cdb33b1775a4af9f924a6/buck_server/buck_parser/profiler.py", line 507, in wrapped return func(*args, **kwargs) This error happened while trying to get dependency '//ReactAndroid/src/main/java/com/facebook/fbreact/specs:FBReactNativeSpec' of target '//ReactAndroid/src/main/java/com/facebook/react/devsupport:devsupport' ``` After fix: ``` react-native $ ./scripts/circleci/buck_fetch.sh + buck fetch ReactAndroid/src/test/java/com/facebook/react/modules Guessing 168a69309928ba16065cdb33b1775a4af9f924a6 as the last one used version. Using additional configuration options from /Users/hramos/.buckconfig.d/experiments, /etc/buckconfig.d/fb_chef.ini, /etc/buckconfig.d/fb_chef_override.ini Invalidating internal cached state: Watchman failed to start. This may cause slower builds. Parsing buck files: finished in 1.1 sec Configuration 'ANDROID_SDK' points to an invalid directory '/opt/android_sdk'. When creating rule //ReactAndroid/src/main/java/com/facebook/hermes/instrumentation:instrumentation. ``` > Note: I don't have the Android SDK configured in this machine. Verified on Circle CI. `test_android` is now green: https://circleci.com/gh/facebook/react-native/140682?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link Reviewed By: cpojer Differential Revision: D20564934 Pulled By: hramos fbshipit-source-id: 5d843b8f113c4db5391ee39addc3ff259d962290 * Fix TextInput left/right padding Summary: This fixes two things: 1) Currently it only respects Start and End padding, and if there's a Theme default, it will override Left/Right padding. Whoops. 2) Currently it doesn't respect when a TextInput starts with padding, but then is removed. This resolves both. It still does not account for RTL support. Changelog: [Internal] Fix AndroidTextInput padding Reviewed By: mdvacca Differential Revision: D20573151 fbshipit-source-id: e89791641b6699e728cde9dbd661a8c21485fbc8 * Validate selection range passed to setTextAndSelection Summary: Changelog: [Internal] # Fabric 1. If `start` and `end` parameters in `setTextAndSelection` are -1, we don't move the cursor. Previously the cursor would be moved to beginning of text input. 2. In view commands, do not validate `eventCount`. It is passed in as undefined from JS because Fabric's text input doesn't use `eventCount`. # Paper 1. If `start` and `end` parameters in `setTextAndSelection` are -1, we don't move the cursor. Previously the cursor would be moved to beginning of text input. Reviewed By: shergin Differential Revision: D20538290 fbshipit-source-id: c7aeddc25f58697254474058ce901df958321f7c * Remove ReactTypes from fbsource and React sync Summary: See https://github.com/facebook/react/pull/18366 This contains a fork of the upstream Flow types. We shouldn't be syncing this since these leads to conflicting types. As a result, these uses have already been codemodded away. Only the imports remained. Changelog: [React Core] - Remove ReactTypes from sync. Reviewed By: gaearon Differential Revision: D20583740 fbshipit-source-id: fc86a934cbdca8ff90fe90282b86ecc945a85e5f * Fix controlled TextInput with child nodes Summary: Changelog: [Internal] # There are three changes in this diff ## _stateRevision is replaced with a BOOL `_stateRevision` was protecting against setting attributed string that is already visible to the user. Previously this was ok because the change was only coming from native, any changes from JS were ignored. Imagine following scenario: 1. User taps key. 2. Update state is called on component initiated by native. 3. New state is created with incremented revision by one. 4. `_stateRevision` gets set to new state's revision + 1. 5. Now JS wants to change something because it just learnt that user tapped the key. 6. New state is created again with incremented revision by one. 7. Update state is called on the component, but the change isn't applied to the text view because `_state->getRevision()` will equal `_stateRevision`. By having a BOOL instead of number, we very explicitly mark the region in which we don't want state changes to be applied to text view. ## Calling [_backedTextInputView setAttributedText] move cursor to the end of text input This is prevented by storing what the current selection is and applying it after `[_backedTextInputView setAttributedText]` is called. This was previously invisible because JS wasn't changing contents of `_backedTextInputView`. ## Storing of previously applied JS attributed string in state This is the mechanism used to detect when value of text input changes come from JavaScript. JavaScript sends text input value changes through props and as children of TextInput. We compare what previously was set from JavaScript to what is currently being send from JavaScript and if they differ, this change is communicated to the component. Previously only first attributed string send from JavaScript was send to the component. # Problem If children are used to set text input's value, then there is a case in which we can't tell what source of truth should be. Let's take following example We have a text field that allows only 4 characters, again this is only a problem if those 4 characters come as children, not as value. This is a controller text input. 1. User types 1234. 2. User types 5th character. 3. JavaScript updates TextInput, saying that the content should stay 1234. 4. In `TextInputShadowNode` `hasJSUpdatedAttributedString` will be set to false, because previous JS value is the same as current JS value. Reviewed By: shergin Differential Revision: D20587681 fbshipit-source-id: 1b8a2efabbfa0fc87cba210570142d162efe61e6 * Daily `arc lint --take BUCKFORMAT` Reviewed By: zertosh Differential Revision: D20593906 fbshipit-source-id: b056947c698508119dc9d4d1bba202295b8f0fda * RN picker - implement background color Summary: add support to the android implementation of the Picker component for setting the background color. Changelog: [Android] [Added] - Support item background color in Dialog Picker Differential Revision: D20566131 fbshipit-source-id: d693b40803fa1051ec955c5728994c820fecd9e9 * Fabric: Modernizing Yoga Dirty flag test. Summary: Now we using TEST_F thing that allows consilidating initialization. Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D20578788 fbshipit-source-id: 103bcb8fdeb3dbf297385cfe56415bd646e16791 * Fabric: Changing signature of `ComponentDescriptor::createState` Summary: This is pure syntactic change. Often we don't have a shared pointer to ShadowNodeFamily and only have just a reference. At the same time, `ComponentDescriptor::createState` does not have to accept a shared pointer. So, it's better to accept just a reference. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D20578787 fbshipit-source-id: 905277001e096d41e75007575b59ea2ea15fbf4b * Fabric: View Test: Changing state should not dirty Yoga tree (in some most cases) Summary: Changelog: [Internal] Fabric-specific internal change. Reviewed By: mdvacca Differential Revision: D20578789 fbshipit-source-id: 4336165217bd39fc8065cfaeb96ef7753433d48a * Get ReactiveNative compiled with Clang 10 (#28362) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/28362 Fixed a few compilation errors emitted by Clang 10. Changelog: [iOS] [Fixed] - Get ready for Clang 10 Differential Revision: D20549970 fbshipit-source-id: dc36a85d90d3e43a05f045feb57c6ab6ded67da7 * Guard against null values in object parameters for bridged methods Summary: Handles the case when a value in an object parameter of a turbo module spec is null (even if the type is nullable). For example, given: ``` export interface Spec extends TurboModule { +myFunc: ({| foo: ?string, |}) => void; } ``` and calling `NativeModule.myFunc({foo: null})`, we see an error like: ``` JSON value '<null>' of type NSNull cannot be converted to NSString ``` Guarding against this by converting NSNull's to nils ## Changelog: [iOS] [Fixed] - Fix crash when passing null value in object parameter of bridged method Reviewed By: fkgozali Differential Revision: D20591590 fbshipit-source-id: fdb90f34131427a235f2e3c99147bf1e6a9c6732 * Modify pending deletion tags to be cross manageChildren Summary: Changelog: [Internal] Removing historic layout animations index adjustment (D20323928) broke the Dating Secret Crush screen. Since flushing animations (D20319824) had to be reverted due to issues with Saved + Privacy Shortcuts (https://fburl.com/tasks/eijtmifu) we need to track pending deletions across `manageChildren` operations. Reviewed By: JoshuaGross Differential Revision: D20601079 fbshipit-source-id: c6f116683750e97abe7f988cf361d2a6449e90e6 * Enable label-actions on the react-native repository (#28374) Summary: Enhance our issue management workflow by having the bot respond automatically whenever a label is applied to the issue. ## Changelog [Internal] - CI Pull Request resolved: https://github.com/facebook/react-native/pull/28374 Test Plan: Not tested. If needed, could be applied to a different, test repository. Reviewed By: cpojer Differential Revision: D20606887 Pulled By: hramos fbshipit-source-id: 874d1464527ea76bf51394a7d3e98e4fd8f69345 * Fix Animated Value initialized with undefined in ScrollView (#28349) Summary: When passing an object to contentOffset that doesn't have `y` prop set it causes the following error: ``` Error: AnimatedValue: Attempting to set value to undefined This error is located at: in ScrollView (at src/index.js:638) ... ``` This happens since a runtime check was added to the `AnimatedValue` constructor. (a3aaa471eca58b31597b9a0669f7ade385ccb175) According to flow types the object passed to contentOffset should always contain both x and y props but since it worked before when y is undefined I think its fine to patch the runtime behaviour defensively, especially since the code change is simple. ## Changelog [General] [Fixed] - Fix Animated Value initialized with undefined in ScrollView Pull Request resolved: https://github.com/facebook/react-native/pull/28349 Test Plan: Tested that the crash no longer reproduces when passing an empty object to contentOffset. Reviewed By: cpojer Differential Revision: D20601664 Pulled By: hramos fbshipit-source-id: b098a2dd1e702f995a9a92fa6e4e9a204187dac4 * xplat/js/react-native-github/ReactCommon/fabric/components/textinput/ Summary: Changelog: [Internal] Reviewed By: scottrice Differential Revision: D20619227 fbshipit-source-id: 674337e6ce585a4e96d020f9624b874ba86e2d80 * Seed ssh known hosts with github's public key (#28370) Summary: The [previous attempt](https://github.com/facebook/react-native/pull/28304) to fix the publish step failed, so now reverting to manually configuring things. This PR adds an entry to SSH’s `known_hosts` file using github.com’s public key that I have verified as per [these instructions](https://serverfault.com/a/807363): ``` ~/C/R/react-native [master] » nmap github.com --script ssh-hostkey Nmap scan report for github.com (140.82.118.4) rDNS record for 140.82.118.4: lb-140-82-118-4-ams.github.com PORT STATE SERVICE 22/tcp open ssh | ssh-hostkey: | 1024 ad:1c:08:a4:40:e3:6f:9c:f5:66:26:5d:4b:33:5d:8c (DSA) |_ 2048 16:27:ac:a5:76:28:2d:36:63:1b:56:4d:eb:df:a6:48 (RSA) ``` These fingerprints line up with [the ones posted by GitHub](https://help.github.com/en/github/authenticating-to-github/githubs-ssh-key-fingerprints), so my setup should be good and can be trusted to grab the public key from the right host: ``` ~/C/R/react-native [master] » ssh-keyscan -t rsa -H github.com # github.com:22 SSH-2.0-babeld-d48c3acd |1|If6MU203eXTaaWL678YEfWkVMrw=|kqLeIAyTy8pzpj8x8Ae4Fr8Mtlc= ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ== ``` ## Changelog [Internal] [Fixed] - Make automated publishing of packages from CI work again Pull Request resolved: https://github.com/facebook/react-native/pull/28370 Test Plan: I used the command being added in this PR in [a failed CI job](https://app.circleci.com/pipelines/github/facebook/react-native/4104/workflows/916127cb-177f-4583-9f90-cae5318041d8/jobs/140810). When I invoked the publish script manually I was not greeted by the blocking prompt and the package was successfully published: https://www.npmjs.com/package/react-native/v/0.0.0-56cf99a96 Reviewed By: cpojer Differential Revision: D20601527 Pulled By: hramos fbshipit-source-id: b1a4405228408cfc4a1b3b44ab88c79522af3a66 * Fix app bundle size diff not always being compared against latest commit (#28368) Summary: - Timestamp of entries in our Firebase instance sometimes get stored as number. This means that we may not always be diffing against the latest master commit. - Size report of Android and iOS gets overwritten depending on which build finishes first. ## Changelog [Internal] [Fixed] - App bundle size diff not always being compared against latest commit [Internal] [Fixed] - Android and iOS app bundle size diff overwrite each other Pull Request resolved: https://github.com/facebook/react-native/pull/28368 Test Plan: - We are now using Firebase's own [firebase.firestore.Timestamp.now](https://firebase.google.com/docs/reference/js/firebase.firestore.Timestamp#now) to ensure that we always get a timestamp in the preferred format. This has been tested locally but can only be verified when merged to master and we start getting new data. In the meantime, I'll manually fix up all the entries in the store. - There should be one app bundle size comment for Android and one for iOS in this PR. Reviewed By: cpojer Differential Revision: D20601620 Pulled By: hramos fbshipit-source-id: 0c3e4b78a74cbd659f1957a6aa74322b016e0646 * Hopefully fix so loading crashes Summary: Changelog: [Android][Internal] Fix potential initializer interruption threading crashes. Reviewed By: mdvacca Differential Revision: D20615755 fbshipit-source-id: 58b706deeb6df1998caff5bf2ae9ec60114313fe * Fix label-actions configuration Summary: Adds back a missing label key, fixes open source issue: https://github.com/facebook/react-native/issues/28378 Changelog: [Internal] [CI] - Fix label-actions config Reviewed By: cpojer Differential Revision: D20625887 fbshipit-source-id: 63c90db249aa9c15369a4b5bcab71cbe75c6d4b8 * Changing Order Of mOverrideColorScheme In Constructor Summary: Changelog: [Android] [Updated] mOverrideColorScheme should be assigned before the first colorSchemeForCurrentConfiguration call, so the initial setting of mColorScheme will reflect the override Reviewed By: zackargyle Differential Revision: D20630173 fbshipit-source-id: a2a2d174d3fc40c14f27dce6a7fa8e67203480c9 * hermes | inspector | Don't include posix headers on non-posix systems Summary: Changelog: [Internal] Hermes inspector includes pthreads, arpa and sys headers on all OSes that would break vanilla Windows builds. This diff adds a check for posix-compliance before inclusion (Note: this ignores all push blocking failures!) Reviewed By: dulinriley Differential Revision: D20564449 fbshipit-source-id: 8e264bc3104065dc4315bb291e8560609fe65184 * Upgrade Prettier from 1.17 to 2.0.2. Summary: This gets us on the latest Prettier 2.x: https://prettier.io/blog/2020/03/21/2.0.0.html Notably, this adds support for TypeScript 3.8, which introduces new syntax, such as `import type`. Reviewed By: zertosh Differential Revision: D20636268 fbshipit-source-id: fca5833d003804333a05ba16325bbbe0e06d6c8a * Back out "Upgrade Prettier from 1.17 to 2.0.2." Differential Revision: D20639755 fbshipit-source-id: 5028563f9cf0527a30b4259daac50cdc03934bfd * Fabric: Additional temporary checks in prop parsing infra Summary: While ViewConfig infra isn't perfect we need to check some value for correctness during prop-parsing. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D20639055 fbshipit-source-id: 193dcd0769bc7777bc8d60c964ede72ebdaa83e4 * Update React package Summary: This just updates the `react` package to the latest stable version. We updated it to experimental internally earlier so this brings the open source version to the latest before the branch cut. This doesn't include any breaking changes. Changelog: [General][Changed] - Update to React 16.13.1 Reviewed By: cpojer Differential Revision: D20642909 fbshipit-source-id: 68a4c74bfe72f1abdb33b0b9071a4f4e8e568318 * Fix sketchy null checks induced by new formatting in Prettier 2.0 Summary: Update code to prepare for Prettier 2.0, which will reformat `a || (b || c)` to `a || b || c`. Changelog: [Internal] prepare for Prettier 2.0 Reviewed By: kassens Differential Revision: D20639483 fbshipit-source-id: c2932b1495884684172ba9291d56c546f51711b8 * RN picker - fix types in AndroidDialogPickerManagerInterface Summary: according to [this crash report](https://our.intern.facebook.com/intern/logview/details/facebook_android_crashes/7ba7056481015482c6166d65cb97e49d/?trace_key=1506fe36a70dd5e50cdc8968f6317f27), `value` was throwing an NPE despite being null-checked. this is because it was an `int` rather than an `Integer`, so the null check wasn't working Changelog: Fix types in AndroidDialogPickerManagerInterface Reviewed By: mdvacca Differential Revision: D20646343 fbshipit-source-id: a27587e0a48f5782bcf5ffddb604018218e65206 * Remove RCTExportModule log spam Summary: The bridge complains if modules aren't exported, which isn't really helpful with lazily loaded modules and turbo modules. I considered only turning this off when TurboModules is enabled, but figured we'd be killing this soon anyways... If anyone feels strongly I can go that approach. Changelog: [iOS][Internal] Remove RCTExportModule log spam Reviewed By: shergin Differential Revision: D20629575 fbshipit-source-id: d32d9fe244c4d06acfee982fca7c7f63da294dc5 * De-jank DevLoadingView Summary: ## Problems Repro steps: 1. Disable Fabric (because CMD + R doesn't work with Fabric right now). 2. Open up Marketplace and hit `CMD + OPT + R` 3. **Observe:** The progress bar doesn't show up right away. It also doesn't actually show progress. https://pxl.cl/140g1 RN Support post: https://fb.workplace.com/groups/rn.support/permalink/3437652016283389/ ## Fixes The first problem is that progress bar doesn't actually show progress. **Fix:** Bundle load progress is updated in `RCTCxxBridge`, where we first require `RCTDevLoadingView`, and then call its `updateProgress` method. Previously, we wouldn't lazily load `RCTDevLoadingView`, it already didn't exist. Lazily loading `RCTDevLoadingView` causes the progress view to show up. Here: https://pxl.cl/140gt If you look at the above video, you'll notice there are two stages to the progress bar: stage 1 displays the actual progress. Stage 2 prompts that we're downloading the JS bundle. As you can see, stage 1 and stage 2 have different background colors, even though both of them are green. **Fix:** I adjusted the JS to match the Native color. Here: https://pxl.cl/140gT We're almost there, but the progress bar is dismissed twice? **Fix:** I dug into the code, and the reason why was because when we hit `CMD + R`, we invalidate the bridge, and immediately re-initialize it. This means that we asynchronously invalidate the old TurboModuleManager, and immediately create a brand new one. Therefore, two `RCTDevLoadingView` modules can (and do) exist at once. So, I moved `RCTDevLoadingView` to be an instance member of `FBReactModule`, to ensure that it doesn't get cleaned up and re-created when TurboModuleManager is deleted and re-created. This finally fixed the progress bar jank: https://pxl.cl/140hn Changelog: [iOS][Fixed] - Remove RCTDevLoadingView jank Reviewed By: rickhanlonii Differential Revision: D20607815 fbshipit-source-id: 05825c67adaf3cfda70be0fa2dc92d413dc8921b * Fix retaining self in block in LogBox impl Summary: Logbox has a retain cycle (see linked task for my deeper investigation). This diff doesn't fix the retain cycle, but it's just good practice to not retain self strongly in blocks. Changelog: [iOS][Internal] Fix retaining self in block in LogBox implementation Reviewed By: shergin Differential Revision: D20630693 fbshipit-source-id: cf399495e9bcd1917932fcc0e9c9d2d2a32bf6f0 * Flow type infoLog Summary: Changelog: [General][Internal] flow type infoLog Reviewed By: zackargyle Differential Revision: D20577939 fbshipit-source-id: eed4401b2ae0a6bf845fdcb54c6abe1fe98fe7c1 * Replace fbsource// with // in xplat/js/ files [1] Summary: `fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency. Changelog: [Internal] Reviewed By: scottrice Differential Revision: D20495655 fbshipit-source-id: a57b72f694c533e2e16dffe74eccb8fdec1f55f5 * Deploy Flow 0.121 to Xplat (#901) Summary: Deploy Flow 0.121 to Xplat bypass-lint allow-large-files Closes https://github.com/facebook/flipper/pull/901 Changelog: [Internal] Reviewed By: panagosg7 Differential Revision: D20570316 fbshipit-source-id: a76983d6f46c8b995ce2dd5cd1e014534790698a * Replace fbsource// with // in xplat/js/ files [3] Summary: `fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D20656211 fbshipit-source-id: deb91b917d349bc500acbb03d734ff621f6e1fc7 * Replace fbsource// with // in xplat/js/ files [4] Summary: `fbsource//xplat` and `//xplat` are equivalent for FB BUCK targets. Removing extra prefix for consistency. Changelog: [Internal] Reviewed By: JoshuaGross, shergin Differential Revision: D20656696 fbshipit-source-id: 10f02decb1dc969fd3491ac90d97f09e2bda59e7 * Add Needs: Repro bot action (#28397) Summary: Add automated response for Needs: Repro ## Changelog [Internal] [Added] - Add automated response for Needs: Repro Pull Request resolved: https://github.com/facebook/react-native/pull/28397 Test Plan: Bot should add a comment with the Needs: Repro label. Reviewed By: cpojer Differential Revision: D20665378 Pulled By: hramos fbshipit-source-id: 1c7d878faacf935a640849f74c81f119e5c7e92d * Daily `arc lint --take CLANGFORMAT` Reviewed By: zertosh Differential Revision: D20666684 fbshipit-source-id: 32255ac7509e0257693969a7b4b044569af30df7 * ✅ Green CI: Fix JavaScript e2e tests, disable failing Android e2e test (#28392) Summary: Jobs now have a `run_disabled_tests` argument that allows for the selective execution of disabled tests. When working on re-enabling a failing test, the contributor just needs to set `run_disabled_tests` to `true` in the appropriate workflow in `.circleci/config.yml`. Tests can be kept green by moving failing tests into the disabled section until a contributor can provide a fix, thus ensuring signal is maintained on master. For example, a failing end-to-end test might be disabled in order to allow the signal from unit tests to be provided, as opposed to flat out failing the entire job. What was done in this PR: * The failing `test_js_e2e` job has been fixed, and merged into the `test_js` job. An empty disabled tests section is added for future use. * The failing `test_ios_e2e` job has been merged into `test_ios`, with all of its steps gated behind the `run_disabled_steps` argument. * The failing Android end-to-end tests have been added to `test_android`, gated behind the `run_disabled_steps` argument * The failing Podspecs test has been added back into `test_ios`, gated behind the `run_disabled_steps` argument ## Changelog [Internal] [CI] - ✅ Green CI, disabled test infrastructure work Pull Request resolved: https://github.com/facebook/react-native/pull/28392 Test Plan: Verified on Circle CI Reviewed By: cpojer Differential Revision: D20665512 Pulled By: hramos fbshipit-source-id: 831738027f90f4b23313893d8342d7e654f34726 * Upgrade internal packages to support ESLint >= 6 (#28393) Summary: Fixes https://github.com/facebook/react-native/issues/28293 I've tested it with https://github.com/react-native-community/react-native-template-typescript and it seems to be working as expected - no warnings, supports typescript 3.8. (note: I didn't upgrade the package version as I don't know how the releases work for this package) ## Changelog [CATEGORY] [TYPE] - Message Pull Request resolved: https://github.com/facebook/react-native/pull/28393 Reviewed By: hramos Differential Revision: D20647112 Pulled By: cpojer fbshipit-source-id: ca6b67971f625dc8125a58f9220dfcd86250ba94 * Fabric: Fixing a deadlock in RCTSurfacePresenter Summary: This is another attempt to fix an issue very similar to T59424871. The previous attempt was in D19249490. I don't know why we don't see production crashes (stalls) but it happened to me (and not to me) in the debugger. The previous attempt didn't work because we still could have a deadlock because we tried to acquired shared mutex already owned exclusively by the `suspend` method. Here is another approach: Instead of using one shared mutex, now we use two. One is similar to what we had and another that protects `suspend` and `resume`. Besides that, now we pass a Scheduler to functions that use that explicitly. This way we can be more explicit about acquiring mutexes and the overall flow of execution. The idea is: Now an arbitrary code that can be reentrant does not cover with mutexes, so the deadlock is not possible. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D20639228 fbshipit-source-id: 98515742f00f2ae94b50b585c9f1f0611e169ebe * Update React Hooks Plugin Summary: Updates `eslint-plugin-react-hooks` to 3.0.0. This introduces a new lint error when you use a Hook inside a class. Changelog: [General][Changed] - Updated the React Hooks ESLint Plugin Reviewed By: cpojer Differential Revision: D20675528 fbshipit-source-id: d0cbe9748fd15df7a4c6de00bd1462610e0a43d6 * Upgrade React DevTools 4.0.6 -> 4.6.0 Summary: Upgrading the embedded version of React DevTools, primarily to pull in [this PR](https://github.com/facebook/react/pull/18397) which will reduce the impact of this package on `node_modules` size. # Update process Following a similar process as D15973709, I began by searching for [all of the references](https://our.intern.facebook.com/intern/biggrep/?corpus=xplat&filename=.json&case=false&view=default&extre=&s=%22react-devtools&engine=apr_strmatch&context=false&filter[uninteresting]=false&filter[intern]=false&filter[test]=false&grep_regex=) to the `react-devtools-core` package and updated all v4 usage to to point to the new 4.6.0 release: 1: Manually update "react-devtools-core" versions: ``` js/package.json js/react-native-github/package.json nuclide/package.json sonar/desktop/app/package.json sonar/desktop/plugins/reactdevtools/package.json vscode/modules/vscode-webview/package.json ``` 2: Setup Yarn proxy: ``` yarn config set proxy http://fwdproxy:8080/ yarn config set https-proxy http://fwdproxy:8080 ``` 3: Run "yarn" in each of the above directories. 4: Run the lockfile shell script: ``` ~/xplat/js/scripts/update-oss-yarn-lockfile.sh ``` 5: Update the generated `MOBILE_JS_NODE_MODULE_DEPS.bzl` by running ``` js1 build buckfiles ``` ## Changelog: [General] [Changed] - Upgrade embedded React DevTools backend from v4.0.6 to v4.6.0. Reviewed By: cpojer, gaearon Differential Revision: D20676091 fbshipit-source-id: 99865bdba9bce45e2a7d582d5fb550cfdbeeca3a * Make ScrollView use ForwardRef Summary: Have ScrollView use forwardRef so that the host component methods like `measure` and `measureLayout` are available without having to call `getNativeScrollRef`. Instead, you can use `<ScrollView ref={myRef} />` and directly call all methods of ScrollView and host components on `myRef`. Previous usage: ``` const myRef = React.createRef<React.ElementRef<typeof ScrollView>>(); <ScrollView ref={myRef} /> const innerViewRef = myRef.current.getNativeScrollRef(); innerViewRef.measure(); ``` New usage: ``` const myRef = React.createRef<React.ElementRef<typeof View>>(); <ScrollView ref={myRef} /> // now, myRef.current can be used directly as the ref myRef.current.measure(); myRef.current.measureLayout(); // Additionally, myRef still has access to ScrollView methods myRef.current.scrollTo(...); ``` Changes: * Added deprecation warnings to ScrollView methods `getNativeScrollRef`, `getScrollableNode`, and `getScrollResponder` * Added the forwardRef call to create `ForwardedScrollView` - this takes in `ref` and passes it into the class ScrollView as `scrollViewRef`. * Forwarded the ref to the native scroll view using `setAndForwardRef`. * Added statics onto `ForwardedScrollView` so that `ScrollView.Context` can still be accessed. * Added type `ScrollViewImperativeMethods`, which lists the public methods of ScrollView. * Converted all public methods of ScrollView to arrow functions. This is because they need to be bound to the forwarded ref. * Bound all public methods of ScrollView to the forwarded ref in the `setAndForwardRef` call. * Flow typed the final output (ForwardedScrollView) as an abstract component that takes in the props of the `ScrollView` class, and has all methods of both the inner host component (`measure`, `measureLayout`, etc) and the public methods (`scrollTo`, etc). Changes to mockScrollView: * Changed mockScrollView to be able to mock the function component instead of a class component * Updated necessary tests Changelog: [General] [Changed] - Make ScrollView use forwardRef Reviewed By: TheSavior Differential Revision: D19304480 fbshipit-source-id: 6c359897526d9d5ac6bc6ab6d5f9d82bfc0d8af4 * Fix issue with onEndReached Summary: onEndReached can be triggered twice when more items are added to the end of the list. This change makes it so that a second call to onEndReached won't happen until the user scrolls down to the new end of the list. Changelog: [General] [Fixed] - Fix double call to onEndReached in VirtualizedList Reviewed By: sahrens Differential Revision: D20066740 fbshipit-source-id: 129d7ae6bfd241eeea18fe0bb12b82be67735874 * Remove console warnings from ScrollView methods Summary: The newly added console warnings in D19304480 are adding a lot of warning noise due to missed infra callsites. Those callsites need to be updated before these warnings can be added. Changelog: [Removed] Remove console warnings from ScrollView methods Reviewed By: rickhanlonii Differential Revision: D20700917 fbshipit-source-id: cb618ee3a291d26e1942e4f91bbc02dee41fb78b * Upgrade react-docgen, jscodeshift and flow-parser Summary: In preparation for upgrading babel, I'm updating some of our source transform tools to the latest versions. Changelog: [Internal] Reviewed By: GijsWeterings Differential Revision: D20675201 fbshipit-source-id: fa4fee2652529c6677087e42cdd1325a8080e46f * Ship State Reconciliation 100% on all platforms Summary: State Reconciliation has been running 50/50 for a while and all metrics look stable. This is necessary for providing a good experience so we should ship to everyone unconditionally. Changelog: [Internal] Fabric diffing reconciliation process improvement Reviewed By: mdvacca Differential Revision: D20715694 fbshipit-source-id: 25b2635ecc29b67e2911679c9db66bc84d37dec1 * Core telemetry tests: update so they pass on my machine Summary: `std::this_thread::sleep_for` is not really precise and will attempt to sleep for "at least" that much time, but may sleep much longer depending on what CPUs are doing and scheduling policies. To get this to pass on my machine, I had to substantially increase the thresholds. Changelog: [Internal] Reviewed By: shergin Differential Revision: D20689571 fbshipit-source-id: f159420d24a95da2b5d95d49ed7882e783291e98 * Optimize diff algorithm to produce fewer remove+insert ("move") paired instructions Summary: An evolution of D20633188 but more performant. There are three optimized paths before the slow path. The first optimized path tries to pair identical nodes from old/new tree, and generate Update mutations, until we hit nodes that are different (indicating either a remove or an insert). This already existed. The next two optimizations, introduced by Tim in his JS pseudocode, were inspired by ReactJS's diffing algorithm. They work in cases where the rest of the nodes are (1) all removals/deletes or (2) all creates+inserts. Finally, if those final two optimized paths can't run, it's because there is a mix of delete+remove, create+insert, and "move" operations, mixed at the beginning, middle, and/or end of the list. This has slightly better average/best-case complexity as the previous implementation. In particularly pathological cases where all nodes are arbitrarily reordered, or reversed, for instance (ABCDE->EDCBA) the algorithm has the same complexity as the previous algorithm (quadratic). For now iOS is pinned to the older differ Changelog: [Internal] Experiment to optimize diffing algorithm in Fabric Reviewed By: shergin Differential Revision: D20684094 fbshipit-source-id: d29fba95a0328156c023e1c87804f23770ee1d91 * Unit test for V2 "minimal instruction" diffing algorithm Summary: This unit test is to verify that the new diffing algorithm generates a "minimal" instruction set, with regards to removes and inserts ("moves"). These unit tests are here to verify the expected behavior in this new algorithm, but these tests may be modified or deleted in the future if we decide we want to change this behavior. Changelog: [Internal] fabric unit test Reviewed By: mdvacca Differential Revision: D20706592 fbshipit-source-id: 5f9991498e0d788ecbf88d938bfe6d3f0f27af40 * Add ES Lint rules for `DynamicColorIOS()`and `ColorAndroid()` (#28398) Summary: The [PlatformColor PR](https://github.com/facebook/react-native/pull/27908) added support for iOS and Android to express platform specific color values. The primary method for an app to specify such colors is via the `PlatformColor()` method that takes string arguments. The `PlatformColor` method returns an opaque Flow type enforcing that apps use the PlatformColor method instead of creating Objects from scratch -- doing so would make it harder to write static analysis tools around Color values in the future. But in addition to `PlatformColor()`, iOS has a `DynamicColorIOS()` method that takes an Object. The Flow type for this Object cannot be opaque, but we still want to enforce that app code doesn't pass variables instead of Object literals or that values in the Objects are variables. To ensure `DynamicColorIOS()` can be statically analyzed this change adds an ESLint rule to enforce that `DynamicColorIOS()` takes an Object literal of a specific shape. A `ColorAndroid()` was also introduced not for practical use but just to test having platform specific methods for more than one platform in the same app. A second ESLint rule is created for `ColorAndroid` as well. ## Changelog [General] [Changed] - Add ES Lint rules for `DynamicColorIOS()`and `ColorAndroid()` Pull Request resolved: https://github.com/facebook/react-native/pull/28398 Test Plan: `yarn lint` passes. Reviewed By: cpojer Differential Revision: D20685383 Pulled By: TheSavior fbshipit-source-id: 9bb37ccc059e74282b119577df0ced63cb9b1f53 * fix: Android gradle config when bundling for release (#28415) Summary: This fix aims to address the issue when bundling an Android app for release and getting the error exhibited in https://github.com/facebook/react-native/issues/28002 which I also encountered myself. The config was changed sometime in November 2019 (as part of https://github.com/facebook/react-native/issues/26940, commit https://github.com/facebook/react-native/commit/a3b08048674e324dbe1f0ca816f35607e9e06a2f) to be very opinionated when it comes to the use of `npx` which Gradle itself cannot find anyway (I have `npx` installed globally and it didn't pick it up). Another issue that the use of `npx` creates is that Gradle should only ever use the currently installed react-native cli rather than a (possibly) higher version which may not always have backward compatibility. The proposed change simply throws a more descriptive error rather than defaulting to a tool which may or may not exist on the machine, be it CI or a development environment. I've also modified the RNTester app to reflect the correct config implementation relative to the RNTester app itself. In real projects, the config inside `android/app/build.gradle` should look similar to the following snippet: ``` project.ext.react = [ cliPath: "$rootDir/../node_modules/react-native/cli.js", entryFile: "index.js" ]; ``` ## Changelog [Android] [Fixed] - Gradle release config Pull Request resolved: https://github.com/facebook/react-native/pull/28415 Test Plan: - [x] Successfully bundled an Android release build with correct config - [x] Works with RNTester app Reviewed By: mdvacca Differential Revision: D20714372 Pulled By: hramos fbshipit-source-id: 4d66139249c6f840582a71a48c64e6a6595f7af0 * Reimplement D19965405: Small improvements in Differentiator/TinyMap Summary: Two things: 1) I reimplement Valentin's idea in D19965405, so that TinyMaps can be iterated over, with a couple of bugfixes (calling front() or back() on an empty vector will crash). 2) I now use TinyMap instead of better::map in the "optimized" diffing algorithm. 3) `erase` now actually removes elements from the vector, but only when more than half of elements have been erased. 4) If you repeatedly erase elements at the beginning of the vector, they will no longer be iterated over. This is a specific optimization for our heaviest TinyMap use-cases. These amount to some small but hopefully somewhat meaningful perf improvements. Changelog: [Internal] Fabric perf Reviewed By: shergin Differential Revision: D20718719 fbshipit-source-id: 91f4b2e2e0f6387ae484e43d5b0095103087baa6 * Remove LayoutInspectingPolicy.includeScrollViewContentOffset Summary: `LayoutInspectingPolicy` has two flags, `includeTransform` and `includeScrollViewContentOffset`. `includeScrollViewContentOffset` seems to be redundant for two reasons. # 1st From looking at callers, they have always the same value. I looked at all call sites, and they are either always both set to true or both set to false. # 2nd The way we include scroll view content offset, is through transformation, so setting `includeTransform` to true and `includeScrollViewContentOffset` to false will include content offset anyway. In order to make both flags work, we would need to introduce further changes to `getRelativeLayoutMetrics`. But since the flag isn't used anyway, I think it is better to get rid of it for now. If we need it in the future, we could re-introduce it. Reviewed By: shergin Differential Revision: D20622256 fbshipit-source-id: fb6156c66b752319ea928239fa723ff90688b0a0 * Add support for translation and rotation to operator * between Rect and Transform Summary: Changelog: [Internal] Until now `Rect operator*(Rect const &rect, Transform const &transform)` supported only scaling. Now it supports translation and rotation as well. Reviewed By: shergin Differential Revision: D20622876 fbshipit-source-id: 1b65393bd3fd6fd9a8941903e0f2681a10097e4a * Include transform property when calling getRelativeLayoutMetrics Summary: Changelog: [Internal] Current implementation of `measure` doesn't take transform into account.. So if you had a view which has width and height 100 and had `Scale(0.5, 0.5, 1)` (this will shrink view by half). Calling `getRelativeLayoutMetrics` would report its size being `{100, 100}`. This applies if view's parent has transformation as well, because transformation is applied to all subviews of the view as well. Reviewed By: mdvacca Differential Revision: D20621590 fbshipit-source-id: 2cf902a0494291c821ecada56f810c5e6620db5a * feat: migrate appveyor to circleci (#28245) Summary: This issue closes https://github.com/facebook/react-native/issues/28241 Migrated Windows test from AppVeyor to CircleCI ## Changelog [Internal] [Changed] - Migrated Windows test from AppVeyor to CircleCI Pull Request resolved: https://github.com/facebook/react-native/pull/28245 Test Plan: For CircleCI to Pass Reviewed By: cpojer Differential Revision: D20689163 Pulled By: hramos fbshipit-source-id: 285c762457ef00f7884ee9157b3f336044c0452f * Remove "Debug with Nuclide" option Summary: This is no longer needed. Reviewed By: cpojer Differential Revision: D20722274 fbshipit-source-id: 5bc3104e90811d724f42aadbf137ab8eff718ca0 * experiment to preload RN bridge after fb4a bookmarks render Summary: Changelog: [Android][Internal] add internal supermodule label Reviewed By: mdvacca Differential Revision: D20434200 fbshipit-source-id: fae50309cdd0df4a4523c2f88d1c8e01a7163575 * Fix CursorDrawable Color Tint for Android 10+ Summary: Accessing this field via reflection is explicitly blacklisted by Google in Android 10 and higher. They have provided a new API to change the color, which I have implemented here. [The old setColorFilter is deprecated](https://developer.android.com/reference/android/graphics/drawable/Drawable#setColorFilter(int,%20android.graphics.PorterDuff.Mode)) so I also updated that method call as well. Changelog: [General] [Fixed] Use new setTextCursorDrawable API for Android 10 Reviewed By: JoshuaGross Differential Revision: D20656068 fbshipit-source-id: 58a92b57c0a892c7c87fc5d735e4ceaa4e987ec7 * Early return on tinting CursorDrawable if no color supplied Summary: There's (potentially) a lot of expensive reflection calls here that, as best I can tell, end up being ignored if the supplied color is null. Better to early return. Changelog: [General] [Internal] Preclude reflection when setting cursor color if color is null Reviewed By: JoshuaGross Differential Revision: D20670594 fbshipit-source-id: 480a988355bbd79008002c4326d4b35035ec2a95 * Partial React Sync for Inspector Summary: Partial sync for React that includes: - https://github.com/facebook/react/pull/18388 - https://github.com/facebook/react/commit/dd7e5e4f5ac2ffac3171ef61daee2cb1edc69635 Created from this branch: https://github.com/facebook/react/compare/master...rickhanlonii:rh-partial-3-24?expand=1 Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D20651395 fbshipit-source-id: 67baf7c407f75d9fd01c17f2203a77a38567100e * Enable inspector for Fabric Summary: ## Overview This diff refactors the Inspector, moving logic to look up view data for a touched view inside the renderer as `getInspectorDataForViewAtPoint`. We then implement that same function for Fabric in order to support the inspector in that renderer. Requires https://github.com/facebook/react/pull/18388 ## Motivation Reason one for this refactor is that, previously, the inspector held all of the logic to look up view data for a given x,y touch coordinate. To do this, it would take the React tag and coordinates, look up the native view tag, measure it, and then ask React internals for the Fiber information of that tag. All of this is deeply coupled to React internals, yet the logic is outside of React core in the Inspector. Reason two is that, for Fabric, the logic for getting the view data is different than Paper. In Fabric, we pass the x,y coordinates to native directly, which returns an instance handle. That handle can be used to measure the ShadowNode, or retrieve the Fiber information. By moving the logic into the renderer in React core, we decouple the implementation details of looking up view data for a tapped point and allow ourselves the ability to add and change renderer-specific code for the actual lookup without impacting outsiders like the Inspector. Changelog: [Internal] Reviewed By: TheSavior Differential Revision: D20291710 fbshipit-source-id: a125223f2e44a6483120c41dc6146ad75a0e3e68 * chore: update url of warning message from deprecated imports (#28452) Summary: Some of the repository name of Lean Core(https://github.com/facebook/react-native/issues/23313) libraries has been renamed. This PR updates the warning message to display the updated url. ## Changelog [General] [Changed] - Update warning message of deprecated imports Pull Request resolved: https://github.com/facebook/react-native/pull/28452 Test Plan: updated URL can be accessed. Reviewed By: cpojer Differential Revision: D20745184 Pulled By: TheSavior fbshipit-source-id: 2c3ed6a000b45022ca6c4862305aa567c4d18b2e * Add `upgrade-support` link on issue creation (#28411) Summary: This PR adds a https://github.com/react-native-community/upgrade-support link for the user when creating an issue. Changelog: [Internal] Pull Request resolved: https://github.com/facebook/react-native/pull/28411 Reviewed By: cpojer Differential Revision: D20714274 Pulled By: hramos fbshipit-source-id: 4ca42224a50e386b95f21f0fb236a917e1b6b982 * Update PixelRatio 'getFontScale' method description (#28407) Summary: Refs facebook/react-native-website#1776. Despite in-code description `PixelRatio.getFontScale()` is working properly on the iOS (it also reflects the user settings). This PR updates the in-code description to match current behaviour. I have decided to skip in the code information about additional setting in `Accessibility` menu and in `Control Centre`, but if you think it is important just let me know, I can update this PR. ## Changelog [Internal] [Fixed] - Fix PixelRatio getFontScale method description Pull Request resolved: https://github.com/facebook/react-native/pull/28407 Test Plan: N/A Differential Revision: D20750260 Pulled By: shergin fbshipit-source-id: c40ec2fd49cd60e2975351c3a1c453aab0045da4 * Remove allowDisablingImmediateExecutionOfScheduleMountItems feature flag Summary: No longer needed. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D20747684 fbshipit-source-id: a8077519b7670d72e23267b1c1423556ec97be3f * RuntimeExecutor helpers that modify the way of the callback is being executed. Summary: Here we implement a bunch of helper methods that allow customizing the behavior of a RuntimeExecutor "on-demand" on the caller side. We will use it in the next diff(s). Changelog: [Internal] Fabric-specific internal change. Reviewed By: PeteTheHeat Differential Revision: D20551411 fbshipit-source-id: 51d3cd02b69753110c0e1155347c6e52eb882c7d * Fabric: Using `executeSynchronouslyOnSameThread_CAN_DEADLOCK` in MainRunLoopEventBeat Summary: We are replacing inline-ed implementation with practically the same one implemented as the helper method. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D20551409 fbshipit-source-id: fcc6f497cd240af65fba534051c217fe5746ce82 * Set RootNodeKind trait in ModalHostViewShadowNode Summary: Changelog: [internal] `ModalHostViewShadowNode` didn't have `RootNodeKit` trait, therefore `getRelativeLayoutMetrics` was including nodes in ancestors that it shouldn't have. Reviewed By: shergin Differential Revision: D20735801 fbshipit-source-id: 6b81e3b174c2f82e530abc2bca2da8bebc2270b0 * mention RNTester app in contributor guide (#28042) Summary: motivation is following - the RNTester app is imho the best place to try out any changes that a contributor would make, yet it is not directly mentioned in the contributor guide. This fixes it. ## Changelog [Internal] - Docs Pull Request resolved: https://github.com/facebook/react-native/pull/28042 Test Plan: not necessary Reviewed By: TheSavior Differential Revision: D20401260 Pulled By: hramos fbshipit-source-id: 01c1b7dff56b59909c94b2feb609650f0baba1a9 * Buck: Use Android SDK 29 during build (#28455) Summary: Fixes `test_android` and `test_docker` build failures. Thanks to dulmandakh for identifying the fix. Changelog: [Internal] [Android] [Changed] - Use Android SDK 29 to build during CI tests Pull Request resolved: https://github.com/facebook/react-native/pull/28455 Test Plan: Circle CI shows `test_android` and `test_docker` passing: https://app.circleci.com/jobs/github/facebook/react-native/142273 Reviewed By: sturmen Differential Revision: D20766589 Pulled By: hramos fbshipit-source-id: 8ef8a8ce3a6e7353ae47425accb3bd26cf1608c4 * Assign orderIndex_ in ConcreteViewShadowNode constructor instead of ViewShadowNode's constructor Summary: Changelog: [Internal] `orderIndex_` was only being assigned for `ViewShadowNode`, not for other `ShadowNodes` that are later represented on the screen. Reviewed By: shergin Differential Revision: D20746477 fbshipit-source-id: c04c2cfea14b9141d22bc3d9e9bb4c0c59925754 * Implement nativePerformanceNow to improve Profiler API results (#27885) Summary: When experimenting with React Profiler API (https://reactjs.org/docs/profiler.html), I noticed that durations are integers without a debugger, but they are doubles with higher precision when debugger is attached. After digging into React Profiler code, I found out that it's using `performance.now()` to accumulate execution times of individual units of work. Since this method does not exist in React Native, it falls back to Javascript `Date`, leading to imprecise results. This PR introduces `global.nativePerformanceNow` function which returns precise native time, and a very basic `performance` polyfill with `now` function. This will greatly improve React Profiler API results, which is essential for profiling and benchmark tools. Solves https://github.com/facebook/react-native/issues/27274 ## Changelog [General] [Added] - Implement `nativePerformanceNow` and `performance.now()` Pull Request resolved: https://github.com/facebook/react-native/pull/27885 Test Plan: ``` const initialTime = global.performance.now(); setTimeout(() => { const newTime = global.performance.now(); console.warn('duration', newTime - initialTime); }, 1000); ``` ### Android + Hermes ![Screenshot_1580198068](https://user-images.githubusercontent.com/13116854/73245757-af0d6c80-41b5-11ea-8130-dde14ebd41a3.png) ### Android + JSC ![Screenshot_1580199089](https://user-images.githubusercontent.com/13116854/73246157-92256900-41b6-11ea-87a6-ac222383200c.png) ### iOS ![Simulator Screen Shot - iPhone 8 - 2020-01-28 at 10 06 49](https://user-images.githubusercontent.com/13116854/73245871-f136ae00-41b5-11ea-9e31-b1eff5717e62.png) Reviewed By: ejanzer Differential Revision: D19888289 Pulled By: rickhanlonii fbshipit-source-id: ab8152382da9aee9b4b3c76f096e45d40f55da6c * Save/restore IP when leaving the interpreter Summary: This diff implements the instruction pointer save/restore trick Tzvetan came up with; allowing us to observe and modify the IP from outside the interpreter loop with negligible overhead. From Tzvetan's internal post on the subject: > [Today] the interpreter IP is just a local variable in the interpreter function, so there is no way to get to its value from outside the function. It lives in a register and we don't want to make it a Runtime field since the overhead [of accessing it via memory in the interpeter loop] would kill us. > However, if you really think about it, it only lives in a register while the interpreter function is running. For the rest of the time, it is spilled by the C++ compiler onto the stack. So, precisely when we need it, it is actually stored in memory. The only problem is, we don't know where! Admittedly, that is an annoying problem, but it feels like it should be solvable. > What if, instead of relying on the compiler to spill the IP register, we manually spill it ourselves, to a known location? It works. Example: https://godbolt.org/z/ftSDnp This diff implements this approach across the whole interpreter loop: whenever we call out of the loop we capture/publish the IP and restore it again immediately after the external call returns. This means we can now see the IP outside the interpret loop and even change it. This is effectively "for free" as the compiler now skips spilling/restoring the IP behind the scenes. The immediate benefit of this is knowing the current IP allows us to have more accurate stack-traces during execution. In future this may enabled tricks like changing the IP before returning to th…

Esemesek added 3 commits October 21, 2019 12:03

fix: Bundle in monorepo

543e275

Remove path from RNTester

fe81c94

Remove whitespace

b7321d7

facebook-github-bot added p: Callstack Partner: Callstack Partner CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Oct 21, 2019

thymikee suggested changes Oct 21, 2019

View reviewed changes

react-native-bot added Tech: Bundler 📦 This issue is related to the bundler (Metro, Haul, etc) used. Bug Platform: Android Android applications. labels Oct 21, 2019

Add Windows compat

c591b08

thymikee approved these changes Oct 21, 2019

View reviewed changes

AshishKapoor approved these changes Oct 22, 2019

View reviewed changes

cpojer suggested changes Oct 28, 2019

View reviewed changes

Remove pickFirst from packaging options

89bed13

cpojer approved these changes Nov 5, 2019

View reviewed changes

facebook-github-bot approved these changes Nov 5, 2019

View reviewed changes

facebook-github-bot closed this in a3b0804 Nov 5, 2019

react-native-bot added the Merged This PR has been merged. label Nov 5, 2019

vdmtrv mentioned this pull request Mar 27, 2020

fix: Android gradle config when bundling for release #28415

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Bundle assets in monorepo #26940

fix: Bundle assets in monorepo #26940

Esemesek commented Oct 21, 2019

facebook-github-bot commented Oct 21, 2019

facebook-github-bot commented Oct 21, 2019

thymikee Oct 21, 2019

grabbou Oct 30, 2019

Kudo Oct 31, 2019

Esemesek Oct 31, 2019

thymikee left a comment

AshishKapoor left a comment

janicduplessis commented Oct 28, 2019

cpojer left a comment

grabbou commented Oct 30, 2019 •

edited

Loading

cpojer left a comment

facebook-github-bot left a comment

react-native-bot commented Nov 5, 2019

fix: Bundle assets in monorepo #26940

fix: Bundle assets in monorepo #26940

Conversation

Esemesek commented Oct 21, 2019

Summary

Changelog

Test Plan

facebook-github-bot commented Oct 21, 2019

facebook-github-bot commented Oct 21, 2019

thymikee Oct 21, 2019

Choose a reason for hiding this comment

grabbou Oct 30, 2019

Choose a reason for hiding this comment

Kudo Oct 31, 2019

Choose a reason for hiding this comment

Esemesek Oct 31, 2019

Choose a reason for hiding this comment

thymikee left a comment

Choose a reason for hiding this comment

AshishKapoor left a comment

Choose a reason for hiding this comment

janicduplessis commented Oct 28, 2019

cpojer left a comment

Choose a reason for hiding this comment

grabbou commented Oct 30, 2019 • edited Loading

cpojer left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

react-native-bot commented Nov 5, 2019

grabbou commented Oct 30, 2019 •

edited

Loading