Use the React default shadowView to allow native animations

[jamesreggio]

Returning a nil shadowView prevents natively-driven animations from working with SVG nodes, despite the fact that most SVG nodes (and node properties) support native animations.

I'm not entirely sure why shadowNode is set to nil, but removing the nil has been working for us in our app for 6+ months. (Perhaps it's set to nil to save on some memory/Yoga-layout overhead?)

I'm fairly confident that this change won't lead to regressions, but I'd be happy to hear from anybody with knowledge of the original code.

[msand]

I'm not exactly sure what the tradeoffs here are. This code has been there since v1.1.2 d700ca5
Have you noticed any performance impact? Perhaps this should be given a boolean prop, set to false/off by default, to give control to the developer. In case it would slow down rendering where shadows aren't needed. What do you think?

[jamesreggio]

@msand, that's not a bad idea (to put it behind a boolean prop). I'll look into it.

[jamesreggio]

@msand, I investigated and it doesn't look like it's possible to affect this with a prop because the shadow view is instantiated before any specific props are available for the given instance. It's also not clearly feasible to control this at the root <Svg> node because the necessary view hierarchy context is not available at the time the shadow node is instantiated.

I spent some time profiling our app and can't say that Yoga layout ever really registers as a performance cost relative to all of the other native concerns. Yoga is only ever going to layout the SVG shadow nodes once, anyhow, since their layout will never be invalidated after initial mount. We have an SVG illustration with several thousand nodes and haven't had a problem, even on iPhone 6.

I'm sympathetic to the fear of introducing a regression, but I also think it would be unhealthy to retain code that nobody understands in perpetuity based upon that fear. (It's a shame that there's not more robust testing norms in the RN space, but that's just how things are.) #684 also appears to be asking for this functionality.

If you merge this, I'd be happy to watch inbound issues for potential regressions. And if there's anything else that would make you more comfortable accepting the patch, please let me know.

[msand]

I'm thinking I would merge this, and then perhaps #757 as well. Would you be willing to help test/review that one as well? It extends the share of animatable properties quite a bit, as it supports string interpolation but requires the pr to react-native, or building it yourself if you're on android, testing the fork in iOS should work with relative ease.

[msand]

I've added support for animation of transforms using the same syntax as react-native views now: #803 (comment)
And also for any number accepting properties. Any help in testing the latest commit in the master branch would be much appreciated!