Skip to content

Commit

Permalink
Fix: Add check version to new arch on TimetoTisplay (#4160)
Browse files Browse the repository at this point in the history
* add check version to new arch

* screen rename, nits on if condition

* lint?

* lint

* lint

* make more data from sample, requested changes

* remove before effect
  • Loading branch information
lucas-zimerman authored Oct 14, 2024
1 parent 4412b4c commit 52c0562
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Fixes

- TimetoTisplay correctly warns about not supporting the new React Native architecture ([#4160](https://github.com/getsentry/sentry-react-native/pull/4160))
- Handles error with string cause ([#4163](https://github.com/getsentry/sentry-react-native/pull/4163))
- Use `appLaunchedInForeground` to determine invalid app start data on Android ([#4146](https://github.com/getsentry/sentry-react-native/pull/4146))
- Upload source maps for all release variants on Android (not only the last found) ([#4125](https://github.com/getsentry/sentry-react-native/pull/4125))
Expand Down
5 changes: 5 additions & 0 deletions samples/react-native/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import { HttpClient } from '@sentry/integrations';
import Ionicons from 'react-native-vector-icons/Ionicons';
import PlaygroundScreen from './Screens/PlaygroundScreen';
import { logWithoutTracing } from './utils';
import HeavyNavigationScreen from './Screens/HeavyNavigationScreen';

LogBox.ignoreAllLogs();

Expand Down Expand Up @@ -168,6 +169,10 @@ const TabTwoStack = Sentry.withProfiler(
name="ManualTracker"
component={ManualTrackerScreen}
/>
<Stack.Screen
name="HeavyNavigation"
component={HeavyNavigationScreen}
/>
<Stack.Screen
name="PerformanceTiming"
component={PerformanceTimingScreen}
Expand Down
71 changes: 71 additions & 0 deletions samples/react-native/src/Screens/HeavyNavigationScreen.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import * as React from 'react';
import { Button, View, StyleSheet, Text, ScrollView } from 'react-native';

import * as Sentry from '@sentry/react-native';
import { StackNavigationProp } from '@react-navigation/stack';

interface Props {
navigation: StackNavigationProp<any, 'HeavyNatigavionScreen'>;
route?: {
params?: {
manualTrack: boolean;
};
};
}
const buttonTitles = Array.from(
{ length: 500 },
(_, index) => `Sample button ${index + 1}`,
);

/**
* this page takes around 300ms to initially display, we navigate to another page in 100ms.
* The time to initial display will never be finished on this page.
*/
const HeavyNavigationScreen = (props: Props) => {
const content = (
<ScrollView style={styles.screen}>
<View style={styles.titleContainer}>
<Text style={styles.title}>
Heavy page only intended for navigating to another page while the page
is loading.
</Text>
</View>
{buttonTitles.map((title, index) => (
<Button key={index} title={title} />
))}
</ScrollView>
);

React.useEffect(() => {
setTimeout(() => {
props.navigation.goBack();
}, 1);
});
return (
<>
{props.route?.params?.manualTrack ? (
<Sentry.TimeToInitialDisplay record={true}>
{content}
</Sentry.TimeToInitialDisplay>
) : (
content
)}
</>
);
};

export default Sentry.withProfiler(HeavyNavigationScreen);

const styles = StyleSheet.create({
screen: {
flex: 1,
padding: 16,
},
titleContainer: {
paddingBottom: 12,
},
title: {
fontSize: 24,
fontWeight: '700',
},
});
21 changes: 21 additions & 0 deletions samples/react-native/src/Screens/PerformanceScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,14 @@ import {
import { StackNavigationProp } from '@react-navigation/stack';
import { CommonActions } from '@react-navigation/native';
import * as Sentry from '@sentry/react-native';
import { Text } from 'react-native';

interface Props {
navigation: StackNavigationProp<any, 'PerformanceScreen'>;
}

const Spacer = () => <View style={styles.spacer} />;

const PerformanceScreen = (props: Props) => {
const onPressPerformanceTiming = () => {
// Navigate with a reset action just to test
Expand Down Expand Up @@ -62,6 +65,24 @@ const PerformanceScreen = (props: Props) => {
props.navigation.navigate('Redux');
}}
/>
<Spacer />
<Text>
Heavy Page Navigation that navigates back automatically, before
finishing the time to display.
</Text>
<Button
title="With Manual TimeToDisplay."
onPress={() => {
props.navigation.navigate('HeavyNavigation', { manualTrack: true });
}}
/>
<Button
title="With Automatic TimeToDisplay."
onPress={() => {
props.navigation.navigate('HeavyNavigation');
}}
/>
<Spacer />
<View style={styles.mainViewBottomWhiteSpace} />
</ScrollView>
</>
Expand Down
7 changes: 5 additions & 2 deletions src/js/tracing/timetodisplay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { Span,StartSpanOptions } from '@sentry/types';
import { fill, logger } from '@sentry/utils';
import * as React from 'react';

import { isTurboModuleEnabled } from '../utils/environment';
import { getRNSentryOnDrawReporter, nativeComponentExists } from './timetodisplaynative';
import type {RNSentryOnDrawNextFrameEvent } from './timetodisplaynative.types';
import { setSpanDurationAsMeasurement } from './utils';
Expand Down Expand Up @@ -59,12 +60,14 @@ function TimeToDisplay(props: {
fullDisplay?: boolean;
}): React.ReactElement {
const RNSentryOnDrawReporter = getRNSentryOnDrawReporter();
const isNewArchitecture = isTurboModuleEnabled();

if (__DEV__ && !nativeComponentMissingLogged && !nativeComponentExists) {
if (__DEV__ && (isNewArchitecture || (!nativeComponentExists && !nativeComponentMissingLogged))){
nativeComponentMissingLogged = true;
// Using setTimeout with a delay of 0 milliseconds to defer execution and avoid printing the React stack trace.
setTimeout(() => {
logger.warn('TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
logger.warn(
'TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
}, 0);
}

Expand Down
26 changes: 26 additions & 0 deletions test/tracing/timetodisplay.test.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import * as mockedtimetodisplaynative from './mockedtimetodisplaynative';
jest.mock('../../src/js/tracing/timetodisplaynative', () => mockedtimetodisplaynative);
import { isTurboModuleEnabled } from '../../src/js/utils/environment';
jest.mock('../../src/js/utils/environment', () => ({
isTurboModuleEnabled: jest.fn().mockReturnValue(false),
}));
jest.spyOn(logger, 'warn');

import type { Span as SpanClass} from '@sentry/core';
import { getActiveSpan, getCurrentScope, getGlobalScope, getIsolationScope, setCurrentClient, spanToJSON, startSpanManual} from '@sentry/core';
import type { Event, Measurements, Span, SpanJSON} from '@sentry/types';
import { logger } from '@sentry/utils';
import React from "react";
import TestRenderer from 'react-test-renderer';

Expand Down Expand Up @@ -373,6 +379,26 @@ describe('TimeToDisplay', () => {
expect(spanToJSON(initialDisplaySpan!).timestamp).toEqual(initialDisplayEndTimestampMs / 1_000);
expect(spanToJSON(fullDisplaySpan!).timestamp).toEqual(fullDisplayEndTimestampMs / 1_000);
});

test('should not log a warning if native component exists and not in new architecture', async () => {

(isTurboModuleEnabled as jest.Mock).mockReturnValue(false);

TestRenderer.create(<TimeToInitialDisplay record={true} />);
await jest.runOnlyPendingTimersAsync(); // Flush setTimeout.

expect(logger.warn).not.toHaveBeenCalled();
});

test('should log a warning if in new architecture', async () => {

(isTurboModuleEnabled as jest.Mock).mockReturnValue(true);
TestRenderer.create(<TimeToInitialDisplay record={true} />);
await jest.runOnlyPendingTimersAsync(); // Flush setTimeout.

expect(logger.warn).toHaveBeenCalledWith(
'TimeToInitialDisplay and TimeToFullDisplay are not supported on the web, Expo Go and New Architecture. Run native build or report an issue at https://github.com/getsentry/sentry-react-native');
});
});

function getInitialDisplaySpan(span?: Span) {
Expand Down

0 comments on commit 52c0562

Please sign in to comment.