Skip to content

Commit

Permalink
Move all header imports to "<React/..>"
Browse files Browse the repository at this point in the history 
Summary:
To make React Native play nicely with our internal build infrastructure we need to properly namespace all of our header includes.

Where previously you could do `#import "RCTBridge.h"`, you must now write this as `#import <React/RCTBridge.h>`. If your xcode project still has a custom header include path, both variants will likely continue to work, but for new projects, we're defaulting the header include path to `$(BUILT_PRODUCTS_DIR)/usr/local/include`, where the React and CSSLayout targets will copy a subset of headers too. To make Xcode copy headers phase work properly, you may need to add React as an explicit dependency to your app's scheme and disable "parallelize build".

Reviewed By: mmmulani

Differential Revision: D4213120

fbshipit-source-id: 84a32a4b250c27699e6795f43584f13d594a9a82
  • Loading branch information
@javache
javache authored and Facebook Github Bot committed Nov 23, 2016
1 parent 5850165 commit e1577df
Show file tree
Hide file tree
Showing 220 changed files with 1,492 additions and 1,197 deletions.
61 changes: 33 additions & 28 deletions Examples/UIExplorer/UIExplorer.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@
2D4BD8E61DA2E20D005AC8A8 /* TestBundle.js in Resources */ = {isa = PBXBuildFile; fileRef = 3DD981D51D33C6FB007DC7BE /* TestBundle.js */; };
2D4BD8E71DA2E20D005AC8A8 /* libOCMock.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 14D6D7101B220EB3001FB087 /* libOCMock.a */; };
2D8C2E321DA40403000EE098 /* RCTMultipartStreamReaderTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 001BFCE31D838343008E587E /* RCTMultipartStreamReaderTests.m */; };
2DD0EFF41DA853B200B0C975 /* libRCTPushNotification-tvOS.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 2DD0EFF31DA853B200B0C975 /* libRCTPushNotification-tvOS.a */; };
2DD323DC1DA2DDBF000FE1B8 /* FlexibleSizeExampleView.m in Sources */ = {isa = PBXBuildFile; fileRef = 27F441E81BEBE5030039B79C /* FlexibleSizeExampleView.m */; };
2DD323DD1DA2DDBF000FE1B8 /* UpdatePropertiesExampleView.m in Sources */ = {isa = PBXBuildFile; fileRef = 272E6B3C1BEA849E001FCF37 /* UpdatePropertiesExampleView.m */; };
2DD323DE1DA2DDBF000FE1B8 /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 13B07FB01A68108700A75B9A /* AppDelegate.m */; };
Expand Down Expand Up @@ -212,13 +211,6 @@
remoteGlobalIDString = 2DD3238F1DA2DD8A000FE1B8;
remoteInfo = "UIExplorer-tvOS";
};
2DD0EFF21DA853B200B0C975 /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 14DC67E71AB71876001358AB /* RCTPushNotification.xcodeproj */;
proxyType = 2;
remoteGlobalIDString = 2DD0EFE61DA8533A00B0C975;
remoteInfo = "RCTPushNotification-tvOS";
};
2DD323A61DA2DD8B000FE1B8 /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 83CBB9F71A601CBA00E9B192 /* Project object */;
Expand Down Expand Up @@ -303,6 +295,20 @@
remoteGlobalIDString = 3D13F83D1D6F6AE000E69E0E;
remoteInfo = UIExplorerBundle;
};
3D3C08801DE3424E00C268FA /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 14AADEFF1AC3DB95002390C9 /* React.xcodeproj */;
proxyType = 2;
remoteGlobalIDString = 3D3C059A1DE3340900C268FA;
remoteInfo = CSSLayout;
};
3D3C08821DE3424E00C268FA /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 14AADEFF1AC3DB95002390C9 /* React.xcodeproj */;
proxyType = 2;
remoteGlobalIDString = 3D3C06751DE3340C00C268FA;
remoteInfo = "CSSLayout-tvOS";
};
58005BED1ABA80530062E044 /* PBXContainerItemProxy */ = {
isa = PBXContainerItemProxy;
containerPortal = 58005BE41ABA80530062E044 /* RCTTest.xcodeproj */;
Expand Down Expand Up @@ -438,9 +444,9 @@
13E501F11D07A84A005F35D8 /* libRCTAnimation.a in Frameworks */,
138DEE241B9EDFB6007F4EA5 /* libRCTCameraRoll.a in Frameworks */,
134A8A2A1AACED7A00945AAE /* libRCTGeolocation.a in Frameworks */,
1341802C1AA9178B003F314A /* libRCTNetwork.a in Frameworks */,
13417FE91AA91432003F314A /* libRCTImage.a in Frameworks */,
3578590A1B28D2CF00341EDB /* libRCTLinking.a in Frameworks */,
1341802C1AA9178B003F314A /* libRCTNetwork.a in Frameworks */,
14DC67F41AB71881001358AB /* libRCTPushNotification.a in Frameworks */,
834C36EC1AF8DED70019C93C /* libRCTSettings.a in Frameworks */,
134180011AA9153C003F314A /* libRCTText.a in Frameworks */,
Expand Down Expand Up @@ -469,7 +475,6 @@
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
2DD0EFF41DA853B200B0C975 /* libRCTPushNotification-tvOS.a in Frameworks */,
2DD323E31DA2DE3F000FE1B8 /* libRCTAnimation-tvOS.a in Frameworks */,
2DD323E41DA2DE3F000FE1B8 /* libRCTImage-tvOS.a in Frameworks */,
2DD323E51DA2DE3F000FE1B8 /* libRCTLinking-tvOS.a in Frameworks */,
Expand Down Expand Up @@ -681,6 +686,8 @@
children = (
14AADF041AC3DB95002390C9 /* libReact.a */,
2DD323D91DA2DD8B000FE1B8 /* libReact-tvOS.a */,
3D3C08811DE3424E00C268FA /* libCSSLayout.a */,
3D3C08831DE3424E00C268FA /* libCSSLayout-tvOS.a */,
);
name = Products;
sourceTree = "<group>";
Expand Down Expand Up @@ -712,7 +719,6 @@
isa = PBXGroup;
children = (
14DC67F11AB71876001358AB /* libRCTPushNotification.a */,
2DD0EFF31DA853B200B0C975 /* libRCTPushNotification-tvOS.a */,
);
name = Products;
sourceTree = "<group>";
Expand Down Expand Up @@ -1134,13 +1140,6 @@
remoteRef = 14DC67F01AB71876001358AB /* PBXContainerItemProxy */;
sourceTree = BUILT_PRODUCTS_DIR;
};
2DD0EFF31DA853B200B0C975 /* libRCTPushNotification-tvOS.a */ = {
isa = PBXReferenceProxy;
fileType = archive.ar;
path = "libRCTPushNotification-tvOS.a";
remoteRef = 2DD0EFF21DA853B200B0C975 /* PBXContainerItemProxy */;
sourceTree = BUILT_PRODUCTS_DIR;
};
2DD323B51DA2DD8B000FE1B8 /* libRCTAnimation-tvOS.a */ = {
isa = PBXReferenceProxy;
fileType = archive.ar;
Expand Down Expand Up @@ -1211,6 +1210,20 @@
remoteRef = 357859001B28D2C500341EDB /* PBXContainerItemProxy */;
sourceTree = BUILT_PRODUCTS_DIR;
};
3D3C08811DE3424E00C268FA /* libCSSLayout.a */ = {
isa = PBXReferenceProxy;
fileType = archive.ar;
path = libCSSLayout.a;
remoteRef = 3D3C08801DE3424E00C268FA /* PBXContainerItemProxy */;
sourceTree = BUILT_PRODUCTS_DIR;
};
3D3C08831DE3424E00C268FA /* libCSSLayout-tvOS.a */ = {
isa = PBXReferenceProxy;
fileType = archive.ar;
path = "libCSSLayout-tvOS.a";
remoteRef = 3D3C08821DE3424E00C268FA /* PBXContainerItemProxy */;
sourceTree = BUILT_PRODUCTS_DIR;
};
58005BEE1ABA80530062E044 /* libRCTTest.a */ = {
isa = PBXReferenceProxy;
fileType = archive.ar;
Expand Down Expand Up @@ -1798,11 +1811,7 @@
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_LABEL = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/../../React/**",
"$(SRCROOT)/../../ReactCommon/**",
"$(SRCROOT)/../../Libraries/**",
);
HEADER_SEARCH_PATHS = "$(BUILT_PRODUCTS_DIR)/usr/local/include";
IPHONEOS_DEPLOYMENT_TARGET = 8.0;
MTL_ENABLE_DEBUG_INFO = YES;
ONLY_ACTIVE_ARCH = YES;
Expand Down Expand Up @@ -1862,11 +1871,7 @@
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_LABEL = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/../../React/**",
"$(SRCROOT)/../../ReactCommon/**",
"$(SRCROOT)/../../Libraries/**",
);
HEADER_SEARCH_PATHS = "$(BUILT_PRODUCTS_DIR)/usr/local/include";
IPHONEOS_DEPLOYMENT_TARGET = 8.0;
MTL_ENABLE_DEBUG_INFO = NO;
OTHER_LDFLAGS = (
Expand Down
2 changes: 1 addition & 1 deletion Examples/UIExplorer/UIExplorer.xcodeproj/xcshareddata/xcschemes/UIExplorer-tvOS.xcscheme
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "0800"
LastUpgradeVersion = "0810"
version = "1.3">
<BuildAction
parallelizeBuildables = "YES"
Expand Down
18 changes: 16 additions & 2 deletions Examples/UIExplorer/UIExplorer.xcodeproj/xcshareddata/xcschemes/UIExplorer.xcscheme
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
<?xml version="1.0" encoding="UTF-8"?>
<Scheme
LastUpgradeVersion = "0730"
LastUpgradeVersion = "0810"
version = "1.3">
<BuildAction
parallelizeBuildables = "YES"
parallelizeBuildables = "NO"
buildImplicitDependencies = "YES">
<BuildActionEntries>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
buildForProfiling = "YES"
buildForArchiving = "YES"
buildForAnalyzing = "YES">
<BuildableReference
BuildableIdentifier = "primary"
BlueprintIdentifier = "83CBBA2D1A601D0E00E9B192"
BuildableName = "libReact.a"
BlueprintName = "React"
ReferencedContainer = "container:../../React/React.xcodeproj">
</BuildableReference>
</BuildActionEntry>
<BuildActionEntry
buildForTesting = "YES"
buildForRunning = "YES"
Expand Down
13 changes: 7 additions & 6 deletions Examples/UIExplorer/UIExplorer/AppDelegate.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@

#import "AppDelegate.h"

#import "RCTBridge.h"
#import "RCTBundleURLProvider.h"
#import "RCTJavaScriptLoader.h"
#import "RCTLinkingManager.h"
#import "RCTRootView.h"
#import <React/RCTBridge.h>
#import <React/RCTBundleURLProvider.h>
#import <React/RCTJavaScriptLoader.h>
#import <React/RCTLinkingManager.h>
#import <React/RCTRootView.h>

#if !TARGET_OS_TV
#import "RCTPushNotificationManager.h"
#import <React/RCTPushNotificationManager.h>
#endif

@interface AppDelegate() <RCTBridgeDelegate>
Expand Down
3 changes: 2 additions & 1 deletion Examples/UIExplorer/UIExplorer/NativeExampleViews/FlexibleSizeExampleView.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
*/

#import <UIKit/UIKit.h>
#import "RCTView.h"

#import <React/RCTView.h>

@interface FlexibleSizeExampleView : RCTView

Expand Down
15 changes: 7 additions & 8 deletions Examples/UIExplorer/UIExplorer/NativeExampleViews/FlexibleSizeExampleView.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,14 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

#import "AppDelegate.h"

#import "FlexibleSizeExampleView.h"
#import "RCTJavaScriptLoader.h"
#import "RCTBridge.h"
#import "RCTRootView.h"
#import "RCTRootViewDelegate.h"

#import "RCTViewManager.h"
#import <React/RCTBridge.h>
#import <React/RCTRootView.h>
#import <React/RCTRootViewDelegate.h>
#import <React/RCTViewManager.h>

#import "AppDelegate.h"

@interface FlexibleSizeExampleViewManager : RCTViewManager

Expand All @@ -38,7 +37,7 @@ - (UIView *)view
@end


@interface FlexibleSizeExampleView ()<RCTRootViewDelegate>
@interface FlexibleSizeExampleView () <RCTRootViewDelegate>

@end

Expand Down
3 changes: 2 additions & 1 deletion Examples/UIExplorer/UIExplorer/NativeExampleViews/UpdatePropertiesExampleView.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
*/

#import <UIKit/UIKit.h>
#import "RCTView.h"

#import <React/RCTView.h>

@interface UpdatePropertiesExampleView : RCTView

Expand Down
11 changes: 5 additions & 6 deletions Examples/UIExplorer/UIExplorer/NativeExampleViews/UpdatePropertiesExampleView.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

#import "AppDelegate.h"

#import "UpdatePropertiesExampleView.h"
#import "RCTJavaScriptLoader.h"
#import "RCTBridge.h"
#import "RCTRootView.h"
#import "RCTViewManager.h"

#import <React/RCTRootView.h>
#import <React/RCTViewManager.h>

#import "AppDelegate.h"

@interface UpdatePropertiesExampleViewManager : RCTViewManager

Expand Down
5 changes: 1 addition & 4 deletions Examples/UIExplorer/UIExplorerUnitTests/OCMock/OCMStubRecorder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* under the License.
*/

#import "OCMRecorder.h"
#import <React/OCMRecorder.h>


@interface OCMStubRecorder : OCMRecorder
Expand Down Expand Up @@ -51,6 +51,3 @@
@property (nonatomic, readonly) OCMStubRecorder *(^ _andForwardToRealObject)(void);

@end



3 changes: 2 additions & 1 deletion Examples/UIExplorer/UIExplorerUnitTests/RCTConvert_NSURLTests.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@

#import <XCTest/XCTest.h>

#import "RCTConvert.h"
#import <React/RCTConvert.h>

#import "RCTUtils.h"

@interface RCTConvert_NSURLTests : XCTestCase
Expand Down
2 changes: 1 addition & 1 deletion Examples/UIExplorer/UIExplorerUnitTests/RCTImageLoaderHelpers.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
* CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
*/

#import "RCTImageLoader.h"
#import <React/RCTImageLoader.h>

typedef BOOL (^RCTImageURLLoaderCanLoadImageURLHandler)(NSURL *requestURL);
typedef RCTImageLoaderCancellationBlock (^RCTImageURLLoaderLoadImageURLHandler)(NSURL *imageURL, CGSize size, CGFloat scale, RCTResizeMode resizeMode, RCTImageLoaderProgressBlock progressHandler, RCTImageLoaderCompletionBlock completionHandler);
Expand Down
36 changes: 3 additions & 33 deletions Libraries/ActionSheetIOS/RCTActionSheet.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,34 +10,12 @@
14C644C41AB0DFC900DE3C65 /* RCTActionSheetManager.m in Sources */ = {isa = PBXBuildFile; fileRef = 14C644C21AB0DFC900DE3C65 /* RCTActionSheetManager.m */; };
/* End PBXBuildFile section */

/* Begin PBXCopyFilesBuildPhase section */
58B511D91A9E6C8500147676 /* CopyFiles */ = {
isa = PBXCopyFilesBuildPhase;
buildActionMask = 2147483647;
dstPath = "include/$(PRODUCT_NAME)";
dstSubfolderSpec = 16;
files = (
);
runOnlyForDeploymentPostprocessing = 0;
};
/* End PBXCopyFilesBuildPhase section */

/* Begin PBXFileReference section */
134814201AA4EA6300B7C361 /* libRCTActionSheet.a */ = {isa = PBXFileReference; explicitFileType = archive.ar; includeInIndex = 0; path = libRCTActionSheet.a; sourceTree = BUILT_PRODUCTS_DIR; };
14C644C11AB0DFC900DE3C65 /* RCTActionSheetManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = RCTActionSheetManager.h; sourceTree = "<group>"; };
14C644C11AB0DFC900DE3C65 /* RCTActionSheetManager.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; lineEnding = 0; path = RCTActionSheetManager.h; sourceTree = "<group>"; xcLanguageSpecificationIdentifier = xcode.lang.objcpp; };
14C644C21AB0DFC900DE3C65 /* RCTActionSheetManager.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = RCTActionSheetManager.m; sourceTree = "<group>"; };
/* End PBXFileReference section */

/* Begin PBXFrameworksBuildPhase section */
58B511D81A9E6C8500147676 /* Frameworks */ = {
isa = PBXFrameworksBuildPhase;
buildActionMask = 2147483647;
files = (
);
runOnlyForDeploymentPostprocessing = 0;
};
/* End PBXFrameworksBuildPhase section */

/* Begin PBXGroup section */
134814211AA4EA7D00B7C361 /* Products */ = {
isa = PBXGroup;
Expand Down Expand Up @@ -66,8 +44,6 @@
buildConfigurationList = 58B511EF1A9E6C8500147676 /* Build configuration list for PBXNativeTarget "RCTActionSheet" */;
buildPhases = (
58B511D71A9E6C8500147676 /* Sources */,
58B511D81A9E6C8500147676 /* Frameworks */,
58B511D91A9E6C8500147676 /* CopyFiles */,
);
buildRules = (
);
Expand Down Expand Up @@ -156,10 +132,7 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/../../React/**",
"$(SRCROOT)/../../ReactCommon/**",
);
HEADER_SEARCH_PATHS = "$(BUILT_PRODUCTS_DIR)/usr/local/include";
IPHONEOS_DEPLOYMENT_TARGET = 8.0;
MTL_ENABLE_DEBUG_INFO = YES;
ONLY_ACTIVE_ARCH = YES;
Expand Down Expand Up @@ -200,10 +173,7 @@
GCC_WARN_UNINITIALIZED_AUTOS = YES_AGGRESSIVE;
GCC_WARN_UNUSED_FUNCTION = YES;
GCC_WARN_UNUSED_VARIABLE = YES;
HEADER_SEARCH_PATHS = (
"$(SRCROOT)/../../React/**",
"$(SRCROOT)/../../ReactCommon/**",
);
HEADER_SEARCH_PATHS = "$(BUILT_PRODUCTS_DIR)/usr/local/include";
IPHONEOS_DEPLOYMENT_TARGET = 8.0;
MTL_ENABLE_DEBUG_INFO = NO;
SDKROOT = iphoneos;
Expand Down
2 changes: 1 addition & 1 deletion Libraries/ActionSheetIOS/RCTActionSheetManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#import <UIKit/UIKit.h>

#import "RCTBridge.h"
#import <React/RCTBridge.h>

@interface RCTActionSheetManager : NSObject <RCTBridgeModule>

Expand Down
10 changes: 5 additions & 5 deletions Libraries/ActionSheetIOS/RCTActionSheetManager.m
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@

#import "RCTActionSheetManager.h"

#import "RCTConvert.h"
#import "RCTLog.h"
#import "RCTUtils.h"
#import "RCTBridge.h"
#import "RCTUIManager.h"
#import <React/RCTBridge.h>
#import <React/RCTConvert.h>
#import <React/RCTLog.h>
#import <React/RCTUIManager.h>
#import <React/RCTUtils.h>

@interface RCTActionSheetManager () <UIActionSheetDelegate>
@end
Expand Down
Loading

39 comments on commit e1577df

@rturk
Copy link

@rturk rturk commented on e1577df Dec 1, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache For clarify maybe headers should be prefixed with ReactNative instead of just React

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache I'm having some issues with building an archive in xcode after updating past this commit. Updated HEADER_SEARCH_PATHS to $(BUILT_PRODUCTS_DIR)/usr/local/include and everything works fine when running with either debug or release scheme (so it's not a scheme issue). However when building an archive to upload on the app store I get header not found errors. After looking a bit I found that xcode uses a different folder structure when archiving and adding "$(BUILD_ROOT)/../InstallationBuildProductsLocation/usr/local/include" to the paths does fix it but kind of feels like a fragile solution. Found a bunch of different solutions on SO that all didn't work probably due to different xcode versions so I ended looking at the build dir to figure out the right path. Any ideas on a better solution? If not I guess we should add this to the header include paths so it doesn't break archive generation.

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache @janicduplessis I've been in a battle trying to build after upgrade to RN0.40, as this commit breaks all the third-party libraries, got errors such as can't find React/RCTXXX.h blablabla , and I ended up to add $(BUILT_PRODUCTS_DIR)/usr/local/include to the HEADER_SEARCH_PATHS of every third-party library, I think it's wired to make such a big change with building settings, I haven't try to archive, and I believe I will encounter with the issue janic said above

So, are there more clever solution?

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also another issue, headers get included in the archive so it cannot get uploaded to the store as an iOS app. Looks like the recommended solution for this is to use a copy files build phase instead of headers build phase but I couldn't get it working so I ended using a pre-archive script to manually delete the headers from the archive. I added rm -rf "${OBJROOT}/../InstallationBuildProductsLocation/usr" in edit scheme -> archive section -> pre-archive -> add script.

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'll try what happens when I do a production build of UIExplorer, but I'm really surprised the copy headers phase is not behaving correctly like that.

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nihgwu: none of the headers inside React moved locations, so I'm wondering why this would cause 3rd party packages with existing header include paths to break. I'd appreciate any help since this xcode config is very frustrating, internally we just use Buck.

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$(DSTROOT)/usr/local/include seems to be the correct path when archiving, but not when doing regular builds...

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache Yeah, from what I read the archive process has a different folder structure -_-, I guess we could include both paths. Also from your previous comment I don't think this change is backwards compatible, I don't know enough about xcode build stuff to know why (I agree it's weird since headers didn't move) but they are not accesible anymore with the old include path so every single lib need to update both their header include path and every import to <React/...>.

Once you manage to generate an archive you will also probably hit the 2nd issue I mentionned because the headers get included in the archive so it cannot be published as an app...

How does this improve builds internally? Is it worth breaking all 3rd party libs and having to work through these issues? I was starting to consider if we should rather just revert this at least until we can have a better solution.

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, yeah, I see how this could break 3rd party libs. Besides reverting this commit, are there any other options that could limit the impact this has?

We do need this internally to have RN behave will with the rest of our build tooling. Having imports as "..." means you're filling the global namespace with RN header references which we had to move away from.

For the xcarchive containing headers issue, I'm now trying https://developer.apple.com/library/content/technotes/tn2215/_index.html as "Headers build phases do not work correctly with static library targets when archiving in Xcode"

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, I'm fine with keeping this as no matter the change we make to avoid polluting the global header namespace it will break 3rd party libs. At least it's a simple fix for library authors.

I also tried using copy files build phase as mentioned in this article but couldn't get it working and gave up since I had to ship a build asap :)

@chirag04
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache This is affecting us badly too. Is there a short term fix? and should we ask 3rd party libs to update their imports/headers?

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3rd parties should only need to update their header search path. I'm working on fixing the setup for archiving builds.

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache not only the header search path, if you need to import the header files from the 3rd party libraries in AppDelegate.m, you will still get errors, you have to change $(BUILT_PRODUCTS_DIR)/usr/local/include to be recursive in your project targets as those libraries are importing "RCTXXX.h" not <React/RCTXXX.h>

So the best way is to update their header search path and import path both

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I'll be landing a change today (and picking it to 0.40), which moves the exported headers to $(BUILT_PRODUCTS_DIR)/include which actually is on Xcode's default include path it seems, so no changes should be required anymore.

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good news, I'm really frustrating with the XCode stuff.

@javache
Copy link
Member Author

@javache javache commented on e1577df Dec 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A new version of this just landed as 59407f3 Can you try it out? @mkonicek will be merging this in v0.40 if all goes well.

@janicduplessis
Copy link
Contributor

@janicduplessis janicduplessis commented on e1577df Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache Just tested and it works really well, thanks for figuring out this frustrating xcode build stuff. Third party libs will still have to update their RN imports and remove React from their headers path otherwise you get header redefinition errors but I think this is fine.

@janicduplessis
Copy link
Contributor

@janicduplessis janicduplessis commented on e1577df Dec 8, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually headers still get included in the archive, looks like they need to be in the 'Project' section instead of 'Private' (as if that makes any sense). After changing that it works properly :)

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javache 59407f3 seems does't solve the 3rd party libraries's search issue, and this commit included CSSLayout->yoga, can't be cherry-picked in v0.40 directly

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

didn't see janic's comment when I commented above, I have to say that frustrated me so and I don't want to make another try, I'll try janic's solution mentioned earlier.

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made the changes that were needed to be able to archive my app here #11395

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@janicduplessis We still need to update the header paths in 3rd party libs, do we? I'll try it later.

@janicduplessis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I don't think there is a way to make this change and not have libs update their headers.

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this is another frustrating change for developers just like 0.26 or 0.29 😞

@sjmueller
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has anyone tested this yet in a swift project? I removed the following from header search paths:

"../node_modules/react-native/React/**",
"../node_modules/react-native/ReactCommon/**",
"../node_modules/react-native/Libraries/**"

Then added $(BUILT_PRODUCTS_DIR)/include/** and still getting tons of errors on both core and 3rd party libs:
image

Please advise...

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sjmueller You definitely missed something:

  1. You need to build within the commit 59407f3,
  2. You don't need to add $(BUILT_PRODUCTS_DIR)/include/** to the search path as it already in the default search path
  3. If you want to archive, you also need to apply @janicduplessis 's patch

I'll send a patch file and write a tutorial to make those changes

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've created a patch for RN0.40-stable branch
https://gist.github.com/nihgwu/7d03ca80fc807d5449a6862f2cbf54d8, this patch reverts the CSSLayout -> yoga change which hasn't been shipped to 0.40-stable branch

  1. copy this patch file to the root path of your project
  2. git apply patch.diff
  3. react-native upgrade and react-native link and other upgrade steps
  4. rename all "RCTXXX.h" to <React/RCTXXX.h> in AppDelegate.m and all your 3rd party libs
  5. remove all the React Native related path from the HEADER_SEARCH_PATHS

@sjmueller
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really appreciate your prompt response @nihgwu. Before your answer, I decided to switch from RN 0.40 to master, which includes 59407f3 as you mentioned. However, I still received the same errors, both in 3rd party libs and core libs. I believe I also tried many combinations with header search paths, including removal of all entries. To be honest I'm not entirely sure that the inclusion of certain paths would break something anyway.

I could try going back to 0.40 and apply your patch, but is this going to be a better combo than what's in latest master as of 2 hours ago?

I fear that there may be more to the story here, people are struggling with something similar using swift and cocoapods in #9014 (even though my project doesn't use CocoaPods at all). Over a year of working with RN, I've occasionally had to fork a third party lib to change an import statement from using brackets to quotes, probably because ObjC projects weren't hitting the same build exceptions. However, this change proposes that every dependency switch to brackets, and we should make sure it doesn't leave swift projects in a broken state that requires post install scripts to fixup dependencies 😢

@nihgwu
Copy link
Contributor

@nihgwu nihgwu commented on e1577df Dec 10, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't switch to the master version because react-native upgrade doesn't work well on that branch.
And I'm sorry I missed you mention that you are in a swift project, I have no experience on this

@robhogan
Copy link
Contributor

@robhogan robhogan commented on e1577df Dec 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there a case for adding #ifndef HEADER_H-style guards to React's public headers? That way we wouldn't be forcing third party libs to accommodate this change. It'd be safe for the user's header/library search paths to point to the same headers in multiple ways.

Edit: This seems to work. PR: #11614

@pvinis
Copy link
Contributor

@pvinis pvinis commented on e1577df Jan 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is disabling parallelization a good thing? isnt the packager slow enough, now we slow down xcode too? why?

@vongohren
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How is the work on swift support going? Any light in the end of the tunnel? Are there any updates in the near future that fixes this?

We are currently looking at RN to be our platform, but we are very interested in using swift with cocoapods. And this issue here has made us stop at version 0.38 so far. But we would really see that we were at the newest version.

@pvinis
Copy link
Contributor

@pvinis pvinis commented on e1577df Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Snorlock we are using react native with objc and swift in one project. its not that hard to set it up actually. and then you can just use swift for the native stuff and js for whatever else. the one annoying thing is the bridging files. so i have some managers and viewmanagers in swift, and for each of them i have to have a file called BlaManagerBridge.m that just has the func declarations for rn to see. not too bad for using swift for now. ping me on email me if you wanna talk.

@vongohren
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah our choice is that we want a pure swift application, meaning no objc. Then searching for issues related led me to this: #11572.
Which is the header issue that have been roaming around. In this issue they talked about this commit, so I mostly refer to that. Is there any light in the end of the tunnel for pure swift applications, based on the breakage in 0.40 - https://github.com/facebook/react-native/releases/tag/v0.40.0

@pvinis
Copy link
Contributor

@pvinis pvinis commented on e1577df Feb 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, i dont know about that. and i will bet its going to take a long time for that to happen. like at least 2 years, would be my guess. they have so many things to do and so many things in objc already done, that (at least the core team) i dont think they will spend time swiftifying everything.
if i were you, wanting to have a pure swift app with rn, i would write a couple of scripts to generate the bridging .m files, and not think about them any more. but i know thats not a perfect solution.

@atticoos
Copy link
Contributor

@atticoos atticoos commented on e1577df Feb 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unable to archive the project at the moment in 0.41.0. Running in debug mode works fine, but in release it appears that the React framework is not found:

screen shot 2017-02-09 at 5 12 06 pm
screen shot 2017-02-09 at 5 12 20 pm

@froatsnook
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajwhite I had the same issue. Sadly the fix was running react-native init again, copying over source files, and redoing the entire project setup.

Should you also take this journey, I wish you Godspeed.

@atticoos
Copy link
Contributor

@atticoos atticoos commented on e1577df Feb 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue boiled down to two things:

  1. First and foremost, we have custom Scheme configurations, those other than Debug and Release.
  2. Secondly, as React is a separate project that needs to be built, and is part of the primary project's build targets after 0.40.0, it will need to know what scheme to build for. However, it only supports Debug and Release. If you use a custom Scheme configuration, the react project is not going to know about it. It's going to put its Build Products in a Release directory, whereas the rest of your application is reading from your custom scheme configuration's build products directory.

Further discussion and solutions can be found in #11813.

I'm all set after having my custom schemes pull in the React Build Products from the Release directories, even though I'm not building a Release scheme.

@froatsnook
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajwhite Ah, that makes sense! I had a custom scheme I was trying to use for archiving. Thanks for the helpful info.

Please sign in to comment.