Summary:
Added basic hook to enable image instrumentation. The hook passes information to the existing image loader, where instrumentation is done, specific for each app, if any.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19047899
fbshipit-source-id: 6c013806cce98bcf1ea240d696a7ede9697e5cd9
Summary:
The `RCTImageURLLoaderWithAttribution` protocol historically only returns a cancellation block to cancel the url request. But for more complex instrumentation, we may need to associate a requestId for the specific URL request. To do this, the protocol now returns an object that has both the unique ID and the cancellation block, so that instrumentation logic can refer to the ID in the future.
Note that the `RCTImageURLLoader` protocol is unchanged, because the request ID is only relevant for instrumentation purpose.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D19047900
fbshipit-source-id: cd029f2470c32fc7bffd674b09a5353fe1dbc80b
Summary:
`RCTImageLoader ` needs loaders & decoders. These can be passed in through init, or looked up at runtime using the bridge. In bridgeless mode, we need to pass them in through init.
This diff just moves the assert to actually protect `_bridge` usage, since there exist other codepaths which don't use the bridge.
Changelog: [iOS][Internal] Move RCTImageLoader asserts to protect bridge usage
Reviewed By: RSNara
Differential Revision: D18990760
fbshipit-source-id: 285008e7b3f473c45381ab472c3d86e5d8e218ae
Summary:
Introduced 2 helper functions to toggle image instrumentation/logging (not in this diff) so that gating check is more efficient and easy to access across RN iOS core.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D18597208
fbshipit-source-id: 0b22031802ab020b16d6fb63e52461cf80a37ab5
Summary:
We are rolling out exact-by-default syntax to xplat/js.
I had to manually move around some comments to preserve proper placement.
Changelog: [Internal]
Reviewed By: jbrown215
Differential Revision: D18633611
fbshipit-source-id: 48f7468dcc55b1d00985419d035a61c6820b3abe
Summary:
There was already a ImageViewNativeComponent used on Android, so I changed `Image.ios.js` to use it as well, and then switched that component to use `codegenNativeComponent` instead of `requireNativeComponent` so that it gets the view config from JS instead of the UIManager.
I'm gating this change behind `RN$Bridgeless` so this only affects bridgeless mode.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18575533
fbshipit-source-id: aa71beb6be65dbb48ad2e1ca748f2cccd72a0e73
Summary:
Changelog: [Internal]
For image loading, the loader may need to inspect the imageView itself, so let's pass its reactTag. This only works pre-Fabric.
Reviewed By: sammy-SC
Differential Revision: D18522073
fbshipit-source-id: 400fc7e04b8d7852f6ecef941dd446fbff423c88
Summary:
Changelog: [Internal]
The main use-case here is to get the rootTag off RCTImageView, for image loading instrumentation. The fact is, each RCTView subclass already has `reactTag` attached today. We already have the `rootTag` when the view is created by the UIManager, so why not just attach it like reactTag? If we don't, looking up the rootTag from the native component is non-trivial and extremely inefficient (have to jump to shadow queue, back to main queue, etc).
Reviewed By: sammy-SC
Differential Revision: D18497002
fbshipit-source-id: 8409e3a1c95e09accedd959592cbf178fab0b2c3
Summary:
Changelog: [iOS] [Changed] - New internal image attribution support, but files importing RCTImageLoader.h must be converted to ObjC++
This new interface is the same as RCTImageURLLoader, but with additional support to pass in optional attribution information. The attribution info is not strictly defined (we may do so in the future though), and it's up to the hosting application and RCTImageURLLoader classes to handle it.
Reviewed By: sammy-SC
Differential Revision: D18492882
fbshipit-source-id: c3870c60e6c2e7c65758fc3235ebf5db369e07dc
Summary:
It turns out the ImageLoader native module has different method signatures on iOS than on Android, so the JS spec we currently have won't work for ANdroid. In this diff I'm splitting up the spec for NativeImageLoader into an Android & iOS versions (similar to PlatformConstants), and updating the Android spec to match the native implementation. I'm also changing `RCTImageLoader` to use the new generated spec, and updating the JS callers (`Image.android.js` and `Image.ios.js`) to use the right one for the platform (instead of importing the untyped `ImageLoader` native module from `react-native`, like we were on Android :-/).
This will be a breaking change for anyone who's directly using `NativeImageLoader.js`, but I think most callsites should be using the `Image` component instead.
Changelog: [General] [Changed] Split NativeImageLoader into NativeImageLoaderAndroid and NativeImageLoaderIOS
Reviewed By: RSNara
Differential Revision: D18439538
fbshipit-source-id: 94c796d3fd27800ea17053e963bee51aca921718
Summary:
Changelog: [Internal]
Reverting the import to the previous local module style since importing from react-native seems to introduce some perf regression. We'll revisit this later in the future.
Reviewed By: yungsters
Differential Revision: D18383893
fbshipit-source-id: f11d46a4545768f39199fd6fd22fcf14905d0a74
Summary:
Changelog: [Internal]
Moved the imports for `TurboModuleRegistry` and `TurboModule` from `react-native`. This was a jscodeshift with the script: P120688078
Reviewed By: yungsters
Differential Revision: D18262538
fbshipit-source-id: 48fac15229c897408928511c5ecbb42f17ec7b42
Summary:
I recently learned that CoreFoundation object lifecycle is not managed automatically by ARC.
RN appears to be leaking a few refs, this small stack of diffs cleans them up.
Changelog: [Internal][Fixed] Fixed memory management of CF objects in RCTImageStoreManager
Reviewed By: fkgozali
Differential Revision: D18308313
fbshipit-source-id: 35c1152753578825871c28e1070599cd409b3a34
Summary:
A very common pattern I've seen in RN codebase:
- (instancetype) init {
[[NSNotificationCenter defaultCenter] addObserver:self ...]
}
- (void) dealloc {
[[NSNotificationCenter defaultCenter] removeObserver:self ...]
}
From Apple:
https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver?language=objc
> If your app targets iOS 9.0 and later or macOS 10.11 and later, you don't need to unregister an observer in its dealloc method.
RN targets iOS9+
Changelog: [Internal][Cleanup] Remove unneeded NSNotification center removeObserver
Reviewed By: shergin
Differential Revision: D18264235
fbshipit-source-id: 684e5f5555cec96b055b13cd83daaeb393f4fac9
Summary:
I kept on running `USE_FRAMEWORKS=1 update-pods && open RNTesterPods.xcworkspace` and adding missing dependencies until `RNTesterPods` started compiling without failure.
**Note:** I made sure to only commit the podfile changes from `update-pods`, **without** `USE_FRAMEWORKS=1`.
Changelog:
[iOS][Fixed] - Fix all RN Podspecs
Reviewed By: fkgozali
Differential Revision: D18284535
fbshipit-source-id: 44d288ae0e52dd2cbbe26bebe7df73ce05644b5d
Summary:
All struct args are passed into NativeModule methods via references. If blocks access those references, we don't move those references to the heap. This means that by the time that the block accesses the struct arg, it could be freed. This can crash the program.
The solution is simple: we copy the struct arg, and access the copy in the block. This ensures that the block will make a copy, which prevents the underlying data structures from being released by the time that the block accesses the struct arg.
Changelog:
[iOS][Fixed] - Retain cropData struct arg in ImageEditingManager.cropImage call
Differential Revision: D18076026
fbshipit-source-id: 1a7bb602606ff1afac38ad5451662c82fa86f205
Summary:
Changelog: [iOS][internal] - Use ImageLoader's `queryCache` in image.ios.js. See D17714521 for native definition
Background:
ImageViewManager is a ViewManager. But we are loading and calling this method as if it is a NativeModule
```
const {ImageViewManager} = NativeModules.
```
This change calls the new method on the actual native module instead of the view manager. This is consistent with the existing behavior on android.
Reviewed By: zackargyle, TheSavior
Differential Revision: D17714520
fbshipit-source-id: b67a62d880978d6b94228cbff8b7e49c54dc44cc
Summary:
Changelog: - [iOS][Internal] Use ImageLoader's `prefetchImage` in image.ios.js. See D17714519 for native definition
Background:
ImageViewManager is a ViewManager. But we are loading and calling this method as if it is a NativeModule
```
const {ImageViewManager} = NativeModules.
```
This change calls the new method on the actual native module instead of the view manager. This is consistent with the existing behavior on android.
Reviewed By: zackargyle, TheSavior
Differential Revision: D17704732
fbshipit-source-id: 510cb8f4a5609258414d3e9d98f57b50e80305c5
Summary:
Changelog: - [iOS][Internal] Use ImageLoader's `getSizeWithHeaders` in image.ios.js. See D17693907 for native definition
Background:
ImageViewManager is a ViewManager. But we are loading and calling this method as if it is a NativeModule
```
const {ImageViewManager} = NativeModules.
```
This change calls the new method on the actual native module instead of the view manager. This is consistent with the existing behavior on android.
Reviewed By: JoshuaGross, TheSavior
Differential Revision: D17704091
fbshipit-source-id: 916ae82fd6f302532f04c1fa590eed8cdd5f63a8
Summary:
Changelog: [Internal][iOS] Use ImageLoader's `getSize` in image.ios.js. See D17607364 for native definition
Background:
ImageViewManager is a ViewManager. But we are loading and calling this method as if it is a NativeModule
```
const {ImageViewManager} = NativeModules.
```
This change calls the new method on the actual native module instead of the view manager. This is consistent with the existing behavior on android.
Reviewed By: TheSavior
Differential Revision: D17607363
fbshipit-source-id: 771e60c54d2c311dee8647ea341a530302895a85
Summary:
**Note:** The specs for these NativeModules live within FBInternal. I just made `fbsource//xplat/js:FBReactNativeSpec` depend on the internal specs.
Changelog: [iOS][Added] Make RCTImageEditingManager and RCTImageStoreManager TurboModule-compatible
Reviewed By: shergin
Differential Revision: D17969820
fbshipit-source-id: c02bdb2e6e62ead98c64c49956d58ca80449892f
Summary:
Couldn't make RCTImageEditingManager and RCTImageStoreManager TurboModule-compatible because their specs live in fb-internal code. I will tackle them in a subsequent diff. See T54946472.
Changelog: [iOS][Added] Make RCTLocalAssetImageLoader and RCTGIFImageDecoder TurboModule-compatible
Reviewed By: PeteTheHeat
Differential Revision: D17936483
fbshipit-source-id: 2266c9ea1ca7ecd52717d9a963e39245da312312
Summary:
In D16805827, I moved `RCTImageLoader`, `RCTImageStoreManager`, and `RCTImageEditingManager` to `CoreModules`. This was necessary to turn `RCTImageLoader` into a TurboModule. However, after D17671288 landed, it's no longer necessary to have OSS NativeModules in `CoreModules`. Therefore, I'm moving these NativeModules back to `RCTImage`.
Changelog: [iOS][Fixed] Move RCTImage NativeModules back to RCTImage
Reviewed By: shergin
Differential Revision: D17921612
fbshipit-source-id: 8ae36d2dc8deaf704313cbe2479bfa011ebcbfbc
Summary:
Some of our NativeModule type specs aren't compatible with our Android codegen and type safety checks - specifically, we don't support `$ReadOnly` in our type checks, and we don't support map types in the codegen. Removing these from a couple of the OSS type specs so we can enable codegen for these modules (eventually).
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D17882093
fbshipit-source-id: 6e8669e4be775347199b2b5346bd8d40d7620886
Summary:
In iOS 13, Apple made a change that results in video URLs returned by UIImagePickerController becoming invalidated as soon as the info object from the delegate callback is released. This commit works around this issue by retaining these info objects by default and giving the application a way to release them once it is done processing the video.
See also https://stackoverflow.com/questions/57798968/didfinishpickingmediawithinfo-returns-different-url-in-ios-13
Reviewed By: olegbl, mmmulani
Differential Revision: D17845889
fbshipit-source-id: 12d0e496508dafa2581ef12730f7537ef98c60e2
Summary: The proper weakproxy usage should be enabled by default from now on.
Reviewed By: PeteTheHeat
Differential Revision: D17866448
fbshipit-source-id: da404a41fd1136d7feebfc7591fa2965a65c4c6b
Summary: Couldn't make RCTImageEditingManager and RCTImageStoreManager TurboModule-compatible because their specs live in fb-internal code. I will tackle them in a subsequent diff. See T54946472.
Reviewed By: PeteTheHeat
Differential Revision: D17720574
fbshipit-source-id: 5d634da475522565f874020301de8e74ff73aa8d
Summary: In D16805827, I moved `RCTImageLoader`, `RCTImageStoreManager`, and `RCTImageEditingManager` to `CoreModules`. This was necessary to turn `RCTImageLoader` into a TurboModule. However, after D17671288 landed, it's no longer necessary to have OSS NativeModules in `CoreModules`. Therefore, I'm moving these NativeModules back to `RCTImage`.
Reviewed By: PeteTheHeat
Differential Revision: D17720575
fbshipit-source-id: 44b07cfa07cbb2b87254132810f86974edc7edab
Summary:
[iOS] [Added] - Add definition for `queryCache` in ImageLoader
This diff is related to moving RCTImageViewManager's commands to a native module, ImageLoader.
Reviewed By: zackargyle, TheSavior
Differential Revision: D17714521
fbshipit-source-id: 722cc17a2ebb03e72d7c080dfc4d0aa6d7440e85
Summary:
[iOS] [Added] - Add `prefetchImage` to ImageLoader native module.
This diff is related to moving RCTImageViewManager's commands to a native module, ImageLoader.
Reviewed By: zackargyle, TheSavior
Differential Revision: D17714519
fbshipit-source-id: 0a50f640cf0c5668a11dd5d40553c257ebbd9d2b
Summary:
Define getSizeWithHeaders in ImageLoader native module.
This diff is related to moving RCTImageViewManager's commands to a native module, ImageLoader.
See it's usage here: D17704091
Reviewed By: TheSavior
Differential Revision: D17693907
fbshipit-source-id: 3c2d7b19ac68ead831e780c4ee23e3ed0643be3a
Summary:
[iOS] [Fixed] - Fix how the amount of free memory is calculated to mimic the logic Apple uses.
For example, see https://opensource.apple.com/source/system_cmds/system_cmds-805.250.2/vm_stat.tproj/vm_stat.c.auto.html for how `vm_stat` does it:
```
sspstat("Pages free:", (uint64_t) (vm_stat.free_count - vm_stat.speculative_count));
```
Reviewed By: shergin
Differential Revision: D17671714
fbshipit-source-id: 18ef31e17a0527a9bef7a408922cd687260866db
Summary:
In React Native there are three types of "Native" components.
```
createReactClass with NativeMethodsMixin
```
```
class MyComponent extends ReactNative.NativeComponent
```
```
requireNativeComponent('RCTView')
```
The implementation for how to handle all three of these exists in the React Native Renderer. Refs attached to components created via these methods provide a set of functions such as
```
.measure
.measureInWindow
.measureLayout
.setNativeProps
```
These methods have been used for our core components in the repo to provide a consistent API. Many of the APIs in React Native require a `reactTag` to a host component. This is acquired by calling `findNodeHandle` with any component. `findNodeHandle` works with the first two approaches.
For a lot of our new Fabric APIs, we will require passing a ref to a HostComponent directly instead of relying on `findNodeHandle` to tunnel through the component tree as that behavior isn't safe with React concurrent mode.
The goal of this change is to enable us to differentiate between components created with `requireNativeComponent` and the other types. This will be needed to be able to safely type the new APIs.
For existing components that should support being a host component but need to use some JS behavior in a wrapper, they should use `forwardRef`. The majority of React Native's core components were migrated to use `forwardRef` last year. Components that can't use forwardRef will need to have a method like `getNativeRef()` to get access to the underlying host component ref.
Note, we will need follow up changes as well as changes to the React Renderer in the React repo to fully utilize this new type.
Changelog:
[Internal] Flow type to differentiate between HostComponent and NativeMethodsMixin and NativeComponent
Reviewed By: jbrown215
Differential Revision: D17551089
fbshipit-source-id: 7a30b4bb4323156c0b2465ca41fcd05f4315becf
Summary: Just need to validate the intended fix properly via simple gating mechanism.
Reviewed By: mmmulani
Differential Revision: D17536264
fbshipit-source-id: 92db4156beabd6dec2a71b6ea7c2d7bf708d44b1
Summary: Now that `ImageURISource` is a proper type, `nativeImageSource`'s return type should simply use that instead of being an untyped `Object`.
Reviewed By: cpojer
Differential Revision: D17246527
fbshipit-source-id: 6ec0c80a93b8794e6c243154875e3560ddacbc59
Summary:
Original commit changeset: 91eb08181f82
Original diff: D16940181
The original commit removed the gating logic to enable the "fix" to T48583301. However, v236 was burned, and v237+ no longer had this gating, making it impossible to measure the impact of the fix.
This diff reverted the original gating removal until we confirm the fix in prod. Note: this is to be picked to RC v238 (but will have merge conflict for sure, will send a separate diff for the branch).
Reviewed By: mmmulani
Differential Revision: D17251340
fbshipit-source-id: 359ac54aeb9c9e728c2735c688346a1f79ed2189
Summary:
@public
The mass majority of RCTUIImageViewAnimated uses are actually for static images. As such, we don't need to create a CADisplayLink.
Reviewed By: shergin
Differential Revision: D16945038
fbshipit-source-id: a7cb63000987d1ea7a8a9b4d596e1e474709d2ac
Summary:
To help determine how severe this issue is, put the fix behind a MC.
We will only pick the parent diff to the RC branch so that the fix immediately goes to master and we don't have to worry about fixing this any further.
Reviewed By: fkgozali
Differential Revision: D16940181
fbshipit-source-id: 91eb08181f82f51aea6a20b3fd489a33bdc0e424
Summary:
@public
CADisplayLink strongly holds onto its target, so you have to use a weak proxy object to pass the target into the CADisplayLink.
Previously we passed a weak-self point (i.e. weakSelf) but this did not have the intended effect, since the pointer to self would still be passed to CADisplayLink, and thus it would hold onto the RCTUIImageViewAnimated strongly.
So is weakSelf doing anything other than using self?
It is but it's very minor and not useful. In the case that the object got de-allocated between assigning self to weakSelf and creating the CADisplayLink, then we would pass a nil target. This is actually impossible though because we are running an instance method, so self is implicitly retained! So semantically it is something different but in practice it is the same as passing self through.
Notes:
* This system was added originally in https://github.com/facebook/react-native/pull/24822
* https://github.com/facebook/react-native/pull/25636 then "enabled" this system by deprecating existing approach
Reviewed By: fkgozali
Differential Revision: D16939869
fbshipit-source-id: 7a0e947896f23aa30ad074d1dcb4d4db7543e00a
Kevin Gozali
Peter Argany
Eli White
George Zahariev
Emily Janzer
Emily Janzer
Ramanpreet Nara
Adam Ernst
Luna Wei
Andres Suarez
Tim Yung
Albert Sun
Ashok Menon
Gunnar Kudrjavets
zhongwuzw
Logan Daniels
Mehdi Mulani
