Skip to content

Commit 2fbc103

Browse files
alwxalwx
committed
fix(tracing): Keep expoRouterIntegration working on Expo Router 55 to 57
Review of #6923 found two regressions for Expo Router versions before 58. Expo Router 55 and older do not ship getRouteInfoFromState, so the require threw and the integration stopped as if expo-router were not installed. The route info module is now optional. Without it, the integration still registers the navigation container and skips the route override, so spans use the React Navigation route names. In Expo Router 56 and 57, useNavigationContainerRef reads a store that stays empty until the Root Layout renders. The ref was read once at Sentry.init, so polling timed out and brought back #6431. The store now reads the ref on every access, and a throwing call returns undefined so it cannot escape the poll timer. The #6431 regression test is restored with the new mocks, and new tests cover a missing getRouteInfoFromState and a throwing useNavigationContainerRef. The CHANGELOG entry moves back to the Unreleased section after the 8.32.0 release.
1 parent f1e1dcf commit 2fbc103

3 files changed

Lines changed: 140 additions & 20 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
### Fixes
1212

1313
- Apply `screenshot` masking options to Android error and feedback screenshots ([#6930](https://github.com/getsentry/sentry-react-native/pull/6930))
14+
- Support Expo Router 58 in `expoRouterIntegration`. Metro no longer warns about the removed `expo-router/build/global-state/router-store` module, and the integration creates navigation spans again ([#6923](https://github.com/getsentry/sentry-react-native/pull/6923))
1415

1516
### Internal
1617

@@ -44,7 +45,6 @@
4445

4546
- Find `ios/.xcode.env` in the Expo plugin "Upload Debug Symbols to Sentry" phase when the iOS project uses Swift Package Manager instead of CocoaPods ([#6911](https://github.com/getsentry/sentry-react-native/pull/6911))
4647
- Fix HTTP timing listeners leaking on React Native below 0.83 and when `expo/fetch` is detected, and respect the `enableHTTPTimings` option ([#6912](https://github.com/getsentry/sentry-react-native/pull/6912))
47-
- Support Expo Router 58 in `expoRouterIntegration`. Metro no longer warns about the removed `expo-router/build/global-state/router-store` module, and the integration creates navigation spans again ([#6923](https://github.com/getsentry/sentry-react-native/pull/6923))
4848
- Remove the `@sentry/browser` feedback modules and the server-only `@sentry/core` modules from native bundles built on Windows. Before this fix, `includeWebFeedback: false` made these bundles fail at launch ([#6915](https://github.com/getsentry/sentry-react-native/pull/6915))
4949
- Stop wrapping the SDK's own `RNSentry` module in `turboModuleContextIntegration` by default. Each wrapped call wrote to the native scope several times, which added CPU time to every SDK call on iOS ([#6921](https://github.com/getsentry/sentry-react-native/pull/6921))
5050
- To wrap `RNSentry` again, add `{ name: 'RNSentry' }` to the `modules` option.

‎packages/core/src/js/tracing/expoRouterStore.ts‎

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -59,36 +59,60 @@ export interface ExpoRouterInternalNavigationEvents {
5959
* Returns the navigation container ref and route info of Expo Router, or `null`
6060
* if `expo-router` is not installed or does not expose the expected API.
6161
*
62-
* Only module paths that exist in every supported Expo Router version are
63-
* required here. Expo Router 58 declares `exports`, so Metro warns about any
64-
* deep path that is missing from the installed version.
62+
* Expo Router 58 declares `exports`, so Metro warns about any deep path that is
63+
* missing from it. Older versions have no `exports`, so a missing deep path
64+
* there is a silent optional dependency.
6565
*/
6666
export function tryGetExpoRouterStore(): ExpoRouterStore | null {
67+
let getNavigationContainerRef: (() => ExpoRouterNavigationRef | undefined) | undefined;
6768
try {
6869
// eslint-disable-next-line @typescript-eslint/no-var-requires
6970
const expoRouter = require('expo-router') as {
7071
useNavigationContainerRef?: () => ExpoRouterNavigationRef | undefined;
7172
};
73+
// Not a React hook: it reads a module-level store, so it is safe to call outside render.
74+
getNavigationContainerRef = expoRouter?.useNavigationContainerRef;
75+
} catch {
76+
return null;
77+
}
78+
if (!getNavigationContainerRef) {
79+
return null;
80+
}
81+
const getRef = getNavigationContainerRef;
82+
const readNavigationContainerRef = (): ExpoRouterNavigationRef | undefined => {
83+
try {
84+
return getRef();
85+
} catch {
86+
return undefined;
87+
}
88+
};
89+
const getRouteInfoFromState = tryGetRouteInfoFromState();
90+
91+
return {
92+
// Before Expo Router 58 the ref is undefined until the Root Layout renders, so read it on every access.
93+
get navigationRef() {
94+
return readNavigationContainerRef();
95+
},
96+
getRouteInfo: getRouteInfoFromState
97+
? () => {
98+
const navigationRef = readNavigationContainerRef();
99+
const state = navigationRef?.current ? navigationRef.getRootState?.() : undefined;
100+
return state ? getRouteInfoFromState(state) : undefined;
101+
}
102+
: undefined,
103+
};
104+
}
105+
106+
/** `getRouteInfoFromState` exists only in Expo Router 56 and later. */
107+
function tryGetRouteInfoFromState(): ((state: unknown) => ExpoRouterUrlObject) | undefined {
108+
try {
72109
// eslint-disable-next-line @typescript-eslint/no-var-requires
73-
const routeInfoModule = require('expo-router/build/global-state/getRouteInfoFromState') as {
110+
const mod = require('expo-router/build/global-state/getRouteInfoFromState') as {
74111
getRouteInfoFromState?: (state: unknown) => ExpoRouterUrlObject;
75112
};
76-
// Not a React hook: it returns a module-level ref in Expo Router 57 and 58.
77-
const getNavigationContainerRef = expoRouter?.useNavigationContainerRef;
78-
const getRouteInfoFromState = routeInfoModule?.getRouteInfoFromState;
79-
if (!getNavigationContainerRef || !getRouteInfoFromState) {
80-
return null;
81-
}
82-
const navigationRef = getNavigationContainerRef();
83-
return {
84-
navigationRef,
85-
getRouteInfo: () => {
86-
const state = navigationRef?.current ? navigationRef.getRootState?.() : undefined;
87-
return state ? getRouteInfoFromState(state) : undefined;
88-
},
89-
};
113+
return mod?.getRouteInfoFromState;
90114
} catch {
91-
return null;
115+
return undefined;
92116
}
93117
}
94118

‎packages/core/test/tracing/expoRouterIntegration.test.ts‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,102 @@ describe('expoRouterIntegration', () => {
9898
});
9999
});
100100

101+
describe('expo-router 56 and 57, navigationRef set when the Root Layout renders', () => {
102+
it('picks up navigationRef when it appears mid-poll (issue #6431)', () => {
103+
const container = createMockNavigationContainer();
104+
const store: { navigationRef?: { current: MockNavigationContainer | null } } = {};
105+
jest.doMock('expo-router', () => ({ useNavigationContainerRef: () => store.navigationRef }), {
106+
virtual: true,
107+
});
108+
109+
const { expoRouterIntegration: integration } = require('../../src/js/tracing/expoRouterIntegration');
110+
const { client, addIntegration } = createMockClient();
111+
112+
const integ = integration();
113+
integ.afterAllSetup?.(client);
114+
115+
expect(addIntegration).not.toHaveBeenCalled();
116+
117+
jest.advanceTimersByTime(200);
118+
store.navigationRef = { current: null };
119+
jest.advanceTimersByTime(50);
120+
121+
// The ref exists but `.current` is still null, so a later timeout must not leave a
122+
// non-functional integration behind.
123+
expect(addIntegration).not.toHaveBeenCalled();
124+
expect(container.addListener).not.toHaveBeenCalled();
125+
126+
store.navigationRef.current = container;
127+
jest.advanceTimersByTime(50);
128+
129+
expect(addIntegration).toHaveBeenCalledTimes(1);
130+
expect(container.addListener).toHaveBeenCalledWith('__unsafe_action__', expect.any(Function));
131+
expect(container.addListener).toHaveBeenCalledWith('state', expect.any(Function));
132+
});
133+
});
134+
135+
describe('expo-router 55 and older, getRouteInfoFromState missing', () => {
136+
it('registers the navigation container without a route override', () => {
137+
const container = createMockNavigationContainer();
138+
jest.doMock('expo-router', () => ({ useNavigationContainerRef: () => ({ current: container }) }), {
139+
virtual: true,
140+
});
141+
jest.doMock(
142+
EXPO_ROUTER_ROUTE_INFO_MODULE,
143+
() => {
144+
throw new Error(`Cannot find module '${EXPO_ROUTER_ROUTE_INFO_MODULE}'`);
145+
},
146+
{ virtual: true },
147+
);
148+
149+
const { expoRouterIntegration: integration } = require('../../src/js/tracing/expoRouterIntegration');
150+
const { client, getIntegrationByName } = createMockClient();
151+
const registerNavigationContainer = jest.fn();
152+
const setRouteOverrideProvider = jest.fn();
153+
getIntegrationByName.mockImplementation((name: string) =>
154+
name === REACT_NAVIGATION_INTEGRATION_NAME
155+
? { name, registerNavigationContainer, _setRouteOverrideProvider: setRouteOverrideProvider }
156+
: undefined,
157+
);
158+
159+
integration().afterAllSetup?.(client);
160+
161+
expect(registerNavigationContainer).toHaveBeenCalledWith({ current: container });
162+
const provider = setRouteOverrideProvider.mock.calls[0][0] as () => unknown;
163+
expect(provider()).toBeUndefined();
164+
});
165+
});
166+
167+
describe('useNavigationContainerRef throws', () => {
168+
it('keeps polling without throwing and warns after the timeout', () => {
169+
jest.doMock(
170+
'expo-router',
171+
() => ({
172+
useNavigationContainerRef: () => {
173+
throw new Error('Invalid hook call');
174+
},
175+
}),
176+
{ virtual: true },
177+
);
178+
179+
const { debug } = require('@sentry/core');
180+
const warnSpy = jest.spyOn(debug, 'warn').mockImplementation(() => undefined);
181+
182+
const { expoRouterIntegration: integration } = require('../../src/js/tracing/expoRouterIntegration');
183+
const { client, addIntegration } = createMockClient();
184+
185+
expect(() => {
186+
integration().afterAllSetup?.(client);
187+
jest.advanceTimersByTime(6_000);
188+
}).not.toThrow();
189+
190+
expect(addIntegration).not.toHaveBeenCalled();
191+
expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining('navigationRef'));
192+
193+
warnSpy.mockRestore();
194+
});
195+
});
196+
101197
describe('expo-router installed, navigationRef pre-populated', () => {
102198
it('registers the navigation container immediately', () => {
103199
const container = createMockNavigationContainer();

0 commit comments

Comments
 (0)