diff --git a/dev-utils/test-config.js b/dev-utils/test-config.js index 1c13d6d12..77fbfff78 100644 --- a/dev-utils/test-config.js +++ b/dev-utils/test-config.js @@ -156,6 +156,12 @@ function getBrowserList(pkg = 'default') { let browsers = [] if (pkg === 'default') { browsers = getDefaultBrowsers() + } else if (pkg === 'react') { + // react router 6 doesn't support IE 11, we get rid of it + // https://github.com/remix-run/react-router/issues/8220#issuecomment-961326123 + return getDefaultBrowsers().filter( + browser => browser.browserName != 'internet explorer' + ) } else if (pkg === 'vue') { // Vue 3 dropped support for IE 11 and older browsers, // so we use modern browsers ro run the tests diff --git a/package-lock.json b/package-lock.json index ea78d7964..5a9400f40 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10689,6 +10689,12 @@ "integrity": "sha512-15spi3V28QdevleWBNXE4pIls3nFZmBbUGrW9IVPwiQczuSb9n76TCB4bsk8TSel+I1OkHEdPhu5QKMfY6rQHA==", "dev": true }, + "@remix-run/router": { + "version": "1.7.2", + "resolved": "https://registry.npmjs.org/@remix-run/router/-/router-1.7.2.tgz", + "integrity": "sha512-7Lcn7IqGMV+vizMPoEl5F0XDshcdDYtMI6uJLQdQz5CfZAwy3vvGKYSUk789qndt5dEC4HfSjviSYlSoHGL2+A==", + "dev": true + }, "@rollup/plugin-commonjs": { "version": "19.0.0", "resolved": "https://registry.npmjs.org/@rollup/plugin-commonjs/-/plugin-commonjs-19.0.0.tgz", @@ -22160,29 +22166,6 @@ "tslib": "^2.0.3" } }, - "history": { - "version": "4.10.1", - "resolved": "https://registry.npmjs.org/history/-/history-4.10.1.tgz", - "integrity": "sha512-36nwAD620w12kuzPAsyINPWJqlNbij+hpK1k9XRloDtym8mxzGYl2c17LnV6IAGB2Dmg4tEa7G7DlawS0+qjew==", - "dev": true, - "requires": { - "@babel/runtime": "^7.1.2", - "loose-envify": "^1.2.0", - "resolve-pathname": "^3.0.0", - "tiny-invariant": "^1.0.2", - "tiny-warning": "^1.0.0", - "value-equal": "^1.0.1" - } - }, - "hoist-non-react-statics": { - "version": "3.3.2", - "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz", - "integrity": "sha512-/gGivxi8JPKWNm/W0jSmzcMPpfpPLc3dY/6GxhX2hQ9iGj3aDfklV4ET7NjKpSinLpJ5vafa9iiGIEZg10SfBw==", - "dev": true, - "requires": { - "react-is": "^16.7.0" - } - }, "hosted-git-info": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/hosted-git-info/-/hosted-git-info-4.0.2.tgz", @@ -27448,16 +27431,6 @@ "integrity": "sha512-I9jwMn07Sy/IwOj3zVkVik2JTvgpaykDZEigL6Rx6N9LbMywwUSMtxET+7lVoDLLd3O3IXwJwvuuns8UB/HeAg==", "dev": true }, - "mini-create-react-context": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/mini-create-react-context/-/mini-create-react-context-0.4.0.tgz", - "integrity": "sha512-b0TytUgFSbgFJGzJqXPKCFCBWigAjpjo+Fl7Vf7ZbKRDptszpppKxXH6DRXEABZ/gcEQczeb0iZ7JvL8e8jjCA==", - "dev": true, - "requires": { - "@babel/runtime": "^7.5.5", - "tiny-warning": "^1.0.3" - } - }, "mini-css-extract-plugin": { "version": "1.5.1", "resolved": "https://registry.npmjs.org/mini-css-extract-plugin/-/mini-css-extract-plugin-1.5.1.tgz", @@ -32536,53 +32509,22 @@ "integrity": "sha512-aUk3bHfZ2bRSVFFbbeVS4i+lNPZr3/WM5jT2J5omUVV1zzcs1nAaf3l51ctA5FFvCRbhrH0bdAsRRQddFJZPtA==" }, "react-router": { - "version": "5.2.0", - "resolved": "https://registry.npmjs.org/react-router/-/react-router-5.2.0.tgz", - "integrity": "sha512-smz1DUuFHRKdcJC0jobGo8cVbhO3x50tCL4icacOlcwDOEQPq4TMqwx3sY1TP+DvtTgz4nm3thuo7A+BK2U0Dw==", + "version": "6.14.2", + "resolved": "https://registry.npmjs.org/react-router/-/react-router-6.14.2.tgz", + "integrity": "sha512-09Zss2dE2z+T1D03IheqAFtK4UzQyX8nFPWx6jkwdYzGLXd5ie06A6ezS2fO6zJfEb/SpG6UocN2O1hfD+2urQ==", "dev": true, "requires": { - "@babel/runtime": "^7.1.2", - "history": "^4.9.0", - "hoist-non-react-statics": "^3.1.0", - "loose-envify": "^1.3.1", - "mini-create-react-context": "^0.4.0", - "path-to-regexp": "^1.7.0", - "prop-types": "^15.6.2", - "react-is": "^16.6.0", - "tiny-invariant": "^1.0.2", - "tiny-warning": "^1.0.0" - }, - "dependencies": { - "isarray": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/isarray/-/isarray-0.0.1.tgz", - "integrity": "sha1-ihis/Kmo9Bd+Cav8YDiTmwXR7t8=", - "dev": true - }, - "path-to-regexp": { - "version": "1.8.0", - "resolved": "https://registry.npmjs.org/path-to-regexp/-/path-to-regexp-1.8.0.tgz", - "integrity": "sha512-n43JRhlUKUAlibEJhPeir1ncUID16QnEjNpwzNdO3Lm4ywrBpBZ5oLD0I6br9evr1Y9JTqwRtAh7JLoOzAQdVA==", - "dev": true, - "requires": { - "isarray": "0.0.1" - } - } + "@remix-run/router": "1.7.2" } }, "react-router-dom": { - "version": "5.2.0", - "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-5.2.0.tgz", - "integrity": "sha512-gxAmfylo2QUjcwxI63RhQ5G85Qqt4voZpUXSEqCwykV0baaOTQDR1f0PmY8AELqIyVc0NEZUj0Gov5lNGcXgsA==", + "version": "6.14.2", + "resolved": "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.14.2.tgz", + "integrity": "sha512-5pWX0jdKR48XFZBuJqHosX3AAHjRAzygouMTyimnBPOLdY3WjzUSKhus2FVMihUFWzeLebDgr4r8UeQFAct7Bg==", "dev": true, "requires": { - "@babel/runtime": "^7.1.2", - "history": "^4.9.0", - "loose-envify": "^1.3.1", - "prop-types": "^15.6.2", - "react-router": "5.2.0", - "tiny-invariant": "^1.0.2", - "tiny-warning": "^1.0.0" + "@remix-run/router": "1.7.2", + "react-router": "6.14.2" } }, "react-shallow-renderer": { @@ -33148,12 +33090,6 @@ "global-dirs": "^0.1.1" } }, - "resolve-pathname": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/resolve-pathname/-/resolve-pathname-3.0.0.tgz", - "integrity": "sha512-C7rARubxI8bXFNB/hqcp/4iUeIXJhJZvFPFPiSPRnhU5UPxzMFIl+2E6yY6c4k9giDJAhtV+enfA+G89N6Csng==", - "dev": true - }, "resolve-url": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/resolve-url/-/resolve-url-0.2.1.tgz", @@ -36103,18 +36039,6 @@ "integrity": "sha1-QFQRqOfmM5/mTbmiNN4R3DHgK9Q=", "dev": true }, - "tiny-invariant": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/tiny-invariant/-/tiny-invariant-1.1.0.tgz", - "integrity": "sha512-ytxQvrb1cPc9WBEI/HSeYYoGD0kWnGEOR8RY6KomWLBVhqz0RgTwVO9dLrGz7dC+nN9llyI7OKAgRq8Vq4ZBSw==", - "dev": true - }, - "tiny-warning": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/tiny-warning/-/tiny-warning-1.0.3.tgz", - "integrity": "sha512-lBN9zLN/oAf68o3zNXYrdCt1kP8WsiGW8Oo2ka41b2IM5JL/S1CTyX1rW0mb/zSuJun0ZUrDxx4sqvYS2FWzPA==", - "dev": true - }, "tmp": { "version": "0.0.33", "resolved": "https://registry.npmjs.org/tmp/-/tmp-0.0.33.tgz", @@ -36743,12 +36667,6 @@ "builtins": "^1.0.3" } }, - "value-equal": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/value-equal/-/value-equal-1.0.1.tgz", - "integrity": "sha512-NOJ6JZCAWr0zlxZt+xqCHNTEKOsrks2HQd4MqhP1qy4z1SkbEP467eNx6TgDKXMvUOb+OENfJCZwM+16n7fRfw==", - "dev": true - }, "vary": { "version": "1.1.2", "resolved": "https://registry.npmjs.org/vary/-/vary-1.1.2.tgz", diff --git a/package.json b/package.json index 671454770..f9d98c7a2 100644 --- a/package.json +++ b/package.json @@ -166,8 +166,8 @@ "puppeteer": "^13.0.0", "react": "^17.0.2", "react-dom": "^17.0.2", - "react-router": "^5.2.0", - "react-router-dom": "^5.2.0", + "react-router": "^6.14.2", + "react-router-dom": "^6.14.2", "regenerator-runtime": "^0.13.3", "rimraf": "^3.0.0", "rxjs": "^6.6.6", diff --git a/packages/rum-react/karma.conf.js b/packages/rum-react/karma.conf.js index fa3df5572..ef0293626 100644 --- a/packages/rum-react/karma.conf.js +++ b/packages/rum-react/karma.conf.js @@ -24,6 +24,7 @@ */ const { baseConfig, prepareConfig } = require('../../dev-utils/karma.js') +const { getBrowserList } = require('../../dev-utils/test-config') const { getWebpackConfig, PACKAGE_TYPES, @@ -33,7 +34,11 @@ const { module.exports = function (config) { config.set(baseConfig) config.set({ - webpack: getWebpackConfig(BUNDLE_TYPES.BROWSER_DEV, PACKAGE_TYPES.REACT) + webpack: getWebpackConfig(BUNDLE_TYPES.BROWSER_DEV, PACKAGE_TYPES.REACT), + customLaunchers: getBrowserList('react').map(launcher => ({ + ...launcher, + base: 'SauceLabs' + })) }) const preparedConfig = prepareConfig(config, 'rum-react') config.set(preparedConfig) diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index f969c099c..bf18b8421 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -46,7 +46,7 @@ }, "peerDependencies": { "react": ">16.0.0", - "react-router-dom": ">4.0.0" + "react-router-dom": ">=6.0.0" }, "browserslist": [ "ie 11" diff --git a/packages/rum-react/src/finish-transaction-effect.js b/packages/rum-react/src/finish-transaction-effect.js new file mode 100644 index 000000000..219d0899d --- /dev/null +++ b/packages/rum-react/src/finish-transaction-effect.js @@ -0,0 +1,53 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import React from 'react' +import { afterFrame } from '@elastic/apm-rum-core' + +function useFinishTransactionEffect(transaction) { + /** + * React guarantees the parent component effects are run after the child components effects + * So once all the child components effects are run, we run the detectFinish logic + * which ensures if the transaction can be completed or not. + */ + React.useEffect(() => { + afterFrame(() => { + transaction && transaction.detectFinish() + }) + return () => { + /** + * Incase the transaction is never ended, we check if the transaction + * can be closed during unmount phase + * + * We call detectFinish instead of forcefully ending the transaction + * since it could be a redirect route and we might prematurely close + * the currently running transaction + */ + transaction && transaction.detectFinish() + } + }, []) +} + +export { useFinishTransactionEffect } diff --git a/packages/rum-react/src/get-apm-route.js b/packages/rum-react/src/get-apm-route.js deleted file mode 100644 index dd3db7cae..000000000 --- a/packages/rum-react/src/get-apm-route.js +++ /dev/null @@ -1,86 +0,0 @@ -/** - * MIT License - * - * Copyright (c) 2017-present, Elasticsearch BV - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - * - */ - -import React from 'react' -import { Route } from 'react-router-dom' -import { getWithTransaction } from './get-with-transaction' - -/** - * If the path/name is given as array, use the computed path - * to get the current transaction name - */ -function getTransactionName(name, props) { - const { match = {} } = props - - if (Array.isArray(name) && match.path) { - return match.path - } - return name -} - -function getApmRoute(apm) { - const withTransaction = getWithTransaction(apm) - - return class ApmRoute extends React.Component { - constructor(props) { - super(props) - this.state = {} - } - - static getDerivedStateFromProps(nextProps, prevState) { - const initial = prevState.apmComponent == null - const { path, component } = nextProps - const pathChanged = path != prevState.path - - /** - * Should update the apmComponent state and re-render the component only on - * initial mount and on route change. - * Ex: Query param changes should not result in new apmComponent - */ - if (initial || pathChanged) { - return { - path, - apmComponent: withTransaction( - path, - 'route-change', - (transaction, props) => { - if (transaction) { - const name = getTransactionName(path, props) - name && (transaction.name = name) - } - } - )(component) - } - } - return null - } - - render() { - return - } - } -} - -export { getApmRoute } diff --git a/packages/rum-react/src/get-apm-routes.js b/packages/rum-react/src/get-apm-routes.js new file mode 100644 index 000000000..0fea1ed2f --- /dev/null +++ b/packages/rum-react/src/get-apm-routes.js @@ -0,0 +1,141 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +import React from 'react' +import { useFinishTransactionEffect } from './finish-transaction-effect' +import { + Routes, + useLocation, + useNavigationType, + matchRoutes, + createRoutesFromChildren +} from 'react-router-dom' +import hoistNonReactStatics from 'hoist-non-react-statics' + +function getApmRoutes(apm) { + function ApmRoutes(props) { + const location = useLocation() + const navigationType = useNavigationType() + + // This effect makes sure the route-change transaction starts before the underlying components are painted + // It's essential for POP navigations (when the user clicks the back or forward buttons in the browser) + // When using useEffect, the agent was not associating ajax requests to + // the route-change transaction. Instead, it was creating a http-request transaction and discarding + // the route-change transaction for lack of spans. + React.useLayoutEffect( + () => { + const routes = createRoutesFromChildren(props.children) + handleRouteChange(apm, routes, location) + }, + // query param changes should not trigger this effect + // that's why we use `location.pathname` instead of `location` + [location.pathname, navigationType, props.children] + ) + + useFinishTransactionEffect(apm.getCurrentTransaction()) + + return + } + + hoistNonReactStatics(ApmRoutes, Routes) + + return ApmRoutes +} + +function handleRouteChange(apm, routes, location) { + if (!apm.isActive()) { + return + } + + // all the navigation types start a transaction, including REPLACE since we want to capture redirections + const name = getRouteFromLocation(routes, location) + apm.startTransaction(name, 'route-change', { + managed: true, + canReuse: true + }) +} + +export function getRouteFromLocation(routes, location) { + if (!routes || routes.length === 0) { + return location.pathname + } + + // There are cases where we can have more than one matchedRoute. + // For instance, when using Nested Routes with a configuration such as `/user`, `/user/profile` and `/user/account` + // if a user clicks on the link '/user/profile' + // matchRoutes will return two results, a `route.path` with `/user` and the another one with `profile` + const branches = matchRoutes(routes, location) + let builtPath = '' + if (branches) { + for (let index = 0; index < branches.length; index++) { + const branch = branches[index] + const route = branch.route + if (route) { + if (route.index) { + // this cover the cases where an index route has been defined. They don't have a `route.path` property + // E.g. } /> + return branch.pathname + } + + let path = route.path + if (path) { + // make sure we don't add an extra slash when concatenating + const hasSlash = + path[0] === '/' || builtPath[builtPath.length - 1] === '/' + path = hasSlash ? path : `/${path}` + builtPath += path + + // E.g `to` property from `` component matches to window.location.pathname + if (branch.pathname === location.pathname) { + // make sure wildcard routes are also handled + const isWildcardRoute = builtPath.slice(-2) === '/*' + + // One case where the number of / can differ is if we have an element like + // }/> + if ( + getNumberOfUrlSegments(builtPath) !== + getNumberOfUrlSegments(branch.pathname) && + !isWildcardRoute + ) { + return path + } + + // This contains each of the `route.path` found concatenated + return builtPath + } + } + } + } + } + + return location.pathname +} + +export function getNumberOfUrlSegments(url) { + return url.split('/').filter(currentSegment => currentSegment.length > 0) + .length +} + +export { getApmRoutes } diff --git a/packages/rum-react/src/get-with-transaction.js b/packages/rum-react/src/get-with-transaction.js index ee957a774..f87f9e6c5 100644 --- a/packages/rum-react/src/get-with-transaction.js +++ b/packages/rum-react/src/get-with-transaction.js @@ -26,6 +26,7 @@ import React from 'react' import hoistStatics from 'hoist-non-react-statics' import { afterFrame, LOGGING_SERVICE } from '@elastic/apm-rum-core' +import { useFinishTransactionEffect } from './finish-transaction-effect' /** * Check if the given component is class based component @@ -97,25 +98,7 @@ function getWithTransaction(apm) { return tr }) - /** - * React guarantees the parent component effects are run after the child components effects - * So once all the child components effects are run, we run the detectFinish logic - * which ensures if the transaction can be completed or not. - */ - React.useEffect(() => { - afterFrame(() => transaction && transaction.detectFinish()) - return () => { - /** - * Incase the transaction is never ended, we check if the transaction - * can be closed during unmount phase - * - * We call detectFinish instead of forcefully ending the transaction - * since it could be a redirect route and we might prematurely close - * the currently running transaction - */ - transaction && transaction.detectFinish() - } - }, []) + useFinishTransactionEffect(transaction) return } diff --git a/packages/rum-react/src/index.js b/packages/rum-react/src/index.js index 97cd4a0b7..530a0d3c6 100644 --- a/packages/rum-react/src/index.js +++ b/packages/rum-react/src/index.js @@ -25,9 +25,9 @@ import { apm } from '@elastic/apm-rum' import { getWithTransaction } from './get-with-transaction' -import { getApmRoute } from './get-apm-route' +import { getApmRoutes } from './get-apm-routes' const withTransaction = getWithTransaction(apm) -const ApmRoute = getApmRoute(apm) +const ApmRoutes = getApmRoutes(apm) -export { withTransaction, ApmRoute } +export { withTransaction, ApmRoutes } diff --git a/packages/rum-react/test/e2e/components/func-component.js b/packages/rum-react/test/e2e/components/func-component.js index 3a7100347..71ec2b150 100644 --- a/packages/rum-react/test/e2e/components/func-component.js +++ b/packages/rum-react/test/e2e/components/func-component.js @@ -45,7 +45,7 @@ export default function FunctionalComponent(props) { return (
- {props.match.path + '\n'} {count} + {props.path + '\n'} {count} Loading...
}> diff --git a/packages/rum-react/test/e2e/components/main-component.js b/packages/rum-react/test/e2e/components/main-component.js index b589bd105..193d726ca 100644 --- a/packages/rum-react/test/e2e/components/main-component.js +++ b/packages/rum-react/test/e2e/components/main-component.js @@ -28,7 +28,7 @@ import React from 'react' class MainComponent extends React.Component { constructor(props) { super(props) - var path = this.props.match.path + var path = this.props.path this.state = { userName: '', path diff --git a/packages/rum-react/test/e2e/components/topic-component.js b/packages/rum-react/test/e2e/components/topic-component.js index 9480137a4..f5a215a89 100644 --- a/packages/rum-react/test/e2e/components/topic-component.js +++ b/packages/rum-react/test/e2e/components/topic-component.js @@ -44,7 +44,7 @@ class TopicComponent extends React.Component { return (

- {this.props.match.path} + {this.props.path}

{this.state.userName}
diff --git a/packages/rum-react/test/e2e/with-router/general.js b/packages/rum-react/test/e2e/with-router/general.js index 5c8a837b5..ab6d4b044 100644 --- a/packages/rum-react/test/e2e/with-router/general.js +++ b/packages/rum-react/test/e2e/with-router/general.js @@ -27,11 +27,17 @@ import '@babel/polyfill' import 'whatwg-fetch' import React, { Suspense, lazy } from 'react' import { render } from 'react-dom' -import { BrowserRouter, Route, Link, Redirect } from 'react-router-dom' +import { + BrowserRouter, + Route, + Link, + Navigate, + useLocation +} from 'react-router-dom' import MainComponent from '../components/main-component' import TopicComponent from '../components/topic-component' import FunctionalComponent from '../components/func-component' -import { ApmRoute } from '../../../src' +import { ApmRoutes } from '../../../src' import createApmBase from '..' const apm = createApmBase({ @@ -55,6 +61,10 @@ const ManualComponent = lazy(() => { }) }) +const RedirectComponent = () => { + const location = useLocation() + return +} class App extends React.Component { render() { return ( @@ -84,30 +94,28 @@ class App extends React.Component {
- ( - - )} - /> - -
about
} /> - - - - ( - Loading...}> - - - )} - /> + + } /> + } /> + } + /> + } /> + } + /> + about} /> + Loading...}> + + + } + /> +
Passed
diff --git a/packages/rum-react/test/e2e/with-router/switch.e2e-spec.js b/packages/rum-react/test/e2e/with-router/routes.e2e-spec.js similarity index 95% rename from packages/rum-react/test/e2e/with-router/switch.e2e-spec.js rename to packages/rum-react/test/e2e/with-router/routes.e2e-spec.js index 788a40804..2f4f8a457 100644 --- a/packages/rum-react/test/e2e/with-router/switch.e2e-spec.js +++ b/packages/rum-react/test/e2e/with-router/routes.e2e-spec.js @@ -25,8 +25,8 @@ const { getLastServerCall } = require('../../../../../dev-utils/webdriver') -describe('Using Switch component of react router', function () { - beforeAll(() => browser.url('/test/e2e/with-router/switch.html')) +describe('Using Routes component of react router', function () { + beforeAll(() => browser.url('/test/e2e/with-router/routes.html')) it('should render the react app on route change', function () { let sendEvents = getLastServerCall(0, 1).sendEvents diff --git a/packages/rum-react/test/e2e/with-router/switch.html b/packages/rum-react/test/e2e/with-router/routes.html similarity index 81% rename from packages/rum-react/test/e2e/with-router/switch.html rename to packages/rum-react/test/e2e/with-router/routes.html index 6f917c1a8..b1a0ac154 100644 --- a/packages/rum-react/test/e2e/with-router/switch.html +++ b/packages/rum-react/test/e2e/with-router/routes.html @@ -10,5 +10,5 @@
- + diff --git a/packages/rum-react/test/e2e/with-router/switch.js b/packages/rum-react/test/e2e/with-router/routes.js similarity index 76% rename from packages/rum-react/test/e2e/with-router/switch.js rename to packages/rum-react/test/e2e/with-router/routes.js index fc340c56b..640dca087 100644 --- a/packages/rum-react/test/e2e/with-router/switch.js +++ b/packages/rum-react/test/e2e/with-router/routes.js @@ -27,17 +27,29 @@ import '@babel/polyfill' import 'whatwg-fetch' import React, { Suspense } from 'react' import { render } from 'react-dom' -import { BrowserRouter, Link, Switch } from 'react-router-dom' +import { + BrowserRouter, + Link, + Route, + Navigate, + useLocation +} from 'react-router-dom' import FunctionalComponent from '../components/func-component' -import { ApmRoute } from '../../../src' +import { ApmRoutes } from '../../../src' import createApmBase from '..' const apm = createApmBase({ logLevel: 'debug', - serviceName: 'apm-agent-rum-switch-e2e-react', + serviceName: 'apm-agent-rum-routes-e2e-react', serviceVersion: '0.0.1', pageLoadTransactionName: '/notfound' }) + +const RedirectNotFoundComponent = () => { + const location = useLocation() + return +} + const NotFound = () =>
Not Found
function App() { @@ -51,10 +63,13 @@ function App() { Loading...}> - - - - + + } /> + } /> + + {/* when no route matches */} + } /> + ) diff --git a/packages/rum-react/test/e2e/with-router/webpack.config.js b/packages/rum-react/test/e2e/with-router/webpack.config.js index bc348a50e..de9b9f615 100644 --- a/packages/rum-react/test/e2e/with-router/webpack.config.js +++ b/packages/rum-react/test/e2e/with-router/webpack.config.js @@ -33,7 +33,7 @@ const { module.exports = { entry: { general: path.join(__dirname, 'general.js'), - switch: path.join(__dirname, 'switch.js') + routes: path.join(__dirname, 'routes.js') }, output: { path: path.resolve(__dirname), diff --git a/packages/rum-react/test/specs/get-apm-route.spec.js b/packages/rum-react/test/specs/get-apm-route.spec.js deleted file mode 100644 index 8bf81c71e..000000000 --- a/packages/rum-react/test/specs/get-apm-route.spec.js +++ /dev/null @@ -1,192 +0,0 @@ -/** - * MIT License - * - * Copyright (c) 2017-present, Elasticsearch BV - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - * - */ - -/** - * shims are required for enzyme to work on older browsers. - */ - -import 'airbnb-js-shims/target/es2015' -import React from 'react' -import Enzyme, { mount } from 'enzyme' -import Adapter from '@wojtekmaj/enzyme-adapter-react-17' - -Enzyme.configure({ adapter: new Adapter() }) - -import { MemoryRouter as Router, Route } from 'react-router-dom' -import { ApmBase } from '@elastic/apm-rum' -import { - createServiceFactory, - TRANSACTION_SERVICE, - LOGGING_SERVICE -} from '@elastic/apm-rum-core' -import { getApmRoute } from '../../src/get-apm-route' -import { getGlobalConfig } from '../../../../dev-utils/test-config' - -function Component(props) { - return

Testing, {props.name}

-} - -describe('ApmRoute', function () { - const { serverUrl, serviceName } = getGlobalConfig().agentConfig - let serviceFactory, apmBase - - beforeEach(() => { - serviceFactory = createServiceFactory() - apmBase = new ApmBase(serviceFactory, false) - apmBase.init({ - active: true, - serverUrl, - serviceName, - disableInstrumentations: ['page-load', 'error'] - }) - }) - - it('should work Route component', function () { - const ApmRoute = getApmRoute(apmBase) - - const rendered = mount( - - - - ) - - var apmRoute = rendered.find(ApmRoute) - var route = rendered.find(Route) - var component = rendered.find(Component) - expect(apmRoute.name()).toBe('ApmRoute') - expect(route.name()).toBe('Route') - expect(route.props()).toEqual( - jasmine.objectContaining({ path: '/', component: jasmine.any(Function) }) - ) - expect(component.text()).toBe('Testing, ') - }) - - it('should work with Route render and log warning', function () { - const loggingService = serviceFactory.getService(LOGGING_SERVICE) - const ApmRoute = getApmRoute(apmBase) - - spyOn(loggingService, 'warn') - - const rendered = mount( - - } /> - - ) - - var apmRoute = rendered.find(ApmRoute) - var route = rendered.find(Route) - var component = rendered.find(Component) - expect(apmRoute.name()).toBe('ApmRoute') - expect(route.name()).toBe('Route') - expect(route.props()).toEqual( - jasmine.objectContaining({ path: '/', render: jasmine.any(Function) }) - ) - expect(component.text()).toBe('Testing, render-test') - expect(loggingService.warn).toHaveBeenCalledWith( - '/ is not instrumented since component property is not provided' - ) - }) - - it('should work correctly with path array in props', function () { - const ApmRoute = getApmRoute(apmBase) - - const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) - const dummyTr = { - name: 'test', - detectFinish: () => {} - } - spyOn(transactionService, 'startTransaction').and.returnValue(dummyTr) - - const Home = () => 'home' - class Topics extends React.Component { - render() { - return 'Topics' - } - } - - const rendered = mount( - - - - - ) - expect(dummyTr.name).toEqual('/') - expect(rendered.text()).toBe('home') - const history = rendered.find(Home).props().history - - history.push({ pathname: '/topic2' }) - expect(dummyTr.name).toEqual('/topic2') - expect(transactionService.startTransaction).toHaveBeenCalledTimes(2) - /** - * Should not create transaction as component is not rerendered - */ - history.push({ pathname: '/topic1' }) - expect(transactionService.startTransaction).toHaveBeenCalledTimes(2) - }) - - it('should not trigger full rerender on query change', function () { - const ApmRoute = getApmRoute(apmBase) - const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) - spyOn(transactionService, 'startTransaction') - - const rendered = mount( - - <> - -

New

} /> - - - ) - - expect(transactionService.startTransaction).toHaveBeenCalledWith( - '/home', - 'route-change', - { - canReuse: true, - managed: true - } - ) - const HomeComponent = rendered.find(Component) - - const history = HomeComponent.props().history - // Move to new route - history.push({ pathname: '/new' }) - expect(transactionService.startTransaction).toHaveBeenCalledWith( - '/new', - 'route-change', - { - canReuse: true, - managed: true - } - ) - - /** - * Update query on existing route - * do not trigger a new transaction - */ - history.push({ pathname: '/new', search: '?test=foo' }) - expect(transactionService.startTransaction).toHaveBeenCalledTimes(2) - }) -}) diff --git a/packages/rum-react/test/specs/get-apm-routes.spec.js b/packages/rum-react/test/specs/get-apm-routes.spec.js new file mode 100644 index 000000000..d1c72cd43 --- /dev/null +++ b/packages/rum-react/test/specs/get-apm-routes.spec.js @@ -0,0 +1,299 @@ +/** + * MIT License + * + * Copyright (c) 2017-present, Elasticsearch BV + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + * + */ + +/** + * shims are required for enzyme to work on older browsers. + */ + +import 'airbnb-js-shims/target/es2015' +import React from 'react' +import Enzyme, { mount } from 'enzyme' +import Adapter from '@wojtekmaj/enzyme-adapter-react-17' + +Enzyme.configure({ adapter: new Adapter() }) + +import { + MemoryRouter as Router, + Route, + useNavigate, + useLocation, + useSearchParams, + Navigate +} from 'react-router-dom' +import { ApmBase } from '@elastic/apm-rum' +import { + createServiceFactory, + TRANSACTION_SERVICE +} from '@elastic/apm-rum-core' +import { getApmRoutes } from '../../src/get-apm-routes' +import { getGlobalConfig } from '../../../../dev-utils/test-config' + +let navigate +function Component(props) { + // React-router util which allows navigating between routes + // Cleaner than handling the history api manually + navigate = useNavigate() + + return

Testing, {props.name}

+} + +describe('ApmRoutes', function () { + const { serverUrl, serviceName } = getGlobalConfig().agentConfig + let serviceFactory, apmBase + + function createApm(active) { + const apmBase = new ApmBase(serviceFactory, false) + return apmBase.init({ + active, + serverUrl, + serviceName, + disableInstrumentations: ['page-load', 'error'] + }) + } + + beforeEach(() => { + serviceFactory = createServiceFactory() + apmBase = createApm(true) + }) + + it('should display Component', function () { + const ApmRoutes = getApmRoutes(apmBase) + + const rendered = mount( + + + } /> + + + ) + + var apmRoutes = rendered.find(ApmRoutes) + var component = rendered.find(Component) + + expect(apmRoutes.name()).toBe('ApmRoutes') + expect(component.text()).toBe('Testing, Elastic') + }) + + it('should not trigger full rerender on query change', function () { + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + mount( + + + } /> + new} /> + + + ) + + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/home', + 'route-change', + { + canReuse: true, + managed: true + } + ) + + transactionService.startTransaction.calls.reset() + + // Move to new route + navigate('/new') + + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/new', + 'route-change', + { + canReuse: true, + managed: true + } + ) + + transactionService.startTransaction.calls.reset() + + /** + * Update query on existing route + * do not trigger a new transaction + */ + navigate('/new?test=foo') + expect(transactionService.startTransaction).not.toHaveBeenCalled() + }) + + it('should start transaction for nested route', function () { + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + mount( + + + }> + } /> + + + + ) + + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/company/careers', + 'route-change', + { + canReuse: true, + managed: true + } + ) + }) + + it('should handle path with wildcard', function () { + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + mount( + + + }> + } /> + + + + ) + + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/company/careers/*', // without the wildcard handling we would end up having /careers/* which is wrong + 'route-change', + { + canReuse: true, + managed: true + } + ) + }) + + it('should handle redirection', function () { + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + const RedirectComponent = () => { + const location = useLocation() + return + } + + mount( + + + } /> + } /> + + + ) + + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/home', + 'route-change', + { + canReuse: true, + managed: true + } + ) + }) + + it('should not startTransaction when rum is inactive', function () { + apmBase = createApm(false) + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + mount( + + + } /> + + + ) + + expect(transactionService.startTransaction).not.toHaveBeenCalled() + }) + + it('should not start transaction when child changes after a queryparam update', function () { + let navigate + function ComponentWithChild() { + navigate = useNavigate() + const [searchParams] = useSearchParams() + + if (searchParams.toString() === 'test=elastic') { + return
hello Elastic
+ } + + return ( +
+ +
+ ) + } + + function Child(props) { + return

Query params: {props.params}

+ } + + const ApmRoutes = getApmRoutes(apmBase) + const transactionService = serviceFactory.getService(TRANSACTION_SERVICE) + spyOn(transactionService, 'startTransaction') + + const rendered = mount( + + + }> + }> + + + ) + + // Navigate to home + navigate('/home') + expect(transactionService.startTransaction).toHaveBeenCalledWith( + '/home', + 'route-change', + { + canReuse: true, + managed: true + } + ) + + var component = rendered.find(ComponentWithChild) + expect(component.text()).toBe('Query params: ') + + transactionService.startTransaction.calls.reset() + + // Update query param + navigate('?test=elastic') + + expect(transactionService.startTransaction).not.toHaveBeenCalled() + // Confirm the component content is different than before + expect(component.text()).toBe('hello Elastic') + }) +}) diff --git a/packages/rum-react/wdio.conf.js b/packages/rum-react/wdio.conf.js index e40d691d3..01e1d4aff 100644 --- a/packages/rum-react/wdio.conf.js +++ b/packages/rum-react/wdio.conf.js @@ -24,5 +24,10 @@ */ const { getWebdriveBaseConfig } = require('../../dev-utils/webdriver') +const { getBrowserList } = require('../../dev-utils/test-config') -exports.config = getWebdriveBaseConfig(__dirname) +exports.config = getWebdriveBaseConfig( + __dirname, + './test/e2e/**/*.e2e-spec.js', + getBrowserList('react') +)