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: publish rnskia prefab to allow RN Skia to be used in third party libraries #1365

Merged
merged 6 commits into from
Feb 20, 2023

Conversation

mrousavy
Copy link
Contributor

need this for VisionCamera v3 :)

@wcandillon
Copy link
Contributor

this may also fix #1358

@chrfalch chrfalch self-requested a review February 16, 2023 07:48
Copy link
Contributor

@chrfalch chrfalch left a comment

Choose a reason for hiding this comment

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

Tested this with clean RN apps version 0.67, 0.68... 0.71, all failed with the following error message:

* What went wrong:
Execution failed for task ':shopify_react-native-skia:buildCMakeDebug[arm64-v8a]'.
> [CXX1404] did not find implicitly required targets: rnskia

Could this be something with the prefab target? Does it need to be declared somewhere?

@mrousavy
Copy link
Contributor Author

@chrfalch I think my latest commit fixes that, but I'm still in the middle of testing.

@chrfalch
Copy link
Contributor

I'll test a bit later - why didn't we just change rnskia -> reactskia in the gradle file under the prefab instead of in the cmake file??

@mrousavy
Copy link
Contributor Author

Because RNSkia is the C++ namespace and that makes more sense to me tbh

Copy link
Contributor

@chrfalch chrfalch left a comment

Choose a reason for hiding this comment

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

I've now tested this on RN 0.68 to 0.71 with the new changes. It seems like adding the prefab true fails on RN 0.68 and below. I added a suggestion on how we can avoid this error.

package/android/build.gradle Outdated Show resolved Hide resolved
package/android/build.gradle Outdated Show resolved Hide resolved
@chrfalch chrfalch merged commit 4f17a0b into Shopify:main Feb 20, 2023
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.

3 participants