Skip to content

Commit d017678

Browse files
committed
Address review comments
1 parent 366b6c4 commit d017678

File tree

1 file changed

+19
-19
lines changed

1 file changed

+19
-19
lines changed

packages/react/src/reactrouter-compat-utils/instrumentation.tsx

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ let _enableAsyncRouteHandlers: boolean = false;
5353
const CLIENTS_WITH_INSTRUMENT_NAVIGATION = new WeakSet<Client>();
5454

5555
// Maximum time (ms) to wait for lazy routes (configured in setup(), default: idleTimeout * 3)
56-
let _maxLazyRouteWaitMs = 3000;
56+
let _lazyRouteTimeout = 3000;
5757

5858
/**
5959
* Adds resolved routes as children to the parent route.
@@ -101,11 +101,11 @@ export interface ReactRouterOptions {
101101
/**
102102
* Maximum time (in milliseconds) to wait for lazy routes to load before finalizing span names.
103103
*
104-
* Defaults to 3× the configured `idleTimeout`. Set to `0` to not wait at all.
104+
* Set to `0` to not wait at all, or `Infinity` to wait indefinitely.
105105
*
106106
* @default idleTimeout * 3
107107
*/
108-
maxLazyRouteWaitMs?: number;
108+
lazyRouteTimeout?: number;
109109
}
110110

111111
type V6CompatibleVersion = '6' | '7';
@@ -501,7 +501,7 @@ export function createReactRouterV6CompatibleTracingIntegration(
501501
enableAsyncRouteHandlers = false,
502502
instrumentPageLoad = true,
503503
instrumentNavigation = true,
504-
maxLazyRouteWaitMs,
504+
lazyRouteTimeout,
505505
} = options;
506506

507507
return {
@@ -514,24 +514,24 @@ export function createReactRouterV6CompatibleTracingIntegration(
514514
// Note: options already contains idleTimeout if user passed it to browserTracingIntegration
515515
// Calculate default: 3× idleTimeout, allow explicit override
516516
const defaultMaxWait = (options.idleTimeout ?? 1000) * 3;
517-
const configuredMaxWait = maxLazyRouteWaitMs ?? defaultMaxWait;
517+
const configuredMaxWait = lazyRouteTimeout ?? defaultMaxWait;
518518

519519
// Validate and set
520520
if (Number.isNaN(configuredMaxWait)) {
521521
DEBUG_BUILD &&
522-
debug.warn('[React Router] maxLazyRouteWaitMs must be a number, falling back to default:', defaultMaxWait);
523-
_maxLazyRouteWaitMs = defaultMaxWait;
522+
debug.warn('[React Router] lazyRouteTimeout must be a number, falling back to default:', defaultMaxWait);
523+
_lazyRouteTimeout = defaultMaxWait;
524524
} else if (configuredMaxWait < 0 && configuredMaxWait !== Infinity) {
525525
DEBUG_BUILD &&
526526
debug.warn(
527-
'[React Router] maxLazyRouteWaitMs must be non-negative or Infinity, got:',
527+
'[React Router] lazyRouteTimeout must be non-negative or Infinity, got:',
528528
configuredMaxWait,
529529
'falling back to:',
530530
defaultMaxWait,
531531
);
532-
_maxLazyRouteWaitMs = defaultMaxWait;
532+
_lazyRouteTimeout = defaultMaxWait;
533533
} else {
534-
_maxLazyRouteWaitMs = configuredMaxWait;
534+
_lazyRouteTimeout = configuredMaxWait;
535535
}
536536

537537
_useEffect = useEffect;
@@ -911,10 +911,13 @@ function patchSpanEnd(
911911

912912
const originalEnd = span.end.bind(span);
913913

914+
// Prevent duplicate work when end() is called multiple times during the async lazy route wait.
915+
// External code (visibility changes, timeouts) can call end() again, which would wastefully
916+
// create promise chains and run expensive route matching before reaching the base span.end() guard.
914917
let endCalled = false;
915918

916919
span.end = function patchedEnd(...args) {
917-
// Prevent multiple calls to end()
920+
// Guard against re-entry
918921
if (endCalled) {
919922
return;
920923
}
@@ -928,7 +931,7 @@ function patchSpanEnd(
928931
const pendingPromises = pendingLazyRouteLoads.get(span);
929932
if (pendingPromises && pendingPromises.size > 0 && currentName && transactionNameHasWildcard(currentName)) {
930933
// Special case: 0 means don't wait at all (legacy behavior)
931-
if (_maxLazyRouteWaitMs === 0) {
934+
if (_lazyRouteTimeout === 0) {
932935
// Don't wait - immediately update and end span
933936
tryUpdateSpanNameBeforeEnd(
934937
span,
@@ -943,17 +946,14 @@ function patchSpanEnd(
943946
return originalEnd(...args);
944947
}
945948

946-
// Take snapshot of current promises to wait for (prevents race conditions with new navigations)
947-
const promiseArray = Array.from(pendingPromises);
948-
949-
// Wait for all lazy routes to settle (never rejects, safe for all outcomes)
950-
const allSettled = Promise.allSettled(promiseArray).then(() => {});
949+
// Wait for lazy routes that are currently loading
950+
const allSettled = Promise.allSettled(pendingPromises).then(() => {});
951951

952952
// Race against timeout or wait indefinitely if Infinity
953953
const waitPromise =
954-
_maxLazyRouteWaitMs === Infinity
954+
_lazyRouteTimeout === Infinity
955955
? allSettled
956-
: Promise.race([allSettled, new Promise<void>(r => setTimeout(r, _maxLazyRouteWaitMs))]);
956+
: Promise.race([allSettled, new Promise<void>(r => setTimeout(r, _lazyRouteTimeout))]);
957957

958958
// Update span name once routes are resolved or timeout expires
959959
waitPromise

0 commit comments

Comments
 (0)