Use v8 instead of JSC + native SVG support #1196
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
brunobar79
added
the
needs-qa
brunobar79
changed the title
ibrahimtaveras00
approved these changes
Nov 13, 2019
rickycodes
pushed a commit
that referenced
this pull request
Jan 31, 2022
* use v8 instead of JSC * replace react-native-remote-svg for react-native-svg * fix svg size * update snapshots * clean up
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR was an attempt to fix the crash happening when going to settings, which took me to try
react-native-v8instead of JSC. Performance is way better, as seen here.After that, I realized the crash was still there and it wasn't JSC fault, so I kept digging until I found it was being caused by
react-native-remote-svgwhile rendering the webviews outside of the viewport (basically the tokens elements that were below the fold). Since the webview wasn't visible android was attempting to remove it from the view stack when other view was displayed on top (ex. going to settings), causing a memory leak that would lead to a crash, and it resulted to be our # 1 crash!That has been fixed by removing the dependency from
react-native-remote-svgand usereact-native-svginstead, which now supports remote svgs too and it renders them natively.I've wrapped into a component
RemoteImagethat we should use to load external images.TL;DR: The app is way faster and we fixed the main crash.