Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
- Fix `TypeError` when `showFeedbackForm`/`showFeedbackButton`/`showScreenshotButton` is called before `FeedbackFormProvider` mounts ([#6435](https://github.com/getsentry/sentry-react-native/pull/6435))
- Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
- Fix iOS retain cycle in `RNSentryOnDrawReporterView` leaking TTID/TTFD reporter views and their frame-tracker listeners ([#6449](https://github.com/getsentry/sentry-react-native/pull/6449))
- Fix `reactNavigationIntegration` reading a stale route from an override provider (e.g. `expoRouterIntegration`), causing navigation transactions to be named/attributed for the previous route ([#6458](https://github.com/getsentry/sentry-react-native/pull/6458))

### Internal

Expand Down
30 changes: 29 additions & 1 deletion packages/core/src/js/tracing/reactnavigation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,14 @@ export const reactNavigationIntegration = ({

// This action is emitted on every dispatch
navigationContainer.addListener('__unsafe_action__', startIdleNavigationSpan);
navigationContainer.addListener('state', updateLatestNavigationSpanWithCurrentRoute);
// React Navigation fires `emit('state')` synchronously BEFORE the
// `onStateChange` prop callback. Integrations like Expo Router refresh
// their route cache (which our route override provider reads) inside that
// `onStateChange`, so reading the override synchronously here would return
// the previous route for the current transition. Defer with a microtask
// so the read happens after the synchronous state-change chain completes
// and downstream caches have caught up. See #6436.
navigationContainer.addListener('state', scheduleUpdateLatestNavigationSpanWithCurrentRoute);
RN_GLOBAL_OBJ.__sentry_rn_v5_registered = true;

if (initialStateHandled) {
Expand Down Expand Up @@ -597,6 +604,27 @@ export const reactNavigationIntegration = ({
stateChangeTimeout = setTimeout(_discardLatestTransaction, routeChangeTimeoutMs);
};

/**
* Defer {@link updateLatestNavigationSpanWithCurrentRoute} until the current
* synchronous state-change chain unwinds so route override providers backed
* by a downstream cache (e.g. Expo Router's router-store, which is refreshed
* via `NavigationContainer.onStateChange`) have picked up the new route. See
* #6436.
*/
const scheduleUpdateLatestNavigationSpanWithCurrentRoute = (): void => {
const g = globalThis as unknown as { queueMicrotask?: (cb: () => void) => void };
if (typeof g.queueMicrotask === 'function') {
g.queueMicrotask(updateLatestNavigationSpanWithCurrentRoute);
return;
}
// Fallback for runtimes without `queueMicrotask`. `.catch()` handler is
// there only to satisfy the `no-floating-promises` lint โ€” the update
// function does not throw.
Promise.resolve()
.then(updateLatestNavigationSpanWithCurrentRoute)
.catch(() => {});
};

/**
* To be called AFTER the state has been changed to populate the transaction with the current route.
*/
Expand Down
39 changes: 39 additions & 0 deletions packages/core/test/tracing/reactnavigation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2007,6 +2007,45 @@ describe('ReactNavigationInstrumentation', () => {
expect(traceData[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('/posts/[...slug]');
expect(traceData[SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_NAME]).toBe('/profile/[id]');
});

// Regression test for #6436. React Navigation's BaseNavigationContainer
// fires `emit('state')` synchronously BEFORE the `onStateChange` prop
// callback, and Expo Router refreshes its router-store cache from that
// prop. Reading the override synchronously in the `state` listener would
// return the previous route for the current transition. The integration
// must defer the read until after the synchronous state-change chain has
// unwound.
it('reads the route override after the synchronous state-change chain (Expo Router ordering)', async () => {
const rNavigation = setupWithOverride();
jest.runOnlyPendingTimers();

// First navigation: settle on '/A' so the provider has a "previous" value.
let latestOverride: { templatedPath: string; params?: Record<string, unknown> } = {
templatedPath: '/A',
};
rNavigation._setRouteOverrideProvider(() => latestOverride);

mockNavigation.navigateToDynamicRoute();
jest.runOnlyPendingTimers();
await client.flush();

// Second navigation: simulate the Expo Router ordering. When the `state`
// event fires the override provider still returns '/A' (Expo Router has
// not yet run its own `onStateChange` prop callback). Flip the provider
// to '/B' AFTER `navigateToCatchAllRoute()` returns but BEFORE the
// pending microtask flushes โ€” this mimics `onStateChange` refreshing
// the store cache right after `emit('state')` returns.
mockNavigation.navigateToCatchAllRoute();
latestOverride = { templatedPath: '/B' };
jest.runOnlyPendingTimers();
await client.flush();

const traceData = client.event?.contexts?.trace?.data as Record<string, unknown>;
expect(client.event?.transaction).toBe('/B');
expect(traceData[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('/B');
expect(traceData['route.path']).toBe('/B');
expect(traceData[SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_NAME]).toBe('/A');
});
});

describe('dispatch breadcrumbs', () => {
Expand Down
Loading