mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Phillip Pan
7c68cf43c4
Summary: Changelog: [Internal] when we try to write a `SharedColor` type prop in the renderer, the template function we match to is the following: https://www.internalfb.com/intern/diffusion/FBS/browsefile/master/xplat/js/react-native-github/ReactCommon/react/renderer/core/propsConversions.h?commit=28dacb972cda702d37dedd4612bc67c212d4d9eb&lines=108-132 this is because `SharedColor` is an alias of `better::optional<Color>`. ultimately, this causes a crash in L130 - the template deduction in L130 interprets the T as an `int`, rather than `SharedColor`, but our `rawValue` is pointing to `SharedColor`. there was a number of options i considered, but didn't feel the most correct: - wrapping `SharedColor` in another `better::optional` - this felt like a hack and didn't really provide any real benefits of the `optional` wrapper. - writing a template specialization on SharedColor - this didn't seem future proof because we could introduce another type that could potentially wrap an integer, which doesn't seem impossible in the future - then we would get some conflict with our `SharedColor` conversion, which is custom to the behavior of colors. - coercing the template during the function call - from an engineering perspective, this didn't seem like a great idea since it isn't clear to the engineer that this would be necessary and they would most likely only find out to do this after seeing a crash on their builds. i ultimately decided to convert `SharedColor` to a simple class wrapper, similar to the implementation already used in Android. this alleviates all of the concerns with the other options above. Reviewed By: sammy-SC Differential Revision: D30149880 fbshipit-source-id: 7e8abafcd9fd7465b13ef227d140e859f8df6ecd