Merge from upstream through 2020-06-30 #816
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: This dependency upgrade ensures that this eslint config now supports the latest TypeScript syntaxes. ## Changelog [General] [Fixed] - Upgrade dependencies. Pull Request resolved: facebook#29048 Test Plan: Try this new version of the plugin on your project. Reviewed By: GijsWeterings Differential Revision: D21879997 Pulled By: cpojer fbshipit-source-id: cb54c2b56361f7c20994d981c61826f7f50350de
Summary: Changelog: [Internal][Yoga] Use double operations during rounding Reviewed By: mdvacca Differential Revision: D21840018 fbshipit-source-id: c5d17fcb8984b1da9832a15ccd4d628e8d742c6a
Reviewed By: ejanzer Differential Revision: D21302418 fbshipit-source-id: a868f6dad3306190c7add26e8f9a976866c16aef
…initializer
Summary:
From the header of `RCTSurfaceHostingProxyRootView`:
This is a RCTRootView-compatible implementation of RCTSurfaceHostingView.
Use this class to replace all usages of RCTRootView in the app for easier migration
I need to do exactly this, but for a bridgeless mode callsite. This proxy class only uses the bridge for some perf logging, which we're fine with not having right now.
Reviewed By: shergin
Differential Revision: D21893522
fbshipit-source-id: 3547cff6143f44714e39e4104d03336010081e2e
Summary: Changelog: [Internal] Clean up animationDriver_ in `Binding::uninstallFabricUIManager`. Reviewed By: JoshuaGross Differential Revision: D21923567 fbshipit-source-id: ecdc727ecbfd1d7052be0372b8d2a0ee7172f93f
Summary: The JavaScriptCore implementation of JSI [does not currently support array buffers](https://github.com/facebook/react-native/blob/master/ReactCommon/jsi/JSCRuntime.cpp#L925-L943). The comments in the code suggest the JSC version used by React Native does not work with array buffers, but this seems to be out of date since the current version of JSC used by React Native does indeed support array buffers. This change just enables array buffers in JSCRuntime.cpp. NOTE: See react-native-community/discussions-and-proposals#91 (comment) for more background on this change. ## Changelog [General] [Added] - Support for array buffers in the JavaScriptCore implementation of JSI Pull Request resolved: facebook#28961 Test Plan: To test these changes, I just made some temporary changes to RNTester to use JSI to inject a test function into the JS runtime that reads from and writes to an array buffer, and call that function from the JS of the RNTester app (see ryantrem@28152ce). For the JS side of this, specifically look at https://github.com/ryantrem/react-native/blob/28152ce3f4ae0fa906557415d106399b3f072118/RNTester/js/RNTesterApp.android.js#L13-L18 For the native side of this, specifically look at https://github.com/ryantrem/react-native/blob/28152ce3f4ae0fa906557415d106399b3f072118/RNTester/android/app/src/main/cpp/JSITest.cpp#L22-L38 Reviewed By: shergin Differential Revision: D21717995 Pulled By: tmikov fbshipit-source-id: 5788479bb33c24d01aa80fa7f509e0ff9dcefea6
Summary: Changelog: [Internal] RCTScheduler was storing Scheduler as `shared_ptr`, but `RCTScheduler` is sole owner of it. `unique_ptr` better expresses this ownership. Reviewed By: JoshuaGross Differential Revision: D21923573 fbshipit-source-id: e382f2d6e0a4875e1441b6063c1ad7056b338e29
Summary: Changelog: [Internal] View gets flattened even though it has `display: none` and therefore it and its children do not get hidden. Reviewed By: shergin, mdvacca Differential Revision: D21929033 fbshipit-source-id: 994a79fb64fbe66273a70218ebe8056d92cd3cd4
Summary: Changelog: [Internal] `CFRelease` crashes app if `cf` is `NULL`, `CGColorRelease` doesn't. From: https://developer.apple.com/documentation/coregraphics/1586340-cgcolorrelease?language=objc# Even though this hasn't happened yet, we had a similar crash in D21943011. Also `CGColorCreate` returns nullable so it could happen. In order to prevent this in the future, let's switch to `CGColorRelease` which doesn't crash if `cf` is `NULL` and also is semantically correct. Reviewed By: shergin Differential Revision: D21953404 fbshipit-source-id: 8b969ee345c2507fcba2442fa20e3fdaf2235d8c
…ObjectWrapper` Summary: ManagedObjectWrapper (`wrapManagedObject`) is used to pass pointers managed by Objective-C runtime object thought C++ parts of the framework. The wrapper consists of a shared pointer to an object with a custom deleter that calls `CFRelease`. Apparently, there is a caveat here: shared ptr implementation always calls a deleter even if the object points to `nullptr`. It's usually fine because in C++ calling `delete` on a nullptr is a no-op, not an error. But it's an error from the `CFRelease` perspective, which checks the pointer and crashes if it's nullptr. More info: https://stackoverflow.com/questions/1135214/why-would-cfreleasenull-crash https://stackoverflow.com/questions/50201967/why-unique-ptr-with-custom-deleter-wont-work-for-nullptr-while-shared-ptr-does Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D21943011 fbshipit-source-id: 442ad5e274a146de112e6bd8f3c2d20f0225bf77
Summary: While I was investigated the previous issue (T59430348) I found this place where we manipulate managed pointers manually. So, to make it error-prone this diff changes it to use `wrapManagedObject`. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D21943010 fbshipit-source-id: ecaeffb419ae8d4880187027ca7ec9563e0dfd46
Summary: Changelog: [iOS][Removed] - Removed DEPRECATED_sendUpdatedChildFrames prop to ScrollView component because there are no callsites of it anymore Reviewed By: shergin Differential Revision: D21941946 fbshipit-source-id: 0b7d6d0986ddff4b250e70e0450a6f7e166b41f4
Summary: This has caused SEVs because the warning has gone unnoticed, so upgrading to error which should be much harder to miss or ignore. https://fb.workplace.com/groups/rn.core/permalink/2680043542227367/ # Changelog [Internal] update nested VList warning to error Reviewed By: TheSavior Differential Revision: D21945364 fbshipit-source-id: 88a9a9ab0b51e0afcf9b25be9854f65a61f419af
…Tree when scrolling stops Summary: In Fabric, some uses of the ScrollViewStickyHeader don't work after scrolling because even though the UI correctly reflects the translateY that the StickyHeader should be at, the underlying C++ Fabric ShadowTree doesn't have the updated parameters. 1. We add a mechanism to pass static props through to animated nodes; these get passed to the platform through the normal commit-diff process. This is to allow passing props to the platform that are also controlled by the animation. This mechanism could be reused elsewhere. 2. In ScrollViewStickyHeader, listen to updates for the translateY value and pass them to the platform when it stops changing - for Fabric only. This noops for non-Fabric since it's not necessary. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21948830 fbshipit-source-id: b203ecde466732203dd12a86e2339e81f66b27e7
Summary: Restore legacy support for mixed union types. These are not type safe and modules should use a different type, but we have a precedent for supporting these in the existing Linking native module. The new codegen will generate native code for these, and show a warning to encourage use of a better type. Generate native code for elements in arrays of objects. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D21848260 fbshipit-source-id: 0b8cbf25e7a02791b4d77e349227a2b0744854f4
Summary: In the past, in Fabric (Android), we never called stopSurface. Ever! This is bad for memory and can cause other issues, so... let's not do that. Instead, when the ReactRootView is being torn down, we check the View ID to see if it's a Fabric RootView, and if it is, we call Fabric's stopSurface method. Fabric stopSurface only has impact on (1) Fabric mount instructions being executed after that point and (2) tells JS to stop running the surface, on the JS thread, asynchronously. Anecdotally it looks like all the MountItems involved in deleting and removing views from the hierarchy are executed before stopSurface is called. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21965837 fbshipit-source-id: 5169c5a1e339fd9e016ae1234d8389b2bd30be70
…face Summary: Because of the previous diffs there's an increased chance of race conditions between JS executing and queuing up PreAllocateViewMountItems for surfaces that are stopped. Make sure those are ignored if they're queued up and a surface has been stopped. Currently stopSurface only happens on the UI thread; PreAllocateViewMountItems can be queued from any thread, but are only executed on the UI thread. So once a batch of items starts executing, there's no race between teardown and execution: we just need to make sure we check that the surface is still running initially. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21965839 fbshipit-source-id: 0241dc171022cc923b7e38dcd110d664096dde79
Summary: See title. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21965838 fbshipit-source-id: a3bd9d48c1fadb19a12cccaab5713cf73566bd93
…k#29090) Summary: Pull Request resolved: facebook#29090 Moving the logic for calling into JS to handle errors into ErrorUtils, where it can be reused outside of the bridge. Reviewed By: RSNara Differential Revision: D21939254 fbshipit-source-id: 0d8f3bd2503720be7619ed8dc8b2389f544049f3
Summary: This diff exposes receiveEvent on the UIManager class. This is necessary to support backward compatibility between Fabric and classic RN changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21979687 fbshipit-source-id: 1ec75896687d55e699f79c520e21f05fac368ee6
Summary: This diff migrates usages of RCTEventEmitter.class to EventDispatcher.dispatchEvent. RCTEventEmitter is not compatible with Fabric, now we need to use EventDispatcher.dispatchEvent instead. changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21982549 fbshipit-source-id: 9ea2d9a00f063a03c2e401fc1e328ce26bcf48df
…EventEmitter Summary: This diff refactors ReactTextView to use uiManager.receiveEvent instead of ReactEventEmitter. ReactEventEmitter.class should be replaced for uiManager.receiveEvent. changelog: [internal] Reviewed By: JoshuaGross Differential Revision: D21982548 fbshipit-source-id: 4ed5825f62c761564aa533f4e8bb1155036df7e2
Summary: Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D21995098 fbshipit-source-id: b387bd45b44fe93743d34d2a3435c165d15f054f
Summary: Changelog: [internal] We were not reseting value of slider when recycling, this would result in wrong value being displayed once slider gets reused. Reviewed By: shergin Differential Revision: D21996786 fbshipit-source-id: 3beac4936d0719c4ac5ac0499209300a912f6986
Summary:
Changelog: [Internal]
Here is what I believe happens.
We have an instance of `RCTSliderComponentView` which has green thumb tint color.
It gets reused, in prepareForRecycle we call `setThumbTintImage:nil` on its UISlider which internally sets an ivar to `nil`.
Next time `RCTSliderComponentView` gets used without explicit thumb tint color, we assign nil to UISlider's thumb tint color.
Internally this nil gets compared to nil that it saved during `prepareForRecycle` and concludes that the value is already sets and exists early.
Since we don't have access to `UISlider` I can't prove this but here is a short video where I showcase this behavior
{F239923204}
The code in video is here P133083862.
Reviewed By: shergin
Differential Revision: D21997324
fbshipit-source-id: 28a11ed817cc863a313217c475042918ee726011
Summary: Changelog: [Internal] There is no need for custom dealloc, let's get rid of it. We also prefer east const over west const. Reviewed By: shergin Differential Revision: D21997545 fbshipit-source-id: aa017f99aa26421fc59b353d0012687cb38fac08
…wNodeFragment::Value Summary: There is no good reason why they are `const` and `private`, in the future diffs we will need to mutate them. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D21992197 fbshipit-source-id: f8aee8db9ea1ff8282b38fbe3a966911678ee2c4
Summary: Changelog: [Internal] Round calculated text size to closest larger pixel size. In Paper we do this as well in https://fburl.com/diffusion/w2pj6ea0 Why do we need this? For example, we calculate height of the text to be 16.707235, yoga takes this value and assigns text to have height 17 anyway. I assume Yoga uses this extra 0.3 points of height to move other things around a little bit because Yoga doesn't deal with exact values. Reviewed By: shergin Differential Revision: D21999032 fbshipit-source-id: 73923dc3e27fa110adb3f76cfa635e0bfdc672d4
Summary: Fixes some comment typos, moves hit testing and accessibility code so it's beside each other. No functionality changes. Changelog:[Internal] Reviewed By: RSNara Differential Revision: D22003047 fbshipit-source-id: 0e785364d7e1a080034d24c9676a56acb45686bb
…` prop (facebook#28760) Summary: motivation: I was just checking out facebook@30cc158 and noticed that the commit, I believe, is missing logic for when `contentOffset` is actually `null`. That is, consider you render `ScrollView` with `contentOffset` { x: 0, y: 100 } and then change that to null / undefined. I'd expect the content offset to invalidate (set to 0 - hope that's the default). ## 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] - ScrollView, HorizontalScrollView: do not ignore `null` `contentOffset` prop Pull Request resolved: facebook#28760 Test Plan: Tested locally within RNTester <details><summary>code</summary> <p> ```js const Ex = () => { let _scrollView: ?React.ElementRef<typeof ScrollView> = React.useRef(null); const [offset, setOffset] = React.useState({x: 0, y: 20}); setTimeout(() => { setOffset(undefined); }, 2000); return ( <View> <ScrollView ref={_scrollView} automaticallyAdjustContentInsets={false} onScroll={() => { console.log('onScroll!'); }} contentOffset={offset} scrollEventThrottle={200} style={styles.scrollView}> {ITEMS.map(createItemRow)} </ScrollView> <Button label="Scroll to top" onPress={() => { nullthrows(_scrollView.current).scrollTo({y: 0}); }} /> <Button label="Scroll to bottom" onPress={() => { nullthrows(_scrollView.current).scrollToEnd({animated: true}); }} /> <Button label="Flash scroll indicators" onPress={() => { nullthrows(_scrollView.current).flashScrollIndicators(); }} /> </View> ); }; ``` </p> </details> Reviewed By: shergin Differential Revision: D22298676 Pulled By: JoshuaGross fbshipit-source-id: e411ba4c8a276908e354d59085d164a38ae253c0
Summary:
Changelog: [Internal]
Do not log warning for "Invalid view set to be the JS responder".
{F242140509}
Fabric doesn't store views in view registry, therefore this warning would be shown to the developer everytime `[RCTUIManager setJSResponder]` is called.
Reviewed By: shergin
Differential Revision: D22309447
fbshipit-source-id: cac8985cdc79ab2d03a246cc2d9377472a64a683
Summary: According to our experiments it's not better than "stop surface on unmount" in any way, and might regress some metrics. Unclear why, but if it's not necessary and doesn't seem to help, it doesn't make sense to continue this experiment. We still have a mechanism on the C++ side to stop outstanding surfaces on teardown that does the same thing. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22318864 fbshipit-source-id: 7e678c63e4884382e57d996a7f4c4b7b24c8193a
Summary: These flags haven't been used in months. They were useful to uncover some race conditions, but will not be iterated further. The Venice project will obviate the concerns that sparked these experiments in the first place. These flags have been hardcoded to false for a while. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22319204 fbshipit-source-id: 09415f3bb1ca56e15f357210e966d0483ff384f2
Summary: Moving property handling functions to their own properties.js file. No changes other than adding types to params. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22208243 fbshipit-source-id: 4a7d2c6e19c151954da793d399af9a256a4a40b7
Summary: Splits the codegen script into functions, and makes use of $YARN_BINARY for portability (defaults to `yarn` if not provided). Changelog: [Internal] Reviewed By: RSNara Differential Revision: D22148360 fbshipit-source-id: 9e86b3e0f7f77bf3a635bf6be204170333dd5e65
Summary: The Hermes inspector always logged the message for `console.assert`. It instead should check the first argument, and only if that argument is false should it log. Also remove the polyfill code that tried to avoid this situation. Changelog: [Internal] Reviewed By: mhorowitz Differential Revision: D22299186 fbshipit-source-id: cdf4f8957d4db3d171d6673a82c7fc32b7152af3
Summary: While in theory we should never delete views before removing them from the hierarchy, there are some exceptions: (1) Some mysterious cases that don't seem like bugs, but where the child still seems to keep a reference to the parent: (2) When deleting views as part of stopSurface. On #1: in the past we had issues when we assumed that ViewManager.getChildCount() would return an accurate count. Sometimes it's just... wrong. Here, I've found at least one case where a View still has a parent after it's removed from the View hierarchy. I assume this is undocumented Android behavior or an Android bug, but either way, there's nothing I can do about it. On #2: there are valid cases where we want to delete a View without explicitly removing it from the View hierarchy (it will eventually be removed from the hierarchy when the Root view is unmounted, but it may still be "in" a View hierarchy when it's deleted). Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22321374 fbshipit-source-id: 9667bbe778c418f0216550638dc26ca48a58e5fa
Summary: (1) As soon as we know we can StopSurface, stop executing all mountitems by clearing out the root tag. (2) If a surface has been stopped and there's a batch of MountItems to execute against it, execute only the "delete" operations to clear views from memory. Both of these should reduce memory usage and improve speed slightly around navigation pops. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D22321389 fbshipit-source-id: 96a8292a8442528f1bba50d35208885cc4168170
Summary:
The current parser behavior flattens out any object type aliases into ObjectTypeAnnotations. Generators can treat these as regular objects and generate the applicable native code. This, however, can lead to repetition whenever an object type alias is re-used in the same native module.
I propose we treat these as a special case, using a TypeAliasTypeAnnotation to denote them as type aliases. Generators can look up the actual signature for a given object alias by referring to the "aliases" array that is provided in the schema.
**Proposed schema change:**
Adds an "aliases" key to each module in the schema:
```
export type NativeModuleShape = $ReadOnly<{|
properties: $ReadOnlyArray<NativeModuleMethodTypeShape>,
aliases: $ReadOnlyArray<{|
name: string,
typeAnnotation:
| $ReadOnly<{|
type: 'ObjectTypeAnnotation',
properties: $ReadOnlyArray<ObjectParamTypeAnnotation>,
|}>
| $ReadOnly<TypeAliasTypeAnnotation>,
|}>,
|}>;
```
Example:
```
{
modules: {
SampleTurboModule: {
nativeModules: {
SampleTurboModule: {
aliases: [],
properties: [],
},
},
},
},
}
```
Method parameters will now support the new 'TypeAliasTypeAnnotation' wherever a param might have used a 'ObjectTypeAnnotation':
```
export type TypeAliasTypeAnnotation = $ReadOnly<{|
type: 'TypeAliasTypeAnnotation',
name: string,
|}>;
```
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D22200700
fbshipit-source-id: 15684620783c752f2fb482ba4b88d1fd1cc07540
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
Summary: Changelog: [internal] Revert a change introduced in facebook#30333 where rickhanlonii asked it to be reverted as well. The change breaks sticky header in Fabric. Reviewed By: rubennorte Differential Revision: D25883861 fbshipit-source-id: b01305c6def390d0664c7be939ab3fc72186a07a
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
…an/0.64-merge-2020-mm-dd
HeyImChris
approved these changes
Aug 24, 2021
Fix Android Build Issues for RN64 Upgrade for patches through 2020-06-30
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please select one of the following
Summary
Merge facebook/react-native's main branch into react-native-macos's main branch through 2020-06-30 23:59:59 PDT (specifically, e261f02).
These changes were made by repeatedly running git merge and resolving merge conflicts as needed.
Notes
yarn lintwarning that later gets fixed byc42a59af7e7f0033668f13424ad6529a70d7cdfc. Therefore, we're also cherry-picking the latter.Validation