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
Summary: In D16805827, I moved ImageLoader to CoreModules. In the process, I migrated usages of `[_bridge moduleForClass:[RCTImageLoader class]]` to `[_bridge moduleForName:@"ImageLoader"]`. These two APIs aren't equivalent, however, since `[_bridge moduleForClass:[RCTImageLoader class]]` by default lazily loads the requested NativeModule, but `[_bridge moduleForName:@"ImageLoader"]` doesn't. So, I had to explicitly set `lazilyLoadIfNecessary` to `YES` in all the call-sites I migrated, to ensure that ImageLoader is correctly initialized when necessary.
Reviewed By: PeteTheHeat
Differential Revision: D16948165
fbshipit-source-id: 434697637dfa5e32de1c398744f9c28c19a6fd94
Summary:
This diff adds a JS spec for RCTImageLoader, and conforms to it in ObjC++. Since RCTImageLoader isn't called from JS, the js spec is empty. Since `/CoreModules/` is the only dir in OSS which supports TM, move the ObjC++ impl there.
The change in `NativeExceptionsManager.js` fixes a weird bug I was hitting in codegen, where the codegen cpp file wouldn't compile due to unused variable.
Reviewed By: JoshuaGross
Differential Revision: D16495674
fbshipit-source-id: 191897b87730a6b0b96022eedc6412551fae04a6
Summary:
This diff moves RCTImageLoader, RCTImageEditingManager, and RCTImageStoreManager to CoreModules. This is necessary for us to convert all these NativeModules to TurboModules.
**Note:** As a part of this diff, I had to break apart `RCTImageLoader.h`. All the protocols that were in `RCTImageLoader` are now in their own headers. Furthermore, `RCTImageLoader`'s methods are defined in `RCTImageLoaderProtocol`, so that we can call them from classes like `RCTImageViewManager` in `RCTImage`.
Reviewed By: PeteTheHeat
Differential Revision: D16805827
fbshipit-source-id: 89f6728b0766c30b74e25f7af1be8e6b8a7e6397
Summary:
This function was used by Touchable*. It was removed from the Touchables in D6494579 in 2017. The only remaining callsite was ImageBackground which is attaching a ref directly to the View so we know it is a native component.
This is needed for some setNativeProps cleanup
Reviewed By: sahrens
Differential Revision: D16796973
fbshipit-source-id: 19379094b3b91920efac4bf1969fc22d4b80bcc6
Summary:
This problem was also affecting Fabric and was fixed in D16708532.
When the image resource is changed and the new image resource fails to load, we expect the display image to fail to load, but the image still shows the image that was successfully loaded last time.
## Changelog
[iOS] [Fixed] - fix display problems when image fails to load
Pull Request resolved: https://github.com/facebook/react-native/pull/25969
Test Plan:
This is catalyst playground with following code P78264143.
TLDR of the code, it sets URL <Image> that is 404.
{F175486515}
Reviewed By: makovkastar
Differential Revision: D16783330
Pulled By: sammy-SC
fbshipit-source-id: 1cb488590ce15d957357f32a73ebf8df6cccf4cd
Summary:
As part of the fix for https://github.com/facebook/react-native/issues/25349 I added `s.static_framework = true` to each podspec in repo (see https://github.com/facebook/react-native/pull/25619#discussion_r306993309 for more context).
This was required to ensure the existing conditional compilation with `#if RCT_DEV` and `__has_include` still worked correctly when `use_frameworks!` is enabled.
However, fkgozali pointed out that it would be ideal if we didn't have this requirement as it could make life difficult for third-party libraries.
This removes the requirement by moving `React-DevSupport.podspec` and `React-RCTWebSocket.podspec` into `React-Core.podspec` as subspecs. This means the symbols are present when `React-Core.podspec` is built dynamically so `s.static_framework = true` isn't required.
This means that any `Podfile` that refers to `React-DevSupport` or `React-RCTWebSocket` will need to be updated to avoid errors.
## Changelog
I don't think this needs a changelog entry since its just a refinement of https://github.com/facebook/react-native/pull/25619.
Pull Request resolved: https://github.com/facebook/react-native/pull/25816
Test Plan:
Check `RNTesterPods` still works both with and without `use_frameworks!`:
1. Go to the `RNTester` directory and run `pod install`.
2. Run the tests in `RNTesterPods.xcworkspace` to see that everything still works fine.
3. Uncomment the `use_frameworks!` line at the top of `RNTester/Podfile` and run `pod install` again.
4. Run the tests again and see that it still works with frameworks enabled.
Reviewed By: hramos
Differential Revision: D16495030
Pulled By: fkgozali
fbshipit-source-id: 2708ac9fd20cd04cb0aea61b2e8ab0d931dfb6d5
Summary:
This is my proposal for fixing `use_frameworks!` compatibility without breaking all `<React/*>` imports I outlined in https://github.com/facebook/react-native/pull/25393#issuecomment-508457700. If accepted, it will fix https://github.com/facebook/react-native/issues/25349.
It builds on the changes I made in https://github.com/facebook/react-native/pull/25496 by ensuring each podspec has a unique value for `header_dir` so that framework imports do not conflict. Every podspec which should be included in the `<React/*>` namespace now includes it's headers from `React-Core.podspec`.
The following pods can still be imported with `<React/*>` and so should not have breaking changes: `React-ART`,`React-DevSupport`, `React-CoreModules`, `React-RCTActionSheet`, `React-RCTAnimation`, `React-RCTBlob`, `React-RCTImage`, `React-RCTLinking`, `React-RCTNetwork`, `React-RCTPushNotification`, `React-RCTSettings`, `React-RCTText`, `React-RCTSettings`, `React-RCTVibration`, `React-RCTWebSocket` .
There are still a few breaking changes which I hope will be acceptable:
- `React-Core.podspec` has been moved to the root of the project. Any `Podfile` that references it will need to update the path.
- ~~`React-turbomodule-core`'s headers now live under `<turbomodule/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- ~~`React-turbomodulesamples`'s headers now live under `<turbomodulesamples/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- ~~`React-TypeSaferty`'s headers now live under `<TypeSafety/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511040967.
- ~~`React-jscallinvoker`'s headers now live under `<jscallinvoker/*>`~~ Replaced by https://github.com/facebook/react-native/pull/25619#issuecomment-511091823.
- Each podspec now uses `s.static_framework = true`. This means that a minimum of CocoaPods 1.5 ([released in April 2018](http://blog.cocoapods.org/CocoaPods-1.5.0/)) is now required. This is needed so that the ` __has_include` conditions can still work when frameworks are enabled.
Still to do:
- ~~Including `React-turbomodule-core` with `use_frameworks!` enabled causes the C++ import failures we saw in https://github.com/facebook/react-native/issues/25349. I'm sure it will be possible to fix this but I need to dig deeper (perhaps a custom modulemap would be needed).~~ Addressed by 33573511f0.
- I haven't got Fabric working yet. I wonder if it would be acceptable to move Fabric out of the `<React/*>` namespace since it is new? �
## Changelog
[iOS] [Fixed] - Fixed compatibility with CocoaPods frameworks.
Pull Request resolved: https://github.com/facebook/react-native/pull/25619
Test Plan:
### FB
```
buck build catalyst
```
### Sample Project
Everything should work exactly as before, where `use_frameworks!` is not in `Podfile`s. I have a branch on my [sample project](https://github.com/jtreanor/react-native-cocoapods-frameworks) here which has `use_frameworks!` in its `Podfile` to demonstrate this is fixed.
You can see that it works with these steps:
1. `git clone git@github.com:jtreanor/react-native-cocoapods-frameworks.git`
2. `git checkout fix-frameworks-subspecs`
3. `cd ios && pod install`
4. `cd .. && react-native run-ios`
The sample app will build and run successfully. To see that it still works without frameworks, remove `use_frameworks!` from the `Podfile` and do steps 3 and 4 again.
### RNTesterPods
`RNTesterPodsPods` can now work with or without `use_frameworks!`.
1. Go to the `RNTester` directory and run `pod install`.
2. Run the tests in `RNTesterPods.xcworkspace` to see that everything still works fine.
3. Uncomment the `use_frameworks!` line at the top of `RNTester/Podfile` and run `pod install` again.
4. Run the tests again and see that it still works with frameworks enabled.
Reviewed By: PeteTheHeat
Differential Revision: D16465247
Pulled By: PeteTheHeat
fbshipit-source-id: cad837e9cced06d30cc5b372af1c65c7780b9e7a
Summary: We no longer want to access RCTImageLoader from the bridge.
Reviewed By: shergin
Differential Revision: D16389383
fbshipit-source-id: 8e006bf0e2e2651f3ac036c09e589213ac9d29f9
Summary:
[After we migrated to new GIF implementation](https://github.com/facebook/react-native/pull/24822), we can remove old implementation now.
## Changelog
[iOS] [Deprecated] - Remove old GIF code
Pull Request resolved: https://github.com/facebook/react-native/pull/25636
Test Plan: GIF still works.
Reviewed By: shergin
Differential Revision: D16280044
Pulled By: osdnk
fbshipit-source-id: 00979280e6c17c93859ce886dd563b0d185c84aa
Summary: We no longer want to access RCTImageLoader from the bridge category. Instead, let's use the `moduleForClass` API.
Reviewed By: shergin
Differential Revision: D16389113
fbshipit-source-id: c638f4b9851698afc53aaaa2b302d21cc19f76e7
Summary:
I discovered failing snapshot tests (T47222928, T47222859). They fail because `<Image>` doesn't work with base64 anymore.
There are two problems that are causing this.
1st is on iOS https://fburl.com/pw246vgw where if the image has 1 frame count, nothing is displayed.
2nd is in https://fburl.com/3im0u38r where if image is not within assets, we don't display it.
Reviewed By: shergin
Differential Revision: D16334151
fbshipit-source-id: 1ea8ef676b7207834ba63f4264e6ef2f05f24b96
Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/25583
We now use CocoaPods for better maintainability.
Reviewed By: hramos
Differential Revision: D16193719
fbshipit-source-id: 26382f2da4eaba14a71771540b587fdc80b41108
Summary:
This PR adds initial support for Project Catalyst a.k.a. UIKitForMac. This is not yet meant for production, but this is enough for RNTester to successfully compile and mostly work :)
Some APIs are not supported on the Mac -- e.g. telephony, and deprecated APIs are removed on Mac ���-- those had to be ifdef'd out via platform checks.
The biggest limitation right now is that I couldn't get Web Socket code to successfully compile, and so there are a lot of temporary platform checks for that , and the RCTWebSocket.xcodeproj is marked as not supporting UIKitForMac. Again -- temporary, until someone with more knowledge knows how to fix this.
https://github.com/react-native-community/discussions-and-proposals/issues/131
## Changelog
[iOS] [Added] - Fixed compilation for macOS (Project Catalyst) -- not meant for production use yet
Pull Request resolved: https://github.com/facebook/react-native/pull/25427
Test Plan:
- Open RNTester/RNTester.xcodeproj with Xcode 10.2, run it like a normal iOS app -- make sure it compiles and runs correctly (no regression)
- Open the same project with Xcode 11 beta 2 (or higher) on macOS Catalina beta, select "My Mac" as device target, and run -- see that it actually compiles and runs. **Note** there are unfortunately some required steps:
- change build configuration to Release (because packager doesn't work correctly yet)
- change development team to yours if Xcode tells you to
- go to RNTester project → Build phases → Link binary with libraries, and change `platforms` for `libRCTWebSocket.a` to `iOS` (without Mac compatibility). I can't commit that change because it breaks compatibility with earlier Xcode versions
The two extra steps for successful compile will disappear once web socket compilation for Catalyst is fixed
Reviewed By: mmmulani
Differential Revision: D16088263
Pulled By: sammy-SC
fbshipit-source-id: 9c0b932b048e50a8e0f336eaa0612851b1909cae
Summary:
The goal of this PR is to improve the pipeline currently used for displaying GIFs / animated images on iOS. It is achieved by not holding all of the decoded frames in memory at the same time, as well as happily releasing existing memory whenever possible. This code is a simplified version of what you would find in SDWebImage (it is nearly 1:1, with unsupported or uneeded things removed). By adopting this API, it also allows classes conforming to RCTImageURLLoader or RCTImageDataDecoder to return any decodable UIImages conforming to RCTAnimatedImage and have improvements to memory consumption. Because RCTAnimatedImage is just a subset of the SDAnimatedImage protocol, it also means that you can use SDWebImage easier with Image directly.
A nice to have would be progressive image loading, but is beyond scope for this PR. It would, however, touch most of these same parts.
## Changelog
[iOS] [Fixed] - Substantially lower chances of crashes from abundant GIF use
Pull Request resolved: https://github.com/facebook/react-native/pull/24822
Test Plan: TBD. (but i am running a version of this in my own app currently)
Reviewed By: shergin
Differential Revision: D15853479
Pulled By: sammy-SC
fbshipit-source-id: 969e0d458da9fa49453aee1dcdf51783c2a45067
Summary: This module is being removed from RN as part of the Lean Core effort.
Reviewed By: TheSavior
Differential Revision: D15714507
fbshipit-source-id: bb5dc2025a25ad450d6971e5948e7a2e678a9a25
Summary: This is being removed from RN as part of Lean Core.
Reviewed By: rickhanlonii
Differential Revision: D15666249
fbshipit-source-id: 00612b999184f216cc3deb72c6b24af359060abe
Summary: It used a wrong name, so this fixed the module lookup.
Reviewed By: yungsters
Differential Revision: D15595099
fbshipit-source-id: f5a711f595d9630541ae08339aa1532ed1d4d5f2
Summary:
This commit more clearly defines the mocks RN sets up and uses paths instead of Haste names to define the mocks. The Jest setup script defined mocks for native modules (Obj-C, Java) and mocks for JS modules in the same data structure. This meant that some non-native modules (that is, JS modules) were in the `mockNativeModules` map -- this commit splits them out and mocks them in typical `jest.mock` fashion.
Additionally, the setup script used to mock the modules using the Haste names. As one of the steps toward migrating to standard path-based imports, the setup script now mocks JS modules using paths (native modules don't need a Haste name nor path since they are just entries in `NativeModules`). This gets us closer to being able to remove `hasteImpl`. (Tracking in https://github.com/facebook/react-native/issues/24772.)
Also, this commit removes mocks that are not referenced anywhere in the RN and React repositories (grepped for the names and found no entries outside of the Jest setup scripts).
## Changelog
[General] [Changed] - Explicitly separate mocked native modules from mocked JS modules
Pull Request resolved: https://github.com/facebook/react-native/pull/24809
Differential Revision: D15316882
Pulled By: cpojer
fbshipit-source-id: 039e4e320121bea9580196fe0a091b8b1e8b41bf
Summary:
This is an ESLint plugin that infers whether an import looks like a Haste module name. To keep the linter fast and simple, it does not look in the Haste map. Instead, it looks for uppercase characters in single-name import paths, since npm has disallowed uppercase letters in package names for a long time. There are some false negatives (e.g. "merge" is a Haste module and this linter rule would not pick it up) but those are about 1.1% of the module names in the RN repo, and unit tests and integration tests will fail anyway once Haste is turned off.
You can disable the lint rule on varying granular levels with ESLint's normal disabling/enabling mechanisms.
Also rewrote more Haste imports so that the linter passes (i.e. fixed lint errors as part of this PR).
## Changelog
[General] [Changed] - Add a lint rule to disallow Haste imports
Pull Request resolved: https://github.com/facebook/react-native/pull/25058
Differential Revision: D15515826
Pulled By: cpojer
fbshipit-source-id: d58a3c30dfe0887f8a530e3393af4af5a1ec1cac
Summary: Moves relevant JS files to fb internal, removes stuff we don't need in the RN repo any more. Android and iOS will happen in a follow-up.
Reviewed By: rickhanlonii
Differential Revision: D15468419
fbshipit-source-id: 39fffc22f87534e557788e398bbae575043353b6
Summary:
In D15367312, I deprecate `$Enum<...>` in favour of `$Keys<...>` (the functionality is identical). Codemod existing usages in xplat and fbcode.
bypass-lint
Reviewed By: samwgoldman
Differential Revision: D15378084
fbshipit-source-id: 251c6b9ac07cb50139a8f03e3a45a5fac0d91812
Summary:
Bugs like https://github.com/facebook/react-native/issues/24789, we don't apply tintColor to GIF. We fixes it by setting a poster image before add animation.
cc. cpojer .
[iOS] [Fixed] - Fixes renderingMode not applied to GIF images
Pull Request resolved: https://github.com/facebook/react-native/pull/24794
Differential Revision: D15316913
Pulled By: cpojer
fbshipit-source-id: 611a07ec17afc962d1eb6b8fc193f118fa623e73
Summary:
This is the next step in moving RN towards standard path-based requires. All the requires in `Libraries` have been rewritten to use relative requires with a few exceptions, namely, `vendor` and `Renderer/oss` since those need to be changed upstream. This commit uses relative requires instead of `react-native/...` so that if Facebook were to stop syncing out certain folders and therefore remove code from the react-native package, internal code at Facebook would not need to change.
See the umbrella issue at https://github.com/facebook/react-native/issues/24316 for more detail.
[General] [Changed] - Migrate "Libraries" from Haste to standard path-based requires
Pull Request resolved: https://github.com/facebook/react-native/pull/24749
Differential Revision: D15258017
Pulled By: cpojer
fbshipit-source-id: a1f480ea36c05c659b6f37c8f02f6f9216d5a323
Summary:
Per a conversation with TheSavior, in #24538, this adds snapshot tests for all components whose mocks will be addressed in that PR. Shallow and deep snapshots are included.
[General] [Added] - Snapshots
Pull Request resolved: https://github.com/facebook/react-native/pull/24554
Differential Revision: D15062197
Pulled By: cpojer
fbshipit-source-id: 70ddbaa5e6d1d2c0fd1130ab04c458d9c49d0ee8
Summary:
Second attempt at adding border properties to `RCTImageView`.
Previous attempt can be found at D14875673 which was reverted.
1. `UIImageView` is no longer laid out in in `layoutSubviews`.
2. `updateWithImage` was not being called, I instead of calling `self.image = image` I was calling `_imageView.image = image` directly which skips calling `updateWithImage`. This meant that image's rendering mode was not changed to `template`.
Reviewed By: shergin
Differential Revision: D14934103
fbshipit-source-id: b74c692f9f8ad520ef1f9c70ec4b4aa68b868cd4
Summary:
`RCTImageView` is now a subclass of `RCTView` and includes `UIImageView` as it's subview.
This enables the use of `borderRadius`, `borderWidth`, `borderColor` properties and all of their derivatives.
Possible problem:
Now `RCTImageView` is backed by two views (`RCTView` + `UIImageView`), not a single one. That's 4 `CALayers`. Possible workaround would be to insert the image directly into `self.layer.contents`.
Reviewed By: RSNara
Differential Revision: D14875673
fbshipit-source-id: 594b2cd1ddffc6627566e07983c6d8f0b37dc2bb
Summary:
We need to remove adding png file extension when path has not extension. Two reasons:
1. `imageWithContentsOfFile` or other `UIKit` methods can load png image correctly, even if path has not `png` file extension.
2. Sometimes, people may have file that actually not have file extension, it's the designated behavior for user. Like #23844 .
CC. sahrens cpojer .
[iOS] [Fixed] - Remove explicitly add png file extension when load local image
Pull Request resolved: https://github.com/facebook/react-native/pull/23864
Reviewed By: shergin
Differential Revision: D14425373
Pulled By: hramos
fbshipit-source-id: 3cc06c9a3d68cadf652c1de742f3cce26258c874
Summary:
@public
This bumps Prettier to v1.16.4
Only format source files were updated.
Reviewed By: mjesun
Differential Revision: D14454893
fbshipit-source-id: 72f9872fe764a79dbf0d9fab9bebb1456b039f2f
Summary: This diff wires up everything from the previous 8 diffs. After this, all codepaths that execute `modulesConformingToProtocol` in `RCTImageLoader.m` will instead use iOS plugins to retrieve the modules on FBiOS.
Reviewed By: fkgozali
Differential Revision: D14360252
fbshipit-source-id: 6f0cecfa8dffa1955ba2f9ed54bc1c130fb23341
Summary:
This adds new functionality to the `Image` component by allowing you to retrieve the width and height of an image just like you'd do with [`Image.getSize`](https://facebook.github.io/react-native/docs/image.html#getsize) but _with_ the ability to provide headers to your request.
Why would you need this you ask? Well, imagine that you have an image that you're loading into your `Image` component that is protected and you get access by using a token in a header (or something similar). That would work. However, getting the dimensions isn't possible since you can't provide those same headers.
This is something that is bothering me when using a third-party library (https://github.com/archriss/react-native-image-gallery) and instead of implementing this just for that single library I imagined that it would be useful for anyone else that needs to get the image dimensions before displaying it.
[Android] [Added] - Added Image.getSizeWithHeaders
[iOS] [Added] - Added Image.getSizeWithHeaders
Pull Request resolved: https://github.com/facebook/react-native/pull/18850
Differential Revision: D14434599
Pulled By: cpojer
fbshipit-source-id: 56d5e58889ddf7ddc12d5f6f7d9dc6921fa17884
Summary:
[General][Fixed] Fixed Flow typing of onError prop on Image component to include `error` field
reason: tried grabbing the error off nativeEvent in D14285176 but flow complained. Docs say it should be there & when actually running the code, it does exist, so I //think// this is ok?
Reviewed By: TheSavior
Differential Revision: D14285688
fbshipit-source-id: 19deb35ba8ab419de209a8deaf12df0b395e5b82
Summary:
We use indent width with 2 spaces, but `RCTImageLoader` use 4 spaces, it's hard to code, code one line, adjust indent, or change Xcode's preference temporary. So let's prettier it.
[iOS] [Fixed] - Prettier Image loader code
Pull Request resolved: https://github.com/facebook/react-native/pull/23701
Differential Revision: D14278082
Pulled By: cpojer
fbshipit-source-id: 6e7db50741b0981f45622c19981cb5fd9bbc0a97
Summary:
We assume `map` is the type of `Map`, but actually it's not, so we would get type error.
[iOS] [Fixed] - [RNTester] fix getter of result from Image query cache
Pull Request resolved: https://github.com/facebook/react-native/pull/23602
Differential Revision: D14221747
Pulled By: cpojer
fbshipit-source-id: 06cf08078a330e4d5731ad72010c87e9e69fcd7b
Summary:
In iOS, seems we have no ways to check wether the cached item is from disk or memory, only `storagePolicy == NSURLCacheStorageAllowedInMemoryOnly ` we can think it's from memory. So we need to add a new result like `disk/memory`?
[iOS] [Added] - Add Image query cache result type
Pull Request resolved: https://github.com/facebook/react-native/pull/23608
Differential Revision: D14205902
Pulled By: cpojer
fbshipit-source-id: 29c253878b5c6776cd4776508e24c57e6bfa7dfa