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

[core] fix use_frameworks with new arch #28451

Conversation

Kudo
Copy link
Contributor

@Kudo Kudo commented Apr 25, 2024

Why

fix header not found issue when using frameworks on new architecture
fixes #28209

How

the header search path is from EXJavaScriptRuntime.mm -> HermesExecutorFactory.h -> <jsinspector-modern/InspectorInterfaces.h>. the correct import should be underlined "jsinspector_modern" in the import. the code is inside react-native core that we cannot touch.
this pr tries to use the add_dependency() from react-native core that we specify extra ":framework_name".

also cleanup some unused search paths that are added through install_modules_dependencies().
we also have to wait 0.74.1 for facebook/react-native#44252

Test Plan

  • use_frameworks=static + hermes + newArchEnabled=true
  • use_frameworks=static + jsc + newArchEnabled=true
  • hermes + newArchEnabled=true
  • jsc + newArchEnabled=true
  • hermes + newArchEnabled=false
  • jsc + newArchEnabled=false
  • use_frameworks=static + hermes + newArchEnabled=false

Checklist

@expo-bot expo-bot added bot: suggestions ExpoBot has some suggestions bot: passed checks ExpoBot has nothing to complain about and removed bot: suggestions ExpoBot has some suggestions labels Apr 25, 2024
@Kudo Kudo force-pushed the @kudo/eng-11972-expo-modulessdk51-build-error-with-useframeworks-and-new branch from fe77c83 to 4904fdf Compare May 2, 2024 01:02
@leonhh
Copy link
Contributor

leonhh commented May 3, 2024

@Kudo I think we can we merge this now, right? Your PR has been released in 0.74.1 and is also already available in expo #28593

@Kudo Kudo force-pushed the @kudo/eng-11972-expo-modulessdk51-build-error-with-useframeworks-and-new branch from 4904fdf to 2f27d5e Compare May 3, 2024 08:42
@Kudo
Copy link
Contributor Author

Kudo commented May 3, 2024

ios updates e2e are failed unrelated. will investigate asides from this pr.

@Kudo Kudo marked this pull request as ready for review May 3, 2024 14:31
@Kudo Kudo merged commit df3fb83 into main May 3, 2024
12 of 13 checks passed
@Kudo Kudo deleted the @kudo/eng-11972-expo-modulessdk51-build-error-with-useframeworks-and-new branch May 3, 2024 15:00
@brentvatne brentvatne added the published Changes from the PR have been published to npm label May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: fingerprint compatible bot: passed checks ExpoBot has nothing to complain about published Changes from the PR have been published to npm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[expo-modules][expo-dev-client][SDK51] build error with useFrameworks: 'static'
6 participants