feat: remove extractColor in favor of RN impl

[WoLewicki]

Summary

PR removing extractColor.ts in favor of using the processColor implementation straight from react-native. It should handle all the cases from the previous implementation and the cases with PlatformColor and DynamicColorIOS. Normally we would just send the returned value to the native side, but in react-native-svg, e.g. fill prop can have more values than just color, e.g. currentColor. Because of it, we cannot use UIColor on iOS, NSColor on macOS and customType="Color" on Android as a prop type there and therefore we need to prepare the custom values on the JS side. It is done by passing the prop as an array with specific first element. In case of colors, it is 0. (hopefully I understood the code right).

Test Plan

ColorTest.tsx in TestsExample.

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS	Implemented
iOS	✅❌
Android	✅❌

Checklist

• I have tested this on a device and a simulator
• I updated the typed files (typescript)

[WoLewicki]

cc @Saadnajmi I think it should work OK on macOS since you add your own implementation of processColor there? Am I right? Can I also test it there somehow?

[Saadnajmi]

@WoLewicki

cc @Saadnajmi I think it should work OK on macOS since you add your own implementation of processColor there? Am I right? Can I also test it there somehow?

Thanks for the feature! I believe this should work, and I'll do my best to test this =).

I'll explain the platform differences here since it's not very well documented:

The main difference color-wise between React Native & React Native macOS is we have two different PlatformColors:

• DynamicColorMacOS (which is the same as DynamicColorIOS except by name)
• ColorWithSystemEffectMacOS (Introduced in Add ColorWithSystemEffectMacOS API to react-native-macos microsoft/react-native-macos#751 , let's you apply a system effect like "pressed" on top of a base color).

To support these types, I actually didn't have to change processColor, I had to define the new flow types (you can see that in my above PR in Libraries/StyleSheet/PlatformColorValueTypes*), and update RCTConvert.m (the native code that converts the color once it's passed across the bridge).

So, processColor on macOS is mostly the same, but RCTConvert has most of the changes. As long as the correct json blob is passed across the bridge, React Native macOS (and React Native Core / any other platform) should do the right thing =)

[Saadnajmi]

How did you find that passing 0 means RCTConvert handles the color?

[WoLewicki]

I went through the native code and here is the code:

• on iOS we go here https://github.com/react-native-svg/react-native-svg/blob/82f0f3a9858de718f831fa41ca670f1e8e2777e7/apple/Utils/RCTConvert%2BRNSVG.m#L64 which invokes this: https://github.com/react-native-svg/react-native-svg/blob/82f0f3a9858de718f831fa41ca670f1e8e2777e7/apple/Brushes/RNSVGSolidColorBrush.m#L23, which goes to this: https://github.com/react-native-svg/react-native-svg/blob/82f0f3a9858de718f831fa41ca670f1e8e2777e7/apple/Utils/RCTConvert%2BRNSVG.m#L137, which is the react-native's RCTConvert for color, so should correctly resolve on macOS too if the implementation for NSColor is there.
• on Android it lands here and is also resolved by react-native's ColorPropConverter

[WoLewicki]

So, processColor on macOS is mostly the same, but RCTConvert has most of the changes. As long as the correct json blob is passed across the bridge, React Native macOS (and React Native Core / any other platform) should do the right thing =)

As seen in the flow of passing the color, it is not passed directly as color through the bridge, but platform-specific RCTConvert should be later invoked, so hopefully it works just right. It needs testing on macOS though before being merged 😅

[Saadnajmi]

So, processColor on macOS is mostly the same, but RCTConvert has most of the changes. As long as the correct json blob is passed across the bridge, React Native macOS (and React Native Core / any other platform) should do the right thing =)

As seen in the flow of passing the color, it is not passed directly as color through the bridge, but platform-specific RCTConvert should be later invoked, so hopefully it works just right. It needs testing on macOS though before being merged 😅

Yep, working on that. I couldn't get the test app working yesterday, so I'll give it another shot today!

[Saadnajmi]

An update, I've still been having issues running the macOS test app locally. I scheduled dedicated time this week for this PR specifically, but I may have to resort to making my own test app locally and running this PR's version of react-native-svg on it. Sorry for the delay!

[Saadnajmi]

OK, I got the existing test app working on macOS with a caveat: The existing test app makes heavy use of Modal which is supported in RN Core but not RN macOS =/, so I just got a bunch of red boxes.

Luckily, we do have a bunch of SVG tests in the other repo I help maintain: FluentUI React Native. So instead, I ran npm pack inside this repo with the PR checked out to generate a zip file of the package, and manually copied that into node_modules/ of FluentUI React Native. Then I could run FluentTester (FluentUI React Native's test app with SVG tests) on macOS with your changes!

I replaced one color usage with ColorWithSystemEffectMacOS (one of our custom PlatformColors) and indeed, RNSVG is calling into RCTConvert and doing what I would expect! Thank you so much for this work!