Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38814
This change enables the Gradle Daemon on CI.
We noticed some flakyness with the Daemon disabled, so we'll give it a try with the daemon enabled which is the default for Gradle.
Changelog:
[Internal] [Changed] - Enable the Gradle Daemon for CI
Reviewed By: cipolleschi
Differential Revision: D48112363
fbshipit-source-id: 4a7f9bbaad33935a97e4e0ea28ea8f2f22c67d0f
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38771
Changelog: [Internal]
Metro has been passing `inlineRequires: false` to Babel transformers for the last 5 years (D9636747), as the inline requires transform had been hoisted into the transform worker. Here we clean up the unused code paths in React Native's implementations of the Metro Babel transformer API, and the unused properties in the API itself.
Reviewed By: GijsWeterings
Differential Revision: D48034441
fbshipit-source-id: d24a793962942a3c87461ffbade847ea749dac95
Summary:
Right now we're building by default Android app in New Architecture, so to align this behaviour between platforms - I added `RCT_NEW_ARCH_ENABLED=1` when installing Pods when running E2E tests, to also build iOS app in New Arch.
## Changelog:
[IOS] [CHANGED] - Build iOS app in New Architecture when running E2E tests.
Pull Request resolved: https://github.com/facebook/react-native/pull/38813
Test Plan: App in `test_e2e_ios` job should build in New Architecture
Reviewed By: dmytrorykun
Differential Revision: D48112814
Pulled By: cipolleschi
fbshipit-source-id: f709fe23f7c3ff65f3e39acb304581c4ec0fdea8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38742
If you try to add an `onScroll` event listener on iOS, you will get a warning that you must additionally add a `scrollEventThrottle`, otherwise you will only receive a single event throughout the duration of the gesturing.
Values under 17ms are unthrottled, so it is common to see components passing magic values `16` and `0.0001` to ask for full events.
This behavior is inconsistent with Android, Web, and Windows.
This change runs an experiment on iOS to continuously fire scroll events to users of `onScroll` without `scrollEventThrottle`. Because there are warnings already, I think this case will be rare, and any code written for Android will already have to be written to be compatible with it.
I intentionally left the warning in for now, so that folks don't add new code relying on this QE while it is being tested.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D47975260
fbshipit-source-id: 8f182c5514cc557e1804586e8cbc68fc54502d83
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38735
UTs around cell measurement results through `getCellMetrics` and `getCellMetricsApprox`. For each orientation, validate basic scenarios for approximation, cached measurement, or measurement by user-provided `getItemLayout`.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D47978630
fbshipit-source-id: 4c9ba6a60599848034f4f23cde3497dd4c2b8788
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38733
I was working under the assumption that Fabric fired layout events bottom up, but it actually fires them top-down, in constrast to Paper.
Previous invalidation logic wasn't quite correct when fired bottom-up. This corrects the logic by:
1. Deriving direction based on initial event ordering
2. Use last cached contentLength if we are on Fabric (top-down)
3. Use future contentLength if we are on Paper (bottom-up)
Changelog:
[General][Fixed] - Fixup contentLength invalidation logic
Reviewed By: rozele
Differential Revision: D47978638
fbshipit-source-id: 3446d08aa34397b4e6bd9924dad0eba36a12a115
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38737
This fixes up behavior on Android so that `scrollToIndex` aligns the right edge of the cell of the given index to the right edge of the scrollview viewport. We do not incorporate RTL on iOS which inverts x/y coordinates from scroller (but not layout).
Changelog:
[General][Fixed] - Right align scrollToIndex in RTL
Reviewed By: lenaic
Differential Revision: D47978637
fbshipit-source-id: 7786b5d97efaced318018409e2c7577a3d8f7402
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38734
Returned measurements from the measurements cache in RTL calculate offset as distance from the left edge of the cell to the right edge of the content, when it should instead be the distance from the right edge of the cell (the logical beginning).
Changelog:
[General][Fixed] - Return right edge in RTL cell metrics
Reviewed By: lenaic
Differential Revision: D47978631
fbshipit-source-id: b0db4e9aff676c5bee81d4491f901a6bbc38e4bf
Summary:
This PR adds better caching for iOS jobs and the cocoapods dependencies.
The changes takes the proper version of the `podfile.lock`, which depends on the job and several other aspects, to decide which cache to restore.
This can save up to 2 minutes per jobs.
Currently, we have 33 iOS jobs that use cocoapods.
This can save 66 minutes per run, which equates to 4950 credits per run.
## Changelog:
[Internal] - Add Caching for Xcodebuild jobs
Pull Request resolved: https://github.com/facebook/react-native/pull/38687
Test Plan: CircleCI stays green and jobs run faster
Reviewed By: cortinico
Differential Revision: D48063823
Pulled By: cipolleschi
fbshipit-source-id: e56ffdb43a689b5630a8d73c8f5d23ccbf732c27
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38781
## Changelog:
[Internal] -
These pop up when compiling with `/Wall`, fixes two legitimate warnings around using YogaShadowNode/Props in RN.
The `suggestedIndex` in `ShadowNode::replaceChild` (and the overriden one in `YogaLayoutableShadowNode`) is used in a way around the code that suggests that it **is** expected to be negative (checking for non-negativity, assigning -1 as default etc), so having it as a `size_t` type argument both makes things confusing and generates the warning.
I believe it's a good idea to be consistent and use the same type for the index throughout. In majority of the cases it's `int32_t` as of now.
Reviewed By: rozele
Differential Revision: D48059620
fbshipit-source-id: c4bab91f5c66c62d00947f96cb48906a8499f282
Summary:
Hello! This PR is a fix for one merged some time ago (https://github.com/facebook/react-native/pull/36216). In the PR check for `nullptr` value of `newRootShadowNode` just after performing commit hooks was overlooked. This PR restores previous behaviour of conditional commit cancellation after commit hook execution.
## Changelog:
[INTERNAL] [FIXED] - Restore checking shadow tree commit cancellation after commit hook execution
Pull Request resolved: https://github.com/facebook/react-native/pull/38715
Test Plan: Just register a commit hook that return `nullptr`. In that case current code crashes due to `nullptr` dereference.
Reviewed By: sammy-SC
Differential Revision: D47972245
Pulled By: ryancat
fbshipit-source-id: 7599ad11ed4b2dcaf25e53f676ec4530e37410d5
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38727
## Changelog:
[General][Fix] - Do not render React DevTools overlays unless they are connected
Fixes https://github.com/facebook/react-native/issues/38024.
Reproducible example: https://github.com/rasaha91/rn-bottomsheet.
- This is a temporary workaround to resolve described problem for DEV bundles without attached React DevTools.
- Still, such problem will be present for DEV bundles with attached React DevTools, but this should be only for brownfield apps with a shrinked React Native window.
Checking that DevTools hook is present is not enough to determine whether the DevTools are connected. These changes fix it.
Short description of what's going on there:
1. When React Native root window is rendered inside some native view, which takes only portion of the screen (like the native bottom sheet in the reproducible example), DevtoolsOverlay / InspectorOverlay takes up space based on application window dimension, this results into resizing the hosting window on the native side.
https://pxl.cl/357r3
2. Right way to fix this would be removing the usage of application window sizes, so that DevtoolsOverlay / InspectorOverlay will be allowed only to take React Native's window.
3. Unfortunately, just removing setting is not enough, we should also have at least 1 of 2 things:
- `collapsable` prop should be set to `true` => View will be flattened
- Remove [`flex: 1` style on both root and inner Views](https://github.com/facebook/react-native/blob/b28e3c16ed7cbc8b3ed3f26d91c58acb4bb28879/packages/react-native/Libraries/ReactNative/AppContainer.js#L145-L147), but this is breaking how LogBox works now.
| {F1062478964} | {F1062492367}
Reviewed By: NickGerleman
Differential Revision: D47954883
fbshipit-source-id: fa9c34137c2711e74bba5b92fa7aafa4173e6bdd
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38739
1. The separator here adds horizontal width, except when the cell is tapped. This is meant to be a visual effect for vertical FlatList, but makes `getItemLayout` incorrect and results in odd layout shifts. Do not use when horizontal.
2. Disabling "fixed height" leads to a barrage of errors because the example sets `initialScrollIndex` but does not set an `onScrollToIndexFailed` prop.
Changelog: [Internal]
Reviewed By: rozele
Differential Revision: D47978628
fbshipit-source-id: eb022c01bfd7df44c2f11d03ccc4580270c36a65
Summary:
updated type for activeThumbColor prop for Switch component
To help community users with all possible properties
## Changelog:
<!-- 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
--> GENERAL CHANGED - added new prop in type of switch component
Pull Request resolved: https://github.com/facebook/react-native/pull/38636
Reviewed By: cortinico
Differential Revision: D47800523
Pulled By: lunaleaps
fbshipit-source-id: 25b22a97a3c02ad01c8100fdd1f3e808f296c14c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38759
As part of refactoring the `ReactSurface`, moving the `SurfaceHandler` interface to `react.interfaces.fabric`.
changelog: [internal] internal
Reviewed By: christophpurrer
Differential Revision: D48006184
fbshipit-source-id: 09297fccc2b405a5e807d35bfa9eb2a9a4c52a16
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38756
In order to expose getSurfaceHandler() from an interface ReactSurface we need this to be public. All methods in an interface are public and the implementing class can not have a restricted visibility than the interface.
Reviewed By: christophpurrer
Differential Revision: D48003816
fbshipit-source-id: 7f0d0bd91cc9cde9c2430fd8dcde7a94966fd217
Summary:
This PR aims to fix an issue where installing iOS pods from a different directory causes the `hermes-engine` `:tag:` not to be properly resolved.
Ever since https://github.com/facebook/react-native/issues/37148 was merged, the `setup_hermes` script tries to resolve the `node_modules/react-native/sdks/.hermesversion` file and use its content to generate the pod tag, in order to verify when it changes over time.
This works perfectly when installing pods within the `ios` folder, as the `react_native_path` should point to the correct relative folder (`../node_modules/react-native` by default).
However, when installing pods from a different directory (the project root, for example) and leveraging the `--project-directory` flag, the file fails to resolve, as the current working directory is considered when resolving the `.hermesversion` file.
### Quick Example:
- `react_native_path`: `../node_modules/react-native` (the default config)
**Installing pods from the `ios` folder:**
- `cd ios`
- `bundle exec pod install`
- `hermestag_file` resolved path: `$project_root/node_modules/react-native` ✅
- `hermes-engine` `:tag:`: `hermes-2023-03-20-RNv0.72.0-49794cfc7c81fb8f69fd60c3bbf85a7480cc5a77` ✅
**Installing pods from the `$project_root` folder**
- `bundle exec pod install --project-directory=ios`
- `hermestag_file` resolved path: `$parent_folder/$project_root/node_modules/react-native` ❌
- `hermes-engine` `:tag:`: `''` ❌
### The fix
Turns out that the same file had a resolved reference to the `react-native` folder, assigned to the `react_native_dir` variable:
```ruby
react_native_dir = Pod::Config.instance.installation_root.join(react_native_path)
```
By resolving the `.hermesversion` using that folder, we guarantee that the relative path will always reference the directory where the `Podfile` is defined, which is the expected behaviour.
## Changelog:
[Internal] - Fix an issue where installing pods from a different directory would fail to resolve `hermes-engine` tags correctly.
For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
Pull Request resolved: https://github.com/facebook/react-native/pull/38754
Test Plan:
- Init a new `react-native` repo
- Remove the generated `Podfile.lock` file
- Navigate to the project root folder
- `bundle exec pod install -project-directory=ios`
- Check that the `hermes-engine` entry has a properly populated `:tag:` attribute:
**Before:**
```
hermes-engine:
:podspec: "../node_modules/react-native/sdks/hermes-engine/hermes-engine.podspec"
:tag: ''
```
**After:**
```
hermes-engine:
:podspec: "../node_modules/react-native/sdks/hermes-engine/hermes-engine.podspec"
:tag: hermes-2023-03-20-RNv0.72.0-49794cfc7c81fb8f69fd60c3bbf85a7480cc5a77
```
Reviewed By: cortinico
Differential Revision: D48029413
Pulled By: cipolleschi
fbshipit-source-id: 82d465abd5c888eeb9eacd32858fa4ecf4f8c217
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38718
> NOTE: Replaces https://github.com/facebook/react-native/pull/38240
## Context
RFC: Decoupling Flipper from React Native core: https://github.com/react-native-community/discussions-and-proposals/pull/641
## Changes
To support incoming new React Native packages around debugging (including migrating over [`react-native-community/cli-plugin-metro`](https://github.com/react-native-community/cli/tree/main/packages/cli-plugin-metro)) — which target Node.js and require a build step, this PR adds a minimal shared build setup across the `react-native` monorepo.
The setup is closely inspired/based on the build scripts in Jest, Metro, and React Native CLI — and is a simple set of script wrappers around Babel. These are available as build commands at the root of the repo:
- `yarn build` — Builds all configured packages. Functionally, this:
- Outputs a `dist/` directory with built files.
- Rewrites package.json `"exports"` to update every `./src/*` reference to `./dist/*` (source of truth).
- `scripts/build/babel-register.js` — Allows running all Node.js entry points from source, similar to the current setup in [facebook/metro](https://github.com/facebook/metro). (Example entry point file in this PR: `packages/dev-middleware/src/index.js`)
Build configuration (i.e. Babel config) is shared as a set standard across the monorepo, and **packages are opted-in to requiring a build**, configured in `scripts/build.config.js`.
```
const buildConfig /*: BuildConfig */ = {
// The packages to include for build and their build options
packages: {
'dev-middleware': {target: 'node'},
},
};
```
For now, there is a single `target: 'node'` option — this is necessary as `react-native`, unlike the above other projects, is a repository with packages targeting several runtimes. We may, in future, introduce a build step for other, non-Node, packages — which may be useful for things such as auto-generated TypeScript definitions.
{F1043312771}
**Differences from the Metro setup**
- References (and compiles out) repo-local `scripts/build/babel-register.js` — removing need for an npm-published dependency.
## Current integration points
- **CircleCI** — `yarn build` is added to the `build_npm_package` and `find_and_publish_bumped_packages` jobs.
**New Node.js package(s) are not load bearing quite yet**: There are not yet any built packages added to the dependencies of `packages/react-native/`, so this will be further tested in a later PR (and is actively being done in an internal commit stack).
### Alternative designs
**Per-package config file**
Replace `scripts/build/config.js` with a package-defined key in in `package.json`, similar to Jest's [`publishConfig`](https://github.com/jestjs/jest/blob/1f019afdcdfc54a6664908bb45f343db4e3d0848/packages/jest-cli/package.json#L87C3-L89C4).
```
"buildConfig": {
"type": "node"
},
```
This would be the only customisation required, with a single Babel config still standardised. Another option this might receive in future is `enableTypeScriptCodgeen`.
**Rollup**
More sophisticated build tool for Node.js, used by the React codebase (albeit within a custom script setup as well).
**Lerna and Nx**
- Most sophisticated setup enabling caching and optimised cloud runs.
- Probably the most likely thing we'll move towards at a later stage.
Changelog: [Internal]
Reviewed By: NickGerleman
Differential Revision: D47760330
fbshipit-source-id: 38ec94708ce3d9946a197d80885781e9707c5841
Summary:
## Changelog:
[Internal] -
This must be some new thing coming from the C++ language service provider (from the VSCode plugins?..), but this keeps popping up since recently as untracked files in the yoga subfolder.
Add it to .gitignore.
Reviewed By: NickGerleman
Differential Revision: D48022954
fbshipit-source-id: dad608f303f3d50b701d776795f6e25f007811b2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38765
## Changelog:
[Internal] -
The `RN_DEBUG_STRING_CONVERTIBLE` was interchangeably used via both `#if` and `#ifdef` (the latter being incorrect), which led to both inconsistency, or even the code plain not compiling it `RN_DEBUG_STRING_CONVERTIBLE=0` explicitly.
Furthermore there was no guarantee it's defined, which can generate warnings with `-Wundef`/`-Wall` (or even errors with `-Werror` on top of that).
This fixes all the usages to be uniform and makes sure the flag is defined when it's used to prevent compiler warnings.
Reviewed By: christophpurrer
Differential Revision: D48021565
fbshipit-source-id: 653b6af96bb0361e6f1d62c1b545ec01309760e8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38736
`scrollToIndex` relies on cached layout information, so we should cache the results from `onContentSizeChange` before attempting the scroll. Otherwise we will fail to scroll in RTL.
Changelog:
[General][Fixed] Cache ScrollView content length before calling `scrollToIndex`
Reviewed By: lenaic
Differential Revision: D47978635
fbshipit-source-id: 27f2a4702650e8a73e8812128821ca03f36216dd
Summary:
`butter::small_vector` is backed by a `folly:small_vector` on non-Android platforms in release mode.
It uses the small vector optimization, keeping a fixed size buffer, then falling back to heap allocation.
When I was debugging through props parsing code to figure out a prop not getting passed to Fabric (was being filtered in ViewConfig), I noticed this vector is filled with 199 elements for ViewProps. So It always overflows the current capacity and falls back to heap allocation. This bumps the capacity basde on what I observed with some more headroom.
Changelog:
[Internal]
Reviewed By: sammy-SC
Differential Revision: D47981105
fbshipit-source-id: 3870d8e4fee9748c487d01cc82284de865ea370d
Summary:
This PR splits the build of Hermes for iOS in multiple jobs.
Before, we were building all the slices of Hermes serially. So, we were:
- building the hermesc
- building hermes for iPhone
- building hermes for iPhonesimulator
- building hermes for Macos
- building hermes for Catalyst
- packaging the framework
This job was taking up to 45-50 minutes.
The the four slices (iPhone, iPhonesimulator, Macos, Catalyst) can be parallelized to harvest a speedup in execution.
The following tables contains the executions before and after this change.
- Full Clean Build -> Before: 51' 35" | After: 17'33" ( 3x improvement)
| BEFORE | AFTER |
| --- | --- |
| <img width="1164" alt="Screenshot 2023-07-28 at 11 48 24" src="https://github.com/facebook/react-native/assets/11162307/49cc519c-16f0-4868-b847-602b1cb21f3e"> | <img width="1120" alt="Screenshot 2023-07-28 at 11 16 32" src="https://github.com/facebook/react-native/assets/11162307/85034cd7-751e-4056-ae4f-ed09ac8343e8"> |
| Total time (critical path): `build_hermes_macos-Debug` = 51' 35" | Total time (critical path): `build_hermesc_apple` (2' 56") + `build_apple_slices_hermes-Debug-macosx` (9'23") + `build_hermes_macos-Debug` (5'14") = 17'33" |
- Fully Cached Build -> Before: 4'35" | After: 32" ( 9x improvement)
| BEFORE | AFTER |
| --- | --- |
| <img width="497" alt="Screenshot 2023-07-28 at 14 38 12" src="https://github.com/facebook/react-native/assets/11162307/978eba4d-3524-45ab-bfa5-d9cb9ba63df1"> | <img width="1099" alt="Screenshot 2023-07-28 at 16 12 17" src="https://github.com/facebook/react-native/assets/11162307/f2a8f0bb-545c-4d6f-9b81-cda87151bb62"> |
| Total Time (critical path): `build_hermes_macos-Debug` (4'35") | Total Time (critical path): `build_hermesc_apple` (7") + `build_apple_slices_hermes-Debug-macosx` (7") + `build_hermes_macos-Debug` (32") = 46" |
## Changelog:
[Internal] - Split hermes build to speedup CI and Release
Pull Request resolved: https://github.com/facebook/react-native/pull/38619
Test Plan:
- CircleCI stays green
- Hermes artifact still works for RNTester and app from the template
Reviewed By: cortinico, dmytrorykun
Differential Revision: D47896833
Pulled By: cipolleschi
fbshipit-source-id: 3b9e8d5de9b2a6fb6671444fda09d77b96123ac2
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38749
changelog: [internal]
# Request for comment
I'll gate this and extend this to TextInput if we think this is appropriate solution.
## Problem
Paragraph and TextInput have custom measure functions. They outsource measurements to the host platform. On Android, this means going through JNI (expensive).
When we architected YogaLayoutableShadowNode, we made intentional decision to dirty those nodes on every clone. This was to lower the complexity.
## Solution
One possible to solution is to just check if children, props or textAttributes have changed. If not, nothing has affected the layout and it is safe to mark the node as clean.
Reviewed By: NickGerleman
Differential Revision: D47914641
fbshipit-source-id: ab448fa18599eb2266984eba9f5d935caad1caed
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/38748
changelog: [internal]
Android was setting flag `swapLeftAndRightInRTL` to true regardless if the context was RTL or LTR. This causes unnecessary tree traversal + invalidation of all yoga nodes. This is a completely unnecessary work when layout direction is left to right.
To fix this, I made sure Android not longer sets `swapLeftAndRightInRTL` to true and in Fabric we only check the flag for RTL context.
Reviewed By: NickGerleman
Differential Revision: D47913605
fbshipit-source-id: 1d938b0dc9ba16a73b076f626055055162e3495f
Nicola Corti
Moti Zilberman
szymonrybczak
Kyaw Thura
Intl Scheduler
Sam Zhou
Pieter Vanderwerff
Nick Gerleman
Facebook Community BotandFacebook Community Bot
Riccardo Cipolleschi
Ruslan Shestopalyuk
Michał Mąka
Alex Hunt
Ruslan Lesiutin
Riccardo Cipolleschi
Luiz Ozorio
Arushi Kesarwani
Vicky Ahuja
Guil Varandas
Samuel Susla