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: support new arch #1961

Merged
merged 40 commits into from
Nov 10, 2023
Merged

Conversation

WoLewicki
Copy link
Contributor

@WoLewicki WoLewicki commented Nov 7, 2023

PR adding support for new architecture and adding fabricexample app where it can be tested.

Test package: https://bit.ly/shopify-react-native-skia-fabric

fixes #387
fixes #676

@WoLewicki
Copy link
Contributor Author

I have signed the CLA!

@@ -1,6 +1,11 @@
#pragma once

#ifdef TARGET_OS_IPHONE
#include <react-native-skia/ImageProps.h>
#else
#include "ImageProps.h"
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to SkImageProps

@@ -0,0 +1,79 @@
This is a new [**React Native**](https://reactnative.dev) project, bootstrapped using [`@react-native-community/cli`](/~https://github.com/react-native-community/cli).

Copy link
Contributor

Choose a reason for hiding this comment

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

Needs to be added to yarn bootstrap, also we need to add it to CI. Add pod install to postinstall on both example and fabricexample

@@ -0,0 +1,8 @@
import type { TurboModule } from "react-native/Libraries/TurboModule/RCTExport";
import { TurboModuleRegistry } from "react-native";
Copy link
Contributor

Choose a reason for hiding this comment

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

here we probably want to use to use platform (Platform is what we use to "polyfill" react native API on web without the need to use react-native-web)

@WoLewicki
Copy link
Contributor Author

I have signed the CLA!

@wcandillon
Copy link
Contributor

You can test the RN Skia on Fabric with this test package: https://bit.ly/shopify-react-native-skia-fabric

@Daavidaviid
Copy link

I get this error testing it with the RNSkia hello world:

Header Header
Screenshot 2023-11-08 at 12 13 28 Screenshot 2023-11-08 at 12 13 18

@wcandillon wcandillon self-requested a review November 10, 2023 13:56
@wcandillon
Copy link
Contributor

@WoLewicki congrats 🥂🙌🏼 and a big thank you ❤️

@wcandillon wcandillon merged commit 3f4f81d into Shopify:main Nov 10, 2023
9 checks passed
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.

Android: Add support for Fabric / new architecture iOS: Add support for Fabric / new architecture
4 participants