-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(react): Add support for cross-usage of React Router instrumentations #15283
Open
onurtemizkan
wants to merge
6
commits into
develop
Choose a base branch
from
onur/react-router-break-recursion
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
93ebab2
fix(react): Break if path is not changed in recursive rebuild.
onurtemizkan 9c031ec
Lint
onurtemizkan cec2d53
Deduplicate route cross-storing logic.
onurtemizkan 07e3ce6
Revisit cross-usage with tests.
onurtemizkan d63ce8b
Add E2E tests.
onurtemizkan 4081708
Lint
onurtemizkan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
29 changes: 29 additions & 0 deletions
29
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/.gitignore
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. | ||
|
||
# dependencies | ||
/node_modules | ||
/.pnp | ||
.pnp.js | ||
|
||
# testing | ||
/coverage | ||
|
||
# production | ||
/build | ||
|
||
# misc | ||
.DS_Store | ||
.env.local | ||
.env.development.local | ||
.env.test.local | ||
.env.production.local | ||
|
||
npm-debug.log* | ||
yarn-debug.log* | ||
yarn-error.log* | ||
|
||
/test-results/ | ||
/playwright-report/ | ||
/playwright/.cache/ | ||
|
||
!*.d.ts |
2 changes: 2 additions & 0 deletions
2
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/.npmrc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
@sentry:registry=http://127.0.0.1:4873 | ||
@sentry-internal:registry=http://127.0.0.1:4873 |
55 changes: 55 additions & 0 deletions
55
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/package.json
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
{ | ||
"name": "react-router-6-cross-usage", | ||
"version": "0.1.0", | ||
"private": true, | ||
"dependencies": { | ||
"@sentry/react": "latest || *", | ||
"@types/react": "18.0.0", | ||
"@types/react-dom": "18.0.0", | ||
"express": "4.20.0", | ||
"react": "18.2.0", | ||
"react-dom": "18.2.0", | ||
"react-router-dom": "^6.28.0", | ||
"react-scripts": "5.0.1", | ||
"typescript": "~5.0.0" | ||
}, | ||
"scripts": { | ||
"build": "react-scripts build", | ||
"start": "run-p start:client start:server", | ||
"start:client": "node server/app.js", | ||
"start:server": "serve -s build", | ||
"test": "playwright test", | ||
"clean": "npx rimraf node_modules pnpm-lock.yaml", | ||
"test:build": "pnpm install && npx playwright install && pnpm build", | ||
"test:build-ts3.8": "pnpm install && pnpm add [email protected] && npx playwright install && pnpm build", | ||
"test:build-canary": "pnpm install && pnpm add react@canary react-dom@canary && npx playwright install && pnpm build", | ||
"test:assert": "pnpm test" | ||
}, | ||
"eslintConfig": { | ||
"extends": [ | ||
"react-app", | ||
"react-app/jest" | ||
] | ||
}, | ||
"browserslist": { | ||
"production": [ | ||
">0.2%", | ||
"not dead", | ||
"not op_mini all" | ||
], | ||
"development": [ | ||
"last 1 chrome version", | ||
"last 1 firefox version", | ||
"last 1 safari version" | ||
] | ||
}, | ||
"devDependencies": { | ||
"@playwright/test": "~1.50.0", | ||
"@sentry-internal/test-utils": "link:../../../test-utils", | ||
"serve": "14.0.1", | ||
"npm-run-all2": "^6.2.0" | ||
}, | ||
"volta": { | ||
"extends": "../../package.json" | ||
} | ||
} |
7 changes: 7 additions & 0 deletions
7
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/playwright.config.mjs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
import { getPlaywrightConfig } from '@sentry-internal/test-utils'; | ||
|
||
const config = getPlaywrightConfig({ | ||
startCommand: `pnpm start`, | ||
}); | ||
|
||
export default config; |
24 changes: 24 additions & 0 deletions
24
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/public/index.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
<head> | ||
<meta charset="utf-8" /> | ||
<meta name="viewport" content="width=device-width, initial-scale=1" /> | ||
<meta name="theme-color" content="#000000" /> | ||
<meta name="description" content="Web site created using create-react-app" /> | ||
<title>React App</title> | ||
</head> | ||
<body> | ||
<noscript>You need to enable JavaScript to run this app.</noscript> | ||
<div id="root"></div> | ||
<!-- | ||
This HTML file is a template. | ||
If you open it directly in the browser, you will see an empty page. | ||
|
||
You can add webfonts, meta tags, or analytics to this file. | ||
The build step will place the bundled scripts into the <body> tag. | ||
|
||
To begin the development, run `npm start` or `yarn start`. | ||
To create a production bundle, use `npm run build` or `yarn build`. | ||
--> | ||
</body> | ||
</html> |
47 changes: 47 additions & 0 deletions
47
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/server/app.js
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove all server related code from this test |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
const express = require('express'); | ||
|
||
const app = express(); | ||
const PORT = 8080; | ||
|
||
const wait = time => { | ||
return new Promise(resolve => { | ||
setTimeout(() => { | ||
resolve(); | ||
}, time); | ||
}); | ||
}; | ||
|
||
async function sseHandler(request, response, timeout = false) { | ||
response.headers = { | ||
'Content-Type': 'text/event-stream', | ||
Connection: 'keep-alive', | ||
'Cache-Control': 'no-cache', | ||
'Access-Control-Allow-Origin': '*', | ||
}; | ||
|
||
response.setHeader('Cache-Control', 'no-cache'); | ||
response.setHeader('Content-Type', 'text/event-stream'); | ||
response.setHeader('Access-Control-Allow-Origin', '*'); | ||
response.setHeader('Connection', 'keep-alive'); | ||
|
||
response.flushHeaders(); | ||
|
||
await wait(2000); | ||
|
||
for (let index = 0; index < 10; index++) { | ||
response.write(`data: ${new Date().toISOString()}\n\n`); | ||
if (timeout) { | ||
await wait(10000); | ||
} | ||
} | ||
|
||
response.end(); | ||
} | ||
|
||
app.get('/sse', (req, res) => sseHandler(req, res)); | ||
|
||
app.get('/sse-timeout', (req, res) => sseHandler(req, res, true)); | ||
|
||
app.listen(PORT, () => { | ||
console.log(`SSE service listening at http://localhost:${PORT}`); | ||
}); |
5 changes: 5 additions & 0 deletions
5
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/src/globals.d.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
interface Window { | ||
recordedTransactions?: string[]; | ||
capturedExceptionId?: string; | ||
sentryReplayId?: string; | ||
} |
107 changes: 107 additions & 0 deletions
107
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/src/index.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
import * as Sentry from '@sentry/react'; | ||
import React from 'react'; | ||
import ReactDOM from 'react-dom/client'; | ||
import { | ||
Outlet, | ||
Route, | ||
RouterProvider, | ||
Routes, | ||
createBrowserRouter, | ||
createRoutesFromChildren, | ||
matchRoutes, | ||
useLocation, | ||
useNavigationType, | ||
useRoutes, | ||
} from 'react-router-dom'; | ||
import Index from './pages/Index'; | ||
|
||
const replay = Sentry.replayIntegration(); | ||
|
||
Sentry.init({ | ||
environment: 'qa', // dynamic sampling bias to keep transactions | ||
dsn: process.env.REACT_APP_E2E_TEST_DSN, | ||
integrations: [ | ||
Sentry.reactRouterV6BrowserTracingIntegration({ | ||
useEffect: React.useEffect, | ||
useLocation, | ||
useNavigationType, | ||
createRoutesFromChildren, | ||
matchRoutes, | ||
trackFetchStreamPerformance: true, | ||
}), | ||
replay, | ||
], | ||
// We recommend adjusting this value in production, or using tracesSampler | ||
// for finer control | ||
tracesSampleRate: 1.0, | ||
release: 'e2e-test', | ||
|
||
// Always capture replays, so we can test this properly | ||
replaysSessionSampleRate: 1.0, | ||
replaysOnErrorSampleRate: 0.0, | ||
|
||
tunnel: 'http://localhost:3031', | ||
}); | ||
|
||
const SentryRoutes = Sentry.withSentryReactRouterV6Routing(Routes); | ||
const sentryUseRoutes = Sentry.wrapUseRoutesV6(useRoutes); | ||
const sentryCreateBrowserRouter = Sentry.wrapCreateBrowserRouterV6(createBrowserRouter); | ||
|
||
const DetailsRoutes = () => | ||
sentryUseRoutes([ | ||
{ | ||
path: ':detailId', | ||
element: <div id="details">Details</div>, | ||
}, | ||
]); | ||
|
||
const DetailsRoutesAlternative = () => ( | ||
<SentryRoutes> | ||
<Route path=":detailId" element={<div id="details">Details</div>} /> | ||
</SentryRoutes> | ||
); | ||
|
||
const ViewsRoutes = () => | ||
sentryUseRoutes([ | ||
{ | ||
index: true, | ||
element: <div id="views">Views</div>, | ||
}, | ||
{ | ||
path: 'views/:viewId/*', | ||
element: <DetailsRoutes />, | ||
}, | ||
{ | ||
path: 'old-views/:viewId/*', | ||
element: <DetailsRoutesAlternative />, | ||
}, | ||
]); | ||
|
||
const ProjectsRoutes = () => ( | ||
<SentryRoutes> | ||
<Route path="projects" element={<Outlet />}> | ||
<Route index element={<div>Project Page Root</div>} /> | ||
<Route path="*" element={<Outlet />}> | ||
<Route path=":projectId/*" element={<ViewsRoutes />} /> | ||
</Route> | ||
</Route> | ||
</SentryRoutes> | ||
); | ||
|
||
const router = sentryCreateBrowserRouter([ | ||
{ | ||
children: [ | ||
{ | ||
path: '/', | ||
element: <Index />, | ||
}, | ||
{ | ||
path: '/*', | ||
element: <ProjectsRoutes />, | ||
}, | ||
], | ||
}, | ||
]); | ||
|
||
const root = ReactDOM.createRoot(document.getElementById('root') as HTMLElement); | ||
root.render(<RouterProvider router={router} />); |
18 changes: 18 additions & 0 deletions
18
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/src/pages/Index.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
// biome-ignore lint/nursery/noUnusedImports: Need React import for JSX | ||
import * as React from 'react'; | ||
import { Link } from 'react-router-dom'; | ||
|
||
const Index = () => { | ||
return ( | ||
<> | ||
<Link to="/projects/123/views/456/789" id="navigation"> | ||
navigate | ||
</Link> | ||
<Link to="/projects/123/old-views/345/654" id="old-navigation"> | ||
navigate old | ||
</Link> | ||
</> | ||
); | ||
}; | ||
|
||
export default Index; |
1 change: 1 addition & 0 deletions
1
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/src/react-app-env.d.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
/// <reference types="react-scripts" /> |
6 changes: 6 additions & 0 deletions
6
dev-packages/e2e-tests/test-applications/react-router-6-cross-usage/start-event-proxy.mjs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import { startEventProxyServer } from '@sentry-internal/test-utils'; | ||
|
||
startEventProxyServer({ | ||
port: 3031, | ||
proxyServerName: 'react-router-6-cross-usage', | ||
}); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wdyt about testing this one on v7?