fix(tracing): don't create a duplicate navigation span for Expo Router's withAnchor POP_TO bookkeeping dispatch#6472
Conversation
…r's withAnchor POP_TO bookkeeping dispatch Expo Router issues a second POP_TO dispatch after a navigation with withAnchor: true, purely to stamp 'initial: false' onto the destination it just navigated to. With useDispatchedActionData enabled this dispatch started a second idle navigation span that stole in-flight child spans (HTTP, TTID/TTFD) from the real navigation and shipped as a spurious duplicate transaction. Two guards, both scoped to useDispatchedActionData: - At dispatch time, skip POP_TO actions whose target route is already focused somewhere on the active route chain. A genuine popTo targets a route behind the focused one and __unsafe_action__ fires before the state change is applied, so real navigations are unaffected. - At state-change time, discard (rather than abandon) a POP_TO span that landed on the same route key, unless a pending deep link claimed it. Fixes getsentry#6434 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
Thanks for taking this on. The state change discard (guard 2) keyed on One issue with guard 1 (the dispatch time Suggested fix: also require the const popToParams = (event?.data?.action?.payload as { params?: { initial?: boolean } } | undefined)?.params;
if (
navigationActionType === 'POP_TO' &&
targetRouteName &&
isRouteFocused(targetRouteName) &&
popToParams?.initial === false
) {
...
}I verified this keeps all current tests green (they already pass A test that fails on the current branch and passes with the change above: test('popTo an earlier same-named route in the stack still creates a span', async () => {
client.eventQueue = [];
mockNavigation.emitWithStateChange(
{ data: { action: { type: 'PUSH', payload: { name: 'Detail' } }, noop: false, stack: undefined } },
{ key: 'detail_2', name: 'Detail' },
);
await jest.runOnlyPendingTimersAsync();
client.eventQueue = [];
mockNavigation.emitWithStateChange(
{ data: { action: { type: 'POP_TO', payload: { name: 'Detail' } }, noop: false, stack: undefined } },
{ key: 'detail_1', name: 'Detail' },
);
await jest.runOnlyPendingTimersAsync();
await client.flush();
expect(client.eventQueue.filter(e => e.type === 'transaction' && e.transaction === 'Detail').length).toBe(1);
});(Add it inside the |
📢 Type of change
📜 Description
Expo Router issues a second
POP_TOdispatch after a navigation withwithAnchor: true, purely to stampinitial: falseonto the destination it just navigated to (expo-routergetNavigationAction:currentParams.initial = !withAnchor). WithuseDispatchedActionData: truethis bookkeeping dispatch started a second idle navigation span for a single user-facing navigation. The spurious span stole in-flight child spans (HTTP, TTID/TTFD) from the real navigation, lingered untilchildSpanTimeoutforce-closed it, and shipped as a confusing duplicate transaction with several children incancelledstatus.Simply adding
POP_TOto the filtered action list would break tracking of genuinepopTo/dismissTonavigations (on React Navigation 7,dismissTois aPOP_TO). Instead this adds two scoped guards, both active only withuseDispatchedActionData:POP_TOwhose target route is already focused somewhere on the active route chain (root → leaf, reusing the same traversal asgetPathFromState). A genuinepopTotargets a route behind the focused one, and__unsafe_action__fires before state changes are applied (documented in React Navigation's types), so its target is never focused at that point. This also protects the real navigation's in-flight span from being discarded by the "noop transaction" branch when the bookkeeping dispatch arrives before the deferred state-change microtask (reactNavigationIntegration: navigation transaction can be named/attributed for the previous route instead of the destination when using a route override provider (e.g.expoRouterIntegration) #6436).POP_TOspan that landed on the same route key (the dispatch-time filter can miss when the payload carries a parent navigator name that matches no focused route) is now discarded instead of being abandoned to run out its idle timeout — unless a pending deep link claimed it, preserving the deep-link-to-current-screen attribution added earlier.No behavior change when
useDispatchedActionDatais disabled (default): the action-type attribute is never set there, so both guards are inert.💡 Motivation and Context
Fixes #6434
💚 How did you test it?
Added 7 unit tests covering: a genuine
POP_TOto a different route still creates a named span; the bookkeepingPOP_TO(leaf and nested/parent-navigator payload names) creates no span; it does not remove an in-flight navigation span from the scope; a same-route-keyPOP_TOspan is discarded unless a deep link claimed it; andNAVIGATElanding on the same route keeps its current behavior. Full suite:yarn jest test/tracing/reactnavigation.test.ts— 121 passed, pluspendingDeepLink/expoRouterIntegration/idleNavigationSpan/ttidsuites green.tsc -p tsconfig.build.jsonandoxlint/oxfmtclean.📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps
If preferred, the same-route-key discard (guard 2) could later be generalized beyond
POP_TO, but that would change behavior for same-routeNAVIGATEparams updates, so it's deliberately left untouched here.