Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable DEFINES_MODULE for DoubleConversion #42591

Closed

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Jan 22, 2024

Summary:

since 32dab7a, DoubleConversion is now added as an implicit dependency for 3rd party module and it breaks swift integration. this pr tries to add the DEFINES_MODULE to DoubleConversion.

Changelog:

[IOS] [FIXED] - Fixed DoubleConversion build error from Swift integration

Test Plan:

i'll need to test this on expo latest and react-native nightly build

# pull latest expo repo and get template tarball
$ git clone --depth 1 https://github.com/expo/expo.git 
$ cd expo
$ yarn install
$ cd templates/expo-template-bare-minimum
$ npx pack --pack-destination ../../

# now create an expo app
$ yarn create expo -t blank@sdk-50 sdk50
$ cd sdk50
$ yarn add react-native@nightly
$ jq '.expo.runtimeVersion = { "policy": "appVersion" }' app.json > app.json.tmp && mv app.json.tmp app.json
$ npx expo prebuild -p ios --template /path/to/expo/expo-template-bare-minimum-50.0.17.tgz
$ cd ios
$ pod install

then it will show the error message:

[!] The following Swift pods cannot yet be integrated as static libraries:

The Swift pod `ExpoModulesCore` depends upon `DoubleConversion`, which does not define modules. To opt into those targets generating module maps (which is necessary to import them from Swift when building as static libraries), you may set `use_modular_headers!` globally in your Podfile, or specify `:modular_headers => true` for particular dependencies.

@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. Contributor A React Native contributor. p: Expo Partner: Expo Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 22, 2024
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.

Thanks for this fix!

@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 Jan 25, 2024
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in 631b6a1.

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. Contributor A React Native contributor. Merged This PR has been merged. p: Expo Partner: Expo 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.

3 participants