Fix hermes and jsc pods for test targets

[louiszawadzki]

Summary

I'm maintaining a library that includes iOS tests (https://github.com/DataDog/dd-sdk-reactnative).

To run these tests, I'm adding the test target to a simple application, from where I am able to run the tests:

Since React Native 0.71, the build for the test target fails with the following errors (depending on whether hermes is enabled or not):

Without hermes:

Undefined symbols for architecture arm64:
  "facebook::jsc::makeJSCRuntime()", referenced from:
      facebook::react::JSCExecutorFactory::createJSExecutor(std::__1::shared_ptr<facebook::react::ExecutorDelegate>, std::__1::shared_ptr<facebook::react::MessageQueueThread>) in libReact-Core.a(JSCExecutorFactory.o)

With hermes:

Undefined symbols for architecture arm64:
  "facebook::react::HermesExecutorFactory::defaultRuntimeConfig()", referenced from:
      std::__1::__shared_ptr_emplace<facebook::react::HermesExecutorFactory, std::__1::allocator<facebook::react::HermesExecutorFactory> >::__shared_ptr_emplace<std::__1::function<void (facebook::jsi::Runtime&)>&>(std::__1::allocator<facebook::react::HermesExecutorFactory>, std::__1::function<void (facebook::jsi::Runtime&)>&) in libReact-Core.a(RCTCxxBridge.o)
  "vtable for facebook::react::HermesExecutorFactory", referenced from:
      facebook::react::HermesExecutorFactory::HermesExecutorFactory(std::__1::function<void (facebook::jsi::Runtime&)>, std::__1::function<void (std::__1::function<void ()> const&, std::__1::function<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > ()>)> const&, hermes::vm::RuntimeConfig) in libReact-Core.a(RCTCxxBridge.o)
  NOTE: a missing vtable usually means the first non-inline virtual member function has no definition.

When I investigated into the testing target for my pod, I realized that React-jsc (or React-hermes if hermes is enabled) were missing from the following Build Settings:

• other linker flags
• header search paths
• library search paths

See screenshots below (with hermes):

After comparing with other pods that were included in these locations, I noticed that the difference was that they were not in the dependencies of React-Core and that React-jsc is missing a header_dir property in its podspec.

After adding both changes, I was able to build the test target again, and there was no regression on my simple application.

I am not sure this is the best way to achieve this, I don't understand fully how header search paths etc. are defined for Pods targets, so feel free to point me to a better solution if you can think of one.

This could also potentially fix #34344

Changelog

[IOS] [FIXED] - Fix builds for pods testing targets

Test Plan

I am still able to build a simple RN 0.71 application.
With the changes, my test target builds and tests are passing.

[facebook-github-bot]

Hi @louiszawadzki!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,463,316	+0
android	hermes	armeabi-v7a	7,783,671	+0
android	hermes	x86	8,936,293	+0
android	hermes	x86_64	8,794,200	+0
android	jsc	arm64-v8a	9,648,925	+0
android	jsc	armeabi-v7a	8,383,202	+0
android	jsc	x86	9,710,897	+0
android	jsc	x86_64	10,187,754	+0

Base commit: e91e468
Branch: main

[cipolleschi]

Hi @louiszawadzki, thanks for the PR. We are about to land a proper fix for the issue, that also adds some tests in CI to ensure that we won't have regressions in the future. You can see the actual changes here: #36003.

Do you think I can close this PR?

[louiszawadzki]

Thanks a lot @cipolleschi!

I'll check the changes, if they still need adjustment for my use case I'll open a new PR to discuss it :)

[louiszawadzki]

Just a heads up, I checked and my use case is solved with 0.71.2 and I was even able to enable hermes :)

Thanks a lot for addressing this!