Summary:
This diff adds `TraceUpdateOverlay` native component to RN `AppContainer.js`. This will enable the overlay when the build is in DEV environment and the DevTools global hook exists. It also closed gap between the JS dev mode and native dev support flag, so that the native component will be available when used by JS.
## Update (2/13/2023)
Instead of the original approach where I put a default value to the devsupport manager flag, I did ui manager check from JS and make sure the native component exists before using it. This is cleaner.
## Problem
Since the `AppContainer` is being used by all RN apps, we need to make sure the native component is registered in UI Manager of the RN app when it's used. Currently, the native component lives in the `DebugCorePackage.java`, which is added to the RN app [when the `DevSupportManager` is used](https://fburl.com/code/muqmqbsa). However, there's no way to tell if an app is using dev support manager in JS, hence there are gaps when the JS code uses `TraceUpdateOverlay`, vs when the native code registered the native component. This issue caused test error in [ReactNativePerfTest](https://fburl.com/testinfra/j24wzh46) from the [previous diff](https://fburl.com/diff/bv9ckhm7), and it actually prevents Flipper from running this properly as shown in this video:
https://pxl.cl/2sqKf
The errors shown in Flipper indicates the RN surface from the plugin is also missing `TraceUpdateOverlay` in its UI Manager:
{F869168865}
## Solution
To fix this issue, we should find a way to expose if the app is using dev support manager in JS. Or we should set to use DevSupportManager whenever it's a dev build as claimed in JS. I will try to find some way to achieve either one of this. I am open to suggestions here for where I should add the native component to. Given that it's used in the AppContainer, and any app could be built in development mode, I don't want to make people to manually add this native component themselves.
## Alternatives
There are some other approaches that could mitigate the issue, but less ideal:
For the test issue
1) Add `setUseDeveloperSupport(true)` to [ReactNativeTestRule.java](https://fburl.com/code/7jaoamdp). That will make the related test pass by using the DevSupportPackages, which has the native component. However, it only fixes tests using that class.
2) Override the package for [ReactNativeTestRule.java](https://fburl.com/code/b4em32fa), or `addPackage` with more packages including the native component. Again this only fixes this test.
3) Add the native component to the [`MainReactPackage`](https://fburl.com/code/nlayho86), which is what I did here in this diff. This would fix more cases as this package is [recommended to be used](https://fburl.com/code/53eweuoh) for all RN app. However, it may not fix all the cases if the RN app didn't manually use it.
4) Add the native component in the [`CoreModulesPackage`](https://fburl.com/code/lfeklztl), which will make all RN apps work, but at the cost of increase package size when this feature is not needed. Or, we could argue that we want to have highlights on trace updates for production build as well?
Changelog:
[Internal] - Enable TraceUpdateOverlay to RN AppContainer
Reviewed By: rubennorte
Differential Revision: D43180893
fbshipit-source-id: a1530cc6e2a9d8c905bdfe5d622d85c4712266f8
Summary:
This diff adds `TraceUpdateOverlay` native component to render highlights when trace update is detected from React JS. This allows a highlight border to be rendered outside of the component with re-renders.
- Created `TraceUpdateOverlay` native component and added to the `DebugCorePackage`
- Added to C++ registry so it's compatible with Fabric
- Added to `AppContainer` for all RN apps when global devtools hook is available
Changelog:
[Android][Internal] - Add trace update overlay to show re-render highlights
Reviewed By: javache
Differential Revision: D42831719
fbshipit-source-id: 30c2e24859a316c27700270087a0d7779d7ad8ed
Summary:
Incorrect TS type disallows use of `hitSlop={number}`. Fixed by using Pressable's hitSlop type.
NOTE: I did not bother to change Flow types in the `.js` file, please add a commit doing that if required.
## Changelog
[GENERAL] [FIXED] - Fix touchable hitSlop type
Pull Request resolved: https://github.com/facebook/react-native/pull/36065
Test Plan: None needed
Reviewed By: christophpurrer
Differential Revision: D43117689
Pulled By: javache
fbshipit-source-id: 96e5ae650f47382c8d7fa1ddf63c76461c65dcc7
Summary:
This adds support for `maintainVisibleContentPosition` on Android. The implementation is heavily inspired from iOS, it works by finding the first visible view and its frame before views are update, then adjusting the scroll position once the views are updated.
Most of the logic is abstracted away in MaintainVisibleScrollPositionHelper to be used in both vertical and horizontal scrollview implementations.
Note that this only works for the old architecture, I have a follow up ready to add fabric support.
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
[Android] [Added] - Add maintainVisibleContentPosition support on Android
Pull Request resolved: https://github.com/facebook/react-native/pull/35049
Test Plan:
Test in RN tester example on Android
https://user-images.githubusercontent.com/2677334/197319855-d81ced33-a80b-495f-a688-4106fc699f3c.mov
Reviewed By: ryancat
Differential Revision: D40642469
Pulled By: skinsshark
fbshipit-source-id: d60f3e2d0613d21af5f150ca0d099beeac6feb91
Summary:
`accessibilityLabelledBy` is missing from `AccessibilityPropsAndroid` TypeScript interface
## Changelog
[GENERAL] [FIXED] - Added missing `accessibilityLabelledBy` TypeScript type
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Pull Request resolved: https://github.com/facebook/react-native/pull/35883
Test Plan: Ran `yarn test-typescript` and `yarn test-typescript-offline` and there were no errors.
Reviewed By: christophpurrer
Differential Revision: D42604287
Pulled By: NickGerleman
fbshipit-source-id: 476d24d1c0257be787b7e84c2c11bcadc3527979
Summary:
`accessibilityLanguage` is missing from `AccessibilityPropsIOS` TypeScript interface
## Changelog
[GENERAL] [FIXED] - Added missing `accessibilityLanguage` TypeScript type
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Pull Request resolved: https://github.com/facebook/react-native/pull/35882
Test Plan: Ran `yarn test-typescript` and `yarn test-typescript-offline` and there were no errors.
Reviewed By: christophpurrer
Differential Revision: D42604363
Pulled By: NickGerleman
fbshipit-source-id: fb8dd4b5bba78a080473a9dc7b49a07587530229
Summary:
The `scrollEventThrottle` prop is documented as only applying to iOS, but in [this commit](https://github.com/facebook/react-native/commit/cf55fd587e6cc82a73079be6076d244ab72fa359) it was also implemented for Android, but the documentation was not updated.
## Changelog
[GENERAL] [CHANGED] - Removed iOS flag from `scrollEventThrottle` docs
<!-- Help reviewers and the release process by writing your own changelog entry.
Pick one each for the category and type tags:
[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->
Pull Request resolved: https://github.com/facebook/react-native/pull/35840
Test Plan: I've not been able to find how to build the docs locally, so I don't have a screenshot of the changes :(
Reviewed By: cipolleschi
Differential Revision: D42530943
Pulled By: sammy-SC
fbshipit-source-id: 2948eca2621712ccd832f4f99f1aab0cbd4af2d0
Summary:
This PR implements logical border-radius as requested on https://github.com/facebook/react-native/issues/34425. This implementation includes the addition of the following style properties
- `borderEndEndRadius`, equivalent to `borderBottomEndRadius`.
- `borderEndStartRadius`, equivalent to `borderBottomStartRadius`.
- `borderStartEndRadius`, equivalent to `borderTopEndRadius`.
- `borderStartStartRadius`, equivalent to `borderTopStartRadius`.
## Changelog
[GENERAL] [ADDED] - Add logical border-radius implementation
Pull Request resolved: https://github.com/facebook/react-native/pull/35572
Test Plan:
1. Open the RNTester app and navigate to the `RTLExample` page
2. Test the new style properties through the `Logical Border Radii Start/End` section
https://user-images.githubusercontent.com/11707729/206623732-6d542347-93f9-40da-be97-f7dcd5f66ca9.mov
Reviewed By: necolas
Differential Revision: D42002043
Pulled By: NickGerleman
fbshipit-source-id: a0aa9783c280398b437aeb7a00c6eb3f767657a5
Summary:
This PR implements `inset` logical properties as requested on https://github.com/facebook/react-native/issues/34425. This implementation includes the addition of the following style properties
- `inset`, equivalent to `top`, `bottom`, `right` and `left`.
- `insetBlock`, equivalent to `top` and `bottom`.
- `insetBlockEnd`, equivalent to `bottom`.
- `insetBlockStart`, equivalent to `top`.
- `insetInline`, equivalent to `right` and `left`.
- `insetInlineEnd`, equivalent to `right` or `left`.
- `insetInlineStart`, equivalent to `right` or `left`.
## Changelog
[GENERAL] [ADDED] - Add Fabric implementation of inset logical properties
Pull Request resolved: https://github.com/facebook/react-native/pull/35692
Test Plan:
1. Open the RNTester app and navigate to the `View` page
2. Test the new style properties through the `Insets` section
<table>
<tr>
<td>Android</td>
<td>iOS</td>
</tr>
<tr>
<td><img src="https://user-images.githubusercontent.com/11707729/208821212-fbbac6ed-09a4-43f4-ba98-dfd2cbabf044.png" alt="1" width="360px" />
</td>
<td>
<img src="https://user-images.githubusercontent.com/11707729/208816997-ef044140-8824-4b1b-a77b-085f18ea9e0e.png" alt="2" width="360px" />
</td>
</tr>
</table>
Reviewed By: NickGerleman
Differential Revision: D42193661
Pulled By: ryancat
fbshipit-source-id: 3db8bcd2c4db0ef4886b9ec49a46424d57362620
Summary:
This was renamed in Flow but not in TS.
Changelog:
[General][Fixed] - Fix types for deprecated scrollTo fields
Reviewed By: christophpurrer
Differential Revision: D42040913
fbshipit-source-id: 5d9746be7b785bea8613f199d39940c4d5d7d138
Summary:
The flow type and inheriting TS types mark animated as optional, mark it here as well.
Changelog:
[General][Fixed] - Mark scrollToEnd animated as optional
Reviewed By: christophpurrer
Differential Revision: D42036647
fbshipit-source-id: 59d408adc639ee1e0ca040301511c64d7ba55bfe
Summary:
changelog: [internal]
Fix Flow types for UIManager measure functions.
Host platform does not accept null value for reactTag parameter. It errors if null is passed. This moves error closer to developers as Flow can catch it.
Reviewed By: javache
Differential Revision: D41614452
fbshipit-source-id: 2216c195808b8a1aae58c7cb2fa15be8a073e222
Summary:
The commit https://github.com/facebook/react-native/commit/3eddc9abb70eb54209c68aab7dbd69e363cc7b29 included on v0.69 introduced a wrong `if` statement inside the `componentDidUpdate` function of the `TouchableOpacity` component. As this `if` statement always evaluates to `true` (`(true || false) !== undefined`) we end up making unnecessary calls to the `_opacityInactive` method every time the component props changes, e.g. every time a `<Text>` inside the TouchableOpacity changes we call this function over and over, and this has been causing some performance issues on big lists.
This PR fixes this problem by adjusting the `componentDidUpdate` function to only call `_opacityInactive` when necessary.
Closes https://github.com/facebook/react-native/issues/34442
Closes https://github.com/facebook/react-native/issues/32476
## Changelog
[General] [Fixed] - Fix TouchableOpacity componentDidUpdate causing an excessive number of pending callbacks
Pull Request resolved: https://github.com/facebook/react-native/pull/35387
Test Plan:
1. Open the RNTester app and navigate to the `Touchable* and onPress` page
2. Test the `TouchableOpacity` component through the many sections
Reviewed By: cipolleschi
Differential Revision: D41397396
Pulled By: ryancat
fbshipit-source-id: 24863b5cbbdd2f3dd1f654b43d7031560937b888
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35342
This is a native implementation of the JS shimmed layout-specific properties in https://github.com/facebook/react-native/pull/35316.
There is an experiment splitting the prop splitting codepath in Fabric, so this change effectively has two implementations depending on whether `enablePropIteratorSetter` is enabled.
None of these changes make sense to propagate Yoga. `inlineEnd`, etc are already mapped to `YGEdgeStart` and `YGEdgeEnd`, but RN's mapping used a different name. Then `blockStart`, `blockEnd`, map directly to existing edges in all cases since we don't support a writing mode.
On web, the last value in the style which computes the given dimension is given precedence. E.g. if "left" comes after "start" it will be chosen. Yoga stylesheets are unordered, and precedence for edges is given based on specificity (left > start > horizontal > all).
We give precedence to new renamings (e.g. start to inlineStart), but to preserve consistent behavior, we give precedence to specific edges before overwriting them with flow relative ones (e.g. marginTop has precedence over marginBlockStar).
Changelog:
[General][Added] - Add Fabric implementation of flow-relative padding and margin
Reviewed By: javache
Differential Revision: D41267765
fbshipit-source-id: 896e2ed71fe8bf83aef00b0a9d70fd20b2ce47a7
Summary:
`aria-labelledby` Flow types were added to the Text props on https://github.com/facebook/react-native/commit/f353119113d6fc85491765ba1e90ac83cb00fd61 but the text component does not support `accessibilityLabelledBy`, which is the prop we were supposed to map `aria-labelledby` to. As the Text component does not really support `aria-labelledby` this PR updates the `TextProps` documentation to explain that this prop is necessary for cross-platform purposes and that other platforms need it.
## Changelog
[Android] [Changed] - Add notes to `aria-labelledby` from Text props
Pull Request resolved: https://github.com/facebook/react-native/pull/35327
Test Plan: Ensure CI is green
Reviewed By: necolas
Differential Revision: D41280877
Pulled By: lunaleaps
fbshipit-source-id: 81cf016bcf3117f15385ddd119c44d6f9fdd6b68
Summary:
Normally when a `ref` prop supplied to a component is changed, the old `ref` receives `null` and the new `ref` receives the active component instance.
However, the current implementation of `ScrollView` has a bug where this does not happen (i.e. changing `ref` or `innerViewRef` does not cause either the old or new refs to be updated). This bug is due to a subtle issue with how `ScrollView` internally memoizes the `ref` (or `innerViewRef`) that is passed into the native component.
This commit fixes this behavior so that `ScrollView`'s `ref` and `innerViewRef` have the correct behavior.
Changelog:
[General][Fixed] When a ScrollView's `ref` or `innnerViewRef` changes, the old ref will now be invoked with `null` and the new ref with the active instance. (Previously, changing `ref` or `innerViewRef` on a `ScrollView` would be treated as though the ref had not changed at all.)
Reviewed By: sammy-SC
Differential Revision: D41208895
fbshipit-source-id: b465f666076edbef410cdf9661e040e1d8fa0404
Summary:
Switches `TextInput` from `setAndForwardRefs` to `useMergeRefs` (and `useCallback`).
Instead of creating a new `_setNativeRef` function on every render, this will now only create a new ref function when either `mostRecentEventCount` changes, `viewCommands` (i.e. `props.multiline` on iOS) changes, or when `props.forwardedRef` changes.
When a text input is being edited, `mostRecentEventCount` will probably change. But when an unfocused `TextInput` is being updated because a parent is being updated, we will now no longer unnecessarily create a new `ref`. The observable behavior of this is that any `ref` supplied onto `TextInput` will now be invoked less frequently.
Changelog:
[General][Changed] Any `ref` set on `TextInput` will now be updated less frequently (when the underlying `ref` has not changed).
Reviewed By: sammy-SC
Differential Revision: D41191439
fbshipit-source-id: c69a061317c51ad6c6ca8acd116539e0f24a1d08
Summary:
While refactoring `TextInput`, I noticed that we are allocating each of the exported imperative instance methods on every invocation of the component. This is wasteful because if the `ref` does not change, this just creates more garbage.
Instead, we only need to allocate them when we are actually going to assign them onto a `ref`.
Changelog:
[Internal]
Reviewed By: sammy-SC
Differential Revision: D41191440
fbshipit-source-id: 0120d67a9e29a3e0c3f42c3fa436381d6bec7619
Summary:
Straightforward refactor of how `TextInput` augments the exported `ref` with instance methods.
This change is in preparation for additional refactors to eliminate `setAndForwadRefs`.
Changelog:
[Internal]
Reviewed By: christophpurrer
Differential Revision: D41191441
fbshipit-source-id: 30560ce489c48409003a9f92875e5096d3f1130e
Summary:
Cleans up `ScrollView-test.js` to be easier to read and to include a unit test for `ref`.
This prepares the Jest test suite to be extended with an additional test to validate a bug with `ref` invalidation.
Changelog:
[Internal]
Reviewed By: sammy-SC
Differential Revision: D41208894
fbshipit-source-id: 6d79adf131602292f7aa3180b1c230643e9baf16
Summary:
Add explicit annotations to React hook callbacks as required for Flow's Local Type Inference project. This codemod prepares the codebase to match Flow's new typechecking algorithm. The new algorithm will make Flow more reliable and predictable.
This diff adds `any` or `$FlowFixMe` in cases where more precise types could not be determined.
Details:
- Codemod script: `.facebook/flowd codemod annotate-react-hooks ../../xplat/js --default-any --write`
- Local Type Inference announcement: [post](https://fb.workplace.com/groups/flowlang/posts/788206301785035)
- Support group: [Flow Support](https://fb.workplace.com/groups/flow)
drop-conflicts
bypass-lint
Reviewed By: SamChou19815
Differential Revision: D41231214
fbshipit-source-id: d5f5ce8d61020baa1138292c9e9f1c69dffd324c
Summary:
Add explicit annotations to underconstrained implicit instantiations as required for Flow's Local Type Inference project. This codemod prepares the codebase to match Flow's new typechecking algorithm. The new algorithm will make Flow more reliable and predictable.
This diff adds `any` or `$FlowFixMe` in cases where more precise types could not be determined.
Details:
- Codemod script: `.facebook/flowd codemod annotate-implicit-instantiations ../../xplat/js --default-any --write`
- Local Type Inference announcement: [post](https://fb.workplace.com/groups/flowlang/posts/788206301785035)
- Support group: [Flow Support](https://fb.workplace.com/groups/flow)
drop-conflicts
bypass-lint
Reviewed By: SamChou19815
Differential Revision: D41226960
fbshipit-source-id: e5e3edbb1aed849f90cc683a4d416a9a2f8f3a19
Summary:
This was left over from an old codemod.
Changelog: [Internal]
Reviewed By: christophpurrer
Differential Revision: D41154548
fbshipit-source-id: 0b5fb3e78491b66ebaf13555f80e0265a25dc7d8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/35169
This reorganizes typing structure a bit.
`Utilities.d.ts` was originally added for utilitiy types but I ended up leaving it a grab bag of types that didn't belong to any individual bit of code. Out of what is in it right now, `Insets` was actually public, and seems to have been imported.
We also run into files around the renderer which are [currently overwritten](https://github.com/facebook/react-native/commits/e286da25fc83324363486eb668806aca179f74b3/Libraries/Renderer/implementations/ReactNativeRenderer.d.ts) by the React sync script.
Finally, all of the top-level imports of `Utilities` were auto-generated by VS Code, but fail in real apps. I think this is because our tsconfig sets a `baseUrl` to allow resolution from the types folder, so the tooling in the RN repo will use that, but it breaks in real apps that don't have that mapping.
This splits all these up into a couple separate directories that are hopefully easier to reason about, and removes `Omit` which has been a builtin type for quite some time (we were actually already using built-in `Omit`).
Changelog:
[General][Fixed] - Fixup TS Organization
Reviewed By: cipolleschi
Differential Revision: D40932319
fbshipit-source-id: 0b6e3e3eda603885b4dc01dcb9f5233aa546d128
Summary:
In 2017, React published v15.5 which extracted the built-in `prop types` to a separate package to reflect the fact that not everybody uses them. In 2018, React Native started to remove `PropTypes` from React Native for the same reason. In 0.68 React Native introduced a deprecation warning which notified users that the change was coming, and in 0.69 we removed the PropTypes entirely.
The feedback we've received from the community is that there has not been enough time to migrate libraries off of PropTypes. This has resulted in users needing to patch the React Native package `index.js` file directly to add back the PropTypes, instead of migrating off of them. We can empathize with this fix short term (it unblocks the upgrade) but long term this patch will cause users to miss important changes to `index.js`, and add a maintenance cost for users.
Part of the reason there was not enough time is that we didn't do a good job surfacing libraries that were using PropTypes. This means, when you got a deprecation warning, it wasn't clear where the source of the usage was (either in your code or in a library). So even if you wanted to migrate, it was difficult to know where to actually make the change.
In the next release, we've made it easier to find call sites using deprecated types by [fixing the code frame in errors](https://github.com/react-native-community/cli/pull/1699) reporting in LogBox, and ensuring that [the app doesn't crash without a warning](https://github.com/facebook/react-native/pull/34650). This should make it easier to identify exactly where the deprecated usage is, so you can migrate it.
To help users get off of the patch, and allow more time to migrate, we're walking back the removal of PropTypes, and keeping it as a deprecation for a couple more versions. We ask that you either migrate off PropTypes to a type system like TypeScript, or migrate to the `deprecated-react-native-prop-types` package.
Once we feel more confident that the community has migrated and will not need to patch React Native in order to fix this issue, we'll remove the PropTypes again. **If you have any trouble finding the source of the PropType usage, please file an issue so we can help track it down with you.**
Changelog:
[General][Changed] - Add back deprecated PropTypes
Reviewed By: yungsters
Differential Revision: D40725705
fbshipit-source-id: 8ce61be30343827efd6dc89a012eeef0b6676deb
Xin Chen
bigcupcoffee
gabrieldonadel
Sebastian Silbermann
Janic Duplessis
Dimitar Nestorov
Sam Zhou
Rob Walker
Ruslan Lesiutin
Luna Wei
XantreGodlike
Nick Gerleman
Samuel Susla
Rujin Cao
Nicolas Gallagher
Tim Yung
Mike Vitousek
Pieter De Baets
Mayank Sunil Pagar
fabriziobertoglio1987
Rick Hanlon
ankit-tailor
