Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merge from upstream through 2020-04-24 #803

Merged
merged 117 commits into from Jul 21, 2021

Conversation

amgleitman
Copy link
Member

@amgleitman amgleitman commented Jul 21, 2021

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos πŸ‘
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos πŸ‘
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Merge facebook/react-native's main branch into react-native-macos's main branch through 2020-04-24 23:59:59 PDT (specifically, 30cc158).

These changes were made by repeatedly running git merge <commit-id> and resolving merge conflicts as needed.

One key difference is that we're keeping babel-eslint at version 10.0.1 because 10.1.0 (as upgraded with 0b68f3c) seems to cause some jest tests to fail. That being said, considering that babel-eslint is deprecated in favor of @babel/eslint-parser, it probably isn't worth sweating this issue too much, and we can resolve this if/when the upstream branch makes the swap.

Validation

  • yarn lint
    • βœ– 63 problems (0 errors, 63 warnings)
  • yarn flow-check-ios
  • yarn flow-check-macos
  • yarn flow-check-android
  • yarn test
    • Test Suites: 128 passed, 128 total
    • Tests: 1 skipped, 1231 passed, 1232 total
    • Snapshots: 561 passed, 561 total
  • RNTester compiles and runs properly on macOS and iOS
    • I paid close attention to test pages that were related to things that saw significant changes, such as things related to platform colors and dark mode.

grabbou and others added 30 commits April 9, 2020 10:30
Summary:
Recently, a default Podfile has been modified to not contain all the React Native pods, but use a helper method `use_react_native!`.

While this is great, it assumes a hardcoded path of `../node_modules/react-native` to be always the correct location of the React Native.

https://github.com/facebook/react-native/blob/d4d8887b5018782eeb3f26efa85125e6bbff73e4/scripts/autolink-ios.rb#L7-L9

Unfortunately, due to the way Ruby works, this completely hides the path away from the users.

Before, they could have seen the wrong path explicitly in a Podfile and knew to update it to resolve path-related issues.

With the current version in `master`, I can see a lot of issues where developers wonder how to resolve the path issues and how to pass the path itself.

https://github.com/facebook/react-native/blob/4118d798265341061105f3a53550db83c66a71cb/template/ios/Podfile#L5-L10

This PR uses React Native CLI configuration (that is already used to link 3rd party dependencies) to explicitly define the correct path to the React Native.

As a result, we don't have to change the paths here whether we're running monorepo or not.

## Changelog

[IOS] [INTERNAL] - Always provide an explicit path to React Native
Pull Request resolved: facebook#28572

Differential Revision: D20945194

Pulled By: TheSavior

fbshipit-source-id: 010f9754f2ed78ef62fd52f4d201f296f5af6d27
Summary:
Upgrades Prettier in Xplat to 1.19.1
Ignores upgrading packages on already on versions greater than 1.19.1

Changelog: [Internal]

allow-large-files
bypass-lint

(Note: this ignores all push blocking failures!)

Reviewed By: gkz, cpojer

Differential Revision: D20879147

fbshipit-source-id: 0deee7ac941e91e1c3c3a1e7d3d3ed20de1d657d
… in xplat

Summary:
Old deprecated function.

Changelog: [Internal]

Reviewed By: nlutsenko

Differential Revision: D20148856

fbshipit-source-id: 79d6fb97824b059e50f67ff5a0b4c38ec7a19469
Summary:
This adds a ProGuard for `hermes` rule so it does not have to be added by users manually.
facebook#28270

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Added] - ProGuard rule for hermes
Pull Request resolved: facebook#28571

Test Plan:
1. Create a project with/without hermes.
2. Enable proguard.

Reviewed By: cpojer

Differential Revision: D20947095

Pulled By: hramos

fbshipit-source-id: 79b166ad2dd060f20041d9f5cfe2f794c754843d
Summary:
Move CheckBox JS files to FB internal

## Changelog:
[General] [Removed] This diff removes the CheckBox export from React Native. Internally, we are requiring CheckBox directly now and externally people will have to use the community maintained module.

Reviewed By: cpojer

Differential Revision: D20910775

fbshipit-source-id: 809e135dc3f68911ac0a004e6eafa8488f0d5327
…#28526)

Summary:
With current master, when you render `<Pressable android_ripple={{borderless: false}}>`, there is no ripple effect at all.

I think the expected behavior is to have ripple with default color and radius, just not borderless.

This was how it was done (by me) in https://github.com/facebook/react-native/pull/28156/files but in the import process, the implementation was changed: facebook@bd38686 so either this PR is a fix or you can just close it (but I'd be curious why).

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [fixed] - ripple should be applied even when borderless == false
Pull Request resolved: facebook#28526

Test Plan:
`<Pressable android_ripple={{borderless: false}}>` on master

![SVID_20200404_123614_1](https://user-images.githubusercontent.com/1566403/78424971-6b315a80-7671-11ea-8be4-5fea428bc556.gif)

`<Pressable android_ripple={{borderless: false}}>` in this PR

![SVID_20200404_122754_1](https://user-images.githubusercontent.com/1566403/78424986-8bf9b000-7671-11ea-9804-37cd58dbb61e.gif)

Differential Revision: D20952026

Pulled By: TheSavior

fbshipit-source-id: df2b95fc6f20d7e958e91805b1a928c4f85904f1
…acebook#28577)

Summary:
This change removes the `ColorAndroid` API.   It was added more as a validation tool than as something useful to a developer.   When making the original [PlatformColor PR](facebook#27908) we felt it was valuable and useful to have working platform specific methods for the two platforms in core to test that the pattern worked in app code (PlatformColorExample.js in RNTester) and that the Flow validation worked, etc.    Practically `PlatformColor()` is more useful to a developer on Android than `ColorAndroid()`.    Now that the construct has served its purpose, this PR removes the `ColorAndroid` function and its related tests and other collateral.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Removed] - Remove ColorAndroid function as it adds no value over PlatfromColor
Pull Request resolved: facebook#28577

Test Plan: RNTester in both iOS and Android was tested.   Jest tests, Flow checks, Lint checks all pass.

Reviewed By: cpojer

Differential Revision: D20952613

Pulled By: TheSavior

fbshipit-source-id: 7d2cbaa2a347fffe59a1f3a26a210676008fdac0
Summary:
Label some BUCK targets properly.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D20960917

fbshipit-source-id: 42fa2266105b6c3dd5108a1b56035a19a95cd61f
Summary:
```
Welcome to Gradle 6.3!

Here are the highlights of this release:
 - Java 14 support
 - Improved error messages for unexpected failures

For more details see https://docs.gradle.org/6.3/release-notes.html
```

## Changelog

[Android] [Changed] - Update Gradle Wrapper to 6.3
Pull Request resolved: facebook#28173

Test Plan: Build project

Differential Revision: D20958894

Pulled By: mdvacca

fbshipit-source-id: a02ab0eb6aff97148c12b844fdd1f9f2617ae53f
…er twice

Summary:
Calling `-[RCTRedBox showErrorMessage]` twice causes a crash

We used `-[UIViewController isBeingPresented]` to tell whether view controller is already presented.
But from the documentation:

> A Boolean value indicating whether the view controller is being presented.

Source: https://developer.apple.com/documentation/uikit/uiviewcontroller/2097564-beingpresented?language=objc#

 ---
So this means that if you present it, wait until presentation animation is finished and then call `-[RCTRedBox showErrorMessage]` again, following exception will be thrown.

```
*** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: 'Application tried to present modally an active controller <UIViewController: 0x7fc33e422f50>.'
```

Changelog: Fix crash caused by presenting view controller twice from RCTRedBox

Reviewed By: PeteTheHeat

Differential Revision: D20946645

fbshipit-source-id: 763066e37db4e56efb0118b2e7867ad0724bae81
Summary:
We currently see a lot of errors happens because of division by zero in `AnimatedDivision` module. We already have a check for that in the module but it happens during the animation tick where the context of execution is already lost and it's hard to find why exactly it happens.
Adding an additional check to the constructor should trigger an error right inside render function which should make the error actionable.

Changelog: [Internal] Early crash in AnimatedDivision in case of division by zero.

Reviewed By: mdvacca

Differential Revision: D20969087

fbshipit-source-id: 0d98774b79be2cc56d468a4f56d2d7c8abf58344
Summary:
Now we can control the differentiator mode via MC.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: fkgozali

Differential Revision: D20978857

fbshipit-source-id: 13264948762f02f874d8d051c873d378062d6db4
Summary:
Use the latest published release of hermes-engine. Update RN to invoke `hermesc` instead of `hermes`.

Changelog: [Android] [Changed] - Upgraded to Hermes 0.5.0

allow-large-files

Reviewed By: mhorowitz

Differential Revision: D20998564

fbshipit-source-id: 4824e273bcb044029a5a7e9379f168d3da47da50
… Undefined

Summary:
Make sure width/height is always passed as Undefined when measure mode is changed to Undefined.

Changelog: [Internal][Yoga] Set width and height as Undefined when we change measure mode to Undefined

Reviewed By: alickbass

Differential Revision: D20029838

fbshipit-source-id: b9931f6ddb13ffd1565889535ade5bbffbe0c304
Summary:
`const ReactNative` is assigned to but never used. Let's get rid of it.

Changelog: [Internal]

Reviewed By: JoshuaGross

Differential Revision: D21016502

fbshipit-source-id: afcb0cfc501adf07e0c4d4452a831160e1cda088
…acebook#28580)

Summary:
This pull request updates RNTester's AppDelegate's Fabric mode to reflect changes made to the SurfacePresenter APIs. It now makes use of `RCTSurfacePresenterBridgeAdapter` to create its `SurfacePresenter`.

## Changelog

[Internal] [Fixed] - Fixed outdated API usage in RNTester's AppDelegate
Pull Request resolved: facebook#28580

Test Plan: `RNTester/RNTester/AppDelegate.mm` now compiles without error when `RN_FABRIC_ENABLED` is enabled.

Reviewed By: hramos

Differential Revision: D20966067

Pulled By: mdvacca

fbshipit-source-id: 8d0168d468240cff61554f2f2df799aaf5d876c1
Summary:
Early ViewCommand Dispatch will solve this category of crashes by going through an entirely different codepath. For users not in that experiment, it might be good to have a mitigation that prevents non-critical issues from crashing (like "blur" failing).

Currently, "blur" failures cause lots of screens to crash. There's no useful signal and those crashes aren't super actionable, so seems better to swallow.

If/when early viewcommand dispatch ships as the default/only mode, we can remove this try/catch entirely.

The only concern I have with landing this is the perf implications of putting a try/catch inside this loop.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21023213

fbshipit-source-id: 310fe2d55a44bc424692a2365ccd5882f35f9d82
Summary:
Changelog: [Internal]

We don't use view command `setMostRecentEventCount`, let's get rid of it.

Reviewed By: JoshuaGross

Differential Revision: D21016600

fbshipit-source-id: 6491c063e9d6a89252300cb47c010b248e473f4b
Summary:
Enhance the label-actions config and support a "Type: Upgrade Issue" label.
- Point to the Upgrade Support repository whenever the Type: Upgrade Issue label is applied.
- Close the issue.

Changelog:
[Internal] label-actions: Add canned response for upgrade issues

Reviewed By: cpojer

Differential Revision: D20974607

fbshipit-source-id: 3cd7890aaeb1e57baf2acc5ca85a9b3ae5117c56
Summary:
This pull request adds `YGNode.h` and `YGStyle.h` to the headers exported by Yoga's podspec. They are required by the new Fabric architecture of React Native.

The modulemap and its umbrella header automatically generated by Cocoapods adds all exported headers to the `modulemap`. Having YGNode and YGStyle exported through here has problems, because they are only available in environments that have C++ available, and will produce errors otherwise.

This pull request fences off the contents of those headers in an `#ifdef __cplusplus` block, so they will not cause errors when imported into environments where C++ isn't available.

I had considered adding a custom modulemap to the podspec as part of this pull request, but this way seems the least "invasive", and this way you are able to add and remove exported headers in the podspec without needing to worry about updating the umbrella header at the same time.

Changelog:

[Internal] - Yoga Podspec: Export YGNore and YGStyle headers

Pull Request resolved: facebook/yoga#997

Reviewed By: hramos

Differential Revision: D20966075

Pulled By: mdvacca

fbshipit-source-id: 5f5caa6b639d11e660b968d681da9a4de6c0eb8e
Summary:
We're still seeing NativeModule eager-init crashes in T46487253. So, just to be extra careful, in case this diff doesn't fix the problem, I'm adding logging into `TurboModuleManager.getModule(moduleName)` to see why TurboModules are showing up as `null`.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21027984

fbshipit-source-id: 74ee62aeac09a4fdb29547e90ef4fa7c07de17a6
Summary:
This cache is unnecessary, because:
1. TurboModuleManager caches all created TurboModules
2. TurboModuleManager calls into the TurboModuleManagerDelegate at most once per NativeModule `moduleName`.

This diff also makes ReactPackageTurboModuleManager thread-safe, which should help get rid of the crashes in T46487253.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D21027998

fbshipit-source-id: c9b5ccc3da7b81787b749e70aa5e55883317eed7
Summary:
In D20659799, I improved `TurboModuleManager.getModule(moduleName)` thread-safety by ensuring that if two threads race to require the same NativeModule, only one thread creates the NativeModule, while the other one waits until it's created.

## The problem:
What I failed to realize was that when two threads race to require two different NativeModules, we can get concurrent calls into `TurboModuleManagerDelegate.getModule(moduleName)`, and `TurboModuleManagerDelegate.getLegacyCxxModule(moduleName)`, which don't have any thread-safe guarantees.

## The fix
`TurboModuleManagerDelegate` is supposed to be an input to the TurboModule system. So, rather than expecting that all TurboModuleManagerDelegates are thread-safe, which might be a reasonable ask (see T65532092), this diff has `TurboModuleManager` acquire the delegate's lock before calling into it. This ensures that we don't get concurrent access into the delegate, which could be reading from, or writing to, some data structure in these method calls. (This was the case with `ReactPackageTurboModuleManagerDelegate`, which is what Fb4a and Workplace use under the hood).

Changelog:
[Android][Fixed] - Control concurrent calls into TMMDelegate from TurboModuleManager

Reviewed By: mdvacca

Differential Revision: D21025965

fbshipit-source-id: d22c4abfe87f9e534717a06f186dde87d3cd24df
Summary:
This release will include the new platform-colors rule.

Changelog: [Internal]

(Note: this ignores all push blocking failures!)

Reviewed By: cpojer

Differential Revision: D21022163

fbshipit-source-id: 65c831b3c820e44f75631b935118b043180ab3c7
Reviewed By: motiz88

Differential Revision: D20697095

fbshipit-source-id: ef35d02da0916109ce528d3026f7ca0956911dda
…ok#28625)

Summary:
The `cliPath` has always been optional value and in fact, even had its default value hardcoded in the React gradle file.

In this PR, I am just taking use of it and remove throwing an error, which is going to be a really annoying breaking change.

## Changelog

[ANDROID] [INTERNAL] - Don't require `cliPath`
Pull Request resolved: facebook#28625

Test Plan:
Run Android project, everything works.
Provide custom `cliPath`, it gets respected

Reviewed By: cpojer

Differential Revision: D21044222

Pulled By: TheSavior

fbshipit-source-id: 8029f988d92abb9f64f30e05932c0d407d0c997e
Summary:
This fixes build failures where `CIRCLE_PR_NUMBER` is not set. This can happen if the PR did not come from a fork.

## Changelog

[Internal] [Fixed] - Fix CIRCLE_PR_NUMBER may not always be set
Pull Request resolved: facebook#28640

Test Plan: Report bundle size step should pass on both this PR and facebook#28641.

Reviewed By: cpojer

Differential Revision: D21045553

Pulled By: TheSavior

fbshipit-source-id: fdfcb1bb88a96345b78ca69c49623df71d4cd608
Summary:
This diff introduces a new "Open Debugger" menu item for VMs that support on device debugging and for opening the React DevTools in Flipper. Provided so that we don't drift too far from the Android code.

Changelog: [Internal]

Reviewed By: RSNara

Differential Revision: D20784270

fbshipit-source-id: 6bb16431d25a6c093a583e2e041b8cffa6765ddd
Summary:
> Starting April 30, 2020, all apps submitted to the App Store must use an Xcode storyboard to provide the app’s launch screen and all iPhone apps must support all iPhone screens.

Updated iOS Launch screen as per [App Store policy change](https://developer.apple.com/news/?id=03042020b).

Community discussion: react-native-community/discussions-and-proposals#209

## Changelog

Changed iOS Launch Screen from a `xib` to `storyboard`. The `LaunchScreen.xib` file has been replaced with `LaunchScreen.storyboard`. Xcode automatically picks up the new Launch Screen no additional change is required.

[iOS] [Deleted] - Deleted LaunchScreen.xib
[iOS] [Added] - Added LaunchScreen.storyboard
Pull Request resolved: facebook#28239

Test Plan: Build the Xcode project under `template/iOS` and verify that the new launch screen is identical to the previous one.

Reviewed By: cpojer

Differential Revision: D20408892

Pulled By: hramos

fbshipit-source-id: 9c38df58d1304088a23f3d73e0fbd87675804f1a
Summary:
## Problem
Every time we want to add, remove, or change the data passed to JavaTurboModule's constructor, we have to modify the C++ TurboModule codegen. (The same is true of `ObjCTurboModule`).

**Why was this necessary?**
- `JavaTurboModule` is effectively an abstract class whose constructor is always invoked by code-generated C++ classes. These C++ code-generated class constructors accept an argument list, and manually foward each and every item in that list to `JavaTurboModule::JavaTurboModule`.

## The fix
In this diff, I introduce a struct `JavaTurboModule::InitParams`, to represent a bag of arguments:
```
class JSI_EXPORT JavaTurboModule : public TurboModule {
 public:
  struct InitParams {
    std::string moduleName;
    jni::alias_ref<JTurboModule> instance;
    std::shared_ptr<CallInvoker> jsInvoker;
    std::shared_ptr<CallInvoker> nativeInvoker;
  };
```

All `JavaTurboModules` will be created with an instance of this `InitParams` struct, instead of a list of arguments.  Our code-generated C++ `jsi::HostObject` sublcasses will simply accept `InitParams` in their constructor, and forward it to `JavaTurboModule`'s constructor. This way, the codegen remains oblivious to what arguments JavaTurboModule requires.

## Okay, but why do we need this change now?
In the future, I plan to modify the constructor for `JavaTurboModule` to accept a performance logger, and a `RuntimeExecutor`. Similar modifications are planned for ObjC. For this reason, to avoid these four codemods, and any potential other codemods that occur because we're making modifications to `JavaTurboModule` or `ObjCTurboModule`, I'm launching this codemod, and the codemods in this stack.

## Misc Fix
- Previously, we were generating the TurboModule name from the Spec filename. This is incorrect because that name represents the spec name. Now, the name will be forwarded from TurboModuleManager in the `JavaTurboModule::InitParams` struct.

## Alternative implementations
I initially considered using `ContextContainer`, but decided against it because:
1. There are no type-safety guarantees.
2. I think it's a bit overkill for this scenario. We just need an opaque bag of data, and for our purposes a simple struct does the job fine.

## Commands run

Reviewed By: fkgozali

Differential Revision: D21035208

fbshipit-source-id: 9542cafea192081bc34d337ab3a7a783083eb06c
willholen and others added 24 commits April 22, 2020 18:46
Summary:
folly_futures was compiled with and exported -DFOLLY_MOBILE=1, while
folly_json did not. This flag disables fancy F14 data structures for
folly::dynamic in favor of a simple std::unordered_map.

This caused inlined/templated code from modules depending on
folly_futures to disagree with the implementations in folly_json,
leading to a crash.

The only such libraries were libhermes-inspector and (transitively)
libhermes-executor-debug, and these only use folly::dynamic for CDP
serialization, which is why the problem was not more apparent.

Changelog: [Internal] Fix crash when attaching a Hermes debugger

Reviewed By: mhorowitz

Differential Revision: D21193307

fbshipit-source-id: 2b795bb6f4f7f991e2adaacec62d62616117322b
Summary:
This is a follow-up pull request to facebook#28280 (reviewed by shergin).
This pull request tried to solve the problem of the default color in a TextInput in dark mode on iOS being white instead of black. I got suggested to solve the problem not on the level of RCTTextAttributes, but on the level of RCTUITextField.

Setting `self.textColor = [UIColor black];` in the constructor did not work, because it gets overwritten by nil in `RCTBaseTextInputView.m`. There I implemented the logic that if NSForegroundColorAttributeName color is nil then the color is being set to black. I think the `defaultTextAttributes` property confuses here, because it ends up being the effective text attributes, e.g. if I unconditionally set the default text color to black, it cannot be changed in React Native anymore. So I put the nil check in.

## Changelog

[iOS] [Fixed] - TextInput color has the same default (#000) on iOS whether in light or dark mode
Pull Request resolved: facebook#28708

Test Plan:
I have manually tested the following:
- The default text color in light mode is black
- The default text color in dark mode is black
- The color can be changed using the `style.color` attribute
- Setting the opacity to 0.5 results in the desired behavior, the whole TextInput becoming half the opacity.
– Setting the `style.color` to rgba(0, 0, 0, 0.5) works as intended, creating a half-opaque text color.

Differential Revision: D21186579

Pulled By: shergin

fbshipit-source-id: ea6405ac6a0243c96677335169b214a2bb9ccc29
Reviewed By: zertosh

Differential Revision: D21202121

fbshipit-source-id: 6acb53e6ca941e465b11aeac4215533c16067eed
Summary:
Straightforward rename to clarify the purpose of this type.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21160790

fbshipit-source-id: eaf5e8c9f51e16134e153a6321857234be1aa338
Summary:
Straightforward rename to clarify the purpose of this type.

The current naming made more sense before the codegen also produced code for NativeModules.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21160793

fbshipit-source-id: 6787ef298e32ff1b4d506afd831af96764f5af6f
Summary:
Straightforward rename to clarify the purpose of this type.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21160791

fbshipit-source-id: 422d09243edda0660815eb2f0ce51f7e56134983
Summary:
Adds a `RootTag` parser test for the new codegen for NativeModules/TurboModules.

I'm doing this in a prerequisite commit in order to make the diff of the diff clearer when I implement proper support for `RootTag`.

This also fixes some of the minor typos and mistakes that I noticed. I also wanted to land these benign snapshot changes independent of the upcoming behavior changes.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21160792

fbshipit-source-id: 5f29f34035da30d7afa2369dbc19e95954553e88
Summary:
Adds support for `RootTag` in the new codegen for NativeModules/TurboModules.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21160788

fbshipit-source-id: 952189f6e8bc8fde8b403d4c0e77b5d66b3f03e4
Summary:
Adds support for `RootTag` in the new codegen for Native Component Commands.

Changelog: [Internal]

Reviewed By: TheSavior

Differential Revision: D21169371

fbshipit-source-id: 3b25433f3328e9c04cfe45bb176fc06d63559f14
Summary:
Following the comment DefinitelyTyped/DefinitelyTyped#42618 (comment)

Modify the flowtype of BackHandler function to have more specific return type.

## Changelog

[General] [Changed] - Changed type of BackHandler to be more specific.
Pull Request resolved: facebook#28192

Test Plan: No flow error in RNTester

Reviewed By: TheSavior

Differential Revision: D20771113

Pulled By: hramos

fbshipit-source-id: 5ca65e2a2b3f8726b8fb4606473d8fad5b0ce730
Summary:
The integration with Yoga was pretty complex from day one. The first attempt to make it simpler was in D19963353 when we removed a bunch of layers of indirection. This is the second iteration that aimed to simplify the structure of methods and their responsibilities.

The only conceptual change (that I am aware of) in this diff is that now we don't support (imaginary) case where a non-leaf YogaLayoutableShadowNode can have a non-YogaLayoutableShadowNode child. In the previous version, it was a no-op, now it's not supported and an assert will fire.

Alongside with refactoring, this diff implements several helper functions that verify the invariants important for the Concurrent Layout in debug mode.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D21198222

fbshipit-source-id: cc085904948056f861562af5bd2571de45a743b9
Summary:
Updating state with a null wrapper is neither desirable, nor possible. The underlying task was closed, just cleaning up comments.

Changelog: [Internal] comments only

Reviewed By: mdvacca

Differential Revision: D21186545

fbshipit-source-id: d14ddd59d42e8fd91c6e7fd50037311d4e8d0b60
…the content wrappers

Summary:
I noticed that in ModalHostShadowNode.java (not used in Fabric), there's an assumption that the Modal will have exactly one child on the native side; this child is explicitly specified in Modal.js.

However, in Fabric, these views are flattened and so the Modal will actually have N children - whatever children the product code passes into the Modal.

In *theory* this should be fine, but might be causing issues. Not sure.

This is an experiment and shouldn't be landed until we verify that (1) this actually matters, (2) that it fixes an issue with Modal on iOS or Android.

Changelog: [Internal] Change to make Fabric consistent with non-Fabric Modal

Reviewed By: mdvacca

Differential Revision: D21191822

fbshipit-source-id: 9d65f346387fd056649d4063d70220f637ba8828
…alScrollView

Summary:
For a very long time, iOS has supported the `contentOffset` property but Android has not:

facebook#6849

This property can be used, primarily, to autoscroll the ScrollView to a starting position when it is first rendered, to avoid "jumps" that occur by asynchronously scrolling to a start position.

Changelog: [Android][Changed] ScrollView now supports `contentOffset`

Reviewed By: mdvacca

Differential Revision: D21198236

fbshipit-source-id: 2b0773569ba42120cb1fcf0f3847ca98af2285e7
Summary:
Fixes text layout so that the parent bounds are correctly respected. This fixes two bugs:

- **Parent width is not respected.** This was caused by the recent change to shrink-wrap text layout.
- **Parent height is not respected.** This has always been a bug.

After this change, Android will behave like iOS.

Changelog:
[Android] [Fixed] - Text layout no longer ignores parent bounds

Reviewed By: mdvacca

Differential Revision: D21199030

fbshipit-source-id: cc072bdcff64167db1f79b7bf965e57a7396cdf4
Summary:
This diff removes unnecessary (int) casts in the calculation of layout for TextInlineViews

changeLog: [Internal][Android] Internal optimization on the calculation of layout for TextInlineViews

Reviewed By: JoshuaGross

Differential Revision: D21211532

fbshipit-source-id: 920c1f88d042f3e1f6bfd0f560371f7482a62064
Summary:
This change upstreams a small change we did in react-native-windows to allow the RTLExample RNTester page to function correctly on Windows.  The change is part of this Pr:
microsoft/react-native-windows#4683
Currently the direction property is gated behind a check for Platform == 'iOS', which means it only works on iOS.  Windows supports direction = 'rtl' so I've chanced this check to Platform != 'android'.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Internal] [Changed] - Changed RTLExample RNTester page to use direction = 'rtl' when Platform is not Android.
Pull Request resolved: facebook#28742

Test Plan: Confirmed this change works correctly in RNTester on Windows.  Have not confirmed iOS as I don't have Mac hardware.

Differential Revision: D21235579

Pulled By: shergin

fbshipit-source-id: 47ab93c2bcd0dbc8347c6746081ae3c64f88faa5
…facebook#28711)

Summary:
This PR adds support for the dark mode and dynamic theme changing to the default App template and to the related `NewAppScreen` components. Using `useColorScheme` hook forced me to refactor a bit main `App.js` file, but I think those changes are step in the right direction according to way in which React Native is used in larger apps, so new `Section` component has been extracted to reduce code redundancy/repetition inside `App`.

Additional color `darker` has been added to the `Colors` statics from `NewAppScreen` because `dark` was too bright for the Dark Mode backgrounds.

Also main `StoryBoard` on iOS has been updated to use theme based colors instead of static or hardcoded ones. There was also an unused, empty `Label` which I have removed.

~~I'm not so much experienced with Android. If someone could also update Android splash screen (if Android requires such change) it will be nice. I want to look at this later using simulator.~~
> I have updated the Android splash screen and tested this change on the Android emulator.

If you have any comment or corrections feel free to post them out, I would like to put more work into this PR if it's needed. Dark Mode this days is a part of near every OS, so it could be considered as a standard feature. I hope those changes helps people which struggle with the basic theming implementation (+ there is now an example of hook and `children` prop usage in the template).

## Changelog

[Internal] [Added] - Add dark mode support to the default app template
Pull Request resolved: facebook#28711

Test Plan:
I have tested the App from the template on the iOS device and in Android emulator with RN `0.63.0-rc`.

Screen recording on iOS (demonstarates both modes, both splash screens and transition):
![ezgif-6-e24ee8e839c9](https://user-images.githubusercontent.com/719641/80025923-a04b0300-84e1-11ea-824a-b4363db48892.gif)

Screenshot of iOS app in Dark Mode:
![IMG_6542](https://user-images.githubusercontent.com/719641/79885748-c98f6480-83f7-11ea-8c73-1351a721d5d6.PNG)

Screenshot of iOS app splash screen in Dark Mode:
![IMG_6544](https://user-images.githubusercontent.com/719641/79960431-add29f80-8485-11ea-985c-b39176024ffa.PNG)

Screenshot of Android app in the emulator:
![Screenshot_1587566100](https://user-images.githubusercontent.com/719641/79995454-88f72000-84b7-11ea-810b-dfb70de03c2a.png)

Differential Revision: D21236148

Pulled By: shergin

fbshipit-source-id: 0c8a9534d3a3f8f8099af939243a889ac4df6cda
Summary:
Previously, a derived class, WithTuple, was used.  This ran
into bugs in MSVC (see microsoft/STL#121).
Instead, use specialization to get the same result using std::tuple
directly.  This avoids the bug, and is a cleaner API.

Changelog: [Internal]

Reviewed By: dulinriley

Differential Revision: D21233677

fbshipit-source-id: 1d75991847164e525b4ba70f65a90627e5f8cd56
Summary:
It's not allowed to return nullptr from the callback. The assert ensures it which is helpful during development.

Probably, we should consider using `gsl::not_null<>` here.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: sammy-SC

Differential Revision: D21149891

fbshipit-source-id: a5f77b35029f22b499491721036405682f812a38
Summary:
It's not immediately obvious from the UI/UX when/if this mechanism breaks, so it's good to have a test.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: JoshuaGross

Differential Revision: D21184718

fbshipit-source-id: 25432a1398cff3ce61f62cf433e3cb73d7a7a93f
Summary:
According to the Flow types, `contentOffset` is nullable. Support that.

Changelog: [Internal] Fix to (1) support null contentOffset in ScrollView and HorizontalScrollView, added on Android after the last release. (2) Correctly add support for contentOffset in ScrollView (I missed that when adding it to HorizontalScrollView in the previous diff).

Reviewed By: alsun2001

Differential Revision: D21243028

fbshipit-source-id: ebef9a9054a3e4dd88556739e836b7ece48fda12
@amgleitman amgleitman requested a review from alloy as a code owner July 21, 2021 00:32
@HeyImChris
Copy link

Can you add the git command(s) you used to merge in this commit?

@HeyImChris
Copy link

Can you be more specific than RNTester runs? Maybe just launch each test once and make sure each control loads as expected?

@amgleitman
Copy link
Member Author

Can you be more specific than RNTester runs? Maybe just launch each test once and make sure each control loads as expected?

The description has been updated to reflect my methodology.

@amgleitman amgleitman merged commit 6d9fdee into microsoft:master Jul 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet