Skip to content

Conversation

@okwasniewski
Copy link
Contributor

Summary:

This PR adds cocoapods utility to set SWIFT_ACTIVE_COMPILATION_CONDITIONS to DEBUG, which is set to this value by default (when generating a new native Xcode project).

This allows to use the #if DEBUG compilator directive in Swift to work out of the box, without any changes on user's side:

override func bundleURL() -> URL? {
#if DEBUG
    RCTBundleURLProvider.sharedSettings().jsBundleURL(forBundleRoot: "index")
#else
    Bundle.main.url(forResource: "main", withExtension: "jsbundle")
#endif
  }

Changelog:

[IOS] [ADDED] - Set SWIFT_ACTIVE_COMPILATION_CONDITIONS to DEBUG

Test Plan:

Run bundle exec pod install and check if the active compilation flags are populated:

CleanShot 2024-01-17 at 13 11 03@2x

@facebook-github-bot 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. p: Callstack Partner: Callstack Partner labels Jan 17, 2024
@okwasniewski okwasniewski changed the title feat: add set_swift_compilation_conditions utility to set Swift compilation conditions feat: set SWIFT_ACTIVE_COMPILATION_CONDITIONS build setting to DEBUG Jan 17, 2024
@okwasniewski okwasniewski changed the title feat: set SWIFT_ACTIVE_COMPILATION_CONDITIONS build setting to DEBUG feat: set Swift active compilation conditions build setting to DEBUG Jan 17, 2024
@facebook-github-bot 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 Jan 17, 2024
@analysis-bot
Copy link

analysis-bot commented Jan 17, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 16,803,066 -435,494
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 20,190,312 -410,683
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: b41a33e
Branch: main

@okwasniewski
Copy link
Contributor Author

Hey @cipolleschi, anything I can do to help you get this one merged?

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Sorry, I completely missed this PR. 😭

The changes look good. Could you add a test in utils-test.rb file to verify that the build settings is actually added? In this way we can make sure not to miss changes in the set_swift_compilation_conditions method

@okwasniewski okwasniewski force-pushed the feat/swift-active-compilation-condtions branch from df39513 to 69f4840 Compare February 12, 2024 16:15
@okwasniewski okwasniewski force-pushed the feat/swift-active-compilation-condtions branch from 69f4840 to d821fef Compare February 12, 2024 16:24
@okwasniewski
Copy link
Contributor Author

@cipolleschi I've refactored the method to set_build_setting which accepts necessary arguments to set proper build settings. This method can remove set_use_hermes_build_setting and set_node_modules_user_settings methods (less code to maintain). I've also changed the tests to only test the utility

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

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

Great job, thanks for refactoring!

@cipolleschi
Copy link
Contributor

/rebase - this command rebase the pr on top of main automatically

@okwasniewski
Copy link
Contributor Author

Hey @cipolleschi, is there anything more we need to get done in order to merge this PR?

@cipolleschi
Copy link
Contributor

/rebase - this comment automatically rebase on top of main

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Jun 6, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 756f89a.

@github-actions
Copy link

github-actions bot commented Jun 6, 2024

This pull request was successfully merged by @okwasniewski in 756f89a.

When will my fix make it into a release? | How to file a pick request?

kosmydel pushed a commit to kosmydel/react-native that referenced this pull request Jun 11, 2024
…acebook#42330)

Summary:
This PR adds cocoapods utility to set `SWIFT_ACTIVE_COMPILATION_CONDITIONS` to DEBUG, which is set to this value by default (when generating a new native Xcode project).

This allows to use the `#if DEBUG` compilator directive in Swift to work out of the box, without any changes on user's side:

```swift
override func bundleURL() -> URL? {
#if DEBUG
    RCTBundleURLProvider.sharedSettings().jsBundleURL(forBundleRoot: "index")
#else
    Bundle.main.url(forResource: "main", withExtension: "jsbundle")
#endif
  }
```

## Changelog:

[IOS] [ADDED] - Set SWIFT_ACTIVE_COMPILATION_CONDITIONS to DEBUG

Pull Request resolved: facebook#42330

Test Plan:
Run `bundle exec pod install` and check if the active compilation flags are populated:

![CleanShot 2024-01-17 at 13 11 03@2x](https://github.com/facebook/react-native/assets/52801365/68f119a6-af47-41bc-a1f1-9085fe4df6a0)

Reviewed By: cortinico

Differential Revision: D58188103

Pulled By: cipolleschi

fbshipit-source-id: 64746f3c7bfbdf47c2dea5e5e8cb2962635b719b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. p: Callstack Partner: Callstack Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants