Skip to content

Commit

Permalink
Make C++ requirement opt-in (#27730)
Browse files Browse the repository at this point in the history
Summary:
When building as a framework these headers get automatically added to the framework umbrella header for React-Core. Instead of converting all the React sources to ObjC++ files and still forcing external users that build native source (and link against a framework build) to also compile as ObjC++, this makes the attribution related methods that were added in fdcdca4 opt-in to ObjC++ builds.

This is also the reason for the current failure of the CI `test_ios_frameworks` run.

## Changelog

I’m unsure if this change really warrants an entry in the CHANGELOG, as it’s more of an amendment of the (afaik) unreleased [change](fdcdca4).

[iOS] [Fixed] - Make framework builds work again by making `RCTImageLoader` C++ requirement opt-in
Pull Request resolved: #27730

Test Plan:
I tested static and dynamic (framework) builds and ran the test suite.

This change should make the `test_ios_frameworks` CI run _build_ again, ~~but it may still fail overall as in my local testing one of the tests leads to a segfault (which I will try to address separately)~~.

Reviewed By: PeteTheHeat

Differential Revision: D19348846

Pulled By: fkgozali

fbshipit-source-id: 8a74e6f7ad3ddce2cf10b080b9a5d7b399bd5fc0
  • Loading branch information
alloy authored and facebook-github-bot committed Jan 28, 2020
1 parent f8a75d5 commit 25571ec
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 0 deletions.
4 changes: 4 additions & 0 deletions Libraries/Image/RCTImageLoaderWithAttributionProtocol.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ RCT_EXTERN void RCTEnableImageLoadingPerfInstrumentation(BOOL enabled);

@protocol RCTImageLoaderWithAttributionProtocol<RCTImageLoaderProtocol>

// TODO (T61325135): Remove C++ checks
#ifdef __cplusplus
/**
* Same as the variant in RCTImageURLLoaderProtocol, but allows passing attribution
* information that each image URL loader can process.
Expand All @@ -30,6 +32,8 @@ RCT_EXTERN void RCTEnableImageLoadingPerfInstrumentation(BOOL enabled);
progressBlock:(RCTImageLoaderProgressBlock)progressBlock
partialLoadBlock:(RCTImageLoaderPartialLoadBlock)partialLoadBlock
completionBlock:(RCTImageLoaderCompletionBlock)completionBlock;
#endif

/**
* Image instrumentation - notify that the image content (UIImage) has been set on the native view.
*/
Expand Down
6 changes: 6 additions & 0 deletions Libraries/Image/RCTImageURLLoaderWithAttribution.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

#import <React/RCTImageURLLoader.h>

// TODO (T61325135): Remove C++ checks
#ifdef __cplusplus
namespace facebook {
namespace react {

Expand All @@ -17,6 +19,7 @@ struct ImageURLLoaderAttribution {

} // namespace react
} // namespace facebook
#endif

@interface RCTImageURLLoaderRequest : NSObject

Expand All @@ -35,6 +38,8 @@ struct ImageURLLoaderAttribution {
*/
@protocol RCTImageURLLoaderWithAttribution <RCTImageURLLoader>

// TODO (T61325135): Remove C++ checks
#ifdef __cplusplus
/**
* Same as the RCTImageURLLoader variant above, but allows optional `attribution` information.
* Caller may also specify a preferred requestId for tracking purpose.
Expand All @@ -48,6 +53,7 @@ struct ImageURLLoaderAttribution {
progressHandler:(RCTImageLoaderProgressBlock)progressHandler
partialLoadHandler:(RCTImageLoaderPartialLoadBlock)partialLoadHandler
completionHandler:(RCTImageLoaderCompletionBlock)completionHandler;
#endif

/**
* Image instrumentation - notify that the image content (UIImage) has been set on the native view.
Expand Down

0 comments on commit 25571ec

Please sign in to comment.