Uh oh!
There was an error while loading. Please reload this page.
ref(react): Remove lazy route machinery in favour of lazyRouteManifest - #19466
ref(react): Remove lazy route machinery in favour of lazyRouteManifest#19466onurtemizkan wants to merge 3 commits into
lazyRouteManifest#19466Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
size-limit report 📦
|
e7ab1da to
38bf328CompareUh oh!
There was an error while loading. Please reload this page.
8f4e810 to
0e3a747CompareThere was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| allRoutes?: RouteObject[]; | ||
| }): void { | ||
| const { location, routes, navigationType, version, matches, basename, allRoutes } = opts; | ||
| const branches = Array.isArray(matches) ? matches : _matchRoutes(allRoutes || routes, location, basename); |
There was a problem hiding this comment.
Manifest fallback bypasses navigation span duplicate detection
Low Severity
The new manifest fallback branch in handleNavigation creates a navigation span via startBrowserTracingNavigationSpan without updating activeNavigationSpans or performing the shouldSkipNavigation duplicate detection that the main branch (lines 831–919) carefully implements. If this code path is ever reached, the created span won't be tracked, so subsequent calls to handleNavigation for the same location won't detect it as a duplicate. While this path is guarded by the setupRouterSubscription deduplication and is unlikely to fire in practice (routes are resolved when idle), the inconsistency with the main branch's tracking pattern could cause untracked spans.
This pull request has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you apply the label |
Closing due to inactivity after stale warning. Comment or reopen when ready to continue, and use |
Following up on #19086
This removes the lazy route machinery (
patchSpanEnd,trackLazyRouteLoad,createDeferredLazyRoutePromise,pendingLazyRouteLoads,deferredLazyRouteResolvers,shouldUpdateWildcardSpanName,tryUpdateSpanNameBeforeEnd, etc.) that was previously used to handle parameterized route naming for lazy routes.The series of fixes for this over time (#17867, #17962, #18098, #18155, #18346, #18881, #18898) added increasingly complex race condition handling, deferred promise mechanisms, and span end patching, all trying to ensure correct transaction names when lazy routes hadn't loaded yet. With
lazyRouteManifestintroduced in #19086, we no longer need any of this. The manifest provides the parameterized route name upfront, without needing to wait for route resolution.Removed:
patchSpanEndand the entirespan.end()monkey-patching mechanismtrackLazyRouteLoad/pendingLazyRouteLoadspromise trackingcreateDeferredLazyRoutePromise/resolveDeferredLazyRoutePromisedeferred resolutionshouldUpdateWildcardSpanName/tryUpdateSpanNameBeforeEndwildcard upgrade logic__sentry_may_have_lazy_routes__span flaghandleNavigationandupdatePageloadTransactionAgnosticDataRouteMatchtype import (only used for the removed matches parameter)lazyRouteTimeoutis deprecated with a runtime warning, pointing users tolazyRouteManifest.The
updatePageloadTransactionretains a manifest fallback for the case wherematchRoutesreturns no branches during initial pageload (lazy routes not yet loaded).handleNavigationdoesn't need this as React Router only updatesstate.locationwhen navigation completes (idle), so routes are already resolved.