feat(tracing): Add Sentry.NavigationContainer wrapper for React Navigation #6199
2 issues
code-review: Found 2 issues (2 low)
Low
Fallback `useEffect` may re-invoke `onReady` on re-renders when `@react-navigation/native` is missing - `packages/core/src/js/index.ts:113`
In NavigationContainer.tsx the fallback useEffect (lines 71-75) lists userOnReady as a dependency and has no guard. When @react-navigation/native is not installed and the parent passes an inline onReady={() => โฆ}, every parent re-render produces a new function reference, causing the effect to re-fire and invoke userOnReady repeatedly instead of once on mount. Consider capturing the callback in a ref and gating with a hasFired ref so it runs at most once, mirroring the real NavigationContainer.onReady semantics. Impact is limited because this only affects the missing-dependency fallback path, but it can surprise consumers who rely on onReady firing a single time.
Also found at:
packages/core/src/js/NavigationContainer.tsx:69-73
Module-level warning flags cause test order dependency - `packages/core/test/NavigationContainer.test.tsx:110-131`
The implementation's module-level _warnedNoClient and _warnedNoIntegration flags are set to true during the warning tests (lines 110โ131) and are never reset between tests, so any future test in this file that re-enters those code paths won't see the expected warnings. Consider calling jest.resetModules() and re-importing in beforeEach, or exporting a reset helper for tests.
โฑ 6m 12s ยท 293.9k in / 28.1k out ยท $1.12