-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
feat(iOS): centralize JS engine dependency configuration #49297
Conversation
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Looks like dynamic frameworks are failing, I'm looking into this |
5a9a02c
to
b84aea5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the cleanup! :D
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Dynamic frameworks are still failing. You can build them locally by using:
|
b84aea5
to
608c743
Compare
@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
After talking with @cipolleschi, I realized that removing those dependencies is impossible because there are build errors about undefined symbols when using dynamic frameworks. I'll create a centralized function that adds those dependencies so its easier to implement logic of linking a 3rd party engine. |
608c743
to
1bf0e64
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left a nit
packages/react-native/ReactCommon/react/nativemodule/samples/ReactCommon-Samples.podspec
Outdated
Show resolved
Hide resolved
1bf0e64
to
83611aa
Compare
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
@cipolleschi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cipolleschi merged this pull request in e7de582. |
This pull request was successfully merged by @okwasniewski in e7de582 When will my fix make it into a release? | How to file a pick request? |
Summary:
Note
This PR is part of JavaScriptCore Extraction to this repository: https://github.com/react-native-community/javascriptcore
This PR centralizes the setup of js engine dependencies which need to be defined when building with dynamic frameworks. This will allow us to change linked framework if using a third party one in the future
Changelog:
[INTERNAL] [CHANGED] - centralize JS engine dependency configuration
Test Plan:
CI Green (Build needs to go properly)