Summary:
See title.
This feature makes LayoutAnimations less stable and isn't needed generally; will be deleted soon.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23382973
fbshipit-source-id: f633f482d463b3ee3e4625b30544a33cd6e36119
Summary:
My goal in this diff is to make LayoutAnimations more stable, and more resilient to challenging situations. Namely, LayoutAnimations already works fine with:
1) Reorders
2) Flattening/unflattening
3) Deletion and recreation of the same hierarchy
4) Updates conflicting with an existing animation
However, what if /all/ of those things are combined? Handling update conflicts with multiple ongoing animations, repeatedly flattening/unflattening the same layer of hierarchy and reordering both parents and children, etc .
This diff does not make LayoutAnimations perfect, but it does make LayoutAnimations much more resilient to situations it was not able to handle before.
My primary method of testing was to use two Playground examples: one just repeatedly queues up mutations (some animated, some not) that create, update, and delete in the same hierarchy layer. The second, more complex one, mutates between random view hierarchies that involve a lot of flattening and unflattening as well as reordering, over a depth of 5. It also exercises animations over TextInlineViews, which is more challenging.
LayoutAnimations works best with the new "Flattening Differ" for now, because the Flattening Differ produces a much smaller, nearly minimal set of instructions in cases of flattening-unflattening. I would like that to not be a hard requirement for using LayoutAnimations, but it's a good starting-point for now.
As part of this work, I also developed a lot of debugging and logging mechanisms that are handy for detecting inconsistencies and debugging crashes. Some are included in this diff behind `#define` statements that are disabled by default, and the rest will be published separately and likely cannot be landed permanently, as they're more invasive changes that are only helpful in debugging.
# Followups:
- Automate testing: write a suite of C++ tests that mutates between random diffs and guarantees that all mutations in a StubViewTree are sensible
- Construct a set of minimal repros that catalogues all remaining crashes and inconsistency issues (these seem to be extremely marginal cases and are very hard to repro - so I think it's fine to run this in prod for now, but I will follow-up as soon as I'm back to catalogue and fix all remaining issues)
- This diff focuses on *not crashing*, but it is still possible to construct a sequence of complex mutations that results in (for example) views having some opacity between 0 and 1 if animations are interrupted repeatedly. Although this is easy enough to prevent in product code - the types of scenarios I'm running in tests are very unlikely to ever happen in production - it would be nice to be *sure* that LayoutAnimations will always converge to a sensible View hierarchy with up-to-date props.
- In general, the index adjustment logic is complicated. I don't know if there's a great way around it, so I need to at least catalogue and test all edge-cases as mentioned above.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23382975
fbshipit-source-id: f379d9aa2a4b9c33fa2ba8fa07870c9e31fad5e7
Summary:
In some cases (BottomSheet?) the parent/ViewManager removes all children of the View before Fabric gets a chance to remove the children.
Apparently prior to D23368229 (https://github.com/facebook/react-native/commit/d344fb4e29a827d1e7d233672a3efe3b2b981a8a) (landed just today!) this sequence of operations happened and just noop'ed. Since we've been doing that happily as long as Fabric
has existed, we'll keep doing that for now.
I suspect that on *some* versions of Android this crashes and others it doesn't, based on logviews and my inability to repro certain crashes.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23387044
fbshipit-source-id: 88a46191adef4f6816bd7babd9103d103ddcef33
Summary:
Cleans up the documentation and types for `ImageSource`.
The only material changes here are:
- `ImageSource`'s array variant will now be `$ReadOnlyArray` instead of `Array`.
- `ImageURISource.headers` is now an object of string properties.
Changelog:
[General][Changed] - Refined `ImageSource` Flow type for array-variant and headers.
Reviewed By: kacieb
Differential Revision: D23355812
fbshipit-source-id: c3407db037dfb1d3514a028d1a237eb76ee6fedd
Summary:
We disabled animations on Android because native driver animations weren't working yet. After recent changes in NativeAnimatedModule, animations are now working as expected. This diff re-enables animations for bridgeless mode on Android.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22605612
fbshipit-source-id: 7a7078bf23bb88e7745fc899e1a5ff551425dbad
Summary:
If we try to delete a view and find the wrong one, when we crash, try to log the *actual* index of the view in question.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23368229
fbshipit-source-id: 7f9835fd07cfe4924d05c7e37b42b9bcdffff4a9
Summary:
Now we store a revision number of a Shadow Tree that leads to a transaction for which the concrete instance of MountingTelemetry corresponds. This is useful to understand how many actual transactions were skipped during a mounting phase (a mounting transaction does not directly correspond to a commit operation).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23364663
fbshipit-source-id: 32b86bcdfc1ae97d8fff3b97a8615cc5a5b4d4a9
Summary:
With this change, we now collect the number of text measurements that we perform during the layout phase of the commit. Text measurements are the most expensive layout operations which pretty much responsible for the vast majority of time spent in the layout phase.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: mdvacca
Differential Revision: D23364664
fbshipit-source-id: 19514b93166b4053c2f3be37e79507f2c5248000
Summary:
Previously this was crashing only in debug, but that's too noisy and isn't giving us any value for now.
Changelog: [Internal]
Differential Revision: D23338800
fbshipit-source-id: bf1535cdda231ccf30af6d00509eec1499a552a1
Summary:
Simplifying the StopSurface flow. Before we would still attempt to execute MountItems, but only the "Delete" operations. This was... well, frankly, overcomplicated. Instead we can just ignore all future MountInstructions for that Surface and delete all views recursively from the root.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23338752
fbshipit-source-id: 6e7ab29ad85572782bfc6a39845a8a619f001559
Summary:
Microsoft’s RN for macOS fork supports the Hermes engine nowadays https://github.com/microsoft/react-native-macos/pull/473. As a longer term work item, we’ve started moving bits that are not invasive for iOS but _are_ a maintenance burden on us—mostly when merging—upstream. Seeing as this one is a recent addition, it seemed like a good candidate to start with.
As to the actual changes, these include:
* Sharing Android’s Hermes executor with the objc side of the codebase.
* Adding a CocoaPods subspec to build the Hermes inspector source and its dependencies (`Folly/Futures`, `libevent`).
* Adding the bits to the Xcode build phase script that creates the JS bundle for release builds to compile Hermes bytecode and source-maps…
* …coincidentally it turns out that the Xcode build phase script did _not_ by default output source-maps for iOS, which is now fixed too.
All of the Hermes bits are automatically enabled, on macOS, when providing the `hermes-engine-darwin` [npm package](https://www.npmjs.com/package/hermes-engine-darwin) and enabling the Hermes pods.
## Changelog
[General] [Added] - Upstream RN macOS Hermes integration bits
Pull Request resolved: https://github.com/facebook/react-native/pull/29748
Test Plan:
Building RNTester for iOS and Android still works as before.
To test the actual changes themselves, you’ll have to use the macOS target in RNTester in the macOS fork, or create a new application from `master`:
<img width="812" alt="Screenshot 2020-08-18 at 16 55 06" src="https://user-images.githubusercontent.com/2320/90547606-160f6480-e18c-11ea-9a98-edbbaa755800.png">
Reviewed By: TheSavior
Differential Revision: D23304618
Pulled By: fkgozali
fbshipit-source-id: 4ef0e0f60d909f3c59f9cfc87c667189df656a3b
Summary:
Changelog: [Internal]
# Problem
`RCTModalHostViewComponentView` reuses the same UIViewController between recycles. This can be a problem if dismissal of the modal is animated as the UIViewController is still presented while it is being dismissed. That's when "Application tried to present modally an active controller" exception can happen.
# Solution
Deallocate UIViewController when `RCTModalHostViewComponentView` is recycled.
Reviewed By: PeteTheHeat
Differential Revision: D23345711
fbshipit-source-id: da540571184afcb88b52758c4a1f0b4ec5874eb1
Summary:
I noticed when porting my iOS app to macOS via Catalyst that the text rendering was somewhat different on the two platforms. Text looked blurry and over-weight on macOS, even when disabling the Catalyst scaling transform.
I hazily remembered that I'd seen this problem before in my old Cocoa development days: this kind of blurring occurs when rendering text with sub-pixel anti-aliasing into an offscreen buffer which will then be traditionally composited, because when the SPAA algorithm attempts to blend with the underlying content (i.e. in the offscreen buffer), there isn't any. SPAA is disabled on iOS, so the issue wouldn't appear there. On macOS, typical approachs to displaying text (e.g. `CATextLayer`) normally disable SPAA, since it's been incompatible with the platform's compositing strategy since the transition to layer-backed views some years ago. But React Native uses `NSLayoutManager` to rasterize text (rather than relying on the system render server via `CATextLayer`), and that class doesn't touch the context's font smoothing bit before drawing.
This change makes macOS/Catalyst text rendering consistent with iOS text rendering by disabling SPAA.
It appears that the code I've modified is in the process of being refactored (for Fabric?). It looks like [this](https://github.com/facebook/react-native/blob/8d6b41e9bcede07fb627d57cf6c11050ae590d57/ReactCommon/react/renderer/textlayoutmanager/platform/ios/RCTTextLayoutManager.mm#L111) is the corresponding place in the new code (sammy-SC, is that right?). I'm happy to include a change to the new renderer in this patch if someone can point me at how to test that change.
## Changelog
[iOS] [Fixed] - Improved text rendering on macOS Catalyst
Pull Request resolved: https://github.com/facebook/react-native/pull/29609
Test Plan:
1. Prepare RNTester for running on macOS (or apply [this patch](https://gist.github.com/andymatuschak/d0f5b4fc1a28efc4f860801aa1deddcd) to handle parts 1 and 2, but you'll still need to do part 3):
1. Open the workspace, navigate to the `RNTester` target's configuration, and check the "Mac" checkbox under "Deployment Info.
2. Flipper doesn't yet compile for Catalyst (https://github.com/facebook/react-native/issues/27845), so you must disable it by: a) commenting out `use_flipper!` and `flipper_post_install` in the Podfile, then running `pod install`; and b) removing the `FB_SONARKIT_ENABLED` preprocessor flags in the Xcode project.
3. macOS has different signing rules from iOS; you must set a development team in the "Signing & Capabilities" tab of the `RNTester` target configuration pane. Unfortunately, you must also do this in the `Pods` project for the `React-Core-AccessibilityResources` target ([this is an issue which CocoaPods must fix](https://github.com/CocoaPods/CocoaPods/issues/8891)).
2. Run RNTester with and without the patch. You'll see that the font hinting is overweight without the patch; see screenshots below (incorrect rendering above, correct rendering below; note that fonts still remain slightly blurred because of Catalyst's window scaling transform, but that's removed on Big Sur).
Reviewed By: PeteTheHeat
Differential Revision: D23344751
Pulled By: sammy-SC
fbshipit-source-id: 1bbf682b681e381a8a90e152245d9b0df8ec7697
Summary:
For some reason, these got out of sync again.
This diff modifies the Java Codegen to sort all the methods.
build-break
overriding_review_checks_triggers_an_audit_and_retroactive_review
Differential Revision:
D23363410
Oncall Short Name: fbandroid_sheriff
Ninja: master broken
fbshipit-source-id: 257d85f92017528e64ced31bc7be011acb333186
Summary:
Changelog: [internal]
According to React Native docs, the default value for `autoCapitalize` is `sentences`.
Fabric's TextInput default value does not align with this.
[Source](https://reactnative.dev/docs/textinput#autocapitalize)
Reviewed By: JoshuaGross
Differential Revision: D23344479
fbshipit-source-id: f9e6f2aa6e1fbba2b08cb4aff23b842e49fa8c21
Summary:
This diff deletes the activityindicator buck module because these files are created by the code gen.
changelog: [internal]
Reviewed By: PeteTheHeat
Differential Revision: D23227860
fbshipit-source-id: 133315a44c9181be3263e0c6227884ed15487c1f
Summary:
This feature has been in [experimental mode since 0.59.0](https://reactnative.dev/blog/2019/03/12/releasing-react-native-059#-faster-app-launches-with-inline-requires). The feature seems quite beneficial.
Most people won't know to look for the inline requires flag, and it's been existing in React Native as an experiment for months. This defaults the flag to true as the next step towards utilizing this optimization for React Native devs initiating brand new projects.
## 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] [Breaking] - Enable `inlineRequires` by default. Gives a performance benefit but slightly different JS execution order
Pull Request resolved: https://github.com/facebook/react-native/pull/29770
Reviewed By: cpojer
Differential Revision: D23338113
Pulled By: TheSavior
fbshipit-source-id: 1b8ff41418c81a324335782d4b4109d89594ccbf
Summary:
The change in the hermes repository fixes the security vulnerability
CVE-2020-1911. This vulnerability only affects applications which
allow evaluation of uncontrolled, untrusted JavaScript code not
shipped with the app, so React Native apps will generally not be affected.
This revision includes a test for the bug. The test is generic JSI
code, so it is included in the hermes and react-native repositories.
Changelog: [Internal]
Reviewed By: tmikov
Differential Revision: D23322992 (https://github.com/facebook/react-native/commit/0dee0e60366645c962848a73bbaf0fc7c49912d0)
fbshipit-source-id: 4e88c974afe1ad33a263f9cac03e9dc98d33649a
Summary:
Changelog: [Internal]
# Problem
The problem was setting `_backedTextInputView.attributedText` to nil inside `[RCTTextInputComponentView prepareForRecycle]`.
Ordinarily this isn't a problem becase `UIManager::updateState` drops the update if the ShadowNode no longer exists. But in certain cases the ShadowNode can exist, empty string being set as its value
# Fix
Fix is trivial, invalidate state before nullifying `_backedTextInputView`. This prevents the state update from being dispatched.
# Discussion
We should go over all other components and make sure state is invalidated as first thing in `[RCTViewComponentView prepareForRecycle]`.
Reviewed By: shergin
Differential Revision: D23324929
fbshipit-source-id: 9568e920d99683ad95f965ef4b63c529f50f3283
Summary:
Deletes `Animated/polyfills` which is no longer necessary (and only creates extra configuration burden).
Changelog:
[Internal]
Reviewed By: cpojer
Differential Revision: D22451963
fbshipit-source-id: 7a9a48b96b3783f2f6340226bdafd2eaa43f32e7
Summary:
Removes the legacy `react-animated` package configuration and collapses the `Animated/src/` directory into `Animated/`.
Also, reconfigures all references to `Animated/src/` to just be `Animated/`.
Changelog:
[Internal]
Reviewed By: cpojer
Differential Revision: D22450848
fbshipit-source-id: 9fd4861e9f357d817d82e9fec71967a2936a3830
Summary:
Replaces `fbjs/warning` call sites in React Native with `console.warn`. A few warnings will now log as warnings without the "Warning:" prefix.
Changelog:
[General][Changed] - Some warnings changed to use `console.warn` without the "Warning:" prefix.
Reviewed By: TheSavior, cpojer
Differential Revision: D22445946
fbshipit-source-id: 96b01e1bdee52b89ff3b808bc9d6cd494f6787f5
Summary:
This implements the full NativeModuleResolvedType for each NativeModule spec, producing .java spec files in the output directory. The output files are now compiled during build time for :ReactAndroid and :packages:rn-tester:android:app.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23312858
fbshipit-source-id: c521b9d6602677fd275891cf44329740c6bd7387
Summary:
For each resolved type, define how to process its own type and whether it should produce generated code/file, minus the NativeModuleResolvedType (will be in the next commit).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23307961
fbshipit-source-id: 87af09867aace7a5ff060a33a00b141048630eda
Summary:
This is the base setup for ResolvedType. Each Type needs to be resolved to its final representation, which knows what Java type it needs, and what standalone Java code (e.g. class) it needs to produce. Individual resolved types are not yet implemented, this commit provides the flow to pass the parsed structure (from schema) to the resolution logic.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23288208
fbshipit-source-id: 18535d5fae8ca15f6d9374bdba38f57dfd4300e8
Summary:
Instead of recreating the schema structure, the generator only needs to collect the list of types that it needs to generate for. So instead, let's just add each parsed type into a map using TypeId as the key. This means every inner types in the schema needs its own unique TypeId. This was a change from the previous commit where we didn't assign unique names to the types. Here's the reasoning:
* In Java, any generated class needs to be in its own file.
* If a NativeModule spec defines a few aliases, and or inner types (function args, return type shape, etc) that needs representation with a dedicated class, we need to track them as well for code generation.
* This means, the schema format is no longer relevant for the code generation step, so let's produce a structure that's more efficient for code generation
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23287818
fbshipit-source-id: 7caf4e95aeafe5c8ba336af290179b85bf87ad6d
Summary:
Changelog: [Internal]
This was set to screensize in D13104172 (https://github.com/facebook/react-native/commit/346c9d5f2c0fd99d6f05828d594d05bfaeed902f) to work around issues at that time.
The issues might not be around anymore.
Reviewed By: PeteTheHeat
Differential Revision: D23298723
fbshipit-source-id: 4f33a2cc65ae4fe5ba20b0b2b270b135878c339f
Summary:
I'm removing an ill-informed "optimization" that resulted in some StateUpdates being dropped. For some components (including TextInput) we rely on State updates to request a layout from the ShadowNode layer.
In the past we were performing an optimization that didn't update the View layer if the data contained in the State container is identical, but in the case of TextInput and other components, we simply pass an opaque
object with no meaningful data to trigger the layouts. In those cases, it could cause a permanent rift between the View layer's StateWrapper and the most recent state object from the C++ perspective.
In the case of TextInput this didn't cause tangible bugs because you can always update state using an out-of-date State object, but it's better this way anyway.
The other issue is that for some components, we want to know when there's a State update from the Cxx layer. This optimization broke certain logic in those components.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23306222
fbshipit-source-id: 8ef83149b814de50776674b5fd22406be1db14ba
Summary:
This crash was introduced in D22962320 (https://github.com/facebook/react-native/commit/ffdfbbec08b1afb1970d2cfe75a78a203d4a79a4), which landed 8/17 and went into 8/23 cut. I plan to pick this diff into that release.
I can't repro this, but there must be a scenario (in bridged mode), where we try to flush event queue while bridge is tearing down. I plan to consider this scenario going forward.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D23306445
fbshipit-source-id: 285d4b94a17423c3b08d83e7041c4ee04b7e6d0c
Summary:
This diff ties the stack together. It completes the long chain of RCTSurfacePresenter ownership:
`FBReactModule` -> `RCTNativeAnimatedTurboModule` (this diff and D23272746)
`RCTNativeAnimatedTurboModule` -> `RCTNativeAnimatedNodesManager` (this diff)
`RCTNativeAnimatedNodesManager` -> `RCTPropsAnimatedNode` (D23272735)
It completes animations working without the bridge.
Changelog: [Internal]
Differential Revision: D23272755
fbshipit-source-id: 137f7ff89993a2cb644bd67869eb685afcec4068
Summary:
`RCTPropsAnimatedNode` uses the bridge to access it's current surface presenter to perform animations.
In bridgeless mode, the surface presenter is not owned by the bridge. Instead, pass the surface presenter through the ownership chain:
`RCTNativeAnimated*Module` -> `RCTNativeAnimatedNodesManager` -> `RCTPropsAnimatedNode`
`RCTSurfacePresenter` should not be strongly held by any of these animation internals. If it gets destroyed at a higher level, animations should not be completed.
Changelog: [Internal]
Differential Revision: D23272735
fbshipit-source-id: ce08ee3b59ac2ba70e31cebb7ba8e9f3a644c848
Summary:
The animated native module relies on `setBridge` to perform generic setup which doesn't rely on the bridge at all. This diff refactors that setup code to an `init` function.
Changelog: [Internal]
Differential Revision: D23272427
fbshipit-source-id: 0c9c5522c9044283f4db25360010465ff42aba25
Joshua Gross
Tim Yung
Frieder Bluemle
Agastya Darma
Emily Janzer
Valentin Shergin
Eloy Durán
Luna Wei
Samuel Susla
Andy Matuschak
Ramanpreet Nara
David Vacca
Gant Laborde
Marc Horowitz
Betty Ni
Kevin Gozali
Jordan Brown
generatedunixname89002005287564@sandcastle995.atn2.facebook.com
Stephy Ma
Peter Argany
