-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(react): Add lazyRouteManifest option to resolve lazy-route names
#19086
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
size-limit report 📦
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| export { createAsyncHandlerProxy, handleAsyncHandlerResult, checkRouteForAsyncHandler } from './lazy-routes'; | ||
|
|
||
| // Route manifest exports | ||
| export { matchRouteManifest } from './route-manifest'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unused export of matchRouteManifest from index.ts
Low Severity
The matchRouteManifest function is exported from index.ts but this export is never used. Both consumers import directly from ./route-manifest instead: utils.ts imports from './route-manifest' and the test file imports from '../../src/reactrouter-compat-utils/route-manifest'. This export is dead code that adds unnecessary noise to the public API surface.
| normalizedPathname = normalizedPathname.slice(base.length) || '/'; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicated basename stripping logic in route-manifest.ts
Low Severity
The inline basename stripping logic in matchRouteManifest (lines 16-26) duplicates the existing stripBasenameFromPathname function in utils.ts (lines 144-163). Both implementations perform case-insensitive basename matching, handle trailing slashes, verify segment boundaries, and return the stripped pathname or '/'. The existing function could be exported and reused to avoid maintaining duplicate logic.
This PR will resolve the core reason for the series of fixes / handling for automatic lazy-route resolution for a while.
Related: #18898, #18881, #18346, #18155, #18098, #17962, #17867, #17438, #17277
The core issue we have been trying to tackle is not having access to the complete route hierarchy when asynchronously loaded lazy routes are used. React Router provides a route manifest that we can use while matching parameterized transaction names with routes in all cases except this lazy-routes pattern.
This problem has been discussed on React Router:
While this has been addressed for Remix / React Router (Framework Mode), it's still not available in Library Mode. The manifest contains the lazily-loaded route, only when it's navigated to. While waiting for navigation, our transactions can be dropped for several reasons, such as user behaviour like switching tabs (
document.hiddenguard), hitting timeouts likeidleTimeout, and potentially other reasons. This results in incomplete transaction naming with leftover wildcards, which caused broken aggregation on the Sentry dashboard.The series of attempts to fix this while keeping automatic route discovery has been prone to race conditions and required special-case handling of each edge case scenario, also requiring a considerable amount of internal logic, affecting our readability and performance. At the end, all failed in giving completely robust and deterministic results on the customers' side.
This PR proposes a new option:
lazyRouteManifestspecifically for lazy routes. This will let us have initial information about the route hierarchy. So we can assign correct parameterized transaction names without needing to wait for navigated state.It's a static array of routes in parameterized format (needs to be maintained by the users on route hierarchy updates) like:
enableAsyncRouteHandlersis set totrueCloses #19090 (added automatically)