Summary:
Update React Native on iOS to do less manual memory management, by replacing uses of `CGColorRef` with `UIColor`, and only calling `UIColor.CGColor` when needed. This results in much less manual memory management, which is probably a good thing in 2024 :D. The downside is there is a breaking change: the signature of a method in `RCTBorderDrawing` changes.
This is a followup to https://github.com/facebook/react-native/issues/46797 . After that PR merged and I tested merging React Native macOS to the `0.76-stable` branch cut commit again, I saw even more places I needed to manually call `CGColorRetain` / `CGColorRelease`. The reason is due to React Native macOS specifics (explained below) and I could update React Native macOS to not need these changes, but I thought I would at least throw up a PR to propose the changes, as it may be good to move away from using Core Graphics' C base API as much as possible.
## Longer Explanation
With https://github.com/microsoft/react-native-macos/pull/2209 , I wrote a shim of [UIGraphicsImageRenderer](https://developer.apple.com/documentation/uikit/uigraphicsimagerenderer) for macOS. The main difference is my shim calls the NSImage API [imageWithSize:flipped:drawingHandler:](https://developer.apple.com/documentation/appkit/nsimage/1519860-imagewithsize?language=objc) to shim the UIGraphicsImageRenderer API [imageWithData:](https://developer.apple.com/documentation/uikit/uiimage/1624137-imagewithdata). The difference between the two is that the macOS API copies the block, and executes it when Appkit is about to draw the image, while the iOS API executes the block right away. Because of this, I think I am hitting way more places where `CGColorRef` variables needed to be retained / released. Additionally, I hit more of them when I merge to the 0.76 branch cut commit (this is why I didn't catch it with https://github.com/facebook/react-native/issues/46797).
Given this constraint, I have a couple of options:
1. Refactor my macOS shim to use the deprecated API [[NSImage lockFocus]](https://developer.apple.com/documentation/appkit/nsimage/1519891-lockfocus)
- I am not a fan of this because `lockFocus` was deprecated for `imageWithSize:flipped:drawingHandler:`
2. Refactor my macOS shim to do what we used to do: Create a CGContext manually and write it to an image
- This is probably OK. Relies on less Appkit specifics, and potentially gives more control to RN on the rendering layer, which I recall lenaic told me is better for Fabric)
3. Refactor React Native to avoid CGColorRef altogether, and use `UIColor` (which ARC will memory manage for us) as much as possible.
- This is the approach of this PR and my preferred approach. The downside is this changes the signature of some of the methods in `RCTBorderDrawing` which is a breaking change. I've seen other PRs do this, so maybe its OK?
## Changelog:
[IOS] [BREAKING] - Replace uses of `CGColorRef` with UIColor to avoid manual memory management
Pull Request resolved: https://github.com/facebook/react-native/pull/46847
Test Plan: Launching RNTester's View example (which tests a lot of the border / outline / shadow rendering) does not crash for me on both iOS and macOS.
Reviewed By: NickGerleman
Differential Revision: D63989547
Pulled By: joevilches
fbshipit-source-id: 5e85e17567e3dd8b4b0388452398954ad2e02464
Summary:
Windows has a stricter set of rules for C++ files where we treat warning as error, the following files have fixes to correctly cast numbers and fix control path errors.
Control Path Error on React-Native-Windows repo:
```
1>react-native-windows\node_modules\react-native\ReactCommon\react\performance\timeline\PerformanceEntryReporter.h(138,1): warning C4715: 'facebook::react::PerformanceEntryReporter::getBufferRef': not all control paths return a value
1>react-native-windows\node_modules\react-native\ReactCommon\react\performance\timeline\PerformanceEntryReporter.h(154,1): warning C4715: 'facebook::react::PerformanceEntryReporter::getBuffer': not all control paths return a value
```
Apply these fixes will allow us to unfork these files in our repo :)
## Changelog:
[GENERAL] [FIXED] - Fix cast and control paths errors on windows
Pull Request resolved: https://github.com/facebook/react-native/pull/46899
Test Plan: Tested on the RNW repo, these files are already forked with the respective fix
Reviewed By: cipolleschi
Differential Revision: D64086963
Pulled By: arushikesarwani94
fbshipit-source-id: fdad72dafa1a01c426536fc1b007dd4a83588d93
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46809
BaseViewManagerInterface isn't adding much value right now. It was added in D16984121 to allow codegen generated ViewManager delegates to apply to view managers which derive from ViewMangager instead of BaseViewManager (if they did some cleverness, to make VM delegate apply to a no-op class, still implementing all of BaseViewManager's methods).
All of the cases where that was used have since been moved to `SimpleViewManager`, and `BaseViewManagerAdapter` (needed to wire this together) doesn't exist anymore, so it's not possible to take any advantage of this interface existing. We should remove it, since its existence is a source of error (e.g. it was missing setters for `accessibilityValue` or those related to pointer events), and is more generally confusing for anyone adding to `BaseViewManager` in the future.
This is a breaking change, because there are some libraries which vendor a copy of generated ViewManagerDelegate when building against legacy arch to be able to share code normally generated at build time. That means these will need to be updated to maintain compatibility with RN versions of 0.77+ with new arch disabled. This will not effect compatibility of these libraries against the default new arch, and the updated delegate is still compatible with older RN version.
```
sourceSets.main {
java {
if (!isNewArchitectureEnabled()) {
srcDirs += [
"src/paper/java",
]
}
}
}
```
1. `react-native-picker/picker`
2. `rnmapbox/maps`
3. `react-native-gesture-handler`
4. `react-native-screens`
5. `react-native-svg`
6. `react-native-safe-area-context`
7. `react-native-pdf`
Changelog:
[Android][Breaking] - Remove BaseViewManagerInterface
Reviewed By: cortinico
Differential Revision: D63819044
fbshipit-source-id: 7e4935c8e43706b168f0f599a6676e8abfa66937
Summary:
Part of this: https://github.com/facebook/react-native/issues/46757
Solves:
ME2E0019
## Changelog:
[ Internal ] [ Added ] - Added a test case for Image
<!-- 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/46831
Test Plan:
yarn e2e-test-ios
yarn e2e-test-android
Reviewed By: cortinico
Differential Revision: D63889719
Pulled By: cipolleschi
fbshipit-source-id: 674879066ec159f11894f24935fe39a86ea1ea63
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46896
On Android Paper UIManager, when calling `ReactRootView.unmountReactApplication`, the ReactRootView tag is unset [here](https://github.com/facebook/react-native/blob/main/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java#L926), before the round trip unmount operation to JS makes it's way back to a `dropView` call on `NativeViewHierarchyManager`.
In practice, this means that legacy architecture apps that unmount surfaces via `ReactRootView.unmountReactApplication` leak references to Views, and the "finalization" step (`onDropViewInstance`) is not universally called.
This is an attempt to fix the issue by skipping the `clearReactRoot` step on Paper, instead waiting for the round trip `UIManager.removeRootView` call.
## Changelog
[Android][Fixed] Fix issue where `onDropViewInstance` cleanup was not being handled after `ReactRootView.unmountReactApplication`
Reviewed By: javache
Differential Revision: D64054042
fbshipit-source-id: b8b8c237796674ca23a332e57a1bf2e07ab5af13
Summary:
While the new `installJSIBindings(..)`/`BindingsInstaller` functionality allows you to synchronously set up stuff in the JS runtime before JS runs, there is no access to the JS CallInvoker, meaning you cannot really set up anything that uses callbacks or asynchronous code.
[Nitro](https://github.com/mrousavy/nitro) needs this.
## 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
-->
[IOS] [ADDED] - Add `CallInvoker` to `installJSIBindings(..)`
[ANDROID] [ADDED] - Add `CallInvoker` to `BindingsInstaller`
Pull Request resolved: https://github.com/facebook/react-native/pull/46851
Test Plan: In `SampleTurboModuleJSIBindings`, we can now use the `CallInvoker`.
Reviewed By: rshest
Differential Revision: D63975546
Pulled By: javache
fbshipit-source-id: 43cd469de2c35581f5362d2500fc27e41d6eab72
Summary:
For users who may have node installed in a path with a space, this requires escaping. For example:
```
NODE_BINARY=/Users/blakef/Library/Application Support/fnm/node-versions/v20.12.0/installation/bin/node
```
Needs to be:
```
NODE_BINARY=/Users/blakef/Library/Application\ Support/fnm/node-versions/v20.12.0/installation/bin/node
```
# Changelog
[iOS][Fixed] Generated NODE_BINARY in .xcode.env.local now supports paths with a space
Reviewed By: cipolleschi
Differential Revision: D64080118
fbshipit-source-id: 1045473e4fd284fc570fa538984618630be1af6d
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46887
Changelog: [internal]
This is a small change to use CommonJS in `ReactNativeFeatureFlags.config.js` so the file can be easily imported from Node.js.
Reviewed By: jorge-cab
Differential Revision: D64039860
fbshipit-source-id: c84ddbdbfe942224e213d12b9496c41b73dd5731
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46817
As I've cleaned up all the RN Tester warnings, I'm enable warningAsErrors for RNTester to make
sure we keep up with our warning count.
Changelog:
[Internal] [Changed] - Enable warningAsErrors for RN-Tester
Reviewed By: cipolleschi
Differential Revision: D63837633
fbshipit-source-id: f83273dadc7aa10ce7ae52ae790279819fb88345
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46890
Add details as this is discussed in the Turbo Native Modules documentation going out in 0.76:
```
Usage: combine-js-to-schema-cli.js <outfile> <file1> [<file2> ...]
Options:
--help Show help [boolean]
--version Show version number [boolean]
-p, --platform Platforms to generate schema for, this works on filenames:
<filename>[.<platform>].(js|tsx?) [default: null]
-e, --exclude Regular expression to exclude files from schema generation
[default: null]
```
## Changelog:
[General][Added] Add cli --help details to combine-js-toschema-cli.js
Reviewed By: dmytrorykun
Differential Revision: D64045337
fbshipit-source-id: a4b977da2cbb0198a5d43f17ca3466ebde21e9a9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46873
## Summary
Adds a small showcase for the different `resizeMethod` options and how downsampling is applied
## Changelog
[Android][Added] - Added showcase for Android `resizeMethod` options when applied to a large image
Reviewed By: rshest
Differential Revision: D62393212
fbshipit-source-id: 46e7ff6310617acb9eb288e3831c0b5e6b1751c8
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46866
## Summary
Adds a new `resizeMethod` called `none`. It disables downsampling on the image request and disregards the global image pipeline default. This has been a pain point raised when rendering large images on Android in issues like [this one](https://github.com/facebook/react-native/issues/21301) and [this one](https://github.com/facebook/fresco/issues/2397).
## Changelog
[Android][Added] - Adds a new `resizeMethod`, `none`, which disables downsampling for an image
Reviewed By: yungsters
Differential Revision: D62393211
fbshipit-source-id: d85e3ed098ad502c8edbdfa817c841271ee9e914
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46891
This was set but never directly used - fix it.
Changelog:
[iOS][Fixed] Removed usage of set but unused variable.
Reviewed By: mg-here
Differential Revision: D64029449
fbshipit-source-id: 0d60428317de95aa235f249c512fa968aa04f879
Summary:
This diff reverts D63643856
`_highestMeasuredFrameIndex` is not properly invalidated causing issues with previous logic when data size shrinks.
bypass-github-export-checks
Changelog:
[General][Changed] - Revert "Fix onEndReached not being called when getItemLayout is present and we scroll past render window"
Reviewed By: NickGerleman
Differential Revision: D64009287
fbshipit-source-id: 8a21b57f5247fc743e65f9a730ff33a9a89d2bc1
Summary:
`FpsDebugFrameCallback.getTotalTimeMS()` implementation loses accuracy due to incorrect order of type casting to int
## Changelog:
[ANDROID] [CHANGED] - Improve FpsDebugFrameCallback.getTotalTimeMS() accuracy
Pull Request resolved: https://github.com/facebook/react-native/pull/46869
Test Plan:
- Launch some test app
- Open up the [Dev Menu](https://reactnative.dev/docs/debugging#accessing-the-dev-menu) in your app and toggle Show Perf Monitor
- Compare results of current & improved implementations
Reviewed By: rshest
Differential Revision: D64024054
Pulled By: arushikesarwani94
fbshipit-source-id: 438792bace0e3443d151cf13364f6e94b66dfb5e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46878
Removes an unused code path from `ResourceDrawableIdHelper`. Vector drawables are now loaded via Fresco decoder so we don't need to offer this utility anymore. While part of the public API, it only made it into 0.76.0 RCs and should have really been marked internal.
Changelog: [Internal]
Reviewed By: rshest
Differential Revision: D64021956
fbshipit-source-id: 6e8073e0f7f079d023fa1e45a5a43f8652ca2826
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46871
changelog: [internal]
Original commit changeset: 748efce7f0b8
Original Phabricator Diff: D59859754
This is temporarily until we find an actual solution that can both cache the colors and also react to theme changes updating the colors value properly.
Reviewed By: javache
Differential Revision: D64002346
fbshipit-source-id: 9ba3bd0d653b9a0d8617f45d42c0c3d966fba01f
Summary:
Modifying the CxxReactPackage by changing the annotation from UnstableReactNativeAPI to FrameworkAPI so as to now support custom C++ TM wrapper
Changelog:
[Android][Changed] - Modify CxxReactPackage to support custom C++ TM wrapper
Reviewed By: shwanton, christophpurrer
Differential Revision: D63740964
fbshipit-source-id: 175b1104dcd11674413199b30415516d4d324a97
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46845
Previously, the parser was throwing away the values of a string union when storing it in the schema. It would only store the union as
```
{
type: 'UnionTypeAnnotation',
memberType: 'StringTypeAnnotation'
}
```
We now track the string literals through the parser and store them in the schema:
```
{
type: 'StringLiteralUnionTypeAnnotation',
types: [
{
type: 'StringLiteralTypeAnnotation'
value: 'light'
},
{
type: 'StringLiteralTypeAnnotation'
value: 'dark'
},
],
}
```
We aren't changing the generators, those still just output "string". They could eventually be made smarter.
The value of this is that the compat checker can now error when the union changes.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D63917685
fbshipit-source-id: 34a10e1f1910d2935837a3659f66049fd4473134
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46867
In multi-root Android React Native apps (e.g., multiple ReactFragments), if the following sequence occurs:
1. a Modal is displayed via the secondary root
2. the secondary root is destroyed
3. the app is backgrounded
4. the app is foregrounded
The LifecycleEventListener on the ReactModalHostView will fire, causing the Modal to be rehydrated in the onHostResume callback.
Removing the lifecycle event listener when the modal is detached from the window resolves the issue.
## Changelog
[Android][Fixed] Fix issues with Modals and lifecycle events in multi-surface apps
Reviewed By: alanleedev
Differential Revision: D64001103
fbshipit-source-id: 10f8304cd9cca0d5c90e39f5e361290f4fc35283
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46864
## Summary
Fresco v3.3.0 contains necessary adjustments to allow downsample overrides per image request (see https://github.com/facebook/fresco/pull/2787), as well as numerous other bug fixes and adjustments across both Drawee and Vito images.
## Changelog
[Android][Changed] - Update Fresco from 3.2.0 to 3.3.0
Reviewed By: oprisnik
Differential Revision: D63983051
fbshipit-source-id: 9ad0c40dfc864d24ffa39ca9e2bc0ddf8210f4f7
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46827
You can now define a native module that takes an argument that is a specific string literal:
Like this:
```
export interface Spec extends TurboModule {
+passString: (arg: string) => void;
+passStringLiteral: (arg: 'A String Literal') => void;
}
```
On the native side, this will still generate `string`, but the schema will now store the string literal, and it will be allowed in JS. This should allow more strict flow / typescript types for modules.
This will also allow the compatibility check to fail if the literal changes.
This is a step towards accepting a union of string literals.
Changelog: [General][Added] Codegen for Native Modules now supports string literals
Reviewed By: GijsWeterings
Differential Revision: D63872440
fbshipit-source-id: e54b97d34af4a3d1af51727db0777f26fb7b778c
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46764
This is a patched fix to earlier attempt (D63700769) and resolving crash due to`java.lang.IllegalArgumentException: View=DecorView@b9f88af[AdsManagerActivity] not attached to window manager`.
- removing ` !dialogWindow.isActive` check as it is always true resulting in always doing early return causing other bugs
- adding try/ catch instead so the code can still run but can catch the known exception without crashing
Changelog:
[Android][Fixed] - Fix crash for Modal not attached to window manager
Reviewed By: mdvacca
Differential Revision: D63712422
fbshipit-source-id: 85fb6df340eb1139f954c92b5f1daf0dc41671d2
Summary:
React Native [app template provided by the CLI](https://github.com/react-native-community/template) currently uses [`metro-config` directly for `MetroConfig` type](https://github.com/react-native-community/template/blob/main/template/metro.config.js#L7).
However, it doesn't have `metro-config` as neither a dependency or dev dependency, which can lead to version mismatches.
While this is obviously a mistake on the template repo side, `metro-config` versions aren't matched with `react-native` versions. Therefore, getting the correct version of `metro-config` from `react-native/metro-config` would require reflecting on `react-native/metro-config`'s package.json etc. which is far from ideal. In my opinion it's would be much better to expose `MetroConfig` type from `react-native/metro-config` directly.
Version mismatching can happen in a monorepo setup. Say we have the monorepo structure using Yarn Modern:
```tree
.
├── RN75-app (workspace)
├── RN76-app (workspace)
│ ├── metro.config.js
│ └── node_modules
│ └── react-native
│ └── metro-config (0.76)
│ └── node_modules
│ └── metro-config (version for 0.76)
└── node_modules
├── react-native
│ └── metro-config (0.75)
└── metro-config (version for 0.75)
```
`react-native@0.75` gets hoisted to the monorepo root while `react-native@0.76` sits in an RN 0.76 app workspace.
Say we have the following `RN76-app/metro.config.js` contents:
```js
const {getDefaultConfig, mergeConfig} = require('react-native/metro-config');
/**
* Metro configuration
* https://reactnative.dev/docs/metro
*
* type {import('metro-config').MetroConfig}
*/
const config = {};
module.exports = mergeConfig(getDefaultConfig(__dirname), config);
```
In this case, `require('react-native/metro-config')` would resolve to `RN76-app/node_modules/react-native/node_modules/metro-config` since `react-native/metro-config` is a (dev) dependency of the App.
However `import('metro-config).MetroConfig` would resolve to `node_modules/metro-config` since it's not a direct dependency.
This is how we have a mismatch - imported functions come from different packages than imported type.
## Changelog:
[GENERAL] [ADDED] - Expose `MetroConfig` type directly from `react-native/metro-config`.
Pull Request resolved: https://github.com/facebook/react-native/pull/46602
Test Plan:
`yarn build` to generate dist for `react-native/metro-config`, see it has the export of `MetroConfig`.
## Notes
If this PR gets approved, I'll submit relevant one to the CLI template.
Reviewed By: huntie
Differential Revision: D63258881
Pulled By: robhogan
fbshipit-source-id: e6f3c880eb4a0aa902c62932d58f243c38b07c2e
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46824
Changelog: [Internal]
Removing the implementation of layout-only shadow node reference updates, since the full version of shadow node reference updates are being released.
Reviewed By: shwanton
Differential Revision: D63831453
fbshipit-source-id: 84cb231be6c16bdc4a4b12a559c41b5a77064813
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46777
Simplifies this internal logic plus ensures there is one source of truth for `reactNativeVersion` in our CLI plugin. Also, renames the `ctx` variable to `cliConfig`, to more clearly signal that this is an integration point with the Community CLI.
Completes this wave of `community-cli-plugin` simplifications.
Changelog: [Internal]
Reviewed By: robhogan
Differential Revision: D63534247
fbshipit-source-id: bf5779fcf971dee1e0caf58b071df360636c53e9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46839
The `ReactNativeFlipper` object is not referenced in the template since 0.75. We can safely remove it as users should not reference it anymore.
Changelog:
[Android] [Breaking] - Remove ReactNativeFlipper object, deprecated in 0.75
Reviewed By: huntie
Differential Revision: D63897187
fbshipit-source-id: a7d73a98550518e46f8dbc0c9b859e0afb16e8a9
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46854
When segment fetch fails, it seems incorrect to call the callback twice: once with error, and once with null.
This seems like a bug.
Created from CodeHub with https://fburl.com/edit-in-codehub
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D63959015
fbshipit-source-id: f3ebb0f4732d99d91b5717a5bf193f44d957bde4
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/46846
As of D61658739, BaseViewManager setting color now goes through BackgroundStyleApplicator, which gives a default implementation of setting a background color, while storing information in a way where we can impement things like border radii and shadows on background for out of the box views.
I knew this could lead to breaks where we previously overrode view-level `setBackgroundColor` to do something custom, but didn't override on VM level, but didn't see any external usages so I assumed it should be relatively safe. Turns out we have some internal usages which run into this pattern (D63913128 already fixed one), including a usage in RN itself! Let's override the view managers in these to delegate to the view's custom drawing.
Changelog:
[Android][Fixed] - Fix some cases where we override setBackgroundColor on View-level instead of VM level
Reviewed By: Abbondanzo
Differential Revision: D63922722
fbshipit-source-id: af988d1436c790be97b2be1325541aa418bf43a3
Saad Najmi
Tatiana Kapos
Nick Gerleman
Fabrizio Cucci
Deepanshu.shukla
sarthak-d11
Eric Rozell
Marc Rousavy
Blake Friedman
Rubén Norte
Nicola Corti
Panos Vekris
Peter Abbondanzo
Arushi Kesarwani
Arushi Kesarwani
David Vacca
Nikita Lutsenko
generatedunixname89002005232357
Anatoliy Magda
Sam Zhou
Thibault Malbranche
Riccardo Cipolleschi
Alonso Paulino
Eli White
Alan Lee
Thomas Nardone
Vitali Zaidman
Tomasz Żelawski
Nick Lefever
Alex Hunt
Ramanpreet Nara