From 708330ca31581595dd172d9ad8948cb7e9fa83f0 Mon Sep 17 00:00:00 2001 From: Antonis Lilis Date: Thu, 1 Oct 2026 15:32:42 +0200 Subject: [PATCH] chore(core): resolve stale TODO comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve four stale/answerable TODOs after verifying each against the source: - reactnavigation.ts / reactnativenavigation.ts "What if it's not SentrySpan?": the latest navigation span is always a SentrySpan or a SentryNonRecordingSpan, and the latter's end() is a no-op, so there is nothing to discard. Replaced with a one-line note at the guard. - reactnativenavigation.ts "Should we include pass props?": passProps is arbitrary app-defined data (potential PII), so it is intentionally omitted. Documented as a one-line note so it is not added back unguarded. - RNSentry.mm "If the callback isn't executed the promise wouldn't be resolved": stale — resolve() is unconditional, and cocoa's configureScope runs the block synchronously when the SDK is initialized (nil scope otherwise, which the JS consumer already handles). Removed the stale comment. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 --- packages/core/ios/RNSentry.mm | 2 -- packages/core/src/js/tracing/reactnativenavigation.ts | 4 ++-- packages/core/src/js/tracing/reactnavigation.ts | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/core/ios/RNSentry.mm b/packages/core/ios/RNSentry.mm index 19daad723f..56398156d9 100644 --- a/packages/core/ios/RNSentry.mm +++ b/packages/core/ios/RNSentry.mm @@ -495,8 +495,6 @@ - (void)handleShakeDetected NSLog(@"Bridge call to: deviceContexts"); } __block NSMutableDictionary *serializedScope; - // Temp work around until sorted out this API in sentry-cocoa. - // TODO: If the callback isnt' executed the promise wouldn't be resolved. [SentrySDKWrapper configureScope:^(SentryScope *_Nonnull scope) { serializedScope = [[scope serialize] mutableCopy]; diff --git a/packages/core/src/js/tracing/reactnativenavigation.ts b/packages/core/src/js/tracing/reactnativenavigation.ts index 5a3eb9e422..c881e1647a 100644 --- a/packages/core/src/js/tracing/reactnativenavigation.ts +++ b/packages/core/src/js/tracing/reactnativenavigation.ts @@ -179,7 +179,7 @@ export const reactNativeNavigationIntegration = ({ latestNavigationSpan.updateName(event.componentName); } latestNavigationSpan.setAttributes({ - // TODO: Should we include pass props? I don't know exactly what it contains, cant find it in the RNavigation docs + // `passProps` is intentionally omitted: arbitrary app data (potential PII); gate on `sendDefaultPii` if ever added. 'route.name': event.componentName, 'route.component_id': event.componentId, 'route.component_type': event.componentType, @@ -227,7 +227,7 @@ export const reactNativeNavigationIntegration = ({ if (isSentrySpan(latestNavigationSpan)) { markRootSpanForDiscard(latestNavigationSpan, 'discarded_latest_navigation'); } - // TODO: What if it's not SentrySpan? + // A non-SentrySpan here is a non-recording span whose end() is a no-op, so nothing to discard. latestNavigationSpan.end(); latestNavigationSpan = undefined; } diff --git a/packages/core/src/js/tracing/reactnavigation.ts b/packages/core/src/js/tracing/reactnavigation.ts index 7b7def8d57..dc303bb1e7 100644 --- a/packages/core/src/js/tracing/reactnavigation.ts +++ b/packages/core/src/js/tracing/reactnavigation.ts @@ -867,7 +867,7 @@ export const reactNavigationIntegration = ({ if (isSentrySpan(latestNavigationSpan)) { markRootSpanForDiscard(latestNavigationSpan, 'discarded_latest_navigation'); } - // TODO: What if it's not SentrySpan? + // A non-SentrySpan here is a non-recording span whose end() is a no-op, so nothing to discard. latestNavigationSpan.end(); latestNavigationSpan = undefined; }