diff --git a/packages/react/src/reactrouterv6.tsx b/packages/react/src/reactrouterv6.tsx index 8f2c2825a2b3..a7950ce0b12f 100644 --- a/packages/react/src/reactrouterv6.tsx +++ b/packages/react/src/reactrouterv6.tsx @@ -2,7 +2,7 @@ // https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536 import { Transaction, TransactionContext, TransactionSource } from '@sentry/types'; -import { getGlobalObject, logger } from '@sentry/utils'; +import { getGlobalObject, getNumberOfUrlSegments, logger } from '@sentry/utils'; import hoistNonReactStatics from 'hoist-non-react-statics'; import React from 'react'; @@ -20,9 +20,23 @@ type Params = { readonly [key in Key]: string | undefined; }; +// https://github.com/remix-run/react-router/blob/9fa54d643134cd75a0335581a75db8100ed42828/packages/react-router/lib/router.ts#L114-L134 interface RouteMatch { + /** + * The names and values of dynamic parameters in the URL. + */ params: Params; + /** + * The portion of the URL pathname that was matched. + */ pathname: string; + /** + * The portion of the URL pathname that was matched before child routes. + */ + pathnameBase: string; + /** + * The route object that was used to match. + */ route: RouteObject; } @@ -94,13 +108,31 @@ function getNormalizedName( const branches = matchRoutes(routes, location); + let pathBuilder = ''; if (branches) { // eslint-disable-next-line @typescript-eslint/prefer-for-of for (let x = 0; x < branches.length; x++) { - if (branches[x].route && branches[x].route.path && branches[x].pathname === location.pathname) { - const path = branches[x].route.path; + const branch = branches[x]; + const route = branch.route; + if (route) { + // Early return if index route + if (route.index) { + return [branch.pathname, 'route']; + } + + const path = route.path; if (path) { - return [path, 'route']; + const newPath = path[0] === '/' ? path : `/${path}`; + pathBuilder += newPath; + if (branch.pathname === location.pathname) { + // If the route defined on the element is something like + // Product} /> + // We should check against the branch.pathname for the number of / seperators + if (getNumberOfUrlSegments(pathBuilder) !== getNumberOfUrlSegments(branch.pathname)) { + return [newPath, 'route']; + } + return [pathBuilder, 'route']; + } } } } diff --git a/packages/react/test/reactrouterv6.test.tsx b/packages/react/test/reactrouterv6.test.tsx index 2a5acb921ae8..0058f257aee5 100644 --- a/packages/react/test/reactrouterv6.test.tsx +++ b/packages/react/test/reactrouterv6.test.tsx @@ -196,4 +196,38 @@ describe('React Router v6', () => { metadata: { source: 'route' }, }); }); + + it('works with nested paths with parameters', () => { + const [mockStartTransaction] = createInstrumentation(); + const SentryRoutes = withSentryReactRouterV6Routing(Routes); + + render( + + + } /> + Account Page} /> + + Project Index} /> + Project Page}> + Project Page Root} /> + Editor}> + View Canvas} /> + Space Canvas} /> + + + + + No Match Page} /> + + , + ); + + expect(mockStartTransaction).toHaveBeenCalledTimes(2); + expect(mockStartTransaction).toHaveBeenLastCalledWith({ + name: '/projects/:projectId/views/:viewId', + op: 'navigation', + tags: { 'routing.instrumentation': 'react-router-v6' }, + metadata: { source: 'route' }, + }); + }); }); diff --git a/packages/tracing/src/integrations/node/express.ts b/packages/tracing/src/integrations/node/express.ts index a211dc279e28..d82d29292e46 100644 --- a/packages/tracing/src/integrations/node/express.ts +++ b/packages/tracing/src/integrations/node/express.ts @@ -1,6 +1,12 @@ /* eslint-disable max-lines */ import { Integration, Transaction } from '@sentry/types'; -import { CrossPlatformRequest, extractPathForTransaction, isRegExp, logger } from '@sentry/utils'; +import { + CrossPlatformRequest, + extractPathForTransaction, + getNumberOfUrlSegments, + isRegExp, + logger, +} from '@sentry/utils'; type Method = | 'all' @@ -384,15 +390,6 @@ function getNumberOfArrayUrlSegments(routesArray: RouteType[]): number { }, 0); } -/** - * Returns number of URL segments of a passed URL. - * Also handles URLs of type RegExp - */ -function getNumberOfUrlSegments(url: string): number { - // split at '/' or at '\/' to split regex urls correctly - return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length; -} - /** * Extracts and returns the stringified version of the layers route path * Handles route arrays (by joining the paths together) as well as RegExp and normal diff --git a/packages/utils/src/index.ts b/packages/utils/src/index.ts index 7c8943da813f..b221bef38c91 100644 --- a/packages/utils/src/index.ts +++ b/packages/utils/src/index.ts @@ -25,3 +25,4 @@ export * from './envelope'; export * from './clientreport'; export * from './ratelimit'; export * from './baggage'; +export * from './url'; diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 19025cbb846a..86ab32278ffe 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -41,40 +41,6 @@ export function uuid4(): string { ); } -/** - * Parses string form of URL into an object - * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B - * // intentionally using regex and not href parsing trick because React Native and other - * // environments where DOM might not be available - * @returns parsed URL object - */ -export function parseUrl(url: string): { - host?: string; - path?: string; - protocol?: string; - relative?: string; -} { - if (!url) { - return {}; - } - - const match = url.match(/^(([^:/?#]+):)?(\/\/([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/); - - if (!match) { - return {}; - } - - // coerce to undefined values to empty string so we don't get 'undefined' - const query = match[6] || ''; - const fragment = match[8] || ''; - return { - host: match[4], - path: match[5], - protocol: match[2], - relative: match[5] + query + fragment, // everything minus origin - }; -} - function getFirstException(event: Event): Exception | undefined { return event.exception && event.exception.values ? event.exception.values[0] : undefined; } @@ -197,17 +163,6 @@ export function addContextToFrame(lines: string[], frame: StackFrame, linesOfCon .map((line: string) => snipLine(line, 0)); } -/** - * Strip the query string and fragment off of a given URL or path (if present) - * - * @param urlPath Full URL or path, including possible query string and/or fragment - * @returns URL or path without query string or fragment - */ -export function stripUrlQueryAndFragment(urlPath: string): string { - // eslint-disable-next-line no-useless-escape - return urlPath.split(/[\?#]/, 1)[0]; -} - /** * Checks whether or not we've already captured the given exception (note: not an identical exception - the very object * in question), and marks it captured if not. diff --git a/packages/utils/src/requestdata.ts b/packages/utils/src/requestdata.ts index 72c12b2e5399..b45f3e874cfc 100644 --- a/packages/utils/src/requestdata.ts +++ b/packages/utils/src/requestdata.ts @@ -15,8 +15,8 @@ import { Event, ExtractedNodeRequestData, Transaction, TransactionSource } from '@sentry/types'; import { isPlainObject, isString } from './is'; -import { stripUrlQueryAndFragment } from './misc'; import { normalize } from './normalize'; +import { stripUrlQueryAndFragment } from './url'; const DEFAULT_INCLUDES = { ip: false, diff --git a/packages/utils/src/url.ts b/packages/utils/src/url.ts new file mode 100644 index 000000000000..8dce48e7428b --- /dev/null +++ b/packages/utils/src/url.ts @@ -0,0 +1,52 @@ +/** + * Parses string form of URL into an object + * // borrowed from https://tools.ietf.org/html/rfc3986#appendix-B + * // intentionally using regex and not href parsing trick because React Native and other + * // environments where DOM might not be available + * @returns parsed URL object + */ +export function parseUrl(url: string): { + host?: string; + path?: string; + protocol?: string; + relative?: string; +} { + if (!url) { + return {}; + } + + const match = url.match(/^(([^:/?#]+):)?(\/\/([^/?#]*))?([^?#]*)(\?([^#]*))?(#(.*))?$/); + + if (!match) { + return {}; + } + + // coerce to undefined values to empty string so we don't get 'undefined' + const query = match[6] || ''; + const fragment = match[8] || ''; + return { + host: match[4], + path: match[5], + protocol: match[2], + relative: match[5] + query + fragment, // everything minus origin + }; +} + +/** + * Strip the query string and fragment off of a given URL or path (if present) + * + * @param urlPath Full URL or path, including possible query string and/or fragment + * @returns URL or path without query string or fragment + */ +export function stripUrlQueryAndFragment(urlPath: string): string { + // eslint-disable-next-line no-useless-escape + return urlPath.split(/[\?#]/, 1)[0]; +} + +/** + * Returns number of URL segments of a passed string URL. + */ +export function getNumberOfUrlSegments(url: string): number { + // split at '/' or at '\/' to split regex urls correctly + return url.split(/\\?\//).filter(s => s.length > 0 && s !== ',').length; +} diff --git a/packages/utils/test/misc.test.ts b/packages/utils/test/misc.test.ts index a92d3963e503..c7c98991efbf 100644 --- a/packages/utils/test/misc.test.ts +++ b/packages/utils/test/misc.test.ts @@ -5,7 +5,6 @@ import { addExceptionMechanism, checkOrSetAlreadyCaught, getEventDescription, - stripUrlQueryAndFragment, uuid4, } from '../src/misc'; @@ -187,27 +186,6 @@ describe('addContextToFrame', () => { }); }); -describe('stripQueryStringAndFragment', () => { - const urlString = 'http://dogs.are.great:1231/yay/'; - const queryString = '?furry=yes&funny=very'; - const fragment = '#adoptnotbuy'; - - it('strips query string from url', () => { - const urlWithQueryString = `${urlString}${queryString}`; - expect(stripUrlQueryAndFragment(urlWithQueryString)).toBe(urlString); - }); - - it('strips fragment from url', () => { - const urlWithFragment = `${urlString}${fragment}`; - expect(stripUrlQueryAndFragment(urlWithFragment)).toBe(urlString); - }); - - it('strips query string and fragment from url', () => { - const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`; - expect(stripUrlQueryAndFragment(urlWithQueryStringAndFragment)).toBe(urlString); - }); -}); - describe('addExceptionMechanism', () => { const defaultMechanism = { type: 'generic', handled: true }; diff --git a/packages/utils/test/url.test.ts b/packages/utils/test/url.test.ts new file mode 100644 index 000000000000..e356662b9b29 --- /dev/null +++ b/packages/utils/test/url.test.ts @@ -0,0 +1,33 @@ +import { getNumberOfUrlSegments, stripUrlQueryAndFragment } from '../src/url'; + +describe('stripQueryStringAndFragment', () => { + const urlString = 'http://dogs.are.great:1231/yay/'; + const queryString = '?furry=yes&funny=very'; + const fragment = '#adoptnotbuy'; + + it('strips query string from url', () => { + const urlWithQueryString = `${urlString}${queryString}`; + expect(stripUrlQueryAndFragment(urlWithQueryString)).toBe(urlString); + }); + + it('strips fragment from url', () => { + const urlWithFragment = `${urlString}${fragment}`; + expect(stripUrlQueryAndFragment(urlWithFragment)).toBe(urlString); + }); + + it('strips query string and fragment from url', () => { + const urlWithQueryStringAndFragment = `${urlString}${queryString}${fragment}`; + expect(stripUrlQueryAndFragment(urlWithQueryStringAndFragment)).toBe(urlString); + }); +}); + +describe('getNumberOfUrlSegments', () => { + test.each([ + ['regular path', '/projects/123/views/234', 4], + ['single param paramaterized path', '/users/:id/details', 3], + ['multi param paramaterized path', '/stores/:storeId/products/:productId', 4], + ['regex path', String(/\/api\/post[0-9]/), 2], + ])('%s', (_: string, input, output) => { + expect(getNumberOfUrlSegments(input)).toEqual(output); + }); +});