-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(react): Add React Router Descendant Routes support. #14304
Conversation
5709dd9
to
d7e6a87
Compare
This comment was marked as outdated.
This comment was marked as outdated.
0dfe3a5
to
1d42620
Compare
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
84caae2
to
bcb1cc7
Compare
size-limit report 📦
|
586129a
to
fe0660b
Compare
fe0660b
to
89751f0
Compare
@@ -222,6 +227,7 @@ export function handleNavigation( | |||
version: V6CompatibleVersion, | |||
matches?: AgnosticDataRouteMatch, | |||
basename?: string, | |||
allRoutes?: RouteObject[], |
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.
M: I think it would make sense to pass the options as object because more than 3 options (and here we have 7) can get messy quickly. In the function usages in this PR undefined
has to be used a lot already and it's cleaner to just pass the params we need. Additionally, if we have to add required params in the future we have to update the usages of the function everywhere.
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.
Yes makes sense, updated 👍
function rebuildRoutePathFromAllRoutes(allRoutes: RouteObject[], location: Location): string { | ||
const matchedRoutes = _matchRoutes(allRoutes, location) as RouteMatch[]; | ||
|
||
if (matchedRoutes) { |
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.
L: we could do an early return here to reduce nesting. And it also makes the other return value more visible
if (matchedRoutes) { | |
if (!matchedRoutes?.length) { | |
return ''; | |
} |
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.
Updated 👍
return trimSlash( | ||
trimSlash(path || '') + | ||
prefixWithSlash( | ||
rebuildRoutePathFromAllRoutes( |
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.
How high is the possibility to run into performance issues here? As this runs recursively.
If we are just considering routes that have a usual, not to crazy length it should be fine though.
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.
I think it's very unlikely that the route definitions would get too deep, especially when the routes are descendant. We only run this when the route accessed falls inside a descendant route.
This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@sentry/node](https://github.com/getsentry/sentry-javascript/tree/master/packages/node) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.42.0` -> `8.47.0`](https://renovatebot.com/diffs/npm/@sentry%2fnode/8.42.0/8.47.0) | | [@sentry/react](https://github.com/getsentry/sentry-javascript/tree/master/packages/react) ([source](https://github.com/getsentry/sentry-javascript)) | dependencies | minor | [`8.42.0` -> `8.47.0`](https://renovatebot.com/diffs/npm/@sentry%2freact/8.42.0/8.47.0) | --- ### Release Notes <details> <summary>getsentry/sentry-javascript (@​sentry/node)</summary> ### [`v8.47.0`](https://github.com/getsentry/sentry-javascript/releases/tag/8.47.0) [Compare Source](getsentry/sentry-javascript@8.46.0...8.47.0) - feat(v8/core): Add `updateSpanName` helper function ([#​14736](getsentry/sentry-javascript#14736)) - feat(v8/node): Do not overwrite prisma `db.system` in newer Prisma versions ([#​14772](getsentry/sentry-javascript#14772)) - feat(v8/node/deps): Bump [@​prisma/instrumentation](https://github.com/prisma/instrumentation) from 5.19.1 to 5.22.0 ([#​14755](getsentry/sentry-javascript#14755)) - feat(v8/replay): Mask srcdoc iframe contents per default ([#​14779](getsentry/sentry-javascript#14779)) - ref(v8/nextjs): Fix typo in source maps deletion warning ([#​14776](getsentry/sentry-javascript#14776)) Work in this release was contributed by [@​aloisklink](https://github.com/aloisklink) and [@​benjick](https://github.com/benjick). Thank you for your contributions! ##### Bundle size 📦 | Path | Size | | ---------------------------------------------------------------- | ----------------- | | [@​sentry/browser](https://github.com/sentry/browser) | 23.29 KB | | [@​sentry/browser](https://github.com/sentry/browser) - with treeshaking flags | 21.96 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing) | 35.81 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) | 73.06 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) - with treeshaking flags | 63.45 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay with Canvas) | 77.37 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay, Feedback) | 89.85 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Feedback) | 40.04 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. sendFeedback) | 27.89 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. FeedbackAsync) | 32.68 KB | | [@​sentry/react](https://github.com/sentry/react) | 25.96 KB | | [@​sentry/react](https://github.com/sentry/react) (incl. Tracing) | 38.64 KB | | [@​sentry/vue](https://github.com/sentry/vue) | 27.52 KB | | [@​sentry/vue](https://github.com/sentry/vue) (incl. Tracing) | 37.67 KB | | [@​sentry/svelte](https://github.com/sentry/svelte) | 23.45 KB | | CDN Bundle | 24.47 KB | | CDN Bundle (incl. Tracing) | 37.51 KB | | CDN Bundle (incl. Tracing, Replay) | 72.71 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) | 78.1 KB | | CDN Bundle - uncompressed | 71.85 KB | | CDN Bundle (incl. Tracing) - uncompressed | 111.23 KB | | CDN Bundle (incl. Tracing, Replay) - uncompressed | 225.3 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed | 238.52 KB | | [@​sentry/nextjs](https://github.com/sentry/nextjs) (client) | 38.9 KB | | [@​sentry/sveltekit](https://github.com/sentry/sveltekit) (client) | 36.32 KB | | [@​sentry/node](https://github.com/sentry/node) | 162.7 KB | | [@​sentry/node](https://github.com/sentry/node) - without tracing | 98.87 KB | | [@​sentry/aws-serverless](https://github.com/sentry/aws-serverless) | 126.53 KB | ### [`v8.46.0`](https://github.com/getsentry/sentry-javascript/releases/tag/8.46.0) [Compare Source](getsentry/sentry-javascript@8.45.1...8.46.0) - feat: Allow capture of more than 1 ANR event \[v8] ([#​14713](getsentry/sentry-javascript#14713)) - feat(node): Detect Railway release name \[v8] ([#​14714](getsentry/sentry-javascript#14714)) - fix: Normalise ANR debug image file paths if appRoot was supplied \[v8] ([#​14709](getsentry/sentry-javascript#14709)) - fix(nuxt): Remove build config from tsconfig ([#​14737](getsentry/sentry-javascript#14737)) Work in this release was contributed by [@​conor-ob](https://github.com/conor-ob). Thank you for your contribution! ##### Bundle size 📦 | Path | Size | | ---------------------------------------------------------------- | ----------------- | | [@​sentry/browser](https://github.com/sentry/browser) | 23.29 KB | | [@​sentry/browser](https://github.com/sentry/browser) - with treeshaking flags | 21.96 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing) | 35.79 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) | 73.01 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) - with treeshaking flags | 63.41 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay with Canvas) | 77.32 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay, Feedback) | 89.81 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Feedback) | 40.04 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. sendFeedback) | 27.89 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. FeedbackAsync) | 32.68 KB | | [@​sentry/react](https://github.com/sentry/react) | 25.96 KB | | [@​sentry/react](https://github.com/sentry/react) (incl. Tracing) | 38.6 KB | | [@​sentry/vue](https://github.com/sentry/vue) | 27.49 KB | | [@​sentry/vue](https://github.com/sentry/vue) (incl. Tracing) | 37.63 KB | | [@​sentry/svelte](https://github.com/sentry/svelte) | 23.45 KB | | CDN Bundle | 24.43 KB | | CDN Bundle (incl. Tracing) | 37.46 KB | | CDN Bundle (incl. Tracing, Replay) | 72.64 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) | 78.01 KB | | CDN Bundle - uncompressed | 71.74 KB | | CDN Bundle (incl. Tracing) - uncompressed | 111.05 KB | | CDN Bundle (incl. Tracing, Replay) - uncompressed | 225.1 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed | 238.32 KB | | [@​sentry/nextjs](https://github.com/sentry/nextjs) (client) | 38.88 KB | | [@​sentry/sveltekit](https://github.com/sentry/sveltekit) (client) | 36.29 KB | | [@​sentry/node](https://github.com/sentry/node) | 162.53 KB | | [@​sentry/node](https://github.com/sentry/node) - without tracing | 98.72 KB | | [@​sentry/aws-serverless](https://github.com/sentry/aws-serverless) | 126.4 KB | ### [`v8.45.1`](https://github.com/getsentry/sentry-javascript/releases/tag/8.45.1) [Compare Source](getsentry/sentry-javascript@8.45.0...8.45.1) - fix(feedback): Return when the `sendFeedback` promise resolves ([#​14683](getsentry/sentry-javascript#14683)) Work in this release was contributed by [@​antonis](https://github.com/antonis). Thank you for your contribution! ##### Bundle size 📦 | Path | Size | | ---------------------------------------------------------------- | ----------------- | | [@​sentry/browser](https://github.com/sentry/browser) | 23.29 KB | | [@​sentry/browser](https://github.com/sentry/browser) - with treeshaking flags | 21.96 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing) | 35.79 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) | 73.01 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay) - with treeshaking flags | 63.41 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay with Canvas) | 77.32 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Tracing, Replay, Feedback) | 89.81 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. Feedback) | 40.04 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. sendFeedback) | 27.89 KB | | [@​sentry/browser](https://github.com/sentry/browser) (incl. FeedbackAsync) | 32.68 KB | | [@​sentry/react](https://github.com/sentry/react) | 25.96 KB | | [@​sentry/react](https://github.com/sentry/react) (incl. Tracing) | 38.6 KB | | [@​sentry/vue](https://github.com/sentry/vue) | 27.49 KB | | [@​sentry/vue](https://github.com/sentry/vue) (incl. Tracing) | 37.63 KB | | [@​sentry/svelte](https://github.com/sentry/svelte) | 23.45 KB | | CDN Bundle | 24.43 KB | | CDN Bundle (incl. Tracing) | 37.46 KB | | CDN Bundle (incl. Tracing, Replay) | 72.64 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) | 78.01 KB | | CDN Bundle - uncompressed | 71.74 KB | | CDN Bundle (incl. Tracing) - uncompressed | 111.05 KB | | CDN Bundle (incl. Tracing, Replay) - uncompressed | 225.1 KB | | CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed | 238.32 KB | | [@​sentry/nextjs](https://github.com/sentry/nextjs) (client) | 38.88 KB | | [@​sentry/sveltekit](https://github.com/sentry/sveltekit) (client) | 36.29 KB | | [@​sentry/node](https://github.com/sentry/node) | 162.52 KB | | [@​sentry/node](https://github.com/sentry/node) - without tracing | 98.71 KB | | [@​sentry/aws-serverless](https://github.com/sentry/aws-serverless) | 126.39 KB | ### [`v8.45.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8450) [Compare Source](getsentry/sentry-javascript@8.44.0...8.45.0) - feat(core): Add `handled` option to `captureConsoleIntegration` ([#​14664](getsentry/sentry-javascript#14664)) - feat(browser): Attach virtual stack traces to `HttpClient` events ([#​14515](getsentry/sentry-javascript#14515)) - feat(replay): Upgrade rrweb packages to 2.31.0 ([#​14689](getsentry/sentry-javascript#14689)) - fix(aws-serverless): Remove v8 layer as it overwrites the current layer for docs ([#​14679](getsentry/sentry-javascript#14679)) - fix(browser): Mark stack trace from `captureMessage` with `attachStacktrace: true` as synthetic ([#​14668](getsentry/sentry-javascript#14668)) - fix(core): Mark stack trace from `captureMessage` with `attatchStackTrace: true` as synthetic ([#​14670](getsentry/sentry-javascript#14670)) - fix(core): Set `level` in server runtime `captureException` ([#​10587](getsentry/sentry-javascript#10587)) - fix(profiling-node): Guard invocation of native profiling methods ([#​14676](getsentry/sentry-javascript#14676)) - fix(nuxt): Inline nitro-utils function ([#​14680](getsentry/sentry-javascript#14680)) - fix(profiling-node): Ensure profileId is added to transaction event ([#​14681](getsentry/sentry-javascript#14681)) - fix(react): Add React Router Descendant Routes support ([#​14304](getsentry/sentry-javascript#14304)) - fix: Disable ANR and Local Variables if debugger is enabled via CLI args ([#​14643](getsentry/sentry-javascript#14643)) Work in this release was contributed by [@​anonrig](https://github.com/anonrig) and [@​Zih0](https://github.com/Zih0). Thank you for your contributions! ### [`v8.44.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8440) [Compare Source](getsentry/sentry-javascript@8.43.0...8.44.0) ##### Deprecations - **feat: Deprecate `autoSessionTracking` ([#​14640](getsentry/sentry-javascript#14640 Deprecates `autoSessionTracking`. To enable session tracking, it is recommended to unset `autoSessionTracking` and ensure that either, in browser environments the `browserSessionIntegration` is added, or in server environments the `httpIntegration` is added. To disable session tracking, it is recommended to unset `autoSessionTracking` and to remove the `browserSessionIntegration` in browser environments, or in server environments configure the `httpIntegration` with the `trackIncomingRequestsAsSessions` option set to `false`. ##### Other Changes - feat: Reword log message around unsent spans ([#​14641](getsentry/sentry-javascript#14641)) - feat(opentelemetry): Set `response` context for http.server spans ([#​14634](getsentry/sentry-javascript#14634)) - fix(google-cloud-serverless): Update homepage link in package.json ([#​14411](getsentry/sentry-javascript#14411)) - fix(nuxt): Add unbuild config to not fail on warn ([#​14662](getsentry/sentry-javascript#14662)) Work in this release was contributed by [@​robinvw1](https://github.com/robinvw1). Thank you for your contribution! ### [`v8.43.0`](https://github.com/getsentry/sentry-javascript/blob/HEAD/CHANGELOG.md#8430) [Compare Source](getsentry/sentry-javascript@8.42.0...8.43.0) ##### Important Changes - **feat(nuxt): Add option autoInjectServerSentry (no default import()) ([#​14553](getsentry/sentry-javascript#14553 Using the dynamic `import()` as the default behavior for initializing the SDK on the server-side did not work for every project. The default behavior of the SDK has been changed, and you now need to **use the `--import` flag to initialize Sentry on the server-side** to leverage full functionality. Example with `--import`: ```bash node --import ./.output/server/sentry.server.config.mjs .output/server/index.mjs ``` In case you are not able to use the `--import` flag, you can enable auto-injecting Sentry in the `nuxt.config.ts` (comes with limitations): ```ts sentry: { autoInjectServerSentry: 'top-level-import', // or 'experimental_dynamic-import' }, ``` - **feat(browser): Adds LaunchDarkly and OpenFeature integrations ([#​14207](getsentry/sentry-javascript#14207 Adds browser SDK integrations for tracking feature flag evaluations through the LaunchDarkly JS SDK and OpenFeature Web SDK: ```ts import * as Sentry from '@​sentry/browser'; Sentry.init({ integrations: [ // Track LaunchDarkly feature flags Sentry.launchDarklyIntegration(), // Track OpenFeature feature flags Sentry.openFeatureIntegration(), ], }); ``` - Read more about the [Feature Flags](https://develop.sentry.dev/sdk/expected-features/#feature-flags) feature in Sentry. - Read more about the [LaunchDarkly SDK Integration](https://docs.sentry.io/platforms/javascript/configuration/integrations/launchdarkly/). - Read more about the [OpenFeature SDK Integration](https://docs.sentry.io/platforms/javascript/configuration/integrations/openfeature/). - **feat(browser): Add `featureFlagsIntegration` for custom tracking of flag evaluations ([#​14582](getsentry/sentry-javascript#14582 Adds a browser integration to manually track feature flags with an API. Feature flags are attached to subsequent error events: ```ts import * as Sentry from '@​sentry/browser'; const featureFlagsIntegrationInstance = Sentry.featureFlagsIntegration(); Sentry.init({ // Initialize the SDK with the feature flag integration integrations: [featureFlagsIntegrationInstance], }); // Manually track a feature flag featureFlagsIntegrationInstance.addFeatureFlag('my-feature', true); ``` - **feat(astro): Add Astro 5 support ([#​14613](getsentry/sentry-javascript#14613 With this release, the Sentry Astro SDK officially supports Astro 5. ##### Deprecations - feat(nextjs): Deprecate typedef for `hideSourceMaps` ([#​14594](getsentry/sentry-javascript#14594)) The functionality of `hideSourceMaps` was removed in version 8 but was forgotten to be deprecated and removed. It will be completely removed in the next major version. - feat(core): Deprecate APIs around `RequestSession`s ([#​14566](getsentry/sentry-javascript#14566)) The APIs around `RequestSession`s are mostly used internally. Going forward the SDK will not expose concepts around `RequestSession`s. Instead, functionality around server-side [Release Health](https://docs.sentry.io/product/releases/health/) will be managed in integrations. ##### Other Changes - feat(browser): Add `browserSessionIntegration` ([#​14551](getsentry/sentry-javascript#14551)) - feat(core): Add `raw_security` envelope types ([#​14562](getsentry/sentry-javascript#14562)) - feat(deps): Bump [@​opentelemetry/instrumentation](https://github.com/opentelemetry/instrumentation) from 0.55.0 to 0.56.0 ([#​14625](getsentry/sentry-javascript#14625)) - feat(deps): Bump [@​sentry/cli](https://github.com/sentry/cli) from 2.38.2 to 2.39.1 ([#​14626](getsentry/sentry-javascript#14626)) - feat(deps): Bump [@​sentry/rollup-plugin](https://github.com/sentry/rollup-plugin) from 2.22.6 to 2.22.7 ([#​14622](getsentry/sentry-javascript#14622)) - feat(deps): Bump [@​sentry/webpack-plugin](https://github.com/sentry/webpack-plugin) from 2.22.6 to 2.22.7 ([#​14623](getsentry/sentry-javascript#14623)) - feat(nestjs): Add fastify support ([#​14549](getsentry/sentry-javascript#14549)) - feat(node): Add [@​vercel/ai](https://github.com/vercel/ai) instrumentation ([#​13892](getsentry/sentry-javascript#13892)) - feat(node): Add `disableAnrDetectionForCallback` function ([#​14359](getsentry/sentry-javascript#14359)) - feat(node): Add `trackIncomingRequestsAsSessions` option to http integration ([#​14567](getsentry/sentry-javascript#14567)) - feat(nuxt): Add option `autoInjectServerSentry` (no default `import()`) ([#​14553](getsentry/sentry-javascript#14553)) - feat(nuxt): Add warning when Netlify or Vercel build is discovered ([#​13868](getsentry/sentry-javascript#13868)) - feat(nuxt): Improve serverless event flushing and scope isolation ([#​14605](getsentry/sentry-javascript#14605)) - feat(opentelemetry): Stop looking at propagation context for span creation ([#​14481](getsentry/sentry-javascript#14481)) - feat(opentelemetry): Update OpenTelemetry dependencies to `^1.29.0` ([#​14590](getsentry/sentry-javascript#14590)) - feat(opentelemetry): Update OpenTelemetry dependencies to `1.28.0` ([#​14547](getsentry/sentry-javascript#14547)) - feat(replay): Upgrade rrweb packages to 2.30.0 ([#​14597](getsentry/sentry-javascript#14597)) - fix(core): Decode `filename` and `module` stack frame properties in Node stack parser ([#​14544](getsentry/sentry-javascript#14544)) - fix(core): Filter out unactionable CEFSharp promise rejection error by default ([#​14595](getsentry/sentry-javascript#14595)) - fix(nextjs): Don't show warning about devtool option ([#​14552](getsentry/sentry-javascript#14552)) - fix(nextjs): Only apply tracing metadata to data fetcher data when data is an object ([#​14575](getsentry/sentry-javascript#14575)) - fix(node): Guard against invalid `maxSpanWaitDuration` values ([#​14632](getsentry/sentry-javascript#14632)) - fix(react): Match routes with `parseSearch` option in TanStack Router instrumentation ([#​14328](getsentry/sentry-javascript#14328)) - fix(sveltekit): Fix git SHA not being picked up for release ([#​14540](getsentry/sentry-javascript#14540)) - fix(types): Fix generic exports with default ([#​14576](getsentry/sentry-javascript#14576)) Work in this release was contributed by [@​lsmurray](https://github.com/lsmurray). Thank you for your contribution! </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS44Mi4wIiwidXBkYXRlZEluVmVyIjoiMzkuODIuMCIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19--> Reviewed-on: https://git.tristess.app/alexandresoro/ouca/pulls/407 Reviewed-by: Alexandre Soro <[email protected]> Co-authored-by: renovate <[email protected]> Co-committed-by: renovate <[email protected]>
We noticed this in Sentry's `/issues/:groupId/` route, which uses SDK v8.43.0. The full route tree is complex, but the relevant parts are: ```js <Route path="/" element={<div>root</div>}> <Route path="/issues/:groupId/" element={<div>issues group</div>}> <Route index element={<div>index</div>} /> </Route> </Route> ``` If you navigate to e.g. `/issues/123` (no trailing slash), the recorded transaction name is `//issues/:groupId/` (notice the double slash). This looks messy but doesn't have too much of a consequence. The worse issue is when you navigate to e.g. `/issues/123/` (with trailing slash), the transaction name is `/issues/123/` - it is not parameterized. This breaks transaction grouping. On the `master` and `develop` branch of the SDK, the transaction name is recorded as `/`. This causes the transactions to be grouped incorrectly with the root, as well as any other routes nested under a route with `path="/"`. (Thanks @JoshFerge for noticing the bad data in Sentry! 🙌) --- Note that this commit effectively reverts a change from #14304 where ```js if (basename + branch.pathname === location.pathname) { ``` became ```js if (location.pathname.endsWith(basename + branch.pathname)) { ``` This is the change that caused the difference in results between SDK versions. This will always match when `basename` is empty, `branch.pathname` is `/`, and `location.pathname` ends with `/` - in other words, if you have a parent route with `path="/"`, every request ending in a slash will match it instead of the more specific child route. (Depending on how often this kind of `Route` nesting happens in the wild, this could be a wide regression in transaction names.) But, no tests failed from the removal of `endsWith`. @onurtemizkan, who wrote the change in question: > I'd expect this to break descendant routes. But in the final > implementation, descendant routes don't end up here. So, I > believe it's safe to revert.
We noticed this in Sentry's `/issues/:groupId/` route, which uses SDK v8.43.0. The full route tree is complex, but the relevant parts are: ```js <Route path="/" element={<div>root</div>}> <Route path="/issues/:groupId/" element={<div>issues group</div>}> <Route index element={<div>index</div>} /> </Route> </Route> ``` If you navigate to e.g. `/issues/123` (no trailing slash), the recorded transaction name is `//issues/:groupId/` (notice the double slash). This looks messy but doesn't have too much of a consequence. The worse issue is when you navigate to e.g. `/issues/123/` (with trailing slash), the transaction name is `/issues/123/` - it is not parameterized. This breaks transaction grouping. On the `master` and `develop` branch of the SDK, the transaction name is recorded as `/`. This causes the transactions to be grouped incorrectly with the root, as well as any other routes nested under a route with `path="/"`. (Thanks @JoshFerge for noticing the bad data in Sentry! 🙌) --- Note that this commit effectively reverts a change from #14304 where ```js if (basename + branch.pathname === location.pathname) { ``` became ```js if (location.pathname.endsWith(basename + branch.pathname)) { ``` This is the change that caused the difference in results between SDK versions. This will always match when `basename` is empty, `branch.pathname` is `/`, and `location.pathname` ends with `/` - in other words, if you have a parent route with `path="/"`, every request ending in a slash will match it instead of the more specific child route. (Depending on how often this kind of `Route` nesting happens in the wild, this could be a wide regression in transaction names.) But, no tests failed from the removal of `endsWith`. @onurtemizkan, who wrote the change in question: > I'd expect this to break descendant routes. But in the final > implementation, descendant routes don't end up here. So, I > believe it's safe to revert.
We noticed this in Sentry's `/issues/:groupId/` route, which uses SDK v8.43.0. The full route tree is complex, but the relevant parts are: ```js <Route path="/" element={<div>root</div>}> <Route path="/issues/:groupId/" element={<div>issues group</div>}> <Route index element={<div>index</div>} /> </Route> </Route> ``` If you navigate to e.g. `/issues/123` (no trailing slash), the recorded transaction name is `//issues/:groupId/` (notice the double slash). This looks messy but doesn't have too much of a consequence. The worse issue is when you navigate to e.g. `/issues/123/` (with trailing slash), the transaction name is `/issues/123/` - it is not parameterized. This breaks transaction grouping. On the `master` and `develop` branch of the SDK, the transaction name is recorded as `/`. This causes the transactions to be grouped incorrectly with the root, as well as any other routes nested under a route with `path="/"`. (Thanks @JoshFerge for noticing the bad data in Sentry! 🙌) --- Note that this commit effectively reverts a change from #14304 where ```js if (basename + branch.pathname === location.pathname) { ``` became ```js if (location.pathname.endsWith(basename + branch.pathname)) { ``` This is the change that caused the difference in results between SDK versions. This will always match when `basename` is empty, `branch.pathname` is `/`, and `location.pathname` ends with `/` - in other words, if you have a parent route with `path="/"`, every request ending in a slash will match it instead of the more specific child route. (Depending on how often this kind of `Route` nesting happens in the wild, this could be a wide regression in transaction names.) But, no tests failed from the removal of `endsWith`. @onurtemizkan, who wrote the change in question: > I'd expect this to break descendant routes. But in the final > implementation, descendant routes don't end up here. So, I > believe it's safe to revert.
Resolves: #5997
Adds support for Descendant Routes, where there are multiple
<Routes />
(wrapped usingSentry.withSentryReactRouterV6Routing
) oruseRoutes
(wrapped usingSentry.withUseRoutes
) are used inside routes declaration./*
, has an element (the inner routes declaration) and no children)This also updates how we check if the
useEffect
hook is triggered the first time in aRoutes
element. The new implementation switches toReact.useRef
, which should be available. We expect users to provideuseEffect
hook when initializing the integration. Maybe we can also make this optional and deprecate it over time.Also, this PR updates the
react-router-6
version used in the tests, to the latest, removing the6.4
tests (the same test set), which IMHO do not seem relevant anymore.