Merge react-native 0.61-stable #323
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 diff implements the JSResponderHandler methods in the core of RN (scheduler API and friends) Reviewed By: ejanzer Differential Revision: D16543437 fbshipit-source-id: dac03e30c4330d182ecf134f3174ba942dbf7289
Summary: This diff implements the JSResponderHandler in Fabric Android code Reviewed By: JoshuaGross Differential Revision: D16543438 fbshipit-source-id: 13680f77a5368e8ba1180383a5f9fb7d7330b90a
Summary: This diff implements the JNI code required for Android to receive JSResponderHandler calls Reviewed By: JoshuaGross, makovkastar Differential Revision: D16543431 fbshipit-source-id: 38cff16a05633fccefa201b189d761d503a9b839
Summary: `codegenNativeCommands` returns an object with functions for each command that has the previous behavior inside the React Renderer, and the new Fabric logic inside of the Fabric React Native Renderer. Changelog: [Internal] - Change AndroidDrawerLayoutNativeComponent to use JS codegen for commands Reviewed By: rickhanlonii Differential Revision: D16529887 fbshipit-source-id: 24a5307944a7f62e18482d60d26052fea3be2051
Summary: This uses a new helper called `dispatchCommand` that now exists on the renderer. This was added to the renderer here: facebook/react#16085 In Paper it calls UIManager.dispatchViewManagerCommand and in Fabric it calls the c++ Fabric UIManager Reviewed By: rickhanlonii Differential Revision: D16578708 fbshipit-source-id: 30f9468a7fd48afb506c0ee49a460b949bc863a1
Summary: ## The Problem 1. `CatalystInstanceImpl` indirectly holds on to the `jsi::Runtime`. When you destroy `CatalystInstanceImpl`, you destroy the `jsi::Runtime`. As a part of reloading React Native, we destroy and re-create `CatalystInstanceImpl`, which destroys and re-creates the `jsi::Runtime`. 2. When JS passes in a callback to a TurboModule method, we take that callback (a `jsi::Function`) and wrap it in a Java `Callback` (implemented by `JCxxCallbackImpl`). This Java `Callback`, when executed, schedules the `jsi::Function` to be invoked on a Java thread at a later point in time. **Note:** The Java NativeModule can hold on to the Java `Callback` (and, by transitivity, the `jsi::Function`) for potentially forever. 3. It is a requirement of `jsi::Runtime` that all objects associated with the Runtime (ex: `jsi::Function`) must be destroyed before the Runtime itself is destroyed. See: https://fburl.com/m3mqk6wt ### jsi.h ``` /// .................................................... In addition, to /// make shutdown safe, destruction of objects associated with the Runtime /// must be destroyed before the Runtime is destroyed, or from the /// destructor of a managed HostObject or HostFunction. Informally, this /// means that the main source of unsafe behavior is to hold a jsi object /// in a non-Runtime-managed object, and not clean it up before the Runtime /// is shut down. If your lifecycle is such that avoiding this is hard, /// you will probably need to do use your own locks. class Runtime { public: virtual ~Runtime(); ``` Therefore, when you delete `CatalystInstanceImpl`, you could end up with a situation where the `jsi::Runtime` is destroyed before all `jsi::Function`s are destroyed. In dev, this leads the program to crash when you reload the app after having used a TurboModule method that uses callbacks. ## The Solution If the only reference to a `HostObject` or a `HostFunction` is in the JS Heap, then the `HostObject` and `HostFunction` destructors can destroy JSI objects. The TurboModule cache is the only thing, aside from the JS Heap, that holds a reference to all C++ TurboModules. But that cache (and the entire native side of `TurboModuleManager`) is destroyed when we call `mHybridData.resetNative()` in `TurboModuleManager.onCatalystInstanceDestroy()` in D16552730. (I verified this by commenting out `mHybridData.resetNative()` and placing a breakpoint in the destructor of `JavaTurboModule`). So, when we're cleaning up `TurboModuleManager`, the only reference to a Java TurboModule is the JS Heap. Therefore, it's safe and correct for us to destroy all `jsi::Function`s created by the Java TurboModule in `~JavaTurboModule`. So, in this diff, I keep a set of all `CallbackWrappers`, and explicitly call `destroy()` on them in the `JavaTurboModule` destructor. Note that since `~JavaTurboModule` accesses `callbackWrappers_`, it must be executed on the JS Thread, since `createJavaCallbackFromJSIFunction` also accesses `callbackWrappers_` on the JS Thread. For additional safety, I also eagerly destroyed the `jsi::Function` after it's been invoked once. I'm not yet sure if we only want JS callbacks to only ever be invoked once. So, I've created a Task to document this work: T48128233. Reviewed By: mdvacca Differential Revision: D16623340 fbshipit-source-id: 3a4c3efc70b9b3c8d329f19fdf4b4423c489695b
Summary: Added missing property to whitelist ## Changelog [General] [Fixed] - Fixed rotateZ native animation Pull Request resolved: facebook#25938 Differential Revision: D16645798 Pulled By: cpojer fbshipit-source-id: ef74d7230fa80068dcceaaff841af27365df92e9
…turn Promise" Summary: Original commit changeset: 420d29d262b6 Reverts facebook#25793 / D16515465 Union type property is not supported by codegen. We don't want to support unions yet and because the improvement is not that big and not yet published as stable for OSS (neither used anywhere internally) we can safely revert it. Reviewed By: RSNara Differential Revision: D16621228 fbshipit-source-id: 2fa416eef1ae353990860026ca97d2b0b429a852
Summary: It's actually the first module in OSS which is typed with taking advantages of codegen. Reviewed By: RSNara Differential Revision: D16620334 fbshipit-source-id: 65d6656506f2a4c68d493939ecfa65ba975abead
Summary: This PR fixes facebook#19637. Summary of the issue: on Android, transformed touchables have press issues because the touchable's measurements are incorrect in the release phase. `UIManager.measure()` returns an incorrect size and position as explained [here](facebook#19637 (comment)) This is easily seen with the inspector : **Screenshot of a { scale: 2 } transform** The real view (scaled) is in pink, the incorrect touchable area is in blue. <img src="https://user-images.githubusercontent.com/110431/41190133-8d07ad02-6bd9-11e8-873d-93776a007309.png" width="200"/> **Screenshot of a { rotateZ: "-45deg" } transform** The real view (rotated) is in pink, the incorrect touchable area is in blue. <img src="https://user-images.githubusercontent.com/110431/41190136-a1a079a6-6bd9-11e8-906d-729015bcab6b.png" width="200"/> ## Changelog [Android] [Fixed] - Fix UIManager.measure() Pull Request resolved: facebook#25836 Test Plan: Android now produces the same results as iOS as seen on these screenshots | Android without fix | Android with fix | iOS | | --- | --- | --- | |  |  |  | Reviewed By: cpojer Differential Revision: D16598914 Pulled By: makovkastar fbshipit-source-id: d56b008b717ea17731fb09001cbd395aa1b044fe
Summary: Fragment was assigned incorrect `tag` and `surfaceID` (`surfaceID` is the important one). Wrong `surfaceID` means that `navigationCoordinator` is never resolved. As a result of navigationCoordinator not being assigned, tapping a video ad on Marketplace results in showing video ad overlay rather than showing full screen video. Reviewed By: JoshuaGross Differential Revision: D16646492 fbshipit-source-id: 0da5c56ecb7c81e9f4a9469a3626ccd430a01558
Reviewed By: cpojer Differential Revision: D16648992 fbshipit-source-id: 4581e2cd6859f27bc384fc3ab328ab5b9414c704
Summary: This diff has three changes: 1. Remove all references to `Stringish` from `NativeDialogManagerAndroid`. (All Fbt objects expose a `.toString` method we could call). 2. Make sure that we only access `DialogManagerAndroid` through `NativeDialogManagerAndroid`. 3. Removed a bunch of `$FlowFixMes` in the files I touched. Probably not the best idea to bite into this cleanup on this diff, but what's done is done. Since this diff is fairly large, I've commented on parts of it I thought were note-worthy. I've also commented on the changes I had to make to fix flow after removing the `$FlowFixMe`s. Reviewed By: PeteTheHeat Differential Revision: D16428855 fbshipit-source-id: 0e6daf2957f4b086ebb1e78e0a59930668c65576
Summary: Hermes has been updated to [v0.1.1](https://github.com/facebook/hermes/releases/tag/v0.1.1) and [renamed from 'hermesvm' to 'hermes-engine'](facebook/hermes@c74842e) ## Changelog [Android] [Changed] - Bump hermes to v0.1.1 Pull Request resolved: facebook#25908 Test Plan: RNTester builds and runs as expected Differential Revision: D16645811 Pulled By: cpojer fbshipit-source-id: 4fb6a3160df2c6d08140dd1fee51acf9ff8baffc
Summary: Some surfaces throw ConcurrentModificationException when logging detailed perf for Fabric. I've refactored the ReactMarker class to use a threadsafe ArrayList and removed synchronization, which is safer and should improve perf everywhere the markers are used, even if there are zero listeners. Reviewed By: mdvacca Differential Revision: D16656139 fbshipit-source-id: 34572f9ad19028a273e0837b0b895c5e8a47976a
Summary: For some reason the conversion from a BOOL object to `bool` (C++ style) may lead to incorrect boolean value. This fixes the value provided to the builder to be of `bool` type instead. Reviewed By: JoshuaGross Differential Revision: D16657766 fbshipit-source-id: b66922aceadd20d16226a07f73b24ee0a3b825dc
Summary:
ErrorUtils is giving an error by eslint. ('ErrorUtils is not defined').
## Changelog
[General] [Fixed] - Add ErrorUtils to eslint globals
Pull Request resolved: facebook#25947
Test Plan: Run eslint on a react native project using ErrorUtils. Eslint verification should pass.
Differential Revision: D16666163
Pulled By: cpojer
fbshipit-source-id: c20c4e21fe06c6863dcfc167d6d03c6217ae1235
Summary: use "shorthand" of `Fragment` No need to import `Fragment` as it can be used via `<></>` vs `<Fragment><Fragment />` ## Changelog Use shorthand for Fragment in App.js [General] [Changed] - Use shorthand for Fragment in App.js Pull Request resolved: facebook#25905 Test Plan: Ci Tests should be sufficient Differential Revision: D16666166 Pulled By: cpojer fbshipit-source-id: 70e2c9793087bf8f5e0a5477c75f178134cbd6a1
Summary: This diff fixes the error message in the codegenNativeComponent fallback Reviewed By: TheSavior Differential Revision: D16579775 fbshipit-source-id: 176f81ea91e11f671407a5e5e5b000c4b83f93b2
Summary: I added this check [a couple of years ago](facebook@1b22d49) to mimic how [React used to check for async roots](https://github.com/facebook/react/blob/acabf112454e5545205da013266d8529599a2a82/packages/react-reconciler/src/ReactFiberReconciler.js#L321-L330). This code doesn't make sense anymore since there's neither an async base class or an `unstable_ConcurrentMode` export, so I'm just cleaning it up. Reviewed By: threepointone, sebmarkbage Differential Revision: D16668567 fbshipit-source-id: 5ccf5feccc4b65ffb3aeb0a09891d8be7490df26
Summary: Adds tests for codegenNativeComponent Reviewed By: TheSavior Differential Revision: D16582627 fbshipit-source-id: 3527126c7838f3e2c0c56b19956c618f0a7fb9f9
Summary: Updated the message from > Loading from pre-bundled file to > Connect to Metro to develop JavaScript I also added a new RCT_PACKAGER_NAME so other packagers can override "Metro" Reviewed By: yungsters, cpojer Differential Revision: D16427501 fbshipit-source-id: 1b7f9e261f7521ba930c6248087fe6f3c3659cb7
Summary: The types-first codemod adds a few of these, so need to sync the suppressions here with the ones in xplat/js/.flowconfig Reviewed By: jbrown215 Differential Revision: D16690168 fbshipit-source-id: 49d3f80a4ab24badf11a9ac54abfe49670989a91
Summary: @public Our usage of C++ bit fields has lead to quite some problems with different compiler setups. Problems include sign bits, alignment, etc. Here we introduce a portable implementation as a variadic template, allowing the user to store a number of booleans and enums (defined with `YG_ENUM_SEQ_DECL`) in an unsigned integer type of their choice. This will replace all usages of bit fields across the Yoga code base. Differential Revision: D16647801 fbshipit-source-id: 230ffab500885a3ad662ea8f19e35a5e9357a563
Summary: @public Removes the style properties bitmask. We have used this for experimentation, and it's no longer necessary. This simplifyies the code, and allows us to cut over to `Bitfield.h` more easily. Reviewed By: astreet Differential Revision: D16648862 fbshipit-source-id: 17c0899807af976f4ba34db54f8f0f6a3cd92519
Summary: @public Replaces the usage of C++ bitfields with our portable `Bitfield` class. Reviewed By: SidharthGuglani Differential Revision: D16649875 fbshipit-source-id: 539f016d5e1c9a8c48cc9bacbbf6ed985e385e69
Summary: Replaces the usage of C++ bitfields with our portable `Bitfield` class. Reviewed By: SidharthGuglani Differential Revision: D16656361 fbshipit-source-id: 05f679e2e994e109b2bd1090c879d6850fabdc40
Summary: Removes the double measure callbacks experiment Original commit changesets: c6cf9c01a173, b157d8137c72 Reviewed By: SidharthGuglani Differential Revision: D16687367 fbshipit-source-id: 9649f8731bd1b27f4d291cee4fa30153165cea02
Summary: No need for a copy here. Pull Request resolved: facebook/yoga#919 Differential Revision: D16701461 Pulled By: davidaurelio fbshipit-source-id: 3a90adbb2b5c43d5aefe693a8525aa3a37e53b3d
Summary: Changing showSoftKeyboard and hideSoftKeyboard to be protected, as we need this change for an internal control that extends ReactEditText. ## Changelog [Android] [Changed] - part of our react native platform, we have a control that extends ReactEditText and we need to be able to override these 2 methods. Pull Request resolved: facebook#25964 Test Plan: The change has been in Microsoft's branch of RN for almost 2 years, and since it's a relatively small change we've done a quick sanity check in RNTester prior to this PR, making sure the TextInput page loads fine and it's functional. Differential Revision: D16686878 Pulled By: cpojer fbshipit-source-id: 63035ee9c58e93bc0fa40e5bec318df05322c6c5
anandrajeswaran
approved these changes
Apr 26, 2020
…0.61.5 .. This builds a clean base upon which all the patches will be rebased ..
…t step configuration files (2) build param to support excluding all SOs from library archives
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
This change merges from upstream react-native branch 0.61-stable into this fork.
The Azure DevOps yml scripts have been updated to build and test using CocaoPods.
Focus areas to test
The full suite of tests pass for all platforms. Manually testing of ios and macos RNTester.
Microsoft Reviewers: Open in CodeFlow