Skip to content

[iOS] Fixes template build failed in release mode#25751

Closed
zhongwuzw wants to merge 1 commit intofacebook:masterfrom
zhongwuzw:fix_template_release_build
Closed

[iOS] Fixes template build failed in release mode#25751
zhongwuzw wants to merge 1 commit intofacebook:masterfrom
zhongwuzw:fix_template_release_build

Conversation

@zhongwuzw
Copy link
Copy Markdown
Contributor

Summary

Fixes #25745, Xcode stripped dead code in Release mode, and in template test code, it should only run in Debug mode, so we can use a macro to fix this issue.

Changelog

[iOS] [Fixed] - Fixes template build failed in release mode

Test Plan

  1. Create a new project using react-native init AwesomProject.
  2. Change project target scheme to Release.
  3. Build and it should success.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 21, 2019
@zhongwuzw zhongwuzw requested a review from elicwhite July 21, 2019 04:47
@react-native-bot react-native-bot added Platform: iOS iOS applications. Bug labels Jul 21, 2019
@elicwhite
Copy link
Copy Markdown
Member

Thanks for this. I wouldn’t have expected a tests file to cause the app to fail to build. Is that normal?

Also, would it make sense to validate that release builds build successfully in CI to keep this kind of issue from sneaking back in?

Should we also have a release scheme in the default template? I can’t imagine that anyone starts an RN project without wanting to build for release so they have to go through all those steps on their own.

@zhongwuzw
Copy link
Copy Markdown
Contributor Author

Thanks for this. I wouldn’t have expected a tests file to cause the app to fail to build. Is that normal?

Yeah, we should avoid.

Also, would it make sense to validate that release builds build successfully in CI to keep this kind of issue from sneaking back in?

We don't use template project in main react-native's CI, I think only cli uses it, so more suitable for cli to add it in CI?

Should we also have a release scheme in the default template? I can’t imagine that anyone starts an RN project without wanting to build for release so they have to go through all those steps on their own.

Emm, we can left users to create, users don't use release mode to develop in most of the time, only when profile or publish app.

Copy link
Copy Markdown
Member

@elicwhite elicwhite left a comment

Choose a reason for hiding this comment

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

I can’t land this from my phone so I’ll land it on Monday at work if someone else hasn’t landed it already by then

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@TheSavior is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @zhongwuzw in 0fcaca8.

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

@mmazzarolo
Copy link
Copy Markdown

Applying this change to /ios/{appname}Tests/{appname}Tests.m fixes the issue.
Thanks!

vovkasm pushed a commit to vovkasm/react-native that referenced this pull request Aug 7, 2019
Summary:
Fixes facebook#25745, Xcode stripped dead code in Release mode, and in template test code, it should only run in Debug mode, so we can use a macro to fix this issue.

## Changelog

[iOS] [Fixed] - Fixes template build failed in release mode
Pull Request resolved: facebook#25751

Test Plan:
1. Create a new project using `react-native init AwesomProject`.
2. Change project target scheme to `Release`.
3. Build and it should success.

Differential Revision: D16442643

Pulled By: TheSavior

fbshipit-source-id: f08ed70523aa1aa418064465f8df367a06e8974f
(cherry picked from commit 0fcaca8)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: iOS iOS applications.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

_RCTSetLogFunction build failure in release mode fixed with Dead Code Stripping = No

5 participants