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

Replace test app with react-native-test-app #120

Merged
merged 10 commits into from
Oct 2, 2024

Conversation

okwasniewski
Copy link
Contributor

In order to help maintenance and improve support for new out of tree platforms (like macOS and visionOS), let's replace the test app with https://github.com/microsoft/react-native-test-app . This simplifies the native code needed to stand up the example apps. In the case of apple platforms, it's now a single pod-sec.

Most of the work was done here: #109 by @Saadnajmi so kudos to him for that!

CleanShot 2024-09-17 at 17 11 12@2x

@okwasniewski
Copy link
Contributor Author

TODO: Fix the CI 😅

Going to do this tomorrow

@wcandillon
Copy link
Owner

nice 🫡 let me know if I can help with the CI

@wcandillon
Copy link
Owner

@okwasniewski silly question, does it support RN Web as well? Sorry I know nothing about this topic yet but it sounds very cool

@okwasniewski
Copy link
Contributor Author

@wcandillon So unfortunately, there is no first class support for web in test-app: microsoft/react-native-test-app#812

I guess you could have two examples (test-app + expo) that would probably give you the best coverage

@okwasniewski
Copy link
Contributor Author

I found an issue with the integration of test-app + turbo

I've contacted @tido64 so we can come up with a solution.

TLDR of the issue:

When running yarn react-native build-ios test app generates a new .xcode.env file and executes: which node command to supply it for Xcode.

However when executing this command through turbo (https://turbo.build/)

which node returns: [DEBUG] output from which node: /private/tmp/xfs-1a72c2db/node <- not correct path

Making the Xcode build fail

@okwasniewski
Copy link
Contributor Author

Hey @wcandillon build is now green!

Its ready to review

@wcandillon
Copy link
Owner

haha no way :)

@wcandillon wcandillon self-requested a review September 19, 2024 10:53
@wcandillon
Copy link
Owner

silly question: can we easily build the app in the new arch? 😇

@okwasniewski
Copy link
Contributor Author

silly question: can we easily build the app in the new arch? 😇

Yeah! For sure.

For iOS:

RCT_NEW_ARCH_ENABLED=1 pod install --project-directory=ios

For Android:

Modify apps/example/android/gradle.properties:

newArchEnabled=true

We can also add new arch build to the CI later

@wcandillon
Copy link
Owner

this is fantastic thank. Right now this doesn't work on iOS (Canvas API crashes, that's a bug on our side) nor Android (e2e not working, probably an issue on our side as well). However are currently fixing a serious bug in our Canvas API.

What I will do on my side is to work on shipping this bug fixes asap so we can test it. But I promise I won't touch the example app so you don't need to fix time fixing conflicts and so on.

Thank you again for this, this is very compelling.

@Saadnajmi
Copy link
Contributor

Awesome work, thanks @okwasniewski for taking the change and getting it ready and @tido64 for helping RNTA work with Turborepo repos!

@wcandillon
Copy link
Owner

None of the examples work for me. First I thought it was related to some canvas API bugs we are currently fixing but now it looks like it might be something else?

@okwasniewski
Copy link
Contributor Author

@wcandillon What is the error you are seeing? Im taking a look

@okwasniewski
Copy link
Contributor Author

@wcandillon Im sorry but I can't reproduce this issue, everything works fine for me. Just to be sure you disabled the Metal API Validation in Xcode?

CleanShot.2024-09-25.at.11.03.44.mp4

Copy link
Owner

@wcandillon wcandillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is big thank you for this.

@wcandillon
Copy link
Owner

@okwasniewski I'm experiencing some crashes on iOS, can you check on your side?

@okwasniewski
Copy link
Contributor Author

@wcandillon Can you hint me what is crashing for you? I can see the build is failing but that's because of missing dawn.json file.

@wcandillon
Copy link
Owner

the e2e tests but the same error happens when browsing the examples. I also cannot install the apk on Android but the issue might be on my side there.

@okwasniewski
Copy link
Contributor Author

@wcandillon I'm going to test Android soon, so the tests were green on previous app and now they are red? Is it the setup of the tests or is it a native crash? I didn't run them

@wcandillon
Copy link
Owner

It's a native crash that I can also reproduce by browsing the examples.

@Saadnajmi
Copy link
Contributor

I'll also try and test

@okwasniewski
Copy link
Contributor Author

@wcandillon Sorry for the delay.

I went to every example and no crash (I built using Android Studio) but it shouldn't matter. Can you provide a crash stack trace? Either way I don't think its related to TestApp more likely its the library.

CleanShot.2024-09-27.at.15.55.39.mp4

@wcandillon
Copy link
Owner

wcandillon commented Oct 1, 2024

Here is the list of issues/regressions I am experiencing. However, these probably shouldn't be called regressions since we are just changing the example app, so the module should still work there.

On iOS, the ExternalTexture.spec.ts e2e test fails, and running the three.js helmet example followed by the r3f example causes a crash.

This behavior is not happening in apps/paper, and I'm not sure why yet.

@Saadnajmi
Copy link
Contributor

Here is the list of issues/regressions I am experiencing. However, these probably shouldn't be called regressions since we are just changing the example app, so the module should still work there.

On iOS, the ExternalTexture.spec.ts e2e test fails, and running the three.js helmet example followed by the r3f example causes a crash.

This behavior is not happening in apps/paper, and I'm not sure why yet.

@wcandillon is there any chance those issues are intermittent and not related to this PR? I can't think of why the test app would cause those things to fail, and I would personally like this PR to unblock new architecture / macOS / potential future Windows support. I just set up a new laptop so I'll try to repro the issues mentioned sometime this week on a clean install.

@wcandillon
Copy link
Owner

I found the issue and it is as we expected unrelated to this PR, sorting it out now.

@wcandillon wcandillon merged commit 264d40a into wcandillon:main Oct 2, 2024
5 checks passed
@okwasniewski okwasniewski deleted the rnta branch October 2, 2024 07:47
@wcandillon
Copy link
Owner

@okwasniewski do you know how I can have the "Metal API Validation" unchecked in the example app? If I uncheck it in xcode, I don't see a change in git, probably because the file is in gitignore?

@Saadnajmi
Copy link
Contributor

Saadnajmi commented Oct 7, 2024

@okwasniewski do you know how I can have the "Metal API Validation" unchecked in the example app? If I uncheck it in xcode, I don't see a change in git, probably because the file is in gitignore?

@tido64 perhaps this is a feature request for RNTA?

Otherwise maybe an expo config plugin?

@wcandillon
Copy link
Owner

@okwasniewski I am also unable to build the app on fabric/android, my bad for not testing it before.
@Saadnajmi Any specific instructions when looking to upgrade the example app?

@okwasniewski
Copy link
Contributor Author

@tido64
Copy link

tido64 commented Oct 8, 2024

@okwasniewski do you know how I can have the "Metal API Validation" unchecked in the example app? If I uncheck it in xcode, I don't see a change in git, probably because the file is in gitignore?

You will have to write an Expo config plugin for this (or patch the .xcscheme some other way). You should find the .xcheme at node_modules/.generated/ios/ReactTestApp.xcodeproj/xcshareddata/xcschemes/<MyApp>.xcscheme, and you need to set enableGPUValidationMode = "0" on the first LaunchAction.

More info on Config Plugins can be found here: https://github.com/microsoft/react-native-test-app/wiki/Config-Plugins

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.

4 participants