Skip to content

Conversation

@chiragsp55
Copy link
Contributor

Resolves https://chartiq.kanbanize.com/ctrl_board/15/cards/53000/details/

RCA: App was crashing when users navigated away from the chart. The chart view got destroyed but async callbacks kept running with invalid references.

Solution: Added safety checks so callbacks gracefully handle destroyed views instead of crashing. Applied the fix to both iOS (where the issue was happening) and Android (as prevention).

Testing: Added a Test Rig screen with a shouldStartInTestRig flag in root navigator (example/src/navigation/root.navigator.tsx). When set to true(defaults to false), app opens directly to Test Rig instead of main chart, making it easy to test navigation scenarios and validate the crash fix. You will see back button only if you are navigating from test-rig screen

import { StyleSheet, View, ViewStyle } from 'react-native';
import { ChartIQ, ChartIQLanguages, ChartIQView } from 'react-native-chartiq';
import { SafeAreaView } from 'react-native-safe-area-context';
import { useNavigation, useRoute } from '@react-navigation/native';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This linter warning are starting to pile up, should we knock out these simple ones that suggest import order?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


const RootNavigator: React.FC = () => {
// Flag to start app in Test Rig screen
const shouldStartInTestRig = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go ahead and set this back to false, just in case we forget.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ops! Thanks for catching. I've mentioned it's false by default in PR description but missed to stage.

Copy link
Contributor

@jdrichards50 jdrichards50 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, testrig also very helpful.

@chiragsp55 chiragsp55 merged commit 2432242 into main Aug 22, 2025
2 checks passed
@chiragsp55 chiragsp55 deleted the users/chirag.vora/53000-rn-crash branch August 22, 2025 03:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants