Skip to content

Commit dca181b

Browse files
authored
fix(core): Fix stale route when reactNavigationIntegration uses a route override provider (#6458)
* fix(core): Defer route override read until after state-change chain React Navigation fires `emit('state')` synchronously before invoking the `onStateChange` prop. Integrations like Expo Router refresh their route cache (which the override provider reads) inside that prop, so reading the override synchronously in the `state` listener returns the previous route for the current transition. Defer the read with a microtask so the override read happens after the synchronous state-change chain completes. Fixes #6436 * docs(changelog): Point #6436 entry at the PR
1 parent 95a05de commit dca181b

3 files changed

Lines changed: 69 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
- Fix `TypeError` when `showFeedbackForm`/`showFeedbackButton`/`showScreenshotButton` is called before `FeedbackFormProvider` mounts ([#6435](https://github.com/getsentry/sentry-react-native/pull/6435))
2020
- Fix orphaned TTID/TTFD spans in the trace view ([#6437](https://github.com/getsentry/sentry-react-native/pull/6437))
2121
- 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))
22+
- 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))
2223

2324
### Internal
2425

packages/core/src/js/tracing/reactnavigation.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -411,7 +411,14 @@ export const reactNavigationIntegration = ({
411411

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

417424
if (initialStateHandled) {
@@ -597,6 +604,27 @@ export const reactNavigationIntegration = ({
597604
stateChangeTimeout = setTimeout(_discardLatestTransaction, routeChangeTimeoutMs);
598605
};
599606

607+
/**
608+
* Defer {@link updateLatestNavigationSpanWithCurrentRoute} until the current
609+
* synchronous state-change chain unwinds so route override providers backed
610+
* by a downstream cache (e.g. Expo Router's router-store, which is refreshed
611+
* via `NavigationContainer.onStateChange`) have picked up the new route. See
612+
* #6436.
613+
*/
614+
const scheduleUpdateLatestNavigationSpanWithCurrentRoute = (): void => {
615+
const g = globalThis as unknown as { queueMicrotask?: (cb: () => void) => void };
616+
if (typeof g.queueMicrotask === 'function') {
617+
g.queueMicrotask(updateLatestNavigationSpanWithCurrentRoute);
618+
return;
619+
}
620+
// Fallback for runtimes without `queueMicrotask`. `.catch()` handler is
621+
// there only to satisfy the `no-floating-promises` lint — the update
622+
// function does not throw.
623+
Promise.resolve()
624+
.then(updateLatestNavigationSpanWithCurrentRoute)
625+
.catch(() => {});
626+
};
627+
600628
/**
601629
* To be called AFTER the state has been changed to populate the transaction with the current route.
602630
*/

packages/core/test/tracing/reactnavigation.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2007,6 +2007,45 @@ describe('ReactNavigationInstrumentation', () => {
20072007
expect(traceData[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('/posts/[...slug]');
20082008
expect(traceData[SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_NAME]).toBe('/profile/[id]');
20092009
});
2010+
2011+
// Regression test for #6436. React Navigation's BaseNavigationContainer
2012+
// fires `emit('state')` synchronously BEFORE the `onStateChange` prop
2013+
// callback, and Expo Router refreshes its router-store cache from that
2014+
// prop. Reading the override synchronously in the `state` listener would
2015+
// return the previous route for the current transition. The integration
2016+
// must defer the read until after the synchronous state-change chain has
2017+
// unwound.
2018+
it('reads the route override after the synchronous state-change chain (Expo Router ordering)', async () => {
2019+
const rNavigation = setupWithOverride();
2020+
jest.runOnlyPendingTimers();
2021+
2022+
// First navigation: settle on '/A' so the provider has a "previous" value.
2023+
let latestOverride: { templatedPath: string; params?: Record<string, unknown> } = {
2024+
templatedPath: '/A',
2025+
};
2026+
rNavigation._setRouteOverrideProvider(() => latestOverride);
2027+
2028+
mockNavigation.navigateToDynamicRoute();
2029+
jest.runOnlyPendingTimers();
2030+
await client.flush();
2031+
2032+
// Second navigation: simulate the Expo Router ordering. When the `state`
2033+
// event fires the override provider still returns '/A' (Expo Router has
2034+
// not yet run its own `onStateChange` prop callback). Flip the provider
2035+
// to '/B' AFTER `navigateToCatchAllRoute()` returns but BEFORE the
2036+
// pending microtask flushes — this mimics `onStateChange` refreshing
2037+
// the store cache right after `emit('state')` returns.
2038+
mockNavigation.navigateToCatchAllRoute();
2039+
latestOverride = { templatedPath: '/B' };
2040+
jest.runOnlyPendingTimers();
2041+
await client.flush();
2042+
2043+
const traceData = client.event?.contexts?.trace?.data as Record<string, unknown>;
2044+
expect(client.event?.transaction).toBe('/B');
2045+
expect(traceData[SEMANTIC_ATTRIBUTE_ROUTE_NAME]).toBe('/B');
2046+
expect(traceData['route.path']).toBe('/B');
2047+
expect(traceData[SEMANTIC_ATTRIBUTE_PREVIOUS_ROUTE_NAME]).toBe('/A');
2048+
});
20102049
});
20112050

20122051
describe('dispatch breadcrumbs', () => {

0 commit comments

Comments
 (0)