Merge from upstream through 2020-09-30 #833
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:
Changelog: [internal]
# Problem
Default padding for TextEdit is top: 26, bottom: 29, left: 10, right: 10 however the default values for LayoutMetrics.contentInsets is {0, 0, 0, 0}.
If you try to construct TextEdit with padding 0, Fabric will drop padding update because padding is already 0, 0, 0, 0.
# Fix
To fix this, I added a special case to `Binding::createUpdatePaddingMountItem`, if the mutation is insert, proceed with updating padding.
Reviewed By: JoshuaGross
Differential Revision: D23731498
fbshipit-source-id: 294ab053e562c05aadf6e743fb6bf12285d50307
Summary: The implementation of this class is too complex for the purpose it serves. Making it simpler will make the code simpler and faster. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23725688 fbshipit-source-id: 5e1ecddb0dd3c4c4f94786e2ba0af9b67e7426ce
…ommited tree Summary: Instead of storing tree separate instance variables, we now store a single object that contains them. We will need it to be able to postpone the mounting stage (we save all info we need for mounting inside the new instance variable). Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23725690 fbshipit-source-id: 09dd4a0912c6f5b34d805636b62f73ca12287329
Summary: Previously, to get a current root shadow node for a shadow tree, we called `tryCommit` method and stole a pointer from this. That was not a very straightforward method to get things done, and most importantly we need to do this to change the shape of the ShadowTreeCommitTransaction signature (remove a shared pointer from the callback) to make it simpler, faster and allow future improvements (see the next diff). Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross, sammy-SC Differential Revision: D23725689 fbshipit-source-id: 51950b843a0e401828b6c6a38e5e2aaaf21ec166
Summary: We don't need a shared_ptr here and without it the code will be faster and simpler. This change is aligned with any clone-line callbacks we have in the Core which accepts a `const &` and return `shared_ptr<>`. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross, sammy-SC Differential Revision: D23725687 fbshipit-source-id: 1cd959f4273913175d342302e2f12752f0114768
… TransactionTelemetry Summary: Apparently, there is C++ keyword for this. Changelog: [Internal] Fabric-specific internal change. Reviewed By: sammy-SC Differential Revision: D23754284 fbshipit-source-id: 5f9bbcc72d9c586173624869d614f12d2319fb7b
Summary: Added an explicit NDK version to the Android template. This allows tooling to detect which NDK version to install automatically. ## 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] - Add an explicit NDK version to Android template Pull Request resolved: facebook#29403 Test Plan: Template builds as it should. NDK version gets installed with initial Android set up. Reviewed By: passy Differential Revision: D23752007 Pulled By: fkgozali fbshipit-source-id: 31c33f275f94a4a62338a61e79b31c4b996969bf
Summary: This diff removes `RCTCGColorRefUnretainedFromSharedColor` function because it's not compatible with future changes we plan to make. Converting SharedColor to unretained CGColorRef is actually quite dangerous because... it's an unowned pointer. Reviewed By: JoshuaGross Differential Revision: D23753509 fbshipit-source-id: df030ade69b63cbb872d6bdbde51575eedc6a4a6
Summary: Even though we have wonderful helper functions converting SharedColor to UIColor and CGColorRef, in many places we still use some hardcoded things. This diff fixes that; it will be important for the next diff. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D23753508 fbshipit-source-id: 09d280b132266252753526c2735ab3e41b96c8d5
…ementations of SharedColor Summary: This diff changes the implementation of `RCTCreateCGColorRefFromSharedColor` and `RCTUIColorFromSharedColor` in such a way that they don't rely on the fact that SharedColor is actually a `shared_ptr<CGColorRef>`. Instead, the methods just extract color components from SharedColor and create UIColor and CGColorRef objects on demand. This allows us to change the implementation of SharedColor without worrying much about the rest of the system, which will do in the next diff. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D23753510 fbshipit-source-id: 340127527888776ebd5d241ed60c7e5220564013
Summary: Finally, this diff changes the internal implementation of SharedColor to be `optional<int>`. Initially, when we started working on the new renderer, it seemed like a good idea to allocated CGColor objects ahead of time and store them with Props. Now, this idea does not look so good, especially because: * Having `SharedColor` as a `shared_ptr` is a quite big perf overhead for copying this thing. And the size of the object is not small. * Having `SharedColor` as a `shared_ptr` creates huge interconnectedness among pieces of the core and rendering. E.g. improper releasing a pointer in some component view can cause a crash somewhere in the core (because both places might use the same shared `blackColor`. On Android, we already use simple `int` as a representation of a color, and this works great. And this diff implements something very similar to Android, but a bit differently: here we use `optional<int>` instead of custom class with a single `int` field and some magic value that represents "empty value". This approach should fix T75836417 because now we don't have allocation and deallocation when we simply assign color values. If this solution works fine on iOS, I will do unify all implementations among all platforms. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D23753507 fbshipit-source-id: 42fd6cee6bf7b39c92c88536da06ba9e8cf4d4db
…colors Summary: This change maps the three most used colors (black, white, clear) to corresponding predefined values in UIColor. This should meaningfully reduce the overall amount of allocated UIColor/CGColor objects. In my non-scientific measures, it reduces the number of CGColor objects from ~1500 to ~1000. And... it no much at least in terms of kilobytes. However, I still think it's a good idea to implement this because I hope that can remove some work from memory allocation infra and maybe enable some optimizations that UIKit hopefully does for black and white colors. (I tend to believe that this optimization exists because UIKit even has a classes called UIDeviceWhiteColor and UICachedDeviceWhiteColor.) Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D23753506 fbshipit-source-id: 46e58dc7e6b0dcab3c83d29c7257c90ffbd95246
Summary: Changelog: [Internal] To align more closely with Paper, Fabric should coalesce touchMove events. on iOS it happens: https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/React/Base/RCTTouchEvent.m?lines=43 Reviewed By: JoshuaGross Differential Revision: D23734212 fbshipit-source-id: a9d324a6481884905d7be6637fcafe4e71f2bd9f
…ation` Summary: `adjustDelayedMutationIndicesForMutation` asserts that the mutation is either Remove or Insert. At one callsite, we weren't checking the mutation type before calling `adjustDelayedMutationIndicesForMutation`. Changelog: [Internal] Reviewed By: shergin Differential Revision: D23746617 fbshipit-source-id: 6046fec2eb4821094937b1b16f40347bbc55c20e
Summary: In MountingCoordinator override mode (used in LayoutAnimations) we must set the start and end `diff` time when no real diff happens, otherwise we will hit an assert in telemetry later. I also ensure that the TransactionNumber is incremented in that case. Changelog: [Internal] Reviewed By: shergin Differential Revision: D23746684 fbshipit-source-id: b1fe3864e453fdba89d43cc827bd37434abf7a4d
Summary: Currently, MountingCoordinator's RN_SHADOW_TREE_INTROSPECTION code will crash often because it assumes there is always a "new" tree to compare the old tree to. In the LayoutAnimations context this is not always the case - in fact, the majority of the time, LayoutAnimations is producing mutations for animation without a "new" tree. Just check that the tree exists before trying to print it. Changelog: [Internal] Reviewed By: shergin Differential Revision: D23747289 fbshipit-source-id: a1ba22aeae32ed8915a53bc33cdc199e8ce5128a
Summary: iOS needs this function to be marked as static. Changelog: [internal] Reviewed By: shergin Differential Revision: D23749613 fbshipit-source-id: a8c160646853450fc7d849448bdbb45e02beb964
…tion Summary: LayoutAnimations: at the end of every animation, issue an update mutation - this is so that the props data on the Mounting layer/StubViewTree layer is pointer-identical to the props data on the ShadowTree. This unblocks iOS debug mode crashes. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D23753606 fbshipit-source-id: 407e0c2746a65e6d3ee29c1cce891cd7c1013593
Summary: Under Fabric only, we can enter an infinite layout loop where the emitted layout event oscillates between two height values that are off by a very small amount. The cause is, in part, components that use layoutEvents to determine their dimensions: for example, using onLayout event "height" parameters to determine the height of a child. If the onLayout height changes rapidly, the child's height will change, causing another layout, ad infinitum. This might seem like an extreme case but there are product use-cases where this is done in non-Fabric and layout stabilizes quickly. In Fabric, currently it may never stabilize. Part of this is due to a longstanding issue that exists in Fabric and non-Fabric, that we cannot immediately fix: If in a single frame, C++ emits 100 layout events to ReactJS, ReactJS may only process 50 before committing the root. That will trigger more layout events, even though product code has only partially processed the layout events. At the next frame, the next 50 events will be processed which may again change the layout, emitting more events... etc. In most cases the tree will converge and layout values will stabilize, but in extreme cases in Fabric, it might not. Part of this is because Fabric does not drop *stale* layout events. If 10 layout events are dispatched to the same node, it will process all 10 events in older. Non-Fabric does not have this behavior, so we're changing Fabric to drop stale events when they queue up. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D23719494 fbshipit-source-id: e44a3b3e40585b59680299db3a4efdc63cdf0de8
Summary: The Apple documentation states: https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Blocks/Articles/bxUsing.html#//apple_ref/doc/uid/TP40007502-CH5-SW1 > Typically, you shouldn’t need to copy (or retain) a block. You only need to make a copy when you expect the block to be used after destruction of the scope within which it was declared. Copying moves a block to the heap. All blocks generated in the TurboModule infra, for callbacks and promise resolve/reject handlers, can be used after the destruction of the scope within which they were declared. Therefore, let's copy them in the hopes that they mitigate T75876134. **Note:** We copy blocks before pushing them into the `retainedObjects` array in the legacy Infra as well. Context: D2559997 (facebook@71da291), D5589246 (facebook@2a6965d) Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D23764329 fbshipit-source-id: e71360977bdff74dc570bd40f0139792860f811f
Summary: Use the latest published release of hermes-engine. Changelog: [Android] [Changed] - Upgraded to Hermes 0.7.0 allow-large-files Reviewed By: cpojer Differential Revision: D23725129 fbshipit-source-id: 50938433147100e97699b717d77a687fbbfe892b
Summary: This enables a new state auto repeating mechanism built-in mechanism for all state updates which we already use for CK interop. This experiment is supposed to help with T74769670 and co. This change is gated with MC. Changelog: [Internal] Fabric-specific internal change. Reviewed By: JoshuaGross Differential Revision: D23762508 fbshipit-source-id: f535513c724ace9ede570177281324eb507329c5
Summary: Inside JavaTurboModule, the native `CallInvoker` is used to schedule work on the NativeModules thread. So, in ~JavaTurboModule(), I scheduled some work on the NativeModules thread. This work holds a copy of the JNI global reference to the Java NativeModule object, and when it's executed, it resets this global reference to the Java NativeModule object. This should ensure that the we don't access the JVM in ~JavaTurboModule, which could crash the program. I also removed the redundant `quitSynchronous()` in `~CatalystInstanceImpl()`, to prevent the NativeModules thread from being deleted before we delete the `jsi::Runtime`. This shouldn't cause an issue, because we delete the NativeModules thread when we call [ReactQueueConfigurationImpl.destroy()](https://fburl.com/codesearch/p7aurwn3). Changelog: [Internal] Reviewed By: ejanzer Differential Revision: D23744777 fbshipit-source-id: a5c8d3f2ac4287dfef9a4b4404a04b335aa0963d
Summary: The way the performance logger is defined now is very unsafe regarding type safety, as all accesses to its properties is untyped (`any`) and it uses several `mixed` types in cases that could be more refined. This migrates the creation of performance loggers to instances of a class to improve its type safety. If there's an impact in performance, it's expected to be positive. Changelog: [Internal][Changed] - Replaced object literals with class instances to create performance loggers Reviewed By: lunaleaps Differential Revision: D23758609 fbshipit-source-id: 0734742eb97d92a4a53f7b66a8ca45a2ae90946c
Summary: The `description` parameter is never used so we can simplify the API. Changelog: [Internal][Changed] Removed `description` option from performance logger timespans Reviewed By: lunaleaps Differential Revision: D23758829 fbshipit-source-id: 10900f86effc3e1f54a408cf8f9fbc9b3b52f569
Summary: Changelog: [Internal][Removed] Removed `addTimeAnnotation` method from performance loggers Reviewed By: lunaleaps Differential Revision: D23758816 fbshipit-source-id: 98e0abae25266f3dcc5953f25f20cde8e3dac190
Summary: Changelog: [Internal][Changed] Removed `update` option from `stopTimestamp` method in performance loggers Reviewed By: lunaleaps Differential Revision: D23759138 fbshipit-source-id: bb83b6f5ff2f640733c2e508779b3bc52800e4f6
Summary: Changelog: [internal] https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/Libraries/Animated/createAnimatedComponent.js?commit=1b6ce6c3a69a&lines=82-112 `_isFabric` in `createAnimatedComponent` returns false for Fabric component, that's why nativeID was not being assigned and view got flattened. To fix this, props.nativeID is propagated. SnackBar already has nativeID https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/RKJSModules/Libraries/FDS/FDSLightweightFeedback/DEPRECATED_FDSSnackBar.js?commit=1b6ce6c3a69a&lines=277 Reviewed By: PeteTheHeat Differential Revision: D23757304 fbshipit-source-id: 9e4b4599c95b8af8767793bc8cdce717a347a273
Summary: This flag was deleted in D23374948 (facebook@6729a3e), reintroduce it. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D23771273 fbshipit-source-id: ae9595194bf14bc740d05b2ca6e7b5e22bdd566f
Summary: Continuing the adding of a "cause" field for logging to GCs. This allows embedders of Hermes (such as React Native) to specify the cause of a call to `collectGarbage`. Notably, this allows Hermes to know when a GC is triggered by a memory warning. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D23742099 fbshipit-source-id: 99453e632328c00045b92a72f789d41c898dc518
Summary: Changelog: [internal] Components can update state multiple times before the state update queue is flushed. This causes unnecessary layout/diff and mount passes. To solve this, drop stale state updates inside `stateUpdateQueue_ ` for specific `ShadowNodeFamily`. Delivering stale status updates is redundant. Let's take SafeAreaView as an example. It schedules 5-6 state updates before `stateUpdateQueue_` is flushed. That's unnecessary work blocking JS thread. We only care about the latest state update. Same for TextInput and other components using state updates. Reviewed By: JoshuaGross Differential Revision: D23987707 fbshipit-source-id: 2e3f92cc93af61d78ac564aa40aef165af64b8c1
Summary:
This was causing a crash in babel:
```
$ babel src --out-dir bin --source-maps
Error: Cannot find module 'babel-plugin-idx' from
'~/fbsource/xplat/js/react-native-github/ReactCommon/hermes/inspector/tools/msggen'
- If you want to resolve "idx", use "module:idx" {
code: 'MODULE_NOT_FOUND'
}
```
It didn't appear that this module was used, so I deleted it.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D23993272
fbshipit-source-id: dd34f0fc652cb27c87c891ca37d0eba66a19a6cf
Summary: A `cat` to file was removed accidentally, preventing the configuration script from executing successfully as part of a `pod install`. Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D24024824 fbshipit-source-id: 94af0c6e663320bfac04ee8f6fb37bd4bdc379a4
…#30051) Summary: This PR fixes a small leftover after the RNTester migration to the monorepo package - an invalid link in the `PanResponder` comment. Refs: * facebook#29567 * facebook/react-native-website#2177 (the similar correction was a part of merged PR in the RN docs) ## 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][Fixed] PanResponder: correct example link in comment Pull Request resolved: facebook#30051 Test Plan: N/A Reviewed By: fkgozali Differential Revision: D24025065 Pulled By: hramos fbshipit-source-id: 190486e458fb8e83a35fa2d3c62f4483f3a4334d
Summary: It was recently upgraded to 4.4, so we don't need the 4.3.1 anymore. Changelog: [Android][Removed] Removed Robolectric 4.3.1 setup Reviewed By: jselbo Differential Revision: D24030822 fbshipit-source-id: 09b3c577d32028723e7bbc02f13459a7ae69b749
Summary: This diff removes an incorrect assert and replaces it with a debug-only verification phase that compares "what we want" with "what we get". Changelog: [Internal] Fabric-specific internal change. Reviewed By: PeteTheHeat Differential Revision: D23983123 fbshipit-source-id: 03a628b4f8baa1f5fe4b55354b7c943e38b5e537
Summary: This is a major version update that needs to be tested thoroughly. Android Studio 4.0.1 is now available in the stable channel https://androidstudio.googleblog.com/2020/05/android-studio-40-available-in-stable.html https://developer.android.com/studio/releases/gradle-plugin#4.0.1 ## Changelog [Android] [Changed] - Update Android Gradle plugin to 4.0.1 Pull Request resolved: facebook#29013 Test Plan: Build project Closes facebook#29044 Reviewed By: shergin Differential Revision: D24041233 Pulled By: fkgozali fbshipit-source-id: 68ef0f313aa773866e65796e323ed0f19f41f834
Summary: If removeViewAt crashes, log the children of the parent view, and all of the parent's ancestors. Changelog: [Internal] Reviewed By: shergin Differential Revision: D24019515 fbshipit-source-id: c5b1ca0948ebc47f2648e161770affa8542ca5dd
Summary: Changelog: [Internal] Reviewed By: mroch Differential Revision: D24040584 fbshipit-source-id: 106caa00cadd6930685c0030ad74685c64572ba9
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
…an/0.64-merge-head
harrieshin
reviewed
Sep 25, 2021
HeyImChris
reviewed
Sep 27, 2021
Summary: This is a revert of D22488870 (facebook@521b167). (facebook#29177) We have to revert it because we realized that it's a breaking change without a very good reason. We have to figure out a better way to unify platform behaviors. Changelog: [iOS][Fixed] - Reverted recent change in TextInput (facebook#29177) Reviewed By: fkgozali Differential Revision: D24200517 fbshipit-source-id: af0e561a6b8d9ade487be6b197a5d79d326442b6
HeyImChris
approved these changes
Sep 28, 2021
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-09-30 23:59:59 PDT (specifically, dd35bf5).
These changes were made by repeatedly running git merge and resolving merge conflicts as needed.
Notes
No notable new diversions have been introduced.
Validation