Merge from upstream through 2020-05-28 #811
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: I removed 1 MiB from `jscodeshift` as it was shipping with docs: facebook/jscodeshift@5885662 Changelog: [Internal] Reviewed By: yungsters Differential Revision: D21368889 fbshipit-source-id: 452fd4cedcc749d972adbb69df5c95117dd55b15
… just return nil Summary: There's a corner case where: * The bridge gets invalidated, and native modules are cleaning up themselves (but not done yet) * Something asks for a NativeModule instance - ideally it should just get nil at this point * If TM Manager is invalidating, we get nil correctly, but we continue thru the old NativeModule lookup logic * The latter will fail anyway, and would throw a redbox (RCTLogError). So, if the bridge is invalidated, if TM Manager returns nil, we should just return nil for old NativeModule lookup. The module of interest is RCTImageLoader, which was requested by RCTImageView on deallocation. The problem is RCTImageView got dealloc'ed **after** the bridge has been invalidated, so the lookup would always fail... Bonus: RCTImageView should just keep a weak ref to the RCTImageLoader, so that: * if the imageLoader is still alive on image dealloc, it can still access them (last minute "meaningless" cleanup) * if the imageLoader is gone, then the image deallocation doesn't do anything Changelog: [iOS] [Fixed] - Fix module lookup race condition on bridge invalidation. Reviewed By: p-sun, sammy-SC Differential Revision: D21371845 fbshipit-source-id: 862dc07de18ddbfb90e87e24b8dbd001147ddce4
Summary: Our parsers accept these but they are invalid. This fixes them. Changelog: [Internal] Reviewed By: yungsters Differential Revision: D21373812 fbshipit-source-id: 6084757b9f842644fe629ae5e6d85baec611588d
Summary: Fixes facebook#28488. ## Changelog [iOS] [Fixed] - Fixes TextInput shaking when typing Chinese Pull Request resolved: facebook#28805 Test Plan: Demo see facebook#28488. Differential Revision: D21376803 Pulled By: shergin fbshipit-source-id: b1fe6cc5f67d42ef98a6c12b8ab9990feac0e2a7
Summary: Migrates `VirtualizedList` off legacy context by creating `VirtualizedListContext`. Changelog: [General][Changed] - Migrated `virtualizedList` legacy context to `React.Context`. Reviewed By: TheSavior Differential Revision: D21370882 fbshipit-source-id: 2fa99ee0bc0e6b747a2d3fe7c66ee402c6b9c5af
Summary: Removes `PropTypes` as a dependency of `VirtualizedList` by no longer validating the return value of `getItemLayout`. Changelog: [Internal] Reviewed By: TheSavior, cpojer Differential Revision: D21370890 fbshipit-source-id: 966db3557b714987aa91179c7654a5ebf27818ad
Summary: `babel/preset-env` pulls in a number of unused deps, like `caniuse-lite` (3 MiB) that knows about which browsers support certain features. We do not ship to browsers and always know which version of node we are using, so we don't need to pull this in. I changed `jscodeshift` to optionally depend on `babel/preset-env` instead of always pulling it in. This reduces node_modules by 7 MiB. Changelog: [Internal] Reviewed By: yungsters Differential Revision: D21374475 fbshipit-source-id: 6f55e96e990ec0ca12f17bb3657bfa5429796b93
Summary: Changelog: [internal] As part of recycle, we should delete state. This is a common pattern used in other components as well. Reviewed By: shergin Differential Revision: D21348782 fbshipit-source-id: a5dee2f4ccee9b19498db31dab1983d8879dca71
Summary: The default application requires Kotlin version that is not supported by the Gradle plugin (should be at least `1.3.10`). However, instead of upgrading, we should remove it entirely. Here's why. This commit facebook@29d3dfb introduced Detox for RNTester Android application. Since the commit doesn't mention Detox for the default application and there are no Detox tests present by default in the default application, I believe that this addition was performed by a mistake. The best way is to remove Kotlin from the default template. This step is described in the Detox documentation and all users that are integrating Detox will be asked to perform it anyway. No need to do it for them. ## Changelog [ANDROID] [INTERNAL] - remove Kotlin from the default template Pull Request resolved: facebook#28626 Test Plan: Building a brand new project with `master` should work Differential Revision: D21388961 Pulled By: shergin fbshipit-source-id: 92666aa67f92b29f4e7f9c036b332bd058cdd49e
Summary: When we call `.erase` on the TinyMap, it sets the key of the element to 0. When we call `.begin()` later, TinyMap will sometimes, but not always, clean up the underlying Vector. This means that *most* of the time, underlying erased elements will be removed from the Vector; but sometimes erased elements will still be there when iterating over it. This was causing us to generate extra "Create" mutations. To fix this, for now we just check for zeroed-out elements when iterating over the vector. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21389371 fbshipit-source-id: 1e641050987d40a3f3e31499dcb373cfb28ae6f8
Summary: Add a `console.warn()` call when calling `_scrollRef.scrollTo`, because `scrollTo` is not guaranteed to exist on `_scrollRef`. Context: `VirtualizedList` holds `_scrollRef`, which is usually a reference to a `ScrollView` component. However, there are several cases where it holds a `View` or other type of component instead. A custom component can be passed in to `renderScrollComponent`, and then `_scrollRef` will point to that custom component. Additionally, if two VirtualizedLists are nested with the same orientation, `_defaultRenderScrollComponent` will return a View instead of a ScrollView. Due to these possibilities, `_scrollRef` is not guaranteed to have a `scrollTo` method. Changelog: [General] [Added] - Add warning when scrollRef does not have a scrollTo method Reviewed By: JoshuaGross, TheSavior Differential Revision: D21386842 fbshipit-source-id: 01e167e0ae0edea8f29853e8b242ce88a5103b49
Summary: While #import is common in Objective C, it's a vendor specific extension in C++, only supported by GCC/Clang, and only when -pedantic is off. Its use causes build breaks with MSVC. Replace it with the standard #include. ## Changelog [Internal] [Fixed] - Don't use #import in C++ Code Pull Request resolved: facebook#28825 Test Plan: We've ben running this change within react-native-windows for some time. Differential Revision: D21391233 Pulled By: shergin fbshipit-source-id: c0f94f314c46d6ac24067bbdcd5aaaeec9da283f
Summary: Before, compilation fails with P130281113. After fixing BUCK target, fails with P130281201. After all changes, the tests succeed. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21390996 fbshipit-source-id: c85aa43b3ef7fc9642d226ae706c937b2a5a408d
Summary: Call Fabric's LayoutAnimation APIs from JS. Changelog: [Internal] A step towards supporting LayoutAnimations on Fabric Reviewed By: shergin, mdvacca Differential Revision: D21297975 fbshipit-source-id: 6d530b01d8152e7c803a7c0299b918a32fb39dc0
…n()`, and prevent erased elements from being iterated over Summary: The core issue solved in D21389371 was that erased elements of a TinyMap were being iterated over, because TinyMap has somewhat-complicated logic around cleaning out the underlying vector. In some very marginal cases, vectors were not being cleaned and an iterator pointing at erased elements was being returned. The diff prevents some possible crashes in `begin()` and `find()` while making it much less likely to iterate over erased elements. We also add a unit test to catch the case fixed in D21389371, in particular. We also are keeping the code added in D21389371 (for now) since it's a cheap check, and will be a safeguard until we have rigorous testing around TinyMap. To be clear that logic should noop currently, but will prevent crashes in case guarantees around TinyMap change in the future. Currently there is only one line of code that actually uses the TinyMap iterator, so this should be safe. Reviewed By: shergin Differential Revision: D21392762 fbshipit-source-id: 36dc998958c230fad01af93338974f8889cbcf55
…k#28817) Summary: Since support for contentOffset was added to horizontal ScrollView on android (30cc158) I'm seeing a crash in my app because of a library. What happens is that it passes a partial object for contentOffset so something like `{x: 1}` which causes a crash on Android. According to the flow types the object should always contain both x and y but I think we should preserve the runtime behaviour and just use 0 like iOS does. ## Changelog [Android] [Fixed] - Allow passing partial contentOffset to ScrollView on Android Pull Request resolved: facebook#28817 Test Plan: Tested that passing partial object for contentOffset does not crash. Reviewed By: JoshuaGross Differential Revision: D21396319 Pulled By: shergin fbshipit-source-id: 4b52c868e3bfe183ff7f68a76ac34d1abd5e1069
Summary: `renderApplication` receives the root component that we need to render and an optional wrapper component. There are cases where we want to use the initial props passed to the root component in the wrapper component as well (e.g.: to provide a specific context to the root component), so this adds modifies `AppContainer` to accept the initial props and inject them into the wrapper component. Changelog: [General] [Added] - Modified `renderApplication` to forward `initialProps` to `WrapperComponent` Reviewed By: fkgozali Differential Revision: D21347486 fbshipit-source-id: 1c4f702a3875077630de1a44d3ac9ef2c80bc10c
Summary: * Brings performance improvements from 25.5.x * Reduces node_modules by 3 MiB, see https://github.com/facebook/jest/pull/9950/files?short_path=63580dd#diff-63580dd1e7078ce037f10f2fee7553b9 * Breaking changes: https://github.com/facebook/jest/blob/master/CHANGELOG.md Changelog: [Internal] Reviewed By: rubennorte Differential Revision: D21369069 fbshipit-source-id: 81a9d50f8e541293a85ce3957cb962930ca05b11
Summary: We are currently very strict about breakpoint location matching. This diff allows some fuzz in the column, but not in the line. Changelog: [Internal] Setting Hermes breakpoints no longer requires exact column match Reviewed By: avp Differential Revision: D21343198 fbshipit-source-id: a59786a9d63f9fe1ed576835ed660ba3343affe1
…acebook#28737) Summary: Fixes facebook#28726 When importing TouchableOpacity, it is treated as any by Flow. Replacing ComponentType with AbstractComponent works. The [Flow documentation](https://flow.org/en/docs/react/types/#toc-react-componenttype) says the following about ComponentType: > Note: In 0.89.0+, React.ComponentType is an alias for React.AbstractComponent<Config, any>, which represents a component with config type Config and any instance type. So I'm thinking that since the instance type is treated as any with ComponentType, Flow treats TouchableOpacity as any as well. ## 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 --> [General] [Fixed] - Fix Touchable{Opacity,Bounce,Highlight} being exported as `any` (Flow) Pull Request resolved: facebook#28737 Test Plan: I have done the same changes to react-native in my project's node_modules and seen that the components TouchableOpacity went from any to AbstractComponent with some props. Now I have a bunch of errors because I'm sending in wrong props to some touchables, which is good! Reviewed By: cpojer Differential Revision: D21362601 Pulled By: TheSavior fbshipit-source-id: 5b98cc79eaef034eccdb7f47242f9f44be2ef2b8
Summary: Import folly to handle optionals (`folly::Optional<__type__>`) Sort modules and indent generated code to match output from the old codegen. While not strictly necessary as these are generated files that should not be edited by hand, I found that matching the old codegen in this regard made it less of a chore when it came to comparing the output of both codebases. Changelog: [Internal] Reviewed By: RSNara Differential Revision: D21395231 fbshipit-source-id: 289d617d7a2d93724456c80afea57a49c108cb9b
Summary: As titled. Reviewed By: fkgozali Differential Revision: D21417307 fbshipit-source-id: 7c6b0179f9f1a5108da241d181a24f707a083deb
Summary: Android ScrollView/HorizontalScrollView `smoothScrollTo` contains some logic that, if called multiple times in a short amount of time, will treat all calls as part of the same animation and will not lengthen the duration of the animation. This means that, for example, if the user is scrolling rapidly, multiple pages could be considered part of one animation, causing some page animations to be animated very rapidly - looking like they're not animated at all. We use a custom animation to perform `smoothScrollTo` to improve the UX. This resolves a longstanding issue in non-Fabric RN, as well as Fabric, since this code is shared between the platforms. Changelog: [Update] Android ScrollView/HorizontalScrollView scrolls using custom animations instead of default Android `smoothScrollTo` implementation, leading to smoother scrolls for paginated ScrollViews Reviewed By: mdvacca Differential Revision: D21416520 fbshipit-source-id: 6ebe63cb054a98336b6e81253d35623fe5522f89
Reviewed By: kassens Differential Revision: D21281288 fbshipit-source-id: cf566ad0628dc179b3753f2f25a11637c33dee24
…amerate Summary: In iOS 11, [CADisplayLink](https://developer.apple.com/documentation/quartzcore/cadisplaylink)'s frameInterval was deprecated in favor of preferredFramesPerSecond, but these two properties have different underlying assumptions. - set frameInterval to 2 for 30fps - set preferredFramesPerSecond to 30 for 30fps. When you use preferredFramesPerSecond, assume frameInterval is 1. This fix ensures gifs in <Image> component will animate at same speed regardless of framerate. Reviewed By: shergin Differential Revision: D21414014 fbshipit-source-id: 40ab23bab1990cf65d2802830b6835f350999537
Summary: This diff fixes an off-by-one error probably caused by my font ligatures where when exactly two exceptions are thrown at the same time we would show the second exception instead of the first. If three or more were thrown, we would show the second. I also fixed some tests that had the wrong descriptions and wrong behavior enforced. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D21413186 fbshipit-source-id: 8e2940c89251dc042b10c6a2a2186089b6e7b53d
Summary: Based on feedback we're updating these titles to be more clear for their source. Changelog: [Internal] Reviewed By: cpojer Differential Revision: D21413486 fbshipit-source-id: c144e7f759a4ff263b7ec80fa643eeb8ffac741b
Summary: For some reason the specs were internal, but the native impl is still in github. So let's move these to github for consistency. Changelog: [Internal] Reviewed By: hramos Differential Revision: D21419934 fbshipit-source-id: f2c4486edca43c4348f3a3c6ce98f76a322bab0b
Summary: Changelog: [Internal] allow-large-files Reviewed By: samwgoldman, cpojer Differential Revision: D21413059 fbshipit-source-id: f3d111b40bfb88c182eab022925f7ae2dc47bc6b
Summary: Workaround for a bug with Fabric when pressing on virtual text. Changelog: [Internal] (Note: this ignores all push blocking failures!) Reviewed By: JoshuaGross Differential Revision: D21432793 fbshipit-source-id: fe20eeadd5365707fb71edae7a76d374e26b4c86
Summary: When a property returns an array type, use the actual element type when generating structs and inlines. Changelog: [Internal] Reviewed By: TheSavior Differential Revision: D21651351 fbshipit-source-id: 14cadf209c38a301c9c65fcaadd8a292c1936349
Summary: We cannot call `parentView.getChildCount()` directly, we must get the child count through the ViewManager: A simple `Log.e` call shows: ``` MountingManager: parentView.getChildCount(): 0 // viewGroupManager.getChildCount(parentView): 7 ``` This difference does not occur for ALL views, but it occurs for... enough that this will crash on basically ~every screen, at least on navigation when all views are removed. Theory about why this is happening: some ViewGroup types compute childCount differently, especially if we do some sort of custom child management for some view type? By coercing to `T` which the ViewManager does, we call getChildCount on the correct class type. This is just a hypothesis, though. But the failing views are all `View`s and it does look like `ReactClippingViewManager` has custom `getChildCount` logic, so that's likely the answer. There's no such thing as an easy diff! Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21750097 fbshipit-source-id: 3d87d8f629a0c12101658050e57e09242dfc2a8c
Summary: As a followup to D21750097 and D21735940, it seems that ANY uses of childCount will be incorrect as they are "often" reported, incorrectly, as just 0. This is likely due to exotic cases like (1) the ViewManager childCount being overridden, or (2) special ViewManagers like BottomSheet where the childCount may actually be 1/0 even though many children are inserted. Anyway, as a more generic fix, let's only rethrow an existing Exception with additional diagnostics instead of trying to detect when this /would/ crash. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21756178 fbshipit-source-id: 17ffb2ed531978bae8d4db19d7b87ec62397e44b
…archy" Summary: Original commit changeset: 5e607f34cf42 Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21757847 fbshipit-source-id: 96caf39f2ee99f64c96ebac9836ca178a7513cf1
Summary: This diff updates our RCTKeyCommands code to be more resilient by copying the [FLEX strategy for key commands](https://github.com/Flipboard/FLEX/blob/master/Classes/Utility/Keyboard/FLEXKeyboardShortcutManager.m). This strategy swizzles UIApplication handleKeyUIEvent which is further upstream than our UIResponder. It also allows for single key hotkeys like pressing just `r` instead of `cmd+r`. It does this without interfering with typing input by checking the first responder first. I've also updated our hotkey handling to support using just the keys like `r` in addition to `cmd+r`. In addition to brining these hotkeys more in line with other iOS tools, they're also easier to use and do not suffer the same issues hotkeys with modifiers like `cmd` have where keys are dropped. Changelog: [iOS] [Added] Allow hotkeys to be used without command key Reviewed By: shergin Differential Revision: D21635129 fbshipit-source-id: 36e0210a62b1f310473e152e8305165024cd338b
Summary: `getStackTracesTreeNodeForAlloc` only works for memory that was tracked with `newAlloc`. For now, that is only memory that goes through GC APIs. Various places were calling this function on native memory which was allocated with `malloc`, not GC memory. This led to SIGSEGV on nullptr when trying to take heap profiles of these objects. `newAlloc` could, in theory, work with native memory as well. But building out support for that is outside the scope of this fix. Changelog: [Internal] Reviewed By: jbower-fb Differential Revision: D21667842 fbshipit-source-id: 8403a6668e5ec607972ce6819f78fedb89da3f37
Summary: This diff add logging on Text Layout Manager. changelog: [Internal][Android] Internal changes to extend logging Reviewed By: JoshuaGross Differential Revision: D21737356 fbshipit-source-id: f34091722ff95f83ca41c9f17c49d31ce6618c9a
Reviewed By: PeteTheHeat Differential Revision: D21706223 fbshipit-source-id: 805628eed004e1469a4b1aa1e69fd2982e448b25
Summary: This diff extends the measurement of Text components in order to support empty strings. This is required for parity with Paper. I created a follow up task to analyze support of empty string as part of the Text infrastructure of Fabric in the future. changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D21761171 fbshipit-source-id: d2aa074052b09732af5d35723f19014090fcabbf
Summary: This diff reverts the workaround added as part of D21432793, now that D21432793 is released in production for FB4A / FBiOS changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D21772129 fbshipit-source-id: 3b39738cb8fc871ec85d87f347c7f0ef408ccc3e
Summary: Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D21703338 fbshipit-source-id: f33cf6dfdf8884ae76500ec55f44411e34e8cb76
Summary: After the experiment in D21198302 ships to 100% and that code is deleted, we no longer need ViewPagerAndroid anywhere in FB so it can be deleted. This is distinct from deleting the native side of the component, which resides in open-source. Changelog: [Internal] Differential Revision: D21211134 fbshipit-source-id: 610e09792b079d34acbfcec836fde58a0b3648a7
Summary: All ReactViewPager functionality should now be possible by using a ScrollView. Changelog: [Internal] ReactViewPager has been deprecated on Android and was removed from core as part of LeanCore Reviewed By: mdvacca Differential Revision: D21751774 fbshipit-source-id: 292475b9ffe7788b745329d13fd88dc3b613819e
…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
reviewed
Aug 13, 2021
Fix for Android Build Issue after RN64 Merge
HeyImChris
approved these changes
Aug 16, 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-05-28 23:59:59 PDT (specifically, f438a6e).
These changes were made by repeatedly running git merge and resolving merge conflicts as needed.
No major divergences were introduced, although one thing that I noticed was the introduction of a new method swizzling function in React/Base/RCTUtils.(h|m). The two previously existing functions return an
IMPcorresponding to the original method implementations in react-native-macos, while they returnvoidupstream. I wasn't 100% sure what would be the best way to extend this pattern here, but since we're not using the result anyway at the moment, it's probably not a big deal. If it comes down to it, we can always create a separate issue for this.Validation