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

feat: move test app to react-native-test-app #2746

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Saadnajmi
Copy link

@Saadnajmi Saadnajmi commented Nov 19, 2024

TODO:

  • Test iOS Paper
  • Test iOS Fabric
  • Test Android Paper
  • Test Android Fabric

Summary

I am interested in (eventually) adding macOS support to react-native-skia. Generally the first step to this is to first move the repository's test app to React Native Test App. This repo offers a few benefits:

  1. RNTA handles the native code of an example app for you (similar to expo's continuous native generation), so you only have to worry about your apps' JS.
  2. Backwards and forwards compatibility across a few Raect Native minor versions
  3. (Most importantly) The ability to easily add support for new platforms. In this case, of the 3 extra platforms RNTA supports ( macOS, visionOS, windows), the ones I know how to port after this change are macOS and visionOS.

Notes

  • I dropped the babel plugin transform-inline-environment-variables because things seemed to bundle fine without it?
  • I dropped a few other dependencies in the test app I didn't know what they were doing. Most likely I'll add a couple back as I try to get the CI to pass.

Test Plan

I got the iOS example app to run with the old architecture so far:

Screenshot 2024-11-18 at 9 28 08 PM

@Saadnajmi
Copy link
Author

@wcandillon Could I get the workflows approved so I can start testing my changes?

@wcandillon
Copy link
Contributor

@Saadnajmi I'm very excited about this and I will try to help as much as I can with it, however transform-inline-environment-variables cannot be dropped as we need it for CI (to open the app on the e2e test screen).

@Saadnajmi
Copy link
Author

@Saadnajmi I'm very excited about this and I will try to help as much as I can with it, however transform-inline-environment-variables cannot be dropped as we need it for CI (to open the app on the e2e test screen).

Makes sense! I should have left the PR in a draft state, my apologies. I figured I'd need to add back 95% of what was dropped as I ran into their use cases :)

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.

2 participants