Summary:
This builds on previous diff to properly parse `<Picker.Item/>` into a cpp struct.
This fixes the dummy text and text color TODOs.
Changelog: [iOS][Fabric] Fabric Picker support
Reviewed By: sammy-SC
Differential Revision: D23964557
fbshipit-source-id: f42c6c9cf410bfc5e66ff078645b6378548481de
Summary:
This is a starting point for the handwritten Fabric Picker component. It is incomplete, and needs to be landed with the rest of the stack above it.
In general, this creates a new `ComponentView`, `ComponentDescriptor`, `ShadowNode`, `Props` and a few other boilerplate classes for Picker. A bunch of the logic in `ComponentView` was copied over from the Paper `RCTPicker` and `RCTPickerManager`.
What works in this diff:
- A `<Picker>` with items can be created in JS, and a corresponding `UIPicker` is created in native with placeholder text, default styling and the correct amount of items
What doesn't work yet (implemented in later diffs):
- Parsing items to use correct text and styling in native
- Events/commands
Reviewed By: sammy-SC
Differential Revision: D23941821
fbshipit-source-id: e049ca6004757fbd1361985644d5dbb8f53e1ce6
Summary:
When switching between non-Fabric and Fabric screens, I believe that `initializeEventListenerForUIManagerType` is not always being called on the NativeAnimatedNodesManager if
`NativeAnimatedModule.initializeLifecycleEventListenersForViewTag` is being called before the NativeAnimatedNodesManager ivar has been set. This should occur very infrequently, but logs
indicate that it /does/ happen in some marginal cases. Protecting against these cases should be trivial, by using the `getNodesManager` method which is responsible for returning a nodes manager or creating a new one.
The existing uses of the `NativeAnimatedNodesManager` ivar also occur on different threads and we were not protecting against this, so I'm changing it to an atomic. It's very likely that
the inconsistency issues in the past were caused not by ordering errors, but thread races.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24267118
fbshipit-source-id: 68abbff7ef3d0b2ecc9aa9977165663ad9447ab8
Summary:
RNTester has some imports left over that operate on directory traversal, assuming it is contained within react-native sources. This change makes these imports relative to react-native, enabling usage outside of the RN repo.
Relates to https://github.com/microsoft/react-native-windows/issues/6210
## Changelog
[Internal] [Fixed] - Avoid File Structure Assumptions in RNTester Imports
Pull Request resolved: https://github.com/facebook/react-native/pull/30141
Test Plan: Validated we can bundle and flow-check both iOS + Android
Reviewed By: cpojer
Differential Revision: D24259628
Pulled By: appden
fbshipit-source-id: 0c21b5d354b01785d8402599da3b0a5be81b4c6d
Summary:
shergin added a new assert to dimensions calculations in D24038911 (e853722981). This crashes in bridgeless mode b/c no bridge.
This diff uses `turboModuleRegistry` as a bridge workaround. The registry is a [weak property](https://www.fburl.com/diffusion/sunv3bx9) so retain cycles shouldn't be an issue here, let me know if that's incorrect.
As a nice bonus, this fixes dynamic font sizing in bridgeless mode. It was broken before since it relied on the bridge.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24256338
fbshipit-source-id: 141d36239ac6a6e9e87ca96eea7aeec56729095d
Summary:
Changelog: [Internal]
If `uiTouch.view` isn't managed by Fabric, go up the view hierarchy to find first view that is.
This is important for host views wrapped by <Touchable>, otherwise touches are not delivered to the component.
Reviewed By: yungsters
Differential Revision: D24219223
fbshipit-source-id: 17b4e3460735371553ee0d30b41776a977f8eafb
Summary:
This PR attempts to fix issue https://github.com/facebook/react-native/issues/28278 and https://github.com/facebook/react-native/issues/29525
On Crashlytics, the following error occurs in file `RCTWebSocketModule.m` at method `-[RCTWebSocketModule webSocket:didFailWithError:]` when a nil value is inserted into a dictionary as a value.
```
Fatal Exception: NSInvalidArgumentException
*** -[__NSPlaceholderDictionary initWithObjects:forKeys:count:]: attempt to insert nil object from objects[0]
```
This PR is following the suggestion of this: https://github.com/facebook/react-native/issues/28278#issuecomment-597461650 and it replaces the values of any property if it is nil. In detail:
- it converts `error` to empty NSString if the original value is nil
- it converts `socketID` to a NSNumber object, which stores `-1` if the original value is nil
## Changelog
[iOS] [Fixed] - A crash in WebSocket module
Pull Request resolved: https://github.com/facebook/react-native/pull/30066
Test Plan: We were not able to reproduce the crash, but the report itself provided enough information to find a solution for this issue.
Reviewed By: shergin
Differential Revision: D24241147
Pulled By: sammy-SC
fbshipit-source-id: d5d632b49ca77b5d8be8b9c32358bef68f17d30a
Summary:
This makes a few change to the template xcodeproj
- Update to recommended settings for Xcode 12
- Remove the main.jsbundle file that doesn't exists, this file is included automatically in the app bundle and doesn't need to be in xcode (it won't even be at that path).
- Add -e to the bundle images script. It seems like Xcode doesn't fail the build on script errors anymore so this makes it so the build stop. If it doesn't stop the app will launch and crash because the bundle doesn't exist (note this is only if running the app in release mode since in debug we don't generate the bundle).
## Changelog
[iOS] [Added] - Update template to xcode 12
Pull Request resolved: https://github.com/facebook/react-native/pull/30150
Test Plan: Made the same changes in an app and made sure it ran well.
Reviewed By: cpojer
Differential Revision: D24237481
Pulled By: sammy-SC
fbshipit-source-id: 2b06a24c510c423eb45a1a840ea365b64506321b
Summary:
When taking a heap timeline, Hermes wasn't showing any data until the timeline
was written to disk and then reloaded.
Turns out we were missing support for two events:
* `HeapProfiler.lastSeenObjectId`: This event reports the most recently
allocated object ID. Used to know when objects were allocated in the
timeline.
* `HeapProfiler.heapStatsUpdate`: Report how many objects and bytes
exist for a "time fragment", represented by a fragment index. Later updates
to the same index can decrease the amount of live memory, which show up
as grey spikes instead of blue spikes
Previously, we only supported these by writing out to a file, and they didn't work
with a "live" profiling view. To fix this, I changed the periodic sampling thread
to instead be a periodic flush of a sample every few allocations. The performance
impact is tucked away only when profiling is turned on, and it's very non-invasive to
the rest of the GC. The flush calls a callback with the relevant information if the
inspector is on, and the inspector sends a message back to the browser.
Changelog: [Internal] Fix for Hermes heap timeline profiling
Reviewed By: neildhar
Differential Revision: D23993363
fbshipit-source-id: 8e0b571130cbb7e839dfb009b04f584f5179085d
Summary:
## Previously
- When TurboModules system was on, we'd only log events from the TurboModules system.
- When TurboModules system was off, we'd only log events from the NativeModule system.
This ultimately gives us less data to analyze both systems in production.
## Changes in This Diff
When perf. logging is on, we'll log events from both systems. Each QPL event now include an annotation of which system the event is coming from. Concretely, this will allow us to see how much of the NativeModule system is being exercised in the TurboModule test group.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D24232594
fbshipit-source-id: 7dff57bd74fc7ef744d3e06ff174304f25790456
Summary:
Index adjustment is tricky. Seems more reliable to adjust each immediate mutation, and then immediately adjust delayed mutations based on it, rinse and repeat.
Previously it was possible to construct examples where the UI would get into a weird state because index adjustment caused items to be inserted in the wrong location.
Changelog: [Internal]
Reviewed By: kacieb
Differential Revision: D24232926
fbshipit-source-id: f8c445213528c2d2aedacf3e0c73c5bbeb62bc3d
Summary:
Changelog: [internal]
RCTViewComponent view should have multipleTouchEnabled set to YES.
Paper has it enabled by default as well.
Reviewed By: JoshuaGross, yungsters
Differential Revision: D24219076
fbshipit-source-id: 74c18632457147b944a8abbacdbecb57e57f62ef
Summary:
We're planning a fix in Flow (D24112595) that uncovers some existing errors that were
previously suppressed. Adding these annotations prevents them from surfacing during the
deploy of the fix.
Changelog: [Internal]
Reviewed By: dsainati1
Differential Revision: D24147994
fbshipit-source-id: fef59a9427da6db79d4824e39768dd5ad0a8d1a3
Summary:
Types First and the various esproposal flags are on by default as of 0.135
Clean up flowconfigs that have to be in sync with the main one
Changelog: [Internal]
Reviewed By: jimmy623
Differential Revision: D24221995
fbshipit-source-id: 5da2867e2f013ad729a436ada192ecab690cb413
Summary:
There are cases under investigation where unmountReactApplication is called before the ReactRootView gets an ID; in some or all of those cases, UIManagerBinding.stopSurface cannot get the ReactFabric JSI module and crashes there.
It's still unclear why `unmountReactApplication` is being called in these circumstances - maybe another crash causing React Native teardown?
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24214712
fbshipit-source-id: 796594653f4ff6d87088c4841b89f06cc873b46f
Summary:
Use the ObjC++ native modules files generated by `react-native-codegen` for the FBReactNativeSpec library.
People working with the core native modules may now re-generate these ObjC++ files using `USE_CODEGEN=1 pod install` or `scripts/generate-native-modules-specs.sh`.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D22249978
fbshipit-source-id: a12db23defacdba36099a24ce0b8475c8973bd6d
Summary:
This is a revert of D22488870 (521b16730d). (https://github.com/facebook/react-native/pull/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 (#29177)
Reviewed By: fkgozali
Differential Revision: D24200517
fbshipit-source-id: af0e561a6b8d9ade487be6b197a5d79d326442b6
Summary:
This PR removes unused `qs` package (query string parser) from the main `package.json` file.
I was not able to find any explicit usage of this package in the repository. Looking at the commit which introduced this package it is not clear why it has been added, query string is not processed in any way.
I have also run ` yarn update-lock` to clean up lock after latest dependencies bumps. There are no `qs` package entry removal in yarn lock, because it is already required by `request` package.
Refs: b4785e5144
## 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] [Removed] - remove unused `qs` dependency
Pull Request resolved: https://github.com/facebook/react-native/pull/30125
Test Plan: This change should not affect the code base.
Reviewed By: appden
Differential Revision: D24184372
Pulled By: cpojer
fbshipit-source-id: 4f159f3c554c48ed21839370c8370848df181046
Summary:
In D23640968 (78b42d7fb7) I introduced a mechanism to update offsetX/offsetY whenever onMeasure/onLayout were called, to ensure that `measureInWindow` had the correct metrics and would work properly.
However, now `uiManager.updateRootLayoutSpecs` gets spammed and is called too often. For example, whenever a TextInput is focused/blurred, `uiManager.updateRootLayoutSpecs` may be called 5+ times even though
the measure specs/offsets may only change once.
Thus, we just compare with previous values before calling into the UIManager. This should give us a very small perf improvement.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24176867
fbshipit-source-id: f0dcc816e651a843607e9e5d40d8f3489894d4ba
Summary:
dispatchDraw, dispatchGetDisplayList, updateDisplayListIfDirty, recreateChildDisplayList, etc, can all crash internally for a variety of reasons and it can be very tricky to track down the root cause. This isn't a fix, this just adds extra logging to hopefully make debugging easier.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24166149
fbshipit-source-id: 1bbaf34a92a9bcac5a594a25522c66e6e0cc80ca
Summary:
According to Node's documentation: https://nodejs.org/api/util.html#util_util_inspect_object_showhidden_depth_colors
> The util.inspect() method returns a string representation of object that is intended for debugging. **The output of util.inspect may change at any time and should not be depended upon programmatically.**
Therefore, this diff switches over our RN Codegen snapshot tests to use a `JSON.stringify` call, followed by a replace of `"` with `'`. This gets the job done without compromising readability.
**Question:** Why do we not use `"`? A: Jest escapes all `"` in the snapshots, which makes reading the snapshots/using them in the console harder.
Changelog: [Internal]
Reviewed By: hramos
Differential Revision: D24157056
fbshipit-source-id: 2f1aa2df28ac3ed4aa17bcdbcd23846ddbf804cf
Summary:
This PR changes ReadableNativeMap.getNullableValue to return null if key not found, instead of throwing exception. This matches method signature and nullable annotation.
## Changelog
[Android] [Changed] - fix ReadableNativeMap.getNullableValue to match signature
Pull Request resolved: https://github.com/facebook/react-native/pull/30121
Test Plan: RNTester app builds and runs as expected, and getNullableValue will return null instead of throwing exception.
Reviewed By: JoshuaGross
Differential Revision: D24164302
Pulled By: fkgozali
fbshipit-source-id: 572c1d4ae5fd493aa0018c2df1dfc7fc91cb4b6b
Summary:
Fix ReadableArray annotations, because these methods throw ArrayIndexOutOfBoundsException instead of null if index is not found.
## Changelog
[Android] [Changed] - fix ReadableArray null annotations. Possibly breaking change for Kotlin apps.
Pull Request resolved: https://github.com/facebook/react-native/pull/30122
Test Plan: RNTester app builds and runs as expected, and show correct type in when used with Kotlin code.
Reviewed By: JoshuaGross
Differential Revision: D24164326
Pulled By: fkgozali
fbshipit-source-id: 0c3f8fa9accbd32cc71c50befe9330e5201643f6
Summary:
The captured `instance_` will result in a call to JVM code to release the reference when the lambda is destroyed. However, Hermes may destroy the lambda in a non-JVM background thread.
Since `instance_` is a member of the class, and `this` is already captured, there is no need to capture it.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D24141179
fbshipit-source-id: e3b268485ed229ee933281a80e5dbe2feba55719
Summary:
# Changelog:
[Internal][Changed] - Close the global perf logger when we create the scoped performance logger and absorb the global perf logger into the scoped logger
Reviewed By: rubennorte
Differential Revision: D23898621
fbshipit-source-id: 0d9bfd95175e660c0bd0bc74974126f92fe409b2
Summary:
Our Babel plugin that strips irrelevant platform-specific code does not currently know how to deal with early return statements.
This minor change to a couple call sites enables the output bundle for Android to omit code specific to iOS.
Changelog:
[Internal]
Reviewed By: ejanzer
Differential Revision: D24139922
fbshipit-source-id: 467c0c38dd45679b889a8a94d85f73fd969e5b36
Summary:
Remove extraneous newlines before and after structs, before copyright headers, and other locations.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24117137
fbshipit-source-id: 194996019b4cadef9239a78334f31c0bc89e3901
Summary:
react-native has moved more code to be flow-strict. Part of this move involved adding custom flow-typed definitions for some third-party dependencies where not previously present. These definitions are not currently published, which prevents type-checking some parts of react-native externally.
This change publishes these type definitions as part of the react-native NPM package. Impact to package size is pretty negligible.
## Changelog
[Internal] [Fixed] - Publish Custom Flow Library Definitions
Pull Request resolved: https://github.com/facebook/react-native/pull/30102
Reviewed By: TheSavior
Differential Revision: D24099502
Pulled By: cpojer
fbshipit-source-id: bf3942aa973df4cfc2843ec3e7dbec82623f0447
Summary:
The UserFlow API takes in a FlowId as a parameter on all of its calls. However, this type is currently not exported so consumers cannot store a generated FlowId without creating their own matching type.
Changelog: [Internal]
Reviewed By: dmitry-voronkevich
Differential Revision: D24115983
fbshipit-source-id: 63afbca01343a31aa60042db3693a96a6b854939
Summary:
When I landed D24042677 (030d2c1931), I had the right idea in spirit but forgot to negate the if statement. Oops.
This means that in non-Fabric, the cached spannable will be updated (potentially causing a crash) and the cached spannable will *never* be updated, meaning that most TextInputs will be measured as 0 in Fabric.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24119952
fbshipit-source-id: dc86137956535e1f2b147bb432d050b3561e2658
Summary:
Fixes an issue where, if a spec uses an array of elements where the element is a type alias, the generated code would use the wrong variable name.
An example of such a spec can be found in `NativeExceptionsManager.js`:
```
+reportSoftException: (
message: string,
stack: Array<StackFrame>,
exceptionId: number,
) => void;
```
The fix ensures the local variable name is passed through, ensuring that either p or itemValue_N is used when appropriate.
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D24116429
fbshipit-source-id: f39b75adb604c751d70a284a11a7fa6649b1344d
Summary:
This is a resubmit of D21499015. It resolves https://github.com/facebook/react-native/pull/28852 and https://github.com/facebook/react-native/issues/27787.
From Harry Yu's original PR (too old to merge now):
Text in TextInputs can't be selected by long press. This happens only when they're inside of FlatLists that are rendered with removeClippedSubview prop set to true on Android.
Fixes https://github.com/facebook/react-native/issues/27787
Issue https://github.com/facebook/react-native/issues/6085 2 years ago had fixed this issue with a quick fix, but the code has since disappeared in another change. It has a longer explanation for why it's fixed, but essentially - the text is assumed to be not selectable since the TextInput is initialized without being attached to the window. We need to explicitly set the text to be selectable on attachment. This change redoes that change with a 1-line fix.
Changelog: [Android] [Fixed] - Fixed TextInput not being selectable in removeClippedSubviews FlatLists
Pull Request resolved: https://github.com/facebook/react-native/pull/28852
Test Plan:
This can be tested with a new toggle in RNTesterApp.
Go to the FlatList in RNTesterApp
Toggle on "removeClippedSubviews"
Try selecting some text in the list header. It should fail without this comment but succeed with it
Reviewed By: sammy-SC
Differential Revision: D24043533
Pulled By: JoshuaGross
fbshipit-source-id: c8e60f8131ccc5f6af31ed976c4184d0a16eb3af
Summary:
Now we check for `_state` being nullptr before dereferencing this.
`accessibilityElements` (as a bunch of other methods that have the same check) can be called by OS any moment (we cannot predict when), so at some very rare moments, it can be called when the view is not fully initialized yet. To prevent crashing we just need to return some default value indicating that the view is empty.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24095654
fbshipit-source-id: d8a37f269c5bf7718acb0b512e7a9313e199398e
Summary:
RCTParagraphComponentView does not need to have the state teller because it does not change the state.
Also, we are thinking about removing State Teller, so to simplify a fix coming in the next diff, we are backout the introducing a state telled in RCTParagraphComponentView.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D24095655
fbshipit-source-id: 90bd21cb8b01056c0db902a604b4088bf8c7750e
Summary:
In theory, view configs can be hierarchical (one can depend on another), at least it was semi-supported on iOS once. And AFAIK there is only one component that seems to use this feature - `RCTComponentHostingView`, all other components essentially inherit RCTView or nothing. At the same time, RCTComponentHostingView does not really need it because RCTComponentHostingView itself does not expose any props.
Changelog: [Internal] Removed outdated warning
Reviewed By: kacieb
Differential Revision: D24095348
fbshipit-source-id: 5629c7433f151b72aae8201c58dd42062990b77e
Summary:
We're seeing that GCs caused by memory warnings on Android are barely collecting
any memory and are operating on much smaller heaps than natural GCs.
This is likely due to some of the TRIM_MEMORY_* events firing too often.
Log which event name it was, instead of the generic "memory warning", to narrow
down the cases where the memory warning was helpful. Unhelpful warnings will
later be moved to not force a GC.
Note that this diff changes Venice, but we don't see any Venice heaps in production,
so it won't matter much other than making the code match up.
Also note that iOS has a similar memory warning in `RCTCxxBridge.mm`, but it hasn't
been called in production. Perhaps iOS is less trigger-happy for memory warnings than
Android.
Changelog: [Internal]
Reviewed By: neildhar
Differential Revision: D24093394
fbshipit-source-id: 03304f0f79083133c4d9b730559aef291319b6eb
Summary:
The `flow-typed/npm` path contains third-party code. The propietary Facebook license header should not be included in these files.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24082894
fbshipit-source-id: 34928c7a6a3d1fdf8d91b4cdd86eb72ca6341545
Summary:
At this point, I think we are ready to enable validation
Changelog: [INTERNAL]
Differential Revision: D24062829
fbshipit-source-id: c82dcd2c376e5cdeb164451e16b6fb3a666106f4
Summary: Renaming methods in UserFlow to match other APIs
Reviewed By: swillard13
Differential Revision: D24078270
fbshipit-source-id: c3a65d440e389d7b3c76de7706372265584353c8