From 1e37368965ef5cdded4db54182e3c91ebc3c8f1e Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 24 Jan 2025 11:02:36 +0100 Subject: [PATCH 01/15] Split V6 and V7 --- .../src/domain/reactRouterV6/createRouter.ts | 2 +- .../src/domain/reactRouterV6/index.ts | 1 + .../domain/reactRouterV6/routesComponent.ts | 4 +- .../reactRouterV6/startReactRouterView.ts | 2 +- .../src/domain/reactRouterV6/useRoutes.ts | 2 +- .../src/domain/reactRouterV7/createRouter.ts | 28 +++++++ .../src/domain/reactRouterV7/index.ts | 4 + .../domain/reactRouterV7/routesComponent.ts | 8 ++ .../reactRouterV7/startReactRouterView.ts | 75 +++++++++++++++++++ .../src/domain/reactRouterV7/useRoutes.ts | 19 +++++ .../rum-react/src/entries/reactRouterV7.ts | 1 + 11 files changed, 141 insertions(+), 5 deletions(-) create mode 100644 packages/rum-react/src/domain/reactRouterV7/createRouter.ts create mode 100644 packages/rum-react/src/domain/reactRouterV7/index.ts create mode 100644 packages/rum-react/src/domain/reactRouterV7/routesComponent.ts create mode 100644 packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts create mode 100644 packages/rum-react/src/domain/reactRouterV7/useRoutes.ts create mode 100644 packages/rum-react/src/entries/reactRouterV7.ts diff --git a/packages/rum-react/src/domain/reactRouterV6/createRouter.ts b/packages/rum-react/src/domain/reactRouterV6/createRouter.ts index e7bf5c7fc2..570e70d83c 100644 --- a/packages/rum-react/src/domain/reactRouterV6/createRouter.ts +++ b/packages/rum-react/src/domain/reactRouterV6/createRouter.ts @@ -2,7 +2,7 @@ import { createBrowserRouter as originalCreateBrowserRouter, createHashRouter as originalCreateHashRouter, createMemoryRouter as originalCreateMemoryRouter, -} from 'react-router-dom' +} from 'react-router-6' import { startReactRouterView } from './startReactRouterView' type Router = ReturnType diff --git a/packages/rum-react/src/domain/reactRouterV6/index.ts b/packages/rum-react/src/domain/reactRouterV6/index.ts index 68f2aa12ba..ee0401bf1b 100644 --- a/packages/rum-react/src/domain/reactRouterV6/index.ts +++ b/packages/rum-react/src/domain/reactRouterV6/index.ts @@ -1,3 +1,4 @@ export { createMemoryRouter, createHashRouter, createBrowserRouter } from './createRouter' export { useRoutes } from './useRoutes' export { Routes } from './routesComponent' +export { startReactRouterView, computeViewName } from './startReactRouterView' diff --git a/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts b/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts index 354f3db95f..55ec10167f 100644 --- a/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts +++ b/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts @@ -1,5 +1,5 @@ -import type { Routes as OriginalRoutes } from 'react-router-dom' -import { createRoutesFromChildren } from 'react-router-dom' +import type { Routes as OriginalRoutes } from 'react-router-6' +import { createRoutesFromChildren } from 'react-router-6' import { useRoutes } from './useRoutes' // Same as react-router-dom Routes but with our useRoutes instead of the original one diff --git a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts index 7961762e50..d026fac840 100644 --- a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts @@ -1,4 +1,4 @@ -import type { RouteMatch } from 'react-router-dom' +import type { RouteMatch } from 'react-router-6' import { display } from '@datadog/browser-core' import { onRumInit } from '../reactPlugin' diff --git a/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts b/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts index 588405dda9..c88515a9d3 100644 --- a/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts +++ b/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts @@ -1,5 +1,5 @@ import { useRef } from 'react' -import { matchRoutes, useLocation, useRoutes as originalUseRoutes } from 'react-router-dom' +import { matchRoutes, useLocation, useRoutes as originalUseRoutes } from 'react-router-6' import { startReactRouterView } from './startReactRouterView' export const useRoutes: typeof originalUseRoutes = (routes, locationArg) => { diff --git a/packages/rum-react/src/domain/reactRouterV7/createRouter.ts b/packages/rum-react/src/domain/reactRouterV7/createRouter.ts new file mode 100644 index 0000000000..b3c19c5346 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouterV7/createRouter.ts @@ -0,0 +1,28 @@ +import { + createBrowserRouter as originalCreateBrowserRouter, + createHashRouter as originalCreateHashRouter, + createMemoryRouter as originalCreateMemoryRouter, +} from 'react-router-7' +import { startReactRouterView } from './startReactRouterView' + +type Router = ReturnType + +export const createBrowserRouter: typeof originalCreateBrowserRouter = (routes, options) => + registerRouter(originalCreateBrowserRouter(routes, options)) +export const createHashRouter: typeof originalCreateHashRouter = (routes, options) => + registerRouter(originalCreateHashRouter(routes, options)) +export const createMemoryRouter: typeof originalCreateMemoryRouter = (routes, options) => + registerRouter(originalCreateMemoryRouter(routes, options)) + +export function registerRouter(router: Router) { + let location = router.state.location.pathname + router.subscribe((routerState) => { + const newPathname = routerState.location.pathname + if (location !== newPathname) { + startReactRouterView(routerState.matches) + location = newPathname + } + }) + startReactRouterView(router.state.matches) + return router +} diff --git a/packages/rum-react/src/domain/reactRouterV7/index.ts b/packages/rum-react/src/domain/reactRouterV7/index.ts new file mode 100644 index 0000000000..ee0401bf1b --- /dev/null +++ b/packages/rum-react/src/domain/reactRouterV7/index.ts @@ -0,0 +1,4 @@ +export { createMemoryRouter, createHashRouter, createBrowserRouter } from './createRouter' +export { useRoutes } from './useRoutes' +export { Routes } from './routesComponent' +export { startReactRouterView, computeViewName } from './startReactRouterView' diff --git a/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts b/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts new file mode 100644 index 0000000000..a06aa9a051 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts @@ -0,0 +1,8 @@ +import type { Routes as OriginalRoutes } from 'react-router-7' +import { createRoutesFromChildren } from 'react-router-7' +import { useRoutes } from './useRoutes' + +// Same as react-router-dom Routes but with our useRoutes instead of the original one +// https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 +export const Routes: typeof OriginalRoutes = ({ children, location }) => + useRoutes(createRoutesFromChildren(children), location) diff --git a/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts new file mode 100644 index 0000000000..c7ce26b8c2 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts @@ -0,0 +1,75 @@ +import type { RouteMatch } from 'react-router-7' +import { display } from '@datadog/browser-core' +import { onReactPluginInit } from '../reactPlugin' + +export function startReactRouterView(routeMatches: RouteMatch[]) { + onReactPluginInit((configuration, rumPublicApi) => { + if (!configuration.router) { + display.warn('`router: true` is missing from the react plugin configuration, the view will not be tracked.') + return + } + rumPublicApi.startView(computeViewName(routeMatches)) + }) +} + +export function computeViewName(routeMatches: RouteMatch[]) { + if (!routeMatches || routeMatches.length === 0) { + return '' + } + + let viewName = '/' + + for (const routeMatch of routeMatches) { + let path = routeMatch.route.path + if (!path) { + continue + } + + path = substitutePathSplats(path, routeMatch.params, routeMatch === routeMatches[routeMatches.length - 1]) + + if (path.startsWith('/')) { + // Absolute path, replace the current view name + viewName = path + } else { + // Relative path, append to the current view name + if (!viewName.endsWith('/')) { + viewName += '/' + } + viewName += path + } + } + + return viewName +} + +/** + * React-Router allows to define routes with "splats" (or "catchall" or "star") segments[1], + * example: /files/*. It has been noticed that keeping those splats in the view name isn't helpful + * as it "hides" too much information. This function replaces the splats with the actual URL path + * name that they are matching. + * + * [1]: https://reactrouter.com/en/main/route/route#splats + * + * @example + * substitutePathSplats('/files/*', { '*': 'path/to/file' }, true) // => '/files/path/to/file' + */ +function substitutePathSplats(path: string, params: Record, isLastMatchingRoute: boolean) { + if ( + !path.includes('*') || + // In some edge cases, react-router does not provide the `*` parameter, so we don't know what to + // replace it with. In this case, we keep the asterisk. + params['*'] === undefined + ) { + return path + } + + // The `*` parameter is only related to the last matching route path. + if (isLastMatchingRoute) { + return path.replace(/\*/, params['*']) + } + + // Intermediary route paths with a `*` are kind of edge cases, and the `*` parameter is not + // relevant for them. We remove it from the path (along with a potential slash preceeding it) to + // have a coherent view name once everything is concatenated (see examples in spec file). + return path.replace(/\/?\*/, '') +} diff --git a/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts b/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts new file mode 100644 index 0000000000..b5f70f9c57 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts @@ -0,0 +1,19 @@ +import { useRef } from 'react' +import { matchRoutes, useLocation, useRoutes as originalUseRoutes } from 'react-router-7' +import { startReactRouterView } from './startReactRouterView' + +export const useRoutes: typeof originalUseRoutes = (routes, locationArg) => { + const location = useLocation() + const pathname = typeof locationArg === 'string' ? locationArg : locationArg?.pathname || location.pathname + const pathnameRef = useRef(null) + + if (pathnameRef.current !== pathname) { + pathnameRef.current = pathname + const matchedRoutes = matchRoutes(routes, pathname) + if (matchedRoutes) { + startReactRouterView(matchedRoutes) + } + } + + return originalUseRoutes(routes, locationArg) +} diff --git a/packages/rum-react/src/entries/reactRouterV7.ts b/packages/rum-react/src/entries/reactRouterV7.ts new file mode 100644 index 0000000000..a244a42a82 --- /dev/null +++ b/packages/rum-react/src/entries/reactRouterV7.ts @@ -0,0 +1 @@ +export * from '../domain/reactRouterV7' From b98d48f47d2990fbe61a214428d154c3b224a9b2 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 24 Jan 2025 11:03:00 +0100 Subject: [PATCH 02/15] update test --- .../reactRooterTests/createRouter.spec.ts | 81 ++++++ .../reactRooterTests/routesComponent.spec.tsx | 205 +++++++++++++ .../startReactRouterView.spec.ts | 144 +++++++++ .../reactRooterTests/useRoutes.spec.tsx | 275 ++++++++++++++++++ .../domain/reactRouterV6/createRouter.spec.ts | 68 ----- .../reactRouterV6/routesComponent.spec.tsx | 191 ------------ .../startReactRouterView.spec.ts | 111 ------- .../domain/reactRouterV6/useRoutes.spec.tsx | 239 --------------- sandbox/react-app/main.tsx | 4 +- 9 files changed, 707 insertions(+), 611 deletions(-) create mode 100644 packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts create mode 100644 packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx create mode 100644 packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts create mode 100644 packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx delete mode 100644 packages/rum-react/src/domain/reactRouterV6/createRouter.spec.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/routesComponent.spec.tsx delete mode 100644 packages/rum-react/src/domain/reactRouterV6/startReactRouterView.spec.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/useRoutes.spec.tsx diff --git a/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts b/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts new file mode 100644 index 0000000000..9e12aee314 --- /dev/null +++ b/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts @@ -0,0 +1,81 @@ +import { initializeReactPlugin } from '../../../test/initializeReactPlugin' +import { createMemoryRouter as createMemoryRouterV6 } from '../reactRouterV6' +import { createMemoryRouter as createMemoryRouterV7 } from '../reactRouterV7' + +describe('createRouter', () => { + const versions = [ + { label: 'react-router v6', createMemoryRouter: createMemoryRouterV6 }, + { label: 'react-router v7', createMemoryRouter: createMemoryRouterV7 }, + ] + + for (const { label, createMemoryRouter } of versions) { + describe(label, () => { + let startViewSpy: jasmine.Spy<(name?: string | object) => void> + let router: ReturnType + + beforeEach(() => { + if (!window.AbortController) { + pending('createMemoryRouter relies on AbortController') + } + + startViewSpy = jasmine.createSpy() + initializeReactPlugin({ + configuration: { + router: true, + }, + publicApi: { + startView: startViewSpy, + }, + }) + + router = createMemoryRouter( + [{ path: '/foo' }, { path: '/bar', children: [{ path: 'nested' }] }, { path: '*' }], + { + initialEntries: ['/foo'], + } + ) + }) + + afterEach(() => { + router?.dispose() + }) + + it('creates a new view when the router is created', () => { + expect(startViewSpy).toHaveBeenCalledWith('/foo') + }) + + it('creates a new view when the router navigates', async () => { + startViewSpy.calls.reset() + await router.navigate('/bar') + expect(startViewSpy).toHaveBeenCalledWith('/bar') + }) + + it('creates a new view when the router navigates to a nested route', async () => { + await router.navigate('/bar') + startViewSpy.calls.reset() + await router.navigate('/bar/nested') + expect(startViewSpy).toHaveBeenCalledWith('/bar/nested') + }) + + it('creates a new view with the fallback route', async () => { + startViewSpy.calls.reset() + await router.navigate('/non-existent') + expect(startViewSpy).toHaveBeenCalledWith('/non-existent') + }) + + it('does not create a new view when navigating to the same URL', async () => { + await router.navigate('/bar') + startViewSpy.calls.reset() + await router.navigate('/bar') + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('does not create a new view when just changing query parameters', async () => { + await router.navigate('/bar') + startViewSpy.calls.reset() + await router.navigate('/bar?baz=1') + expect(startViewSpy).not.toHaveBeenCalled() + }) + }) + } +}) diff --git a/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx b/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx new file mode 100644 index 0000000000..a4b4950593 --- /dev/null +++ b/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx @@ -0,0 +1,205 @@ +import React from 'react' +import { flushSync } from 'react-dom' +import { MemoryRouter as MemoryRouterV6, Route as RouteV6, useNavigate as useNavigateV6 } from 'react-router-6' +import { MemoryRouter as MemoryRouterV7, Route as RouteV7, useNavigate as useNavigateV7 } from 'react-router-7' +import { initializeReactPlugin } from '../../../test/initializeReactPlugin' +import { appendComponent } from '../../../test/appendComponent' +import { Routes as RoutesV6 } from '../reactRouterV6' +import { Routes as RoutesV7 } from '../reactRouterV7' +;[ + { + version: 'react-router-6', + MemoryRouter: MemoryRouterV6, + Route: RouteV6, + useNavigate: useNavigateV6, + Routes: RoutesV6, + }, + { + version: 'react-router-7', + MemoryRouter: MemoryRouterV7, + Route: RouteV7, + useNavigate: useNavigateV7, + Routes: RoutesV7, + }, +].forEach(({ version, MemoryRouter, Route, useNavigate, Routes }) => { + describe(`Routes component (${version})`, () => { + let startViewSpy: jasmine.Spy<(name?: string | object) => void> + + beforeEach(() => { + startViewSpy = jasmine.createSpy() + initializeReactPlugin({ + configuration: { + router: true, + }, + publicApi: { + startView: startViewSpy, + }, + }) + }) + + it('starts a new view as soon as it is rendered', () => { + appendComponent( + + + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + + it('renders the matching route', () => { + const container = appendComponent( + + + + + + ) + + expect(container.innerHTML).toBe('foo') + }) + + it('does not start a new view on re-render', () => { + let forceUpdate: () => void + + function App() { + const [, setState] = React.useState(0) + forceUpdate = () => setState((s) => s + 1) + return ( + + + + + + ) + } + + appendComponent() + + expect(startViewSpy).toHaveBeenCalledTimes(1) + + flushSync(() => { + forceUpdate!() + }) + + expect(startViewSpy).toHaveBeenCalledTimes(1) + }) + + it('starts a new view on navigation', async () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + return null + } + + appendComponent( + + + + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/bar') + }) + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') + }) + + it('does not start a new view if the URL is the same', () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + return null + } + + appendComponent( + + + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/foo') + }) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('does not start a new view if the path is the same but with different parameters', () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + return null + } + + appendComponent( + + + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/foo?bar=baz') + }) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('does not start a new view if it does not match any route', () => { + // Prevent react router from showing a warning in the console when a route does not match + spyOn(console, 'warn') + + appendComponent( + + + + + + ) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('allows passing a location object', () => { + appendComponent( + + + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + + it('allows passing a location string', () => { + appendComponent( + + + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + }) +}) diff --git a/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts b/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts new file mode 100644 index 0000000000..c4d999bc2c --- /dev/null +++ b/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts @@ -0,0 +1,144 @@ +import { display } from '@datadog/browser-core' +import { + createMemoryRouter as createMemoryRouterV6, + type RouteObject as RouteObjectV6, + type RouteMatch as RouteMatchV6, +} from 'react-router-6' +import { + createMemoryRouter as createMemoryRouterV7, + type RouteObject as RouteObjectV7, + type RouteMatch as RouteMatchV7, +} from 'react-router-7' +import { registerCleanupTask } from '../../../../core/test' +import { initializeReactPlugin } from '../../../test/initializeReactPlugin' +import * as ViewV6 from '../reactRouterV6' +import * as ViewV7 from '../reactRouterV7' + +const routerVersions = [ + { + version: 'react-router-6', + createMemoryRouter: createMemoryRouterV6, + startReactRouterView: ViewV6.startReactRouterView, + computeViewName: ViewV6.computeViewName, + }, + { + version: 'react-router-7', + createMemoryRouter: createMemoryRouterV7, + startReactRouterView: ViewV7.startReactRouterView, + computeViewName: ViewV7.computeViewName, + }, +] + +routerVersions.forEach(({ version, createMemoryRouter, startReactRouterView, computeViewName }) => { + describe(`startReactRouterView (${version})`, () => { + describe('startReactRouterView', () => { + it('creates a new view with the computed view name', () => { + const startViewSpy = jasmine.createSpy() + initializeReactPlugin({ + configuration: { + router: true, + }, + publicApi: { + startView: startViewSpy, + }, + }) + + startReactRouterView([ + { route: { path: '/' } }, + { route: { path: 'user' } }, + { route: { path: ':id' } }, + ] as unknown as RouteMatchV6[] & RouteMatchV7[]) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/user/:id') + }) + + it('displays a warning if the router integration is not enabled', () => { + const displayWarnSpy = spyOn(display, 'warn') + initializeReactPlugin({ + configuration: {}, + }) + + startReactRouterView([] as unknown as RouteMatchV6[] & RouteMatchV7[]) + expect(displayWarnSpy).toHaveBeenCalledOnceWith( + '`router: true` is missing from the react plugin configuration, the view will not be tracked.' + ) + }) + }) + + describe('computeViewName', () => { + it('returns an empty string if there is no route match', () => { + expect(computeViewName([] as unknown as RouteMatchV6[] & RouteMatchV7[])).toBe('') + }) + + it('ignores routes without a path', () => { + expect( + computeViewName([ + { route: { path: '/foo' } }, + { route: {} }, + { route: { path: ':id' } }, + ] as unknown as RouteMatchV6[] & RouteMatchV7[]) + ).toBe('/foo/:id') + }) + + // prettier-ignore + const cases = [ + // route paths, path, expected view name + + // Simple paths + ['/foo', '/foo', '/foo'], + ['/foo', '/bar', '/foo'], // apparently when the path doesn't match any route, React Router returns the last route as a matching route + ['/foo > bar', '/foo/bar', '/foo/bar'], + ['/foo > bar > :p', '/foo/bar/1', '/foo/bar/:p'], + [':p', '/foo', '/:p'], + ['/foo/:p', '/foo/bar', '/foo/:p'], + ['/foo > :p', '/foo/bar', '/foo/:p'], + ['/:a/:b', '/foo/bar', '/:a/:b'], + ['/:a > :b', '/foo/bar', '/:a/:b'], + ['/foo-:a', '/foo-1', '/foo-:a'], + ['/foo/ > bar/ > :id/', '/foo/bar/1/', '/foo/bar/:id/'], + ['/foo > /foo/bar > /foo/bar/:id', + '/foo/bar/1', '/foo/bar/:id'], + + // Splats + ['*', '/foo/1', '/foo/1'], + ['*', '/', '/'], + ['/foo/*', '/foo/1', '/foo/1'], + ['/foo > *', '/foo/1', '/foo/1'], + ['* > *', '/foo/1', '/foo/1'], + ['* > *', '/', '/'], + ['/foo/* > *', '/foo/1', '/foo/1'], + ['* > foo/*', '/foo/1', '/foo/1'], + ['/foo/* > bar/*', '/foo/bar/1', '/foo/bar/1'], + ['/foo/* > bar', '/foo/bar', '/foo/bar'], + ['/foo/:p > *', '/foo/bar/baz', '/foo/:p/baz'], + ['/:p > *', '/foo/bar/1', '/:p/bar/1'], + ['/foo/* > :p', '/foo/bar', '/foo/:p'], + + // Extra edge cases - React Router does not provide the matched path in those case + ['/foo/*/bar', '/foo/1/bar', '/foo/*/bar'], + ['/foo/*-bar', '/foo/1-bar', '/foo/*-bar'], + ['*/*', '/foo/1', '/*/*'], + ] as const + + cases.forEach(([routePaths, path, expectedViewName]) => { + it(`compute the right view name for paths ${routePaths}`, () => { + // Convert the routePaths representing nested routes paths delimited by ' > ' to an actual + // react-router route object. Example: '/foo > bar > :p' would be turned into + // { path: '/foo', children: [{ path: 'bar', children: [{ path: ':p' }] }] } + const route = routePaths + .split(' > ') + .reduceRight( + (childRoute, path) => ({ path, children: childRoute ? [childRoute] : undefined }), + undefined as RouteObjectV6 | RouteObjectV7 | undefined + )! + + const router = createMemoryRouter([route] as any, { + initialEntries: [path], + }) + registerCleanupTask(() => router.dispose()) + expect(computeViewName(router.state.matches as any)).toEqual(expectedViewName) + }) + }) + }) + }) +}) diff --git a/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx new file mode 100644 index 0000000000..9a88a1b02a --- /dev/null +++ b/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx @@ -0,0 +1,275 @@ +import React from 'react' +import { flushSync } from 'react-dom' +import { MemoryRouter as MemoryRouterV6, useNavigate as useNavigateV6 } from 'react-router-6' +import type { RouteObject as RouteObjectV6 } from 'react-router-6' +import { MemoryRouter as MemoryRouterV7, useNavigate as useNavigateV7 } from 'react-router-7' +import type { RouteObject as RouteObjectV7 } from 'react-router-7' +import { appendComponent } from '../../../test/appendComponent' +import { initializeReactPlugin } from '../../../test/initializeReactPlugin' +import { useRoutes as useRoutesV6 } from '../reactRouterV6' +import { useRoutes as useRoutesV7 } from '../reactRouterV7' + +const versions = [ + { + version: 'react-router-6', + MemoryRouter: MemoryRouterV6, + useNavigate: useNavigateV6, + useRoutes: useRoutesV6, + }, + { + version: 'react-router-7', + MemoryRouter: MemoryRouterV7, + useNavigate: useNavigateV7, + useRoutes: useRoutesV7, + }, +] + +versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { + describe(`useRoutes (${version})`, () => { + let startViewSpy: jasmine.Spy<(name?: string | object) => void> + + beforeEach(() => { + startViewSpy = jasmine.createSpy() + initializeReactPlugin({ + configuration: { + router: true, + }, + publicApi: { + startView: startViewSpy, + }, + }) + }) + + it('starts a new view as soon as it is rendered', () => { + appendComponent( + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + + it('renders the matching route', () => { + const container = appendComponent( + + + + ) + + expect(container.innerHTML).toBe('foo') + }) + + it('does not start a new view on re-render', () => { + let forceUpdate: () => void + + function App() { + const [, setState] = React.useState(0) + forceUpdate = () => setState((s) => s + 1) + return ( + + + + ) + } + + appendComponent() + + expect(startViewSpy).toHaveBeenCalledTimes(1) + + flushSync(() => { + forceUpdate!() + }) + + expect(startViewSpy).toHaveBeenCalledTimes(1) + }) + + it('starts a new view on navigation', async () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + + return null + } + + appendComponent( + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/bar') + }) + + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') + }) + + it('does not start a new view if the URL is the same', () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + + return null + } + + appendComponent( + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/foo') + }) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('does not start a new view if the path is the same but with different parameters', () => { + let navigate: (path: string) => void + + function NavBar() { + navigate = useNavigate() + + return null + } + + appendComponent( + + + + + ) + + startViewSpy.calls.reset() + flushSync(() => { + navigate!('/foo?bar=baz') + }) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('does not start a new view if it does not match any route', () => { + // Prevent react router from showing a warning in the console when a route does not match + spyOn(console, 'warn') + + appendComponent( + + + + ) + + expect(startViewSpy).not.toHaveBeenCalled() + }) + + it('allows passing a location object', () => { + appendComponent( + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + + it('allows passing a location string', () => { + appendComponent( + + + + ) + + expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') + }) + }) +}) + +function RoutesRenderer({ + routes, + location, + useRoutes, +}: { + routes: RouteObjectV6[] & RouteObjectV7[] + location?: { pathname: string } | string + useRoutes: typeof useRoutesV6 | typeof useRoutesV7 +}) { + return useRoutes(routes, location) +} diff --git a/packages/rum-react/src/domain/reactRouterV6/createRouter.spec.ts b/packages/rum-react/src/domain/reactRouterV6/createRouter.spec.ts deleted file mode 100644 index 2a673ae330..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/createRouter.spec.ts +++ /dev/null @@ -1,68 +0,0 @@ -import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { createMemoryRouter } from './createRouter' - -describe('createRouter', () => { - let startViewSpy: jasmine.Spy<(name?: string | object) => void> - let router: ReturnType - - beforeEach(() => { - if (!window.AbortController) { - pending('createMemoryRouter rely on AbortController') - } - - startViewSpy = jasmine.createSpy() - initializeReactPlugin({ - configuration: { - router: true, - }, - publicApi: { - startView: startViewSpy, - }, - }) - - router = createMemoryRouter([{ path: '/foo' }, { path: '/bar', children: [{ path: 'nested' }] }, { path: '*' }], { - initialEntries: ['/foo'], - }) - }) - - afterEach(() => { - router?.dispose() - }) - - it('creates a new view when the router is created', () => { - expect(startViewSpy).toHaveBeenCalledWith('/foo') - }) - - it('creates a new view when the router navigates', async () => { - startViewSpy.calls.reset() - await router.navigate('/bar') - expect(startViewSpy).toHaveBeenCalledWith('/bar') - }) - - it('creates a new view when the router navigates to a nested route', async () => { - await router.navigate('/bar') - startViewSpy.calls.reset() - await router.navigate('/bar/nested') - expect(startViewSpy).toHaveBeenCalledWith('/bar/nested') - }) - - it('creates a new view with the fallback route', async () => { - startViewSpy.calls.reset() - await router.navigate('/non-existent') - expect(startViewSpy).toHaveBeenCalledWith('/non-existent') - }) - - it('does not create a new view when the router navigates to the same URL', async () => { - await router.navigate('/bar') - startViewSpy.calls.reset() - await router.navigate('/bar') - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('does not create a new view when the router navigates to the same path but different parameters', async () => { - await router.navigate('/bar') - startViewSpy.calls.reset() - await router.navigate('/bar?baz=1') - expect(startViewSpy).not.toHaveBeenCalled() - }) -}) diff --git a/packages/rum-react/src/domain/reactRouterV6/routesComponent.spec.tsx b/packages/rum-react/src/domain/reactRouterV6/routesComponent.spec.tsx deleted file mode 100644 index f169f7be3e..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/routesComponent.spec.tsx +++ /dev/null @@ -1,191 +0,0 @@ -import React from 'react' -import { flushSync } from 'react-dom' - -import { MemoryRouter, Route, useNavigate } from 'react-router-dom' -import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { appendComponent } from '../../../test/appendComponent' -import { Routes } from './routesComponent' - -describe('Routes component', () => { - let startViewSpy: jasmine.Spy<(name?: string | object) => void> - - beforeEach(() => { - startViewSpy = jasmine.createSpy() - initializeReactPlugin({ - configuration: { - router: true, - }, - publicApi: { - startView: startViewSpy, - }, - }) - }) - - it('starts a new view as soon as it is rendered', () => { - appendComponent( - - - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) - - it('renders the matching route', () => { - const container = appendComponent( - - - - - - ) - - expect(container.innerHTML).toBe('foo') - }) - - it('does not start a new view on re-render', () => { - let forceUpdate: () => void - - function App() { - const [, setState] = React.useState(0) - forceUpdate = () => setState((s) => s + 1) - return ( - - - - - - ) - } - - appendComponent() - - expect(startViewSpy).toHaveBeenCalledTimes(1) - - flushSync(() => { - forceUpdate!() - }) - - expect(startViewSpy).toHaveBeenCalledTimes(1) - }) - - it('starts a new view on navigation', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/bar') - }) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') - }) - - it('does not start a new view if the URL is the same', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/foo') - }) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('does not start a new view if the path is the same but with different parameters', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/foo?bar=baz') - }) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('does not start a new view if it does not match any route', () => { - // Prevent react router from showing a warning in the console when a route does not match - spyOn(console, 'warn') - - appendComponent( - - - - - - ) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('allows passing a location object', () => { - appendComponent( - - - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) - - it('allows passing a location string', () => { - appendComponent( - - - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) -}) diff --git a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.spec.ts b/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.spec.ts deleted file mode 100644 index 315bf81a3b..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.spec.ts +++ /dev/null @@ -1,111 +0,0 @@ -import { type RouteObject, createMemoryRouter, type RouteMatch } from 'react-router-dom' -import { display } from '@datadog/browser-core' -import { registerCleanupTask } from '../../../../core/test' -import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { computeViewName, startReactRouterView } from './startReactRouterView' - -describe('startReactRouterView', () => { - it('creates a new view with the computed view name', () => { - const startViewSpy = jasmine.createSpy() - initializeReactPlugin({ - configuration: { - router: true, - }, - publicApi: { - startView: startViewSpy, - }, - }) - - startReactRouterView([ - { route: { path: '/' } }, - { route: { path: 'user' } }, - { route: { path: ':id' } }, - ] as RouteMatch[]) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/user/:id') - }) - - it('displays a warning if the router integration is not enabled', () => { - const displayWarnSpy = spyOn(display, 'warn') - initializeReactPlugin({ - configuration: {}, - }) - - startReactRouterView([] as RouteMatch[]) - expect(displayWarnSpy).toHaveBeenCalledOnceWith( - '`router: true` is missing from the react plugin configuration, the view will not be tracked.' - ) - }) -}) - -describe('computeViewName', () => { - it('returns an empty string if there is no route match', () => { - expect(computeViewName([] as RouteMatch[])).toBe('') - }) - - it('ignores routes without a path', () => { - expect( - computeViewName([{ route: { path: '/foo' } }, { route: {} }, { route: { path: ':id' } }] as RouteMatch[]) - ).toBe('/foo/:id') - }) - - // prettier-ignore - const cases = [ - // route paths, path, expected view name - - // Simple paths - ['/foo', '/foo', '/foo'], - ['/foo', '/bar', '/foo'], // apparently when the path doesn't match any route, React Router returns the last route as a matching route - ['/foo > bar', '/foo/bar', '/foo/bar'], - ['/foo > bar > :p', '/foo/bar/1', '/foo/bar/:p'], - [':p', '/foo', '/:p'], - ['/foo/:p', '/foo/bar', '/foo/:p'], - ['/foo > :p', '/foo/bar', '/foo/:p'], - ['/:a/:b', '/foo/bar', '/:a/:b'], - ['/:a > :b', '/foo/bar', '/:a/:b'], - ['/foo-:a', '/foo-1', '/foo-:a'], - ['/foo/ > bar/ > :id/', '/foo/bar/1/', '/foo/bar/:id/'], - ['/foo > /foo/bar > /foo/bar/:id', - '/foo/bar/1', '/foo/bar/:id'], - - // Splats - ['*', '/foo/1', '/foo/1'], - ['*', '/', '/'], - ['/foo/*', '/foo/1', '/foo/1'], - ['/foo > *', '/foo/1', '/foo/1'], - ['* > *', '/foo/1', '/foo/1'], - ['* > *', '/', '/'], - ['/foo/* > *', '/foo/1', '/foo/1'], - ['* > foo/*', '/foo/1', '/foo/1'], - ['/foo/* > bar/*', '/foo/bar/1', '/foo/bar/1'], - ['/foo/* > bar', '/foo/bar', '/foo/bar'], - ['/foo/:p > *', '/foo/bar/baz', '/foo/:p/baz'], - ['/:p > *', '/foo/bar/1', '/:p/bar/1'], - ['/foo/* > :p', '/foo/bar', '/foo/:p'], - - // Extra edge cases - React Router does not provide the matched path in those case - ['/foo/*/bar', '/foo/1/bar', '/foo/*/bar'], - ['/foo/*-bar', '/foo/1-bar', '/foo/*-bar'], - ['*/*', '/foo/1', '/*/*'], - ] as const - - cases.forEach(([routePaths, path, expectedViewName]) => { - it(`compute the right view name for paths ${routePaths}`, () => { - // Convert the routePaths representing nested routes paths delimited by ' > ' to an actual - // react-router route object. Example: '/foo > bar > :p' would be turned into - // { path: '/foo', children: [{ path: 'bar', children: [{ path: ':p' }] }] } - const route = routePaths - .split(' > ') - .reduceRight( - (childRoute, path) => ({ path, children: childRoute ? [childRoute] : undefined }), - undefined as RouteObject | undefined - )! - - const router = createMemoryRouter([route], { - initialEntries: [path], - }) - registerCleanupTask(() => router.dispose()) - expect(computeViewName(router.state.matches)).toEqual(expectedViewName) - }) - }) -}) diff --git a/packages/rum-react/src/domain/reactRouterV6/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRouterV6/useRoutes.spec.tsx deleted file mode 100644 index 27998ddef3..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/useRoutes.spec.tsx +++ /dev/null @@ -1,239 +0,0 @@ -import React from 'react' -import { flushSync } from 'react-dom' - -import type { RouteObject } from 'react-router-dom' -import { MemoryRouter, useNavigate } from 'react-router-dom' -import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { appendComponent } from '../../../test/appendComponent' -import { useRoutes } from './useRoutes' - -describe('useRoutes', () => { - let startViewSpy: jasmine.Spy<(name?: string | object) => void> - - beforeEach(() => { - startViewSpy = jasmine.createSpy() - initializeReactPlugin({ - configuration: { - router: true, - }, - publicApi: { - startView: startViewSpy, - }, - }) - }) - - it('starts a new view as soon as it is rendered', () => { - appendComponent( - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) - - it('renders the matching route', () => { - const container = appendComponent( - - - - ) - - expect(container.innerHTML).toBe('foo') - }) - - it('does not start a new view on re-render', () => { - let forceUpdate: () => void - - function App() { - const [, setState] = React.useState(0) - forceUpdate = () => setState((s) => s + 1) - return ( - - - - ) - } - - appendComponent() - - expect(startViewSpy).toHaveBeenCalledTimes(1) - - flushSync(() => { - forceUpdate!() - }) - - expect(startViewSpy).toHaveBeenCalledTimes(1) - }) - - it('starts a new view on navigation', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/bar') - }) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') - }) - - it('does not start a new view if the URL is the same', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/foo') - }) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('does not start a new view if the path is the same but with different parameters', () => { - let navigate: (path: string) => void - - function NavBar() { - navigate = useNavigate() - - return null - } - - appendComponent( - - - - - ) - - startViewSpy.calls.reset() - flushSync(() => { - navigate!('/foo?bar=baz') - }) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('does not start a new view if it does not match any route', () => { - // Prevent react router from showing a warning in the console when a route does not match - spyOn(console, 'warn') - - appendComponent( - - - - ) - - expect(startViewSpy).not.toHaveBeenCalled() - }) - - it('allows passing a location object', () => { - appendComponent( - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) - - it('allows passing a location string', () => { - appendComponent( - - - - ) - - expect(startViewSpy).toHaveBeenCalledOnceWith('/foo') - }) -}) - -function RoutesRenderer({ routes, location }: { routes: RouteObject[]; location?: { pathname: string } | string }) { - return useRoutes(routes, location) -} diff --git a/sandbox/react-app/main.tsx b/sandbox/react-app/main.tsx index 07188e9b33..8ce0a8b8b5 100644 --- a/sandbox/react-app/main.tsx +++ b/sandbox/react-app/main.tsx @@ -1,8 +1,8 @@ -import { Link, Outlet, RouterProvider, useParams } from 'react-router-dom' +import { Link, Outlet, RouterProvider, useParams } from 'react-router-7' import React, { useEffect, useState } from 'react' import ReactDOM from 'react-dom/client' import { datadogRum } from '@datadog/browser-rum' -import { createBrowserRouter } from '@datadog/browser-rum-react/react-router-v6' +import { createBrowserRouter } from '@datadog/browser-rum-react/react-router-v7' import { reactPlugin, ErrorBoundary, UNSTABLE_ReactComponentTracker } from '@datadog/browser-rum-react' datadogRum.init({ From 006e4e72a4f217737bf4e5bf33f69b938464deb2 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 24 Jan 2025 11:03:31 +0100 Subject: [PATCH 03/15] Update dependency --- eslint-local-rules/disallowSideEffects.js | 2 + packages/rum-react/package.json | 5 +- .../rum-react/react-router-v7/package.json | 6 ++ yarn.lock | 65 ++++++++++++++++++- 4 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 packages/rum-react/react-router-v7/package.json diff --git a/eslint-local-rules/disallowSideEffects.js b/eslint-local-rules/disallowSideEffects.js index 21859edb6e..322882493d 100644 --- a/eslint-local-rules/disallowSideEffects.js +++ b/eslint-local-rules/disallowSideEffects.js @@ -37,6 +37,8 @@ const packagesWithoutSideEffect = new Set([ '@datadog/browser-rum-core', 'react', 'react-router-dom', + 'react-router-6', + 'react-router-7', ]) /** diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index 2b92dc67fc..596cd6c0fa 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -17,7 +17,7 @@ }, "peerDependencies": { "react": "18", - "react-router-dom": "6" + "react-router-dom": "6 || 7" }, "peerDependenciesMeta": { "@datadog/browser-rum": { @@ -38,7 +38,8 @@ "@types/react-dom": "18.3.5", "react": "18.3.1", "react-dom": "18.3.1", - "react-router-dom": "6.28.2" + "react-router-6": "npm:react-router-dom@6.28.2", + "react-router-7": "npm:react-router-dom@7.1.3" }, "repository": { "type": "git", diff --git a/packages/rum-react/react-router-v7/package.json b/packages/rum-react/react-router-v7/package.json new file mode 100644 index 0000000000..f152762656 --- /dev/null +++ b/packages/rum-react/react-router-v7/package.json @@ -0,0 +1,6 @@ +{ + "private": true, + "main": "../cjs/entries/reactRouterV7.js", + "module": "../esm/entries/reactRouterV7.js", + "types": "../cjs/entries/reactRouterV7.d.ts" +} diff --git a/yarn.lock b/yarn.lock index 7c95732835..0aa6e3dc09 100644 --- a/yarn.lock +++ b/yarn.lock @@ -355,10 +355,11 @@ __metadata: "@types/react-dom": "npm:18.3.5" react: "npm:18.3.1" react-dom: "npm:18.3.1" - react-router-dom: "npm:6.28.2" + react-router-6: "npm:react-router-dom@6.28.2" + react-router-7: "npm:react-router-dom@7.1.3" peerDependencies: react: 18 - react-router-dom: 6 + react-router-dom: 6 || 7 peerDependenciesMeta: "@datadog/browser-rum": optional: true @@ -1769,6 +1770,13 @@ __metadata: languageName: node linkType: hard +"@types/cookie@npm:^0.6.0": + version: 0.6.0 + resolution: "@types/cookie@npm:0.6.0" + checksum: 10c0/5b326bd0188120fb32c0be086b141b1481fec9941b76ad537f9110e10d61ee2636beac145463319c71e4be67a17e85b81ca9e13ceb6e3bb63b93d16824d6c149 + languageName: node + linkType: hard + "@types/cors@npm:2.8.17, @types/cors@npm:^2.8.12": version: 2.8.17 resolution: "@types/cors@npm:2.8.17" @@ -4382,6 +4390,13 @@ __metadata: languageName: node linkType: hard +"cookie@npm:^1.0.1": + version: 1.0.2 + resolution: "cookie@npm:1.0.2" + checksum: 10c0/fd25fe79e8fbcfcaf6aa61cd081c55d144eeeba755206c058682257cb38c4bd6795c6620de3f064c740695bb65b7949ebb1db7a95e4636efb8357a335ad3f54b + languageName: node + linkType: hard + "cookie@npm:~0.4.1": version: 0.4.2 resolution: "cookie@npm:0.4.2" @@ -11287,7 +11302,7 @@ __metadata: languageName: node linkType: hard -"react-router-dom@npm:6.28.2": +"react-router-6@npm:react-router-dom@6.28.2": version: 6.28.2 resolution: "react-router-dom@npm:6.28.2" dependencies: @@ -11300,6 +11315,18 @@ __metadata: languageName: node linkType: hard +"react-router-7@npm:react-router-dom@7.1.3": + version: 7.1.3 + resolution: "react-router-dom@npm:7.1.3" + dependencies: + react-router: "npm:7.1.3" + peerDependencies: + react: ">=18" + react-dom: ">=18" + checksum: 10c0/84752b90e3f9e9168fc29d7dd5eb0ff622a831de15e4e5d899694f19bc988387d831d180d07f503071d944fa4a666c1d07004e4526268215cc5dbe5bba98f52c + languageName: node + linkType: hard + "react-router@npm:6.28.2": version: 6.28.2 resolution: "react-router@npm:6.28.2" @@ -11311,6 +11338,24 @@ __metadata: languageName: node linkType: hard +"react-router@npm:7.1.3": + version: 7.1.3 + resolution: "react-router@npm:7.1.3" + dependencies: + "@types/cookie": "npm:^0.6.0" + cookie: "npm:^1.0.1" + set-cookie-parser: "npm:^2.6.0" + turbo-stream: "npm:2.4.0" + peerDependencies: + react: ">=18" + react-dom: ">=18" + peerDependenciesMeta: + react-dom: + optional: true + checksum: 10c0/f42f7b245533d1adaa00779a0287993a836d5b56039d97a6643a8b3a721ffb92ff47c97cfb36409fec8794ac3c8a884339f588cf21fcd7f6dccdfc834520c76f + languageName: node + linkType: hard + "react-style-singleton@npm:^2.2.1, react-style-singleton@npm:^2.2.2": version: 2.2.3 resolution: "react-style-singleton@npm:2.2.3" @@ -11995,6 +12040,13 @@ __metadata: languageName: node linkType: hard +"set-cookie-parser@npm:^2.6.0": + version: 2.7.1 + resolution: "set-cookie-parser@npm:2.7.1" + checksum: 10c0/060c198c4c92547ac15988256f445eae523f57f2ceefeccf52d30d75dedf6bff22b9c26f756bd44e8e560d44ff4ab2130b178bd2e52ef5571bf7be3bd7632d9a + languageName: node + linkType: hard + "set-function-length@npm:^1.2.1": version: 1.2.2 resolution: "set-function-length@npm:1.2.2" @@ -13076,6 +13128,13 @@ __metadata: languageName: node linkType: hard +"turbo-stream@npm:2.4.0": + version: 2.4.0 + resolution: "turbo-stream@npm:2.4.0" + checksum: 10c0/e68b2569f1f16e6e9633d090c6024b2ae9f0e97bfeacb572451ca3732e120ebbb546f3bc4afc717c46cb57b5aea6104e04ef497f9912eef6a7641e809518e98a + languageName: node + linkType: hard + "type-check@npm:^0.4.0, type-check@npm:~0.4.0": version: 0.4.0 resolution: "type-check@npm:0.4.0" From 4f2b1a797cfe48a05d5ac3869d1e30552c0d5775 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 24 Jan 2025 11:15:52 +0100 Subject: [PATCH 04/15] startReactRouterView --- .../src/domain/reactRouterV7/startReactRouterView.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts index c7ce26b8c2..d13b2afa33 100644 --- a/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts @@ -1,9 +1,9 @@ import type { RouteMatch } from 'react-router-7' import { display } from '@datadog/browser-core' -import { onReactPluginInit } from '../reactPlugin' +import { onRumInit } from '../reactPlugin' export function startReactRouterView(routeMatches: RouteMatch[]) { - onReactPluginInit((configuration, rumPublicApi) => { + onRumInit((configuration, rumPublicApi) => { if (!configuration.router) { display.warn('`router: true` is missing from the react plugin configuration, the view will not be tracked.') return From 55599e39af5d7582efdb9b43d7f34074bfa37b32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 24 Jan 2025 12:17:30 +0100 Subject: [PATCH 05/15] =?UTF-8?q?=F0=9F=8F=B7=EF=B8=8F=20fix=20typecheck?= =?UTF-8?q?=20issues?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/core/src/domain/connectivity/connectivity.ts | 1 + packages/core/test/emulate/mockNavigator.ts | 2 +- tsconfig.base.json | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/core/src/domain/connectivity/connectivity.ts b/packages/core/src/domain/connectivity/connectivity.ts index 529c1df627..4d6551457c 100644 --- a/packages/core/src/domain/connectivity/connectivity.ts +++ b/packages/core/src/domain/connectivity/connectivity.ts @@ -8,6 +8,7 @@ interface BrowserNavigator extends Navigator { export interface NetworkInformation { type?: NetworkInterface effectiveType?: EffectiveType + saveData: boolean } export interface Connectivity { diff --git a/packages/core/test/emulate/mockNavigator.ts b/packages/core/test/emulate/mockNavigator.ts index c9e38bc5f6..6a43c2332b 100644 --- a/packages/core/test/emulate/mockNavigator.ts +++ b/packages/core/test/emulate/mockNavigator.ts @@ -13,7 +13,7 @@ export function setNavigatorOnLine(onLine: boolean) { }) } -export function setNavigatorConnection(connection: NetworkInformation | undefined) { +export function setNavigatorConnection(connection: Partial | undefined) { Object.defineProperty(navigator, 'connection', { get() { return connection diff --git a/tsconfig.base.json b/tsconfig.base.json index 8230854f3a..a7af2f80b0 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -11,6 +11,7 @@ "target": "ES2018", "sourceMap": true, "jsx": "react", + "skipLibCheck": true, "types": [], "lib": ["ES2020", "DOM"], From 71e3ee81a85e5890726db32264e24c2922179102 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Fri, 24 Jan 2025 14:19:14 +0100 Subject: [PATCH 06/15] change module resolution --- tsconfig.base.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tsconfig.base.json b/tsconfig.base.json index a7af2f80b0..b7c1afe798 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -4,14 +4,13 @@ "experimentalDecorators": true, "esModuleInterop": true, "importHelpers": false, - "moduleResolution": "node", + "moduleResolution": "node16", "module": "ES2020", "resolveJsonModule": true, "strict": true, "target": "ES2018", "sourceMap": true, "jsx": "react", - "skipLibCheck": true, "types": [], "lib": ["ES2020", "DOM"], From 81f6de43a60049dda03674524bbc1d326bf286a5 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Jan 2025 11:33:52 +0100 Subject: [PATCH 07/15] Router V6 and V7 refactor --- .../src/domain/reactRouter/createRouter.ts | 20 +++++ .../rum-react/src/domain/reactRouter/index.ts | 4 + .../src/domain/reactRouter/routesComponent.ts | 11 +++ .../startReactRouterView.ts | 17 +---- .../rum-react/src/domain/reactRouter/types.ts | 14 ++++ .../src/domain/reactRouter/useRoutes.ts | 29 +++++++ .../src/domain/reactRouterV6/createRouter.ts | 28 ------- .../src/domain/reactRouterV6/index.ts | 4 - .../domain/reactRouterV6/routesComponent.ts | 8 -- .../reactRouterV6/startReactRouterView.ts | 75 ------------------- .../src/domain/reactRouterV6/useRoutes.ts | 19 ----- .../src/domain/reactRouterV7/createRouter.ts | 28 ------- .../src/domain/reactRouterV7/index.ts | 4 - .../domain/reactRouterV7/routesComponent.ts | 8 -- .../src/domain/reactRouterV7/useRoutes.ts | 19 ----- .../rum-react/src/entries/reactRouterV6.ts | 27 ++++++- .../rum-react/src/entries/reactRouterV7.ts | 27 ++++++- sandbox/react-app/main.tsx | 2 +- 18 files changed, 135 insertions(+), 209 deletions(-) create mode 100644 packages/rum-react/src/domain/reactRouter/createRouter.ts create mode 100644 packages/rum-react/src/domain/reactRouter/index.ts create mode 100644 packages/rum-react/src/domain/reactRouter/routesComponent.ts rename packages/rum-react/src/domain/{reactRouterV7 => reactRouter}/startReactRouterView.ts (69%) create mode 100644 packages/rum-react/src/domain/reactRouter/types.ts create mode 100644 packages/rum-react/src/domain/reactRouter/useRoutes.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/createRouter.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/index.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/routesComponent.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV6/useRoutes.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV7/createRouter.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV7/index.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV7/routesComponent.ts delete mode 100644 packages/rum-react/src/domain/reactRouterV7/useRoutes.ts diff --git a/packages/rum-react/src/domain/reactRouter/createRouter.ts b/packages/rum-react/src/domain/reactRouter/createRouter.ts new file mode 100644 index 0000000000..e0786c40e3 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouter/createRouter.ts @@ -0,0 +1,20 @@ +import { startReactRouterView } from './startReactRouterView' +import type { AnyCreateRouter } from './types' + +export function wrapCreateRouter>( + originalCreateRouter: CreateRouter +): CreateRouter { + return ((routes, options) => { + const router = originalCreateRouter(routes, options) + let location = router.state.location.pathname + router.subscribe((routerState) => { + const newPathname = routerState.location.pathname + if (location !== newPathname) { + startReactRouterView(routerState.matches) + location = newPathname + } + }) + startReactRouterView(router.state.matches) + return router + }) as CreateRouter +} diff --git a/packages/rum-react/src/domain/reactRouter/index.ts b/packages/rum-react/src/domain/reactRouter/index.ts new file mode 100644 index 0000000000..6984407e86 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouter/index.ts @@ -0,0 +1,4 @@ +export { wrapCreateRouter } from './createRouter' +export { wrapUseRoutes } from './useRoutes' +export { createRoutesComponent } from './routesComponent' +export { startReactRouterView, computeViewName } from './startReactRouterView' diff --git a/packages/rum-react/src/domain/reactRouter/routesComponent.ts b/packages/rum-react/src/domain/reactRouter/routesComponent.ts new file mode 100644 index 0000000000..6916e4920c --- /dev/null +++ b/packages/rum-react/src/domain/reactRouter/routesComponent.ts @@ -0,0 +1,11 @@ +import type { AnyLocation, AnyRouteObject, AnyUseRoute } from './types' +// Same as react-router-dom Routes but with our useRoutes instead of the original one +// https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 +export function createRoutesComponent( + useRoutes: AnyUseRoute, + createRoutesFromChildren: (children: React.ReactNode, parentPath?: number[]) => AnyRouteObject[] +) { + return function Routes({ children, location }: { children: React.ReactNode; location?: Location }) { + return useRoutes(createRoutesFromChildren(children), location) + } +} diff --git a/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts similarity index 69% rename from packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts rename to packages/rum-react/src/domain/reactRouter/startReactRouterView.ts index d13b2afa33..6b2b1157a1 100644 --- a/packages/rum-react/src/domain/reactRouterV7/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts @@ -1,8 +1,8 @@ -import type { RouteMatch } from 'react-router-7' import { display } from '@datadog/browser-core' import { onRumInit } from '../reactPlugin' +import type { AnyRouteMatch } from './types' -export function startReactRouterView(routeMatches: RouteMatch[]) { +export function startReactRouterView(routeMatches: AnyRouteMatch[]) { onRumInit((configuration, rumPublicApi) => { if (!configuration.router) { display.warn('`router: true` is missing from the react plugin configuration, the view will not be tracked.') @@ -12,7 +12,7 @@ export function startReactRouterView(routeMatches: RouteMatch[]) { }) } -export function computeViewName(routeMatches: RouteMatch[]) { +export function computeViewName(routeMatches: AnyRouteMatch[]) { if (!routeMatches || routeMatches.length === 0) { return '' } @@ -54,22 +54,13 @@ export function computeViewName(routeMatches: RouteMatch[]) { * substitutePathSplats('/files/*', { '*': 'path/to/file' }, true) // => '/files/path/to/file' */ function substitutePathSplats(path: string, params: Record, isLastMatchingRoute: boolean) { - if ( - !path.includes('*') || - // In some edge cases, react-router does not provide the `*` parameter, so we don't know what to - // replace it with. In this case, we keep the asterisk. - params['*'] === undefined - ) { + if (!path.includes('*') || params['*'] === undefined) { return path } - // The `*` parameter is only related to the last matching route path. if (isLastMatchingRoute) { return path.replace(/\*/, params['*']) } - // Intermediary route paths with a `*` are kind of edge cases, and the `*` parameter is not - // relevant for them. We remove it from the path (along with a potential slash preceeding it) to - // have a coherent view name once everything is concatenated (see examples in spec file). return path.replace(/\/?\*/, '') } diff --git a/packages/rum-react/src/domain/reactRouter/types.ts b/packages/rum-react/src/domain/reactRouter/types.ts new file mode 100644 index 0000000000..a05538a4e1 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouter/types.ts @@ -0,0 +1,14 @@ +export type AnyRouteObject = { path?: string | undefined } +export type AnyUseRoute = ( + routes: AnyRouteObject[], + location?: Location +) => React.ReactElement | null +export type AnyRouteMatch = { route: AnyRouteObject; params: Record } +export type AnyLocation = { pathname: string } | string +export type AnyCreateRouter = ( + routes: AnyRouteObject[], + options?: Options +) => { + state: { location: { pathname: string }; matches: AnyRouteMatch[] } + subscribe: (callback: (routerState: { location: { pathname: string }; matches: AnyRouteMatch[] }) => void) => void +} diff --git a/packages/rum-react/src/domain/reactRouter/useRoutes.ts b/packages/rum-react/src/domain/reactRouter/useRoutes.ts new file mode 100644 index 0000000000..baadc88b83 --- /dev/null +++ b/packages/rum-react/src/domain/reactRouter/useRoutes.ts @@ -0,0 +1,29 @@ +import { useRef } from 'react' +import { startReactRouterView } from './startReactRouterView' +import type { AnyUseRoute, AnyRouteObject, AnyRouteMatch } from './types' + +export function wrapUseRoutes>({ + useRoutes, + useLocation, + matchRoutes, +}: { + useRoutes: T + useLocation: () => { pathname: string } + matchRoutes: (routes: AnyRouteObject[], pathname: string) => AnyRouteMatch[] | null +}): T { + return ((routes, locationArg) => { + const location = useLocation() + const pathname = typeof locationArg === 'string' ? locationArg : locationArg?.pathname || location.pathname + const pathnameRef = useRef(null) + + if (pathnameRef.current !== pathname) { + pathnameRef.current = pathname + const matchedRoutes = matchRoutes(routes, pathname) + if (matchedRoutes) { + startReactRouterView(matchedRoutes) + } + } + + return useRoutes(routes, locationArg) + }) as T +} diff --git a/packages/rum-react/src/domain/reactRouterV6/createRouter.ts b/packages/rum-react/src/domain/reactRouterV6/createRouter.ts deleted file mode 100644 index 570e70d83c..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/createRouter.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { - createBrowserRouter as originalCreateBrowserRouter, - createHashRouter as originalCreateHashRouter, - createMemoryRouter as originalCreateMemoryRouter, -} from 'react-router-6' -import { startReactRouterView } from './startReactRouterView' - -type Router = ReturnType - -export const createBrowserRouter: typeof originalCreateBrowserRouter = (routes, options) => - registerRouter(originalCreateBrowserRouter(routes, options)) -export const createHashRouter: typeof originalCreateHashRouter = (routes, options) => - registerRouter(originalCreateHashRouter(routes, options)) -export const createMemoryRouter: typeof originalCreateMemoryRouter = (routes, options) => - registerRouter(originalCreateMemoryRouter(routes, options)) - -export function registerRouter(router: Router) { - let location = router.state.location.pathname - router.subscribe((routerState) => { - const newPathname = routerState.location.pathname - if (location !== newPathname) { - startReactRouterView(routerState.matches) - location = newPathname - } - }) - startReactRouterView(router.state.matches) - return router -} diff --git a/packages/rum-react/src/domain/reactRouterV6/index.ts b/packages/rum-react/src/domain/reactRouterV6/index.ts deleted file mode 100644 index ee0401bf1b..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -export { createMemoryRouter, createHashRouter, createBrowserRouter } from './createRouter' -export { useRoutes } from './useRoutes' -export { Routes } from './routesComponent' -export { startReactRouterView, computeViewName } from './startReactRouterView' diff --git a/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts b/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts deleted file mode 100644 index 55ec10167f..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/routesComponent.ts +++ /dev/null @@ -1,8 +0,0 @@ -import type { Routes as OriginalRoutes } from 'react-router-6' -import { createRoutesFromChildren } from 'react-router-6' -import { useRoutes } from './useRoutes' - -// Same as react-router-dom Routes but with our useRoutes instead of the original one -// https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 -export const Routes: typeof OriginalRoutes = ({ children, location }) => - useRoutes(createRoutesFromChildren(children), location) diff --git a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts deleted file mode 100644 index d026fac840..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/startReactRouterView.ts +++ /dev/null @@ -1,75 +0,0 @@ -import type { RouteMatch } from 'react-router-6' -import { display } from '@datadog/browser-core' -import { onRumInit } from '../reactPlugin' - -export function startReactRouterView(routeMatches: RouteMatch[]) { - onRumInit((configuration, rumPublicApi) => { - if (!configuration.router) { - display.warn('`router: true` is missing from the react plugin configuration, the view will not be tracked.') - return - } - rumPublicApi.startView(computeViewName(routeMatches)) - }) -} - -export function computeViewName(routeMatches: RouteMatch[]) { - if (!routeMatches || routeMatches.length === 0) { - return '' - } - - let viewName = '/' - - for (const routeMatch of routeMatches) { - let path = routeMatch.route.path - if (!path) { - continue - } - - path = substitutePathSplats(path, routeMatch.params, routeMatch === routeMatches[routeMatches.length - 1]) - - if (path.startsWith('/')) { - // Absolute path, replace the current view name - viewName = path - } else { - // Relative path, append to the current view name - if (!viewName.endsWith('/')) { - viewName += '/' - } - viewName += path - } - } - - return viewName -} - -/** - * React-Router allows to define routes with "splats" (or "catchall" or "star") segments[1], - * example: /files/*. It has been noticed that keeping those splats in the view name isn't helpful - * as it "hides" too much information. This function replaces the splats with the actual URL path - * name that they are matching. - * - * [1]: https://reactrouter.com/en/main/route/route#splats - * - * @example - * substitutePathSplats('/files/*', { '*': 'path/to/file' }, true) // => '/files/path/to/file' - */ -function substitutePathSplats(path: string, params: Record, isLastMatchingRoute: boolean) { - if ( - !path.includes('*') || - // In some edge cases, react-router does not provide the `*` parameter, so we don't know what to - // replace it with. In this case, we keep the asterisk. - params['*'] === undefined - ) { - return path - } - - // The `*` parameter is only related to the last matching route path. - if (isLastMatchingRoute) { - return path.replace(/\*/, params['*']) - } - - // Intermediary route paths with a `*` are kind of edge cases, and the `*` parameter is not - // relevant for them. We remove it from the path (along with a potential slash preceeding it) to - // have a coherent view name once everything is concatenated (see examples in spec file). - return path.replace(/\/?\*/, '') -} diff --git a/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts b/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts deleted file mode 100644 index c88515a9d3..0000000000 --- a/packages/rum-react/src/domain/reactRouterV6/useRoutes.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { useRef } from 'react' -import { matchRoutes, useLocation, useRoutes as originalUseRoutes } from 'react-router-6' -import { startReactRouterView } from './startReactRouterView' - -export const useRoutes: typeof originalUseRoutes = (routes, locationArg) => { - const location = useLocation() - const pathname = typeof locationArg === 'string' ? locationArg : locationArg?.pathname || location.pathname - const pathnameRef = useRef(null) - - if (pathnameRef.current !== pathname) { - pathnameRef.current = pathname - const matchedRoutes = matchRoutes(routes, pathname) - if (matchedRoutes) { - startReactRouterView(matchedRoutes) - } - } - - return originalUseRoutes(routes, locationArg) -} diff --git a/packages/rum-react/src/domain/reactRouterV7/createRouter.ts b/packages/rum-react/src/domain/reactRouterV7/createRouter.ts deleted file mode 100644 index b3c19c5346..0000000000 --- a/packages/rum-react/src/domain/reactRouterV7/createRouter.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { - createBrowserRouter as originalCreateBrowserRouter, - createHashRouter as originalCreateHashRouter, - createMemoryRouter as originalCreateMemoryRouter, -} from 'react-router-7' -import { startReactRouterView } from './startReactRouterView' - -type Router = ReturnType - -export const createBrowserRouter: typeof originalCreateBrowserRouter = (routes, options) => - registerRouter(originalCreateBrowserRouter(routes, options)) -export const createHashRouter: typeof originalCreateHashRouter = (routes, options) => - registerRouter(originalCreateHashRouter(routes, options)) -export const createMemoryRouter: typeof originalCreateMemoryRouter = (routes, options) => - registerRouter(originalCreateMemoryRouter(routes, options)) - -export function registerRouter(router: Router) { - let location = router.state.location.pathname - router.subscribe((routerState) => { - const newPathname = routerState.location.pathname - if (location !== newPathname) { - startReactRouterView(routerState.matches) - location = newPathname - } - }) - startReactRouterView(router.state.matches) - return router -} diff --git a/packages/rum-react/src/domain/reactRouterV7/index.ts b/packages/rum-react/src/domain/reactRouterV7/index.ts deleted file mode 100644 index ee0401bf1b..0000000000 --- a/packages/rum-react/src/domain/reactRouterV7/index.ts +++ /dev/null @@ -1,4 +0,0 @@ -export { createMemoryRouter, createHashRouter, createBrowserRouter } from './createRouter' -export { useRoutes } from './useRoutes' -export { Routes } from './routesComponent' -export { startReactRouterView, computeViewName } from './startReactRouterView' diff --git a/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts b/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts deleted file mode 100644 index a06aa9a051..0000000000 --- a/packages/rum-react/src/domain/reactRouterV7/routesComponent.ts +++ /dev/null @@ -1,8 +0,0 @@ -import type { Routes as OriginalRoutes } from 'react-router-7' -import { createRoutesFromChildren } from 'react-router-7' -import { useRoutes } from './useRoutes' - -// Same as react-router-dom Routes but with our useRoutes instead of the original one -// https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 -export const Routes: typeof OriginalRoutes = ({ children, location }) => - useRoutes(createRoutesFromChildren(children), location) diff --git a/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts b/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts deleted file mode 100644 index b5f70f9c57..0000000000 --- a/packages/rum-react/src/domain/reactRouterV7/useRoutes.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { useRef } from 'react' -import { matchRoutes, useLocation, useRoutes as originalUseRoutes } from 'react-router-7' -import { startReactRouterView } from './startReactRouterView' - -export const useRoutes: typeof originalUseRoutes = (routes, locationArg) => { - const location = useLocation() - const pathname = typeof locationArg === 'string' ? locationArg : locationArg?.pathname || location.pathname - const pathnameRef = useRef(null) - - if (pathnameRef.current !== pathname) { - pathnameRef.current = pathname - const matchedRoutes = matchRoutes(routes, pathname) - if (matchedRoutes) { - startReactRouterView(matchedRoutes) - } - } - - return originalUseRoutes(routes, locationArg) -} diff --git a/packages/rum-react/src/entries/reactRouterV6.ts b/packages/rum-react/src/entries/reactRouterV6.ts index 37e4e7ab39..2f28b39c1b 100644 --- a/packages/rum-react/src/entries/reactRouterV6.ts +++ b/packages/rum-react/src/entries/reactRouterV6.ts @@ -1 +1,26 @@ -export * from '../domain/reactRouterV6' +/* eslint-disable local-rules/disallow-side-effects */ + +import { + createBrowserRouter as originalCreateBrowserRouter, + createHashRouter as originalCreateHashRouter, + createMemoryRouter as originalCreateMemoryRouter, + useRoutes as originalUseRoutes, + useLocation, + matchRoutes, + createRoutesFromChildren, +} from 'react-router-dom-6' +import { wrapCreateRouter, createRoutesComponent, wrapUseRoutes } from '../domain/reactRouter' + +export const createBrowserRouter = wrapCreateRouter(originalCreateBrowserRouter) +export const createHashRouter = wrapCreateRouter(originalCreateHashRouter) +export const createMemoryRouter = wrapCreateRouter(originalCreateMemoryRouter) + +export const useRoutes = wrapUseRoutes({ + useRoutes: originalUseRoutes, + useLocation, + matchRoutes, +}) + +export const Routes = createRoutesComponent(useRoutes, createRoutesFromChildren) + +export * from '../domain/reactRouter' diff --git a/packages/rum-react/src/entries/reactRouterV7.ts b/packages/rum-react/src/entries/reactRouterV7.ts index a244a42a82..a087154157 100644 --- a/packages/rum-react/src/entries/reactRouterV7.ts +++ b/packages/rum-react/src/entries/reactRouterV7.ts @@ -1 +1,26 @@ -export * from '../domain/reactRouterV7' +/* eslint-disable local-rules/disallow-side-effects */ + +import { + createBrowserRouter as originalCreateBrowserRouter, + createHashRouter as originalCreateHashRouter, + createMemoryRouter as originalCreateMemoryRouter, + useRoutes as originalUseRoutes, + useLocation, + matchRoutes, + createRoutesFromChildren, +} from 'react-router-dom-7' +import { wrapCreateRouter, createRoutesComponent, wrapUseRoutes } from '../domain/reactRouter' + +export const createBrowserRouter = wrapCreateRouter(originalCreateBrowserRouter) +export const createHashRouter = wrapCreateRouter(originalCreateHashRouter) +export const createMemoryRouter = wrapCreateRouter(originalCreateMemoryRouter) + +export const useRoutes = wrapUseRoutes({ + useRoutes: originalUseRoutes, + useLocation, + matchRoutes, +}) + +export const Routes = createRoutesComponent(useRoutes, createRoutesFromChildren) + +export * from '../domain/reactRouter' diff --git a/sandbox/react-app/main.tsx b/sandbox/react-app/main.tsx index 8ce0a8b8b5..8c88f4d7f2 100644 --- a/sandbox/react-app/main.tsx +++ b/sandbox/react-app/main.tsx @@ -1,4 +1,4 @@ -import { Link, Outlet, RouterProvider, useParams } from 'react-router-7' +import { Link, Outlet, RouterProvider, useParams } from 'react-router-dom-7' import React, { useEffect, useState } from 'react' import ReactDOM from 'react-dom/client' import { datadogRum } from '@datadog/browser-rum' From 2a8f2c2766998db396fc74fc4a9b9cab17fbc17f Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Jan 2025 11:34:03 +0100 Subject: [PATCH 08/15] license / nitpick / fix test --- LICENSE-3rdparty.csv | 3 ++- packages/rum-react/package.json | 4 ++-- .../domain/reactRooterTests/createRouter.spec.ts | 4 ++-- .../reactRooterTests/routesComponent.spec.tsx | 8 ++++---- .../reactRooterTests/startReactRouterView.spec.ts | 10 +++++----- .../domain/reactRooterTests/useRoutes.spec.tsx | 15 ++++++--------- tsconfig.base.json | 5 +++-- yarn.lock | 8 ++++---- 8 files changed, 28 insertions(+), 29 deletions(-) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 20ecee5b51..00c2c33266 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -65,7 +65,8 @@ dev,npm-run-all,MIT,Copyright 2015 Toru Nagashima dev,pako,MIT,(C) 2014-2017 Vitaly Puzrin and Andrey Tupitsin dev,prettier,MIT,Copyright James Long and contributors dev,puppeteer,Apache-2.0,Copyright 2017 Google Inc. -dev,react-router-dom,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 +dev,react-router-dom-6,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 +dev,react-router-dom-7,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 dev,style-loader,MIT,Copyright JS Foundation and other contributors dev,terser-webpack-plugin,MIT,Copyright JS Foundation and other contributors dev,ts-loader,MIT,Copyright 2015 TypeStrong diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index 596cd6c0fa..38455a82fb 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -38,8 +38,8 @@ "@types/react-dom": "18.3.5", "react": "18.3.1", "react-dom": "18.3.1", - "react-router-6": "npm:react-router-dom@6.28.2", - "react-router-7": "npm:react-router-dom@7.1.3" + "react-router-dom-6": "npm:react-router-dom@6.28.2", + "react-router-dom-7": "npm:react-router-dom@7.1.3" }, "repository": { "type": "git", diff --git a/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts b/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts index 9e12aee314..bc5cbce8b8 100644 --- a/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts +++ b/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts @@ -1,6 +1,6 @@ import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { createMemoryRouter as createMemoryRouterV6 } from '../reactRouterV6' -import { createMemoryRouter as createMemoryRouterV7 } from '../reactRouterV7' +import { createMemoryRouter as createMemoryRouterV6 } from '../../entries/reactRouterV6' +import { createMemoryRouter as createMemoryRouterV7 } from '../../entries/reactRouterV7' describe('createRouter', () => { const versions = [ diff --git a/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx b/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx index a4b4950593..f7bfb2d557 100644 --- a/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx +++ b/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx @@ -1,11 +1,11 @@ import React from 'react' import { flushSync } from 'react-dom' -import { MemoryRouter as MemoryRouterV6, Route as RouteV6, useNavigate as useNavigateV6 } from 'react-router-6' -import { MemoryRouter as MemoryRouterV7, Route as RouteV7, useNavigate as useNavigateV7 } from 'react-router-7' +import { MemoryRouter as MemoryRouterV6, Route as RouteV6, useNavigate as useNavigateV6 } from 'react-router-dom-6' +import { MemoryRouter as MemoryRouterV7, Route as RouteV7, useNavigate as useNavigateV7 } from 'react-router-dom-7' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' import { appendComponent } from '../../../test/appendComponent' -import { Routes as RoutesV6 } from '../reactRouterV6' -import { Routes as RoutesV7 } from '../reactRouterV7' +import { Routes as RoutesV6 } from '../../entries/reactRouterV6' +import { Routes as RoutesV7 } from '../../entries/reactRouterV7' ;[ { version: 'react-router-6', diff --git a/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts b/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts index c4d999bc2c..a13fe659ef 100644 --- a/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts +++ b/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts @@ -3,16 +3,16 @@ import { createMemoryRouter as createMemoryRouterV6, type RouteObject as RouteObjectV6, type RouteMatch as RouteMatchV6, -} from 'react-router-6' +} from 'react-router-dom-6' import { createMemoryRouter as createMemoryRouterV7, type RouteObject as RouteObjectV7, type RouteMatch as RouteMatchV7, -} from 'react-router-7' +} from 'react-router-dom-7' import { registerCleanupTask } from '../../../../core/test' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import * as ViewV6 from '../reactRouterV6' -import * as ViewV7 from '../reactRouterV7' +import * as ViewV6 from '../../entries/reactRouterV6' +import * as ViewV7 from '../../entries/reactRouterV7' const routerVersions = [ { @@ -128,7 +128,7 @@ routerVersions.forEach(({ version, createMemoryRouter, startReactRouterView, com const route = routePaths .split(' > ') .reduceRight( - (childRoute, path) => ({ path, children: childRoute ? [childRoute] : undefined }), + (childRoute, routePath) => ({ path: routePath, children: childRoute ? [childRoute] : undefined }), undefined as RouteObjectV6 | RouteObjectV7 | undefined )! diff --git a/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx index 9a88a1b02a..be9737d5ec 100644 --- a/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx +++ b/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx @@ -1,13 +1,13 @@ import React from 'react' import { flushSync } from 'react-dom' -import { MemoryRouter as MemoryRouterV6, useNavigate as useNavigateV6 } from 'react-router-6' -import type { RouteObject as RouteObjectV6 } from 'react-router-6' -import { MemoryRouter as MemoryRouterV7, useNavigate as useNavigateV7 } from 'react-router-7' -import type { RouteObject as RouteObjectV7 } from 'react-router-7' +import { MemoryRouter as MemoryRouterV6, useNavigate as useNavigateV6 } from 'react-router-dom-6' +import type { RouteObject as RouteObjectV6 } from 'react-router-dom-6' +import { MemoryRouter as MemoryRouterV7, useNavigate as useNavigateV7 } from 'react-router-dom-7' +import type { RouteObject as RouteObjectV7 } from 'react-router-dom-7' import { appendComponent } from '../../../test/appendComponent' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import { useRoutes as useRoutesV6 } from '../reactRouterV6' -import { useRoutes as useRoutesV7 } from '../reactRouterV7' +import { useRoutes as useRoutesV6 } from '../../entries/reactRouterV6' +import { useRoutes as useRoutesV7 } from '../../entries/reactRouterV7' const versions = [ { @@ -113,7 +113,6 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { function NavBar() { navigate = useNavigate() - return null } @@ -150,7 +149,6 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { function NavBar() { navigate = useNavigate() - return null } @@ -182,7 +180,6 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { function NavBar() { navigate = useNavigate() - return null } diff --git a/tsconfig.base.json b/tsconfig.base.json index b7c1afe798..286a236709 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -4,7 +4,7 @@ "experimentalDecorators": true, "esModuleInterop": true, "importHelpers": false, - "moduleResolution": "node16", + "moduleResolution": "node", "module": "ES2020", "resolveJsonModule": true, "strict": true, @@ -19,7 +19,8 @@ "@datadog/browser-core": ["./packages/core/src"], "@datadog/browser-rum-core": ["./packages/rum-core/src"], "@datadog/browser-rum-react": ["./packages/rum-react/src/entries/main"], - "@datadog/browser-rum-react/react-router-v6": ["./packages/rum-react/src/entries/reactRouterV6"] + "@datadog/browser-rum-react/react-router-v6": ["./packages/rum-react/src/entries/reactRouterV6"], + "@datadog/browser-rum-react/react-router-v7": ["./packages/rum-react/src/entries/reactRouterV7"] } } } diff --git a/yarn.lock b/yarn.lock index 0aa6e3dc09..befa2f6430 100644 --- a/yarn.lock +++ b/yarn.lock @@ -355,8 +355,8 @@ __metadata: "@types/react-dom": "npm:18.3.5" react: "npm:18.3.1" react-dom: "npm:18.3.1" - react-router-6: "npm:react-router-dom@6.28.2" - react-router-7: "npm:react-router-dom@7.1.3" + react-router-dom-6: "npm:react-router-dom@6.28.2" + react-router-dom-7: "npm:react-router-dom@7.1.3" peerDependencies: react: 18 react-router-dom: 6 || 7 @@ -11302,7 +11302,7 @@ __metadata: languageName: node linkType: hard -"react-router-6@npm:react-router-dom@6.28.2": +"react-router-dom-6@npm:react-router-dom@6.28.2": version: 6.28.2 resolution: "react-router-dom@npm:6.28.2" dependencies: @@ -11315,7 +11315,7 @@ __metadata: languageName: node linkType: hard -"react-router-7@npm:react-router-dom@7.1.3": +"react-router-dom-7@npm:react-router-dom@7.1.3": version: 7.1.3 resolution: "react-router-dom@npm:7.1.3" dependencies: From f4e3802684d90a4ca2e4174e37bee29ce8c41a77 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Tue, 28 Jan 2025 12:24:13 +0100 Subject: [PATCH 09/15] typecheck --- tsconfig.base.json | 1 + 1 file changed, 1 insertion(+) diff --git a/tsconfig.base.json b/tsconfig.base.json index 286a236709..56005e1f71 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -11,6 +11,7 @@ "target": "ES2018", "sourceMap": true, "jsx": "react", + "skipLibCheck": true, "types": [], "lib": ["ES2020", "DOM"], From 430bdc10e0dc75936a2100a205bb8bdcc62ede48 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 14 Feb 2025 11:17:10 +0100 Subject: [PATCH 10/15] cleaning code / updating license / add comment --- LICENSE-3rdparty.csv | 3 +- eslint-local-rules/disallowSideEffects.js | 2 - .../createRouter.spec.ts | 0 .../routesComponent.spec.tsx | 14 ++- .../startReactRouterView.spec.ts | 3 +- .../reactRouter/startReactRouterView.ts | 4 + .../rum-react/src/domain/reactRouter/types.ts | 13 ++- .../useRoutes.spec.tsx | 87 +++++++------------ scripts/check-licenses.js | 14 ++- 9 files changed, 70 insertions(+), 70 deletions(-) rename packages/rum-react/src/domain/{reactRooterTests => reactRouter}/createRouter.spec.ts (100%) rename packages/rum-react/src/domain/{reactRooterTests => reactRouter}/routesComponent.spec.tsx (94%) rename packages/rum-react/src/domain/{reactRooterTests => reactRouter}/startReactRouterView.spec.ts (98%) rename packages/rum-react/src/domain/{reactRooterTests => reactRouter}/useRoutes.spec.tsx (75%) diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv index 00c2c33266..20ecee5b51 100644 --- a/LICENSE-3rdparty.csv +++ b/LICENSE-3rdparty.csv @@ -65,8 +65,7 @@ dev,npm-run-all,MIT,Copyright 2015 Toru Nagashima dev,pako,MIT,(C) 2014-2017 Vitaly Puzrin and Andrey Tupitsin dev,prettier,MIT,Copyright James Long and contributors dev,puppeteer,Apache-2.0,Copyright 2017 Google Inc. -dev,react-router-dom-6,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 -dev,react-router-dom-7,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 +dev,react-router-dom,MIT,Copyright (c) React Training LLC 2015-2019 Copyright (c) Remix Software Inc. 2020-2021 Copyright (c) Shopify Inc. 2022-2023 dev,style-loader,MIT,Copyright JS Foundation and other contributors dev,terser-webpack-plugin,MIT,Copyright JS Foundation and other contributors dev,ts-loader,MIT,Copyright 2015 TypeStrong diff --git a/eslint-local-rules/disallowSideEffects.js b/eslint-local-rules/disallowSideEffects.js index 322882493d..21859edb6e 100644 --- a/eslint-local-rules/disallowSideEffects.js +++ b/eslint-local-rules/disallowSideEffects.js @@ -37,8 +37,6 @@ const packagesWithoutSideEffect = new Set([ '@datadog/browser-rum-core', 'react', 'react-router-dom', - 'react-router-6', - 'react-router-7', ]) /** diff --git a/packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts b/packages/rum-react/src/domain/reactRouter/createRouter.spec.ts similarity index 100% rename from packages/rum-react/src/domain/reactRooterTests/createRouter.spec.ts rename to packages/rum-react/src/domain/reactRouter/createRouter.spec.ts diff --git a/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx b/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx similarity index 94% rename from packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx rename to packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx index f7bfb2d557..0d5988213f 100644 --- a/packages/rum-react/src/domain/reactRooterTests/routesComponent.spec.tsx +++ b/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx @@ -1,5 +1,4 @@ -import React from 'react' -import { flushSync } from 'react-dom' +import React,{act} from 'react' import { MemoryRouter as MemoryRouterV6, Route as RouteV6, useNavigate as useNavigateV6 } from 'react-router-dom-6' import { MemoryRouter as MemoryRouterV7, Route as RouteV7, useNavigate as useNavigateV7 } from 'react-router-dom-7' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' @@ -80,14 +79,14 @@ import { Routes as RoutesV7 } from '../../entries/reactRouterV7' expect(startViewSpy).toHaveBeenCalledTimes(1) - flushSync(() => { + act(() => { forceUpdate!() }) expect(startViewSpy).toHaveBeenCalledTimes(1) }) - it('starts a new view on navigation', async () => { + it('starts a new view on navigation', () => { let navigate: (path: string) => void function NavBar() { @@ -106,10 +105,9 @@ import { Routes as RoutesV7 } from '../../entries/reactRouterV7' ) startViewSpy.calls.reset() - flushSync(() => { + act(() => { navigate!('/bar') }) - await new Promise((resolve) => setTimeout(resolve, 0)) expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') }) @@ -131,7 +129,7 @@ import { Routes as RoutesV7 } from '../../entries/reactRouterV7' ) startViewSpy.calls.reset() - flushSync(() => { + act(() => { navigate!('/foo') }) @@ -156,7 +154,7 @@ import { Routes as RoutesV7 } from '../../entries/reactRouterV7' ) startViewSpy.calls.reset() - flushSync(() => { + act(() => { navigate!('/foo?bar=baz') }) diff --git a/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts similarity index 98% rename from packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts rename to packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts index a13fe659ef..f28272f967 100644 --- a/packages/rum-react/src/domain/reactRooterTests/startReactRouterView.spec.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts @@ -13,6 +13,7 @@ import { registerCleanupTask } from '../../../../core/test' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' import * as ViewV6 from '../../entries/reactRouterV6' import * as ViewV7 from '../../entries/reactRouterV7' +import type { AnyRouteMatch } from './types' const routerVersions = [ { @@ -47,7 +48,7 @@ routerVersions.forEach(({ version, createMemoryRouter, startReactRouterView, com { route: { path: '/' } }, { route: { path: 'user' } }, { route: { path: ':id' } }, - ] as unknown as RouteMatchV6[] & RouteMatchV7[]) + ] as unknown as AnyRouteMatch[]) expect(startViewSpy).toHaveBeenCalledOnceWith('/user/:id') }) diff --git a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts index 6b2b1157a1..62d008c124 100644 --- a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts @@ -62,5 +62,9 @@ function substitutePathSplats(path: string, params: Record = ( routes: AnyRouteObject[], location?: Location diff --git a/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx similarity index 75% rename from packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx rename to packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx index be9737d5ec..61bd95ace8 100644 --- a/packages/rum-react/src/domain/reactRooterTests/useRoutes.spec.tsx +++ b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx @@ -1,13 +1,11 @@ -import React from 'react' -import { flushSync } from 'react-dom' +import React, { act } from 'react' import { MemoryRouter as MemoryRouterV6, useNavigate as useNavigateV6 } from 'react-router-dom-6' -import type { RouteObject as RouteObjectV6 } from 'react-router-dom-6' import { MemoryRouter as MemoryRouterV7, useNavigate as useNavigateV7 } from 'react-router-dom-7' -import type { RouteObject as RouteObjectV7 } from 'react-router-dom-7' import { appendComponent } from '../../../test/appendComponent' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' import { useRoutes as useRoutesV6 } from '../../entries/reactRouterV6' import { useRoutes as useRoutesV7 } from '../../entries/reactRouterV7' +import type { AnyRouteObject } from './types' const versions = [ { @@ -25,6 +23,16 @@ const versions = [ ] versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { + function RoutesRenderer({ + routes, + location, + }: { + routes: AnyRouteObject[] + location?: { pathname: string } | string + }) { + return useRoutes(routes, location) + } + describe(`useRoutes (${version})`, () => { let startViewSpy: jasmine.Spy<(name?: string | object) => void> @@ -50,7 +58,6 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { element: null, }, ]} - useRoutes={useRoutes} /> ) @@ -68,7 +75,6 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { element: 'foo', }, ]} - useRoutes={useRoutes} /> ) @@ -81,7 +87,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { function App() { const [, setState] = React.useState(0) - forceUpdate = () => setState((s) => s + 1) + forceUpdate = () => setState(s => s + 1) return ( { element: null, }, ]} - useRoutes={useRoutes} /> ) @@ -101,14 +106,14 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { expect(startViewSpy).toHaveBeenCalledTimes(1) - flushSync(() => { + act(() => { forceUpdate!() }) expect(startViewSpy).toHaveBeenCalledTimes(1) }) - it('starts a new view on navigation', async () => { + it('starts a new view on navigation', () => { let navigate: (path: string) => void function NavBar() { @@ -121,26 +126,19 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { ) startViewSpy.calls.reset() - flushSync(() => { + + act(() => { navigate!('/bar') }) - await new Promise((resolve) => setTimeout(resolve, 0)) expect(startViewSpy).toHaveBeenCalledOnceWith('/bar') }) @@ -157,18 +155,15 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { ) startViewSpy.calls.reset() - flushSync(() => { + + act(() => { navigate!('/foo') }) @@ -188,18 +183,15 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { ) startViewSpy.calls.reset() - flushSync(() => { + + act(() => { navigate!('/foo?bar=baz') }) @@ -207,12 +199,14 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { }) it('does not start a new view if it does not match any route', () => { - // Prevent react router from showing a warning in the console when a route does not match + // On empêche react-router d'afficher un avertissement dans la console si aucune route ne correspond spyOn(console, 'warn') appendComponent( - + ) @@ -224,13 +218,9 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { ) @@ -243,13 +233,12 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { ) @@ -258,15 +247,3 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { }) }) }) - -function RoutesRenderer({ - routes, - location, - useRoutes, -}: { - routes: RouteObjectV6[] & RouteObjectV7[] - location?: { pathname: string } | string - useRoutes: typeof useRoutesV6 | typeof useRoutesV7 -}) { - return useRoutes(routes, location) -} diff --git a/scripts/check-licenses.js b/scripts/check-licenses.js index c9e47d2729..40db83ab8f 100644 --- a/scripts/check-licenses.js +++ b/scripts/check-licenses.js @@ -8,6 +8,11 @@ const { findBrowserSdkPackageJsonFiles } = require('./lib/filesUtils') const LICENSE_FILE = 'LICENSE-3rdparty.csv' +const aliasMapping = { + 'react-router-dom-6': 'react-router-dom', + 'react-router-dom-7': 'react-router-dom', +} + runMain(async () => { const packageJsonFiles = findBrowserSdkPackageJsonFiles() @@ -36,9 +41,16 @@ runMain(async () => { }) function retrievePackageDependencies(packageJsonFile) { - return Object.keys(packageJsonFile.content.dependencies || {}) + const deps = Object.keys(packageJsonFile.content.dependencies || {}) .concat(Object.keys(packageJsonFile.content.devDependencies || {})) .filter((dependency) => !dependency.includes('@datadog')) + return deps.map((dependency) => { + let packageName = dependency + if (aliasMapping[packageName]) { + packageName = aliasMapping[packageName] + } + return packageName + }) } function withoutDuplicates(a) { From c0bc2ce3929373da5b47c3363e509c1ebeb97b39 Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Fri, 14 Feb 2025 11:20:19 +0100 Subject: [PATCH 11/15] prettier --- .../reactRouter/routesComponent.spec.tsx | 2 +- .../src/domain/reactRouter/useRoutes.spec.tsx | 29 +++++-------------- 2 files changed, 8 insertions(+), 23 deletions(-) diff --git a/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx b/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx index 0d5988213f..5b39d26fa4 100644 --- a/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx +++ b/packages/rum-react/src/domain/reactRouter/routesComponent.spec.tsx @@ -1,4 +1,4 @@ -import React,{act} from 'react' +import React, { act } from 'react' import { MemoryRouter as MemoryRouterV6, Route as RouteV6, useNavigate as useNavigateV6 } from 'react-router-dom-6' import { MemoryRouter as MemoryRouterV7, Route as RouteV7, useNavigate as useNavigateV7 } from 'react-router-dom-7' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' diff --git a/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx index 61bd95ace8..2a7b5cf4ae 100644 --- a/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx +++ b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx @@ -87,7 +87,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { function App() { const [, setState] = React.useState(0) - forceUpdate = () => setState(s => s + 1) + forceUpdate = () => setState((s) => s + 1) return ( { appendComponent( - + ) @@ -181,11 +177,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { appendComponent( - + ) @@ -204,9 +196,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { appendComponent( - + ) @@ -216,12 +206,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { it('allows passing a location object', () => { appendComponent( - + ) @@ -233,9 +218,9 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { Date: Fri, 14 Feb 2025 12:06:03 +0100 Subject: [PATCH 12/15] pipeline --- packages/rum-react/src/domain/reactRouter/routesComponent.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/rum-react/src/domain/reactRouter/routesComponent.ts b/packages/rum-react/src/domain/reactRouter/routesComponent.ts index 6916e4920c..9b78d8caf0 100644 --- a/packages/rum-react/src/domain/reactRouter/routesComponent.ts +++ b/packages/rum-react/src/domain/reactRouter/routesComponent.ts @@ -1,6 +1,8 @@ import type { AnyLocation, AnyRouteObject, AnyUseRoute } from './types' + // Same as react-router-dom Routes but with our useRoutes instead of the original one // https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 + export function createRoutesComponent( useRoutes: AnyUseRoute, createRoutesFromChildren: (children: React.ReactNode, parentPath?: number[]) => AnyRouteObject[] From c361bea41795b8d5392b23a6d2ddc17af998171f Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Mon, 17 Feb 2025 13:26:37 +0100 Subject: [PATCH 13/15] fix comment / check license --- .../src/domain/reactRouter/useRoutes.spec.tsx | 2 +- scripts/check-licenses.js | 24 ++++++++----------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx index 2a7b5cf4ae..dd2bb4a7fe 100644 --- a/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx +++ b/packages/rum-react/src/domain/reactRouter/useRoutes.spec.tsx @@ -191,7 +191,7 @@ versions.forEach(({ version, MemoryRouter, useNavigate, useRoutes }) => { }) it('does not start a new view if it does not match any route', () => { - // On empêche react-router d'afficher un avertissement dans la console si aucune route ne correspond + // Prevent react router from showing a warning in the console when a route does not match spyOn(console, 'warn') appendComponent( diff --git a/scripts/check-licenses.js b/scripts/check-licenses.js index 40db83ab8f..5678842074 100644 --- a/scripts/check-licenses.js +++ b/scripts/check-licenses.js @@ -8,11 +8,6 @@ const { findBrowserSdkPackageJsonFiles } = require('./lib/filesUtils') const LICENSE_FILE = 'LICENSE-3rdparty.csv' -const aliasMapping = { - 'react-router-dom-6': 'react-router-dom', - 'react-router-dom-7': 'react-router-dom', -} - runMain(async () => { const packageJsonFiles = findBrowserSdkPackageJsonFiles() @@ -41,16 +36,17 @@ runMain(async () => { }) function retrievePackageDependencies(packageJsonFile) { - const deps = Object.keys(packageJsonFile.content.dependencies || {}) - .concat(Object.keys(packageJsonFile.content.devDependencies || {})) + return Object.entries(packageJsonFile.content.dependencies || {}) + .concat(Object.entries(packageJsonFile.content.devDependencies || {})) + .map(([dependency, version]) => { + if (version.startsWith('npm:')) { + // Extract the original dependency name from the npm protocol version string. Example: + // npm:react@17 -> react + return version.slice(4).split('@')[0] + } + return dependency + }) .filter((dependency) => !dependency.includes('@datadog')) - return deps.map((dependency) => { - let packageName = dependency - if (aliasMapping[packageName]) { - packageName = aliasMapping[packageName] - } - return packageName - }) } function withoutDuplicates(a) { From 0c7bb7de18e6414c9cdc5bd9433eb617743a6f2c Mon Sep 17 00:00:00 2001 From: "roman.gaignault" Date: Mon, 17 Feb 2025 15:43:32 +0100 Subject: [PATCH 14/15] Update entries files, no need to export twice. Restore comments --- .../rum-react/src/domain/reactRouter/routesComponent.ts | 2 -- .../src/domain/reactRouter/startReactRouterView.spec.ts | 9 ++------- .../src/domain/reactRouter/startReactRouterView.ts | 3 +++ packages/rum-react/src/entries/reactRouterV6.ts | 2 -- packages/rum-react/src/entries/reactRouterV7.ts | 2 -- 5 files changed, 5 insertions(+), 13 deletions(-) diff --git a/packages/rum-react/src/domain/reactRouter/routesComponent.ts b/packages/rum-react/src/domain/reactRouter/routesComponent.ts index 9b78d8caf0..6916e4920c 100644 --- a/packages/rum-react/src/domain/reactRouter/routesComponent.ts +++ b/packages/rum-react/src/domain/reactRouter/routesComponent.ts @@ -1,8 +1,6 @@ import type { AnyLocation, AnyRouteObject, AnyUseRoute } from './types' - // Same as react-router-dom Routes but with our useRoutes instead of the original one // https://github.com/remix-run/react-router/blob/5d66dbdbc8edf1d9c3a4d9c9d84073d046b5785b/packages/react-router/lib/components.tsx#L503-L508 - export function createRoutesComponent( useRoutes: AnyUseRoute, createRoutesFromChildren: (children: React.ReactNode, parentPath?: number[]) => AnyRouteObject[] diff --git a/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts index f28272f967..fd5494ad25 100644 --- a/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.spec.ts @@ -11,26 +11,21 @@ import { } from 'react-router-dom-7' import { registerCleanupTask } from '../../../../core/test' import { initializeReactPlugin } from '../../../test/initializeReactPlugin' -import * as ViewV6 from '../../entries/reactRouterV6' -import * as ViewV7 from '../../entries/reactRouterV7' +import { startReactRouterView, computeViewName } from './startReactRouterView' import type { AnyRouteMatch } from './types' const routerVersions = [ { version: 'react-router-6', createMemoryRouter: createMemoryRouterV6, - startReactRouterView: ViewV6.startReactRouterView, - computeViewName: ViewV6.computeViewName, }, { version: 'react-router-7', createMemoryRouter: createMemoryRouterV7, - startReactRouterView: ViewV7.startReactRouterView, - computeViewName: ViewV7.computeViewName, }, ] -routerVersions.forEach(({ version, createMemoryRouter, startReactRouterView, computeViewName }) => { +routerVersions.forEach(({ version, createMemoryRouter }) => { describe(`startReactRouterView (${version})`, () => { describe('startReactRouterView', () => { it('creates a new view with the computed view name', () => { diff --git a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts index 62d008c124..59035378d4 100644 --- a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts @@ -55,9 +55,12 @@ export function computeViewName(routeMatches: AnyRouteMatch[]) { */ function substitutePathSplats(path: string, params: Record, isLastMatchingRoute: boolean) { if (!path.includes('*') || params['*'] === undefined) { + // In some edge cases, react-router does not provide the `*` parameter, so we don't know what to + // replace it with. In this case, we keep the asterisk. return path } + // The `*` parameter is only related to the last matching route path. if (isLastMatchingRoute) { return path.replace(/\*/, params['*']) } diff --git a/packages/rum-react/src/entries/reactRouterV6.ts b/packages/rum-react/src/entries/reactRouterV6.ts index 2f28b39c1b..0c91ad1856 100644 --- a/packages/rum-react/src/entries/reactRouterV6.ts +++ b/packages/rum-react/src/entries/reactRouterV6.ts @@ -22,5 +22,3 @@ export const useRoutes = wrapUseRoutes({ }) export const Routes = createRoutesComponent(useRoutes, createRoutesFromChildren) - -export * from '../domain/reactRouter' diff --git a/packages/rum-react/src/entries/reactRouterV7.ts b/packages/rum-react/src/entries/reactRouterV7.ts index a087154157..6461dab32c 100644 --- a/packages/rum-react/src/entries/reactRouterV7.ts +++ b/packages/rum-react/src/entries/reactRouterV7.ts @@ -22,5 +22,3 @@ export const useRoutes = wrapUseRoutes({ }) export const Routes = createRoutesComponent(useRoutes, createRoutesFromChildren) - -export * from '../domain/reactRouter' From 26b90427d82105754cebcfae0a3b6df7760ed120 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 18 Feb 2025 10:03:39 +0100 Subject: [PATCH 15/15] revert unexpected changes to startReactRouterView.ts --- .../src/domain/reactRouter/startReactRouterView.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts index 59035378d4..4aabc5a2d0 100644 --- a/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts +++ b/packages/rum-react/src/domain/reactRouter/startReactRouterView.ts @@ -54,9 +54,12 @@ export function computeViewName(routeMatches: AnyRouteMatch[]) { * substitutePathSplats('/files/*', { '*': 'path/to/file' }, true) // => '/files/path/to/file' */ function substitutePathSplats(path: string, params: Record, isLastMatchingRoute: boolean) { - if (!path.includes('*') || params['*'] === undefined) { + if ( + !path.includes('*') || // In some edge cases, react-router does not provide the `*` parameter, so we don't know what to // replace it with. In this case, we keep the asterisk. + params['*'] === undefined + ) { return path } @@ -68,6 +71,5 @@ function substitutePathSplats(path: string, params: Record