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

Workaround React Native 0.71 and XCFramework setup issues #5912

Conversation

mokagio
Copy link
Contributor

@mokagio mokagio commented Jun 28, 2023

Same as #5908 but without Hermes, just to see how it works in Jetpack/WordPress.

I disabled Hermes because adding it required some additional setup (see #5908) which I'm not sure I have worked out yet.

This is the first build I get to compile and run successfully on my machine when integrated. A WordPress CI build is running here.

Update: I have builds that compile and on which I can load the editor for both implementations, with and without Hermes. I made whether to use engine configurable at build time with the HERMES_ENABLED environment variable. It's now on by default because my understanding is that is the preferred solution.

The issue is due to RNReanimated looking for react-native in the
node_modules relative to its location, which is how most projects would
be setup. Unfortunately, our projects is not standard.

When running `pod install` or `pod update`, we'd get:

```
Installing RNReanimated 2.17.0 (was 2.9.1-wp-4)
internal/modules/cjs/loader.js:934
  throw err;
  ^

Error: Cannot find module 'react-native/package.json'
Require stack:
- /Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/[eval]
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:931:15)
    at Function.resolve (internal/modules/cjs/helpers.js:113:19)
    at [eval]:1:9
    at Script.runInThisContext (vm.js:134:12)
    at Object.runInThisContext (vm.js:310:38)
    at internal/process/execution.js:81:19
    at [eval]-wrapper:6:22
    at evalScript (internal/process/execution.js:80:60)
    at internal/main/eval_string.js:27:3 {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/[eval]'
  ]
}

[!] Invalid `RNReanimated.podspec` file: no implicit conversion of nil into String.

 #  from /var/folders/dq/cdqxvx3s5ps75564rpmb_dc00000gn/T/d20230627-89428-abwplk/RNReanimated.podspec:5
 #  -------------------------------------------
 #  reanimated_package_json = JSON.parse(File.read(File.join(__dir__, "package.json")))
 >  config = find_config()
 #  assert_no_multiple_instances(config)
 #  -------------------------------------------
```
@mokagio mokagio force-pushed the mokagio/workaround-rnreanimated-2.17.0 branch from b77ceee to f1fde4c Compare June 28, 2023 11:58
@mokagio mokagio changed the title Disable Hermes when building XCFramework Workaround React Native 0.71 and XCFramework setup issues (without Hermes) Jun 28, 2023
@mokagio mokagio changed the base branch from mokagio/workaround-rnreanimated-2.17.0 to upgrade/react-native-0.71-ios June 28, 2023 12:34
mokagio and others added 13 commits June 29, 2023 20:49
Using only our custom specs, the build failed with

```
Multiple commands produce '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/DerivedData/XCFrameworkScaffold/Build/Products/Debug-iphonesimulator/ReactCommon/ReactCommon.framework/Headers/CallbackWrapper.h'

- Target 'ReactCommon' (project 'Pods') has copy command from '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/Pods/ReactCommon/react/bridging/CallbackWrapper.h' to '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/DerivedData/XCFrameworkScaffold/Build/Products/Debug-iphonesimulator/ReactCommon/ReactCommon.framework/Headers/CallbackWrapper.h'
- Target 'ReactCommon' (project 'Pods') has copy command from '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/Pods/ReactCommon/react/nativemodule/core/ReactCommon/CallbackWrapper.h' to '/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/DerivedData/XCFrameworkScaffold/Build/Products/Debug-iphonesimulator/ReactCommon/ReactCommon.framework/Headers/CallbackWrapper.h'
```

However, with this setup the build fails with:

```
/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/Pods/../../gutenberg/node_modules/react-native/React/FBReactNativeSpec/../../scripts/xcode/with-environment.sh: line 35: .xcode.env: command not found
/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/Pods/../../gutenberg/node_modules/react-native/React/FBReactNativeSpec/../../scripts/xcode/with-environment.sh: line 35: node: command not found
[Warning] You need to configure your node path in the  environment.  You can set it up quickly by running:  echo 'export NODE_BINARY=' > .xcode.env  in the ios folder. This is needed by React Native to work correctly.  We fallback to the DEPRECATED behavior of finding . This will be REMOVED in a future version.  You can read more about this here: https://reactnative.dev/docs/environment-setup#optional-configuring-your-environment
/Users/gio/Developer/a8c/gutenberg-mobile/ios-xcframework/Pods/../../gutenberg/node_modules/react-native/React/FBReactNativeSpec/../../scripts/xcode/with-environment.sh: line 41: /scripts/find-node-for-xcode.sh: No such file or directory
[Error] Could not find node. It looks like that the .xcode.env or .xcode.env.local
```
This way, users can override what CocoaPods does without modifying the
codebase via the `.local` file.
@mokagio mokagio force-pushed the mokagio/workaround-rnreanimated-2.17.0-no-hermes branch from 4f31ae2 to c2bfebc Compare June 29, 2023 11:00
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 29, 2023

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@mokagio mokagio force-pushed the mokagio/workaround-rnreanimated-2.17.0-no-hermes branch from 27b232f to 91bacf5 Compare June 29, 2023 23:10
@mokagio mokagio changed the title Workaround React Native 0.71 and XCFramework setup issues (without Hermes) Workaround React Native 0.71 and XCFramework setup issues Jun 30, 2023
@fluiddot
Copy link
Contributor

iOS Ci jobs are passing with this workaround 🎊 (we can omit Test iOS on Device - Canaries as it's also failing in the base branch). @mokagio In order to unblock the iOS installable build, we'd like to set this PR ready to review, and eventually merge it. Let us know if you have any concerns about this, thanks 🙇 !

@fluiddot fluiddot marked this pull request as ready for review June 30, 2023 10:29
@mokagio mokagio force-pushed the mokagio/workaround-rnreanimated-2.17.0-no-hermes branch from 135dcb1 to 6f175c1 Compare July 3, 2023 07:59
@mokagio mokagio force-pushed the mokagio/workaround-rnreanimated-2.17.0-no-hermes branch from 8d171f3 to dbfac03 Compare July 3, 2023 11:24
@mokagio
Copy link
Contributor Author

mokagio commented Jul 3, 2023

@fluiddot sorry to hold you up. I've been investigating this branch for a good part of my day, starting from CI to run the tests when using the XCFramework built from f2d1b716bcf0ec89c637dd0f035c0188dcce6f38:

image

The problem, as can be seen when using that version locally, is that Hermes cannot be found

dyld[99988]: Library not loaded: @rpath/hermes.framework/hermes
  Referenced from: <931F58EC-3AC7-39DE-BEB7-1D44D8A5CF15> /Users/gio/Library/Developer/CoreSimulator/Devices/90F54127-B854-4ED8-93FA-D58167331941/data/Containers/Bundle/Application/3DA26EC7-B308-4029-9974-426839535542/WordPress.app/Frameworks/Gutenberg.framework/Gutenberg

I think what happened is that the fix for that issue that I had implemented in b77ceee got lost when I ported over the work from #5908 to this PR.

I'm working on addressing that... Apologies for the delay.

@fluiddot
Copy link
Contributor

fluiddot commented Jul 3, 2023

I think what happened is that the fix for that issue that I had implemented in b77ceee got lost when I ported over the work from #5908 to this PR.

I'm working on addressing that... Apologies for the delay.

@mokagio No worries, let me know if I can help any how on this workaround 🙇 . I tried exploring part of my day building the WP-iOS app using Gutenberg locally, but no luck so far. For now, and for the sake of unblocking the iOS builds for the RN upgrade, I'd advocate focusing on the CI installable builds. After that, we could explore how to enable local builds.

@mokagio
Copy link
Contributor Author

mokagio commented Jul 3, 2023

FYI, I am working on the said fixes in #5924

For now, and for the sake of unblocking the iOS builds for the RN upgrade, I'd advocate focusing on the CI installable builds. After that, we could explore how to enable local builds.

+1

@mokagio
Copy link
Contributor Author

mokagio commented Jul 3, 2023

Closing in favor of #5924

@mokagio mokagio closed this Jul 3, 2023
@mokagio mokagio deleted the mokagio/workaround-rnreanimated-2.17.0-no-hermes branch July 3, 2023 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants