Bump version of react-native-svg dependency.

[aloukissas]

Looks like react-native-svg is broken for react-native 0.40.0 (see [1]).
This was fixed in 4.5.0. Bump up to that version to fix that issue. Also
need to bump the version of react-native to 0.40.0 (see [2]).

[1] software-mansion/react-native-svg#214
[2] https://github.com/react-native-community/react-native-svg

Thank you for contributing a pull request.

Please ensure that you have signed the CLA.

• I have signed the CLA

[marzolfb]

Please see my comments on #58. I've been hesitant to bump to react-native 0.40.0 because I haven't found stability in react-native-svg versions above 4.4.1. While I did try to go to react-native-svg version 4.6.1, I don't remember if I had similar issues with 4.5 like I did with 4.6.1.

So, a few things to address here...

• If you go to react-native-svg 4.5.0, is this no longer an issue if you try to view charts in the example app?

When switching to react-native-svg 4.6.1 and react-native 0.40.0, a number of charts replace the (blue colored) grid lines with bold black lines and the radar chart didn't render some of its (blue) lines at all

• There are actually quite a few file updates that should be part of the commit when you upgrade react-native from 0.38.0 to 0.40.0. This process is automated in the handy react-native cli tooling by doing this:

react-native upgrade

after upgrading your version in package.json

• We want to keep the react-native version in the example app consistent with the version in the main lib. So please update example/package.json as well

[aloukissas]

@marzolfb indeed the problem is gone with this. Your call; for now, I've resorted to using a local module /w the above change. Thought I'd share my findings with the community, since this took a bit to trace back.

[marzolfb]

Since the one issue is resolved, I don't mind going to react-native 0.40.0, if you can take care of the other two points for me (do react-native upgrade and update the version on the example app).

[aloukissas]

Sounds good. I'll update the patch.

[marzolfb]

Quick clarification...the react-native upgrade is targeted at the example app... so you'll want to cd example there before running that. Sorry that wasn't clear.

[aloukissas]

@marzolfb ack that.

[marzolfb]

Just that one Android-specific concern I have. Otherwise this looks good.

[marzolfb]

This automated change has caused the example app to break in the past. Did you get a chance to run through the example app successfully in an Android simulator after you made these changes? If its broken, we probably will need to manually add back in the new RNSvgPackage() to make it work

[aloukissas]

I've only been playing with iOS. I'll have to setup an Android environment and test it.

[marzolfb]

I tried out your changes on both iOS and Android and got errors on both

On iOS:

No component found for view with name "RNSVGGroup"

On Android:

No ViewManager defined for class RNSVGGroup

Doing:

react-native link react-native-svg

in the example dir fixes this problem and modifies the following 4 files:

git status

...
modified:   android/app/build.gradle
modified:   android/app/src/main/java/com/example/MainApplication.java
modified:   android/settings.gradle
modified:   ios/example.xcodeproj/project.pbxproj

The change that react-native link ... makes to example/MainApplication.java is the same one I mention above in "we probably will need to manually add back in the new RNSvgPackage() to make it work". So, no need to do the manual edit - we just need to do react-native link react-native-svg instead.

That should be all thats left to do to button this up.

[aloukissas]

Got it - apologies, I'm just now getting my feet wet with the framework. What would be the ideal case for CI-run tests on each PR, so such errors are caught by automation?

[marzolfb]

Well, on the library source itself we have some rudimentary jest snapshot tests that are set up that can be run with npm test (see src/__tests__, src/__mocks__, ./circle.yml, ./package.json) but I think what you are asking above is automated tests on the example app. Unfortunately, we don't have anything setup right now for the example but would love to add something. Ironically, the src tests are really just duplicates of portions of the example app. Jest snapshot tests couple probably work specifically for the example app. There is a ton of room for improvement in both the src tests and getting something started on the example app. I would welcome any contributions that you or others are willing to contribute in a separate PR(s).

[marzolfb]

@aloukissas - Any update on this? I'd rather us get this rn/rn-svg update pushed through as soon as we can and tackle the tests for the example app in a separate PR.

[marzolfb]

Closing this now that I merged #78 which is similar but uses a newer react-native version (0.41.2)

[aloukissas]

@marzolfb apologies, have been afk for the past week or so. Thanks for following up.