-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
docs(js): streamline React Router v6 and v7 documentation #13138
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
Changes from 1 commit
a8ed4d4
8584284
a5ff0ca
8d509ae
3c4f6b5
4c68e87
0417ac9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -4,38 +4,14 @@ description: "Learn about Sentry's React Router v6 integration." | |||
| sidebar_order: 20 | ||||
| --- | ||||
|
|
||||
| <Alert> | ||||
| - React Router v6 support is included in the `@sentry/react` package since | ||||
| version `7`. | ||||
| </Alert> | ||||
|
|
||||
| Update your `Sentry.browserTracingIntegration` to `Sentry.reactRouterV6BrowserTracingIntegration` and provide the required React hooks and router functions: | ||||
|
|
||||
| - `useEffect` hook from `react` | ||||
| - `useLocation` and `useNavigationType` hooks from `react-router-dom` or `react-router` | ||||
| - `createRoutesFromChildren` and `matchRoutes` functions from `react-router-dom` or `react-router` | ||||
|
|
||||
| <Alert level="warning"> | ||||
|
|
||||
| To ensure proper routing instrumentation, initialize Sentry by calling `Sentry.init` **before**: | ||||
|
|
||||
| - Wrapping your `<Routes />` component | ||||
| - Using `useRoutes` | ||||
| - Using `wrapCreateBrowserRouterV6` or `wrapCreateMemoryRouterV6` | ||||
|
|
||||
| </Alert> | ||||
| _React Router v6 support is included in the `@sentry/react` package since version `7`._ | ||||
|
||||
| _React Router v6 support is included in the `@sentry/react` package since version `7`._ |
IMHO we can drop this, we are on v9 now 😅
inventarSarah marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
inventarSarah marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
inventarSarah marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
inventarSarah marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
inventarSarah marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
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.
Is this section helpful? What does it tell me? 😅 I should set this up anyhow to get all errors, I'd say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean this whole alert here!
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.
Removed it vor v6 and v7 👍
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.
Not related to any change in this PR but generally: We should clarify here, because it is not clear to me: What If I am not using a custom error boundary, what should I do then? Should I add one to capture errors, or not? cc @lforst & @AbhiPrasad
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for react router you need to add one, otherwise component-related errors get swallowed by the router itself.
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.
yeah, so let's reword this (can be a different PR as well) to make it clear you def should add this, not just if you're using a custom boundary!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added a sentence to the first paragraph in this section (v6 and v7 pages) -- hope this is what you meant (if not, let's chat later today @mydea :) )
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.
Would it make sense to lead with something like this:
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.
and something similar for react-router v7 :)
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.
@mydea Good point! I updated the start of the page with an adapted version of your suggestion.
Sadly, the TableOfContents component does not work with our structure as it seems to be created to work with the SDK options pages, and our structure here is quite different -> I created an issue #13159 so we can consider upgrading the functionality of this component
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.
should be fixed here #13171 😅
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.
the toc is now included - looks good!