Fix ios build error when use_frameworks is on and fabric is off (v2) #33409

Kudo · 2022-03-11T08:24:48Z

Summary

alternative solution for #33379

when use_frameworks! is on, there are errors like:
'FBReactNativeSpec/FBReactNativeSpec.h' file not found
#import <FBReactNativeSpec/FBReactNativeSpec.h>
this error may come from from f7e4c07c84b6 regression.

when use_frameworks! is on, xcode will search headers from framework directories, the correct imports would be #import <React_Codegen/FBReactNativeSpec/FBReactNativeSpec.h> (xcode will transform dash to underscore, so it is React_Codegen but not React-Codegen). in the other hand, when use_frameworks! is off, the correct import is #import <React-Codegen/FBReactNativeSpec/FBReactNativeSpec.h>.

this fix is specific for old architecture (fabric is off).

when fabric is on, there are other errors from duplicated headers when copying to build folder. the reason is that framework build would try to flatten headers. we have primitives.h in different folders and they would be flattened into React_Fabric.framework/Headers. to be honest, i don't know how to deal with the problem in the meantime, maybe subspecs are not enough, we should separate them from subspecs to dedicated podspecs so that we can have these targets as different frameworks.

in this alternative fix, i try to add React-Codegen/React_Codegen.framework/Headers into header search paths and make original #import <FBReactNativeSpec/FBReactNativeSpec.h> reachable.

this change in the pr is just a workaround to solve breaking in latest main branch and this is not important to the use_frameworks! fix at all. this breaking was coming from 1804951.

Changelog

[iOS] [Fixed] - Fix iOS build error when Podfile use_frameworks! is on and Fabric is off

Test Plan

verify with rn-tester

change fabric_enabled to false in packages/rn-tester/Podfile
USE_FRAMEWORKS=1 pod install
build rn-tester in xcode

…orted

analysis-bot · 2022-03-11T08:33:30Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 009d80b
Branch: main

analysis-bot · 2022-03-11T09:01:30Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,214,079	+5
android	hermes	armeabi-v7a	7,799,788	+1
android	hermes	x86	8,587,566	+4
android	hermes	x86_64	8,538,497	-6
android	jsc	arm64-v8a	9,882,731	+2
android	jsc	armeabi-v7a	8,852,976	+0
android	jsc	x86	9,852,297	-10
android	jsc	x86_64	10,447,370	-6

Base commit: 009d80b
Branch: main

facebook-github-bot · 2022-03-11T10:41:49Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Kudo · 2022-03-11T12:42:32Z

@mikehardy could you help to verify this solution for your cases when you get a chance?
this is the patch file for v0.68.0-rc.2: https://gist.github.com/Kudo/7a590cbd6f8ca33fdf9f706ac96049ea

cortinico · 2022-03-11T12:43:40Z

It's all green interally so we should be good to go 👍

mikehardy · 2022-03-11T14:52:25Z

Fantastic! Verifying now @Kudo

mikehardy · 2022-03-11T16:19:29Z

It appears to work, applying this patch https://gist.githubusercontent.com/Kudo/7a590cbd6f8ca33fdf9f706ac96049ea/raw/00ff89b8c56b30f0db90bf1c74d4cdbe7c9f1612/react-native+0.68.0-rc.2.patch during my normal test of static frameworks build on iOS resulted in successful compilation + execution of the app

Nice!

See facebook/react-native#33409 for discussion

dmytrorykun · 2022-03-11T16:27:06Z

React/CoreModules/RCTExceptionsManager.h

@@ -37,7 +36,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)reportFatalException:(nullable NSString *)message
                       stack:(nullable NSArray<NSDictionary *> *)stack
                 exceptionId:(double)exceptionId;
- (void)reportEarlyJsException:(std::string)errorMap;
+- (void)reportEarlyJsException:(nullable NSString *)errorMap;


@Kudo please revert this change. It is not related to the original issue, and it breaks our internal tests.
Other than that, this solution looks good!

mikehardy · 2022-03-11T16:32:11Z

React/CoreModules/RCTExceptionsManager.mm

+- (void)reportEarlyJsException:(NSString *)errorMap
 {
-  NSString *errprStr = [NSString stringWithUTF8String:errorMap.c_str()];
+  NSString *errprStr = errorMap;


would these also need revert @dmitryrykun ? just scanning this after seeing your comment and they seem similar

Yeah, I meant all the changes in RCTExceptionsManager.

…t be exported" This reverts commit 7a0398c.

Kudo · 2022-03-11T16:39:31Z

reverted the commit. thanks @dmitryrykun for review and @mikehardy for kindly help to verify the fix 💛

facebook-github-bot · 2022-03-11T16:41:53Z

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

cortinico · 2022-03-11T16:47:57Z

Thank you all for the great work addressing this issue 🙏

react-native-bot · 2022-03-11T17:15:05Z

This pull request was successfully merged by @Kudo in a6c2846.

^{When will my fix make it into a release? | Upcoming Releases}

…33409) Summary: alternative solution for #33379 > when `use_frameworks!` is on, there are errors like: > ``` > 'FBReactNativeSpec/FBReactNativeSpec.h' file not found > #import <FBReactNativeSpec/FBReactNativeSpec.h> > ``` > this error may come from from f7e4c07c84b6 regression. > > when `use_frameworks!` is on, xcode will search headers from framework directories, the correct imports would be `#import <React_Codegen/FBReactNativeSpec/FBReactNativeSpec.h>` (xcode will transform dash to underscore, so it is `React_Codegen` but not `React-Codegen`). in the other hand, when `use_frameworks!` is off, the correct import is `#import <React-Codegen/FBReactNativeSpec/FBReactNativeSpec.h>`. > > > this fix is specific for old architecture (fabric is off). > > when fabric is on, there are other errors from duplicated headers when copying to build folder. [the reason is that framework build would try to flatten headers](https://mkonrad.net/2015/03/29/xcode-static-libraries-preserving-header-directory-structure.html). we have `primitives.h` in different folders and they would be flattened into `React_Fabric.framework/Headers`. to be honest, i don't know how to deal with the problem in the meantime, maybe subspecs are not enough, we should separate them from subspecs to dedicated podspecs so that we can have these targets as different frameworks. in this alternative fix, i try to add `React-Codegen/React_Codegen.framework/Headers` into header search paths and make original `#import <FBReactNativeSpec/FBReactNativeSpec.h>` reachable. [this change](7a0398c) in the pr is just a workaround to solve breaking in latest main branch and this is not important to the `use_frameworks!` fix at all. this breaking was coming from 1804951. [iOS] [Fixed] - Fix iOS build error when Podfile `use_frameworks!` is on and Fabric is off Pull Request resolved: #33409 Test Plan: verify with rn-tester 1. change `fabric_enabled` to false in `packages/rn-tester/Podfile` 2. `USE_FRAMEWORKS=1 pod install` 3. build rn-tester in xcode Reviewed By: dmitryrykun Differential Revision: D34817041 Pulled By: cortinico fbshipit-source-id: 4d1a610e99a807793eb3f64461e0d735c0a9ca9c

…acebook#33409) Summary: alternative solution for facebook#33379 > when `use_frameworks!` is on, there are errors like: > ``` > 'FBReactNativeSpec/FBReactNativeSpec.h' file not found > #import <FBReactNativeSpec/FBReactNativeSpec.h> > ``` > this error may come from from facebook@f7e4c07c84b6 regression. > > when `use_frameworks!` is on, xcode will search headers from framework directories, the correct imports would be `#import <React_Codegen/FBReactNativeSpec/FBReactNativeSpec.h>` (xcode will transform dash to underscore, so it is `React_Codegen` but not `React-Codegen`). in the other hand, when `use_frameworks!` is off, the correct import is `#import <React-Codegen/FBReactNativeSpec/FBReactNativeSpec.h>`. > > > this fix is specific for old architecture (fabric is off). > > when fabric is on, there are other errors from duplicated headers when copying to build folder. [the reason is that framework build would try to flatten headers](https://mkonrad.net/2015/03/29/xcode-static-libraries-preserving-header-directory-structure.html). we have `primitives.h` in different folders and they would be flattened into `React_Fabric.framework/Headers`. to be honest, i don't know how to deal with the problem in the meantime, maybe subspecs are not enough, we should separate them from subspecs to dedicated podspecs so that we can have these targets as different frameworks. in this alternative fix, i try to add `React-Codegen/React_Codegen.framework/Headers` into header search paths and make original `#import <FBReactNativeSpec/FBReactNativeSpec.h>` reachable. [this change](facebook@7a0398c) in the pr is just a workaround to solve breaking in latest main branch and this is not important to the `use_frameworks!` fix at all. this breaking was coming from facebook@1804951. ## Changelog [iOS] [Fixed] - Fix iOS build error when Podfile `use_frameworks!` is on and Fabric is off Pull Request resolved: facebook#33409 Test Plan: verify with rn-tester 1. change `fabric_enabled` to false in `packages/rn-tester/Podfile` 2. `USE_FRAMEWORKS=1 pod install` 3. build rn-tester in xcode Reviewed By: dmitryrykun Differential Revision: D34817041 Pulled By: cortinico fbshipit-source-id: 4d1a610e99a807793eb3f64461e0d735c0a9ca9c

Kudo added 2 commits March 11, 2022 16:13

[ios] Fix non-fabric mode use_frameworks build error

4003274

workaround framework build error because c++ header should not be exp…

7a0398c

…orted

facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Contributor A React Native contributor. p: Expo Partner: Expo Partner labels Mar 11, 2022

react-native-bot added Bug Platform: iOS iOS applications. labels Mar 11, 2022

facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Mar 11, 2022

mikehardy added a commit to mikehardy/rnfbdemo that referenced this pull request Mar 11, 2022

fix(ios, rn68): a different take on static frameworks fix

2039e6b

See facebook/react-native#33409 for discussion

dmytrorykun requested changes Mar 11, 2022

View reviewed changes

mikehardy reviewed Mar 11, 2022

View reviewed changes

Revert "workaround framework build error because c++ header should no…

bf2df84

…t be exported" This reverts commit 7a0398c.

facebook-github-bot closed this in a6c2846 Mar 11, 2022

cortinico mentioned this pull request Mar 11, 2022

Fix ios build error when use_frameworks is on and fabric is off #33379

Closed

react-native-bot added the Merged This PR has been merged. label Mar 11, 2022

Kudo deleted the fix-pod-errors-2 branch March 11, 2022 18:36

This was referenced Apr 21, 2022

[Snyk] Security upgrade react-native from 0.63.3 to 0.68.0 NOUIY/react-native-macos#28

Open

[Snyk] Upgrade react-native from 0.63.3 to 0.68.0 NOUIY/react-native-macos#32

Open

snyk-bot mentioned this pull request May 5, 2022

[Snyk] Upgrade react-native from 0.63.3 to 0.68.1 NOUIY/react-native-macos#99

Open

NOUIY mentioned this pull request Oct 5, 2022

[Snyk] Security upgrade react-native from 0.63.3 to 0.68.0 NOUIY/react-native-macos#789

Open

This was referenced Nov 14, 2022

i18n fallback testing discord/react-native#30

Closed

Alter font loading code to support fallback fonts in fontFamily discord/react-native#31

Merged

stevenpetryk mentioned this pull request Nov 21, 2022

Remove RN Android's bizarre font weight extension pattern discord/react-native#32

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ios build error when use_frameworks is on and fabric is off (v2) #33409

Fix ios build error when use_frameworks is on and fabric is off (v2) #33409

Kudo commented Mar 11, 2022

analysis-bot commented Mar 11, 2022 •

edited

Loading

analysis-bot commented Mar 11, 2022 •

edited

Loading

facebook-github-bot commented Mar 11, 2022

Kudo commented Mar 11, 2022

cortinico commented Mar 11, 2022

mikehardy commented Mar 11, 2022

mikehardy commented Mar 11, 2022

dmytrorykun Mar 11, 2022

mikehardy Mar 11, 2022 •

edited

Loading

dmytrorykun Mar 11, 2022

Kudo commented Mar 11, 2022

facebook-github-bot commented Mar 11, 2022

cortinico commented Mar 11, 2022

react-native-bot commented Mar 11, 2022

Fix ios build error when use_frameworks is on and fabric is off (v2) #33409

Fix ios build error when use_frameworks is on and fabric is off (v2) #33409

Conversation

Kudo commented Mar 11, 2022

Summary

Changelog

Test Plan

analysis-bot commented Mar 11, 2022 • edited Loading

analysis-bot commented Mar 11, 2022 • edited Loading

facebook-github-bot commented Mar 11, 2022

Kudo commented Mar 11, 2022

cortinico commented Mar 11, 2022

mikehardy commented Mar 11, 2022

mikehardy commented Mar 11, 2022

dmytrorykun Mar 11, 2022

Choose a reason for hiding this comment

mikehardy Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

dmytrorykun Mar 11, 2022

Choose a reason for hiding this comment

Kudo commented Mar 11, 2022

facebook-github-bot commented Mar 11, 2022

cortinico commented Mar 11, 2022

react-native-bot commented Mar 11, 2022

analysis-bot commented Mar 11, 2022 •

edited

Loading

analysis-bot commented Mar 11, 2022 •

edited

Loading

mikehardy Mar 11, 2022 •

edited

Loading