feat(react): Add tracing support for React Router 6.4 createBrowserRouter.
#6172
Conversation
onurtemizkan
force-pushed
the
onur/support-react-router-6.4
branch
from
c23ef51
to
7d486d3
Compare
|
Any thoughts on merging this? Further concerns? It's becoming an issue of concern about whether my project will continue to pay for Sentry without it. |
size-limit report 📦
|
There was a problem hiding this comment.
We don't need to access useEffect and react-router functions anymore. It's still required by reactRouterV6Instrumentation, so we may have another instrumentation setup function like reactRouterV64Instrumentation, to avoid this. WDYT
Introducing another function is just more cost (I'm thinking docs and support questions) - let's just stick with the existing function signature.
This doesn't fix the issue
That's fine, we can try digging into this separately.
Thinking about the previous issues about hydration, I'm not very keen to introduce another React component wrapper, but it's also possible
Maybe we ship the createBrowserRouter wrapper as a first step to unblock users, and then try to wrap RouterProvider as a next step.
packages/react/package.json
Outdated
| "react": "^18.0.0", | ||
| "react-dom": "^18.0.0", | ||
| "react-router-3": "npm:react-router@3.2.0", | ||
| "react-router-4": "npm:react-router@4.1.0", | ||
| "react-router-5": "npm:react-router@5.0.0", | ||
| "react-router-6": "npm:react-router@6.3.0", | ||
| "react-router-6": "npm:react-router@6.4.2", |
There was a problem hiding this comment.
let's add a new alias for 6.4?
onurtemizkan
force-pushed
the
onur/support-react-router-6.4
branch
from
7d486d3
to
a3b39b7
Compare
onurtemizkan
force-pushed
the
onur/support-react-router-6.4
branch
from
a4a328f
to
26e44c6
Compare
onurtemizkan
changed the title
createBrowserRouter.
| @@ -13,9 +14,17 @@ import { | |||
| useNavigationType, | |||
| useRoutes, | |||
| } from 'react-router-6'; | |||
| import { createMemoryRouter, Navigate as Navigate_6_4,RouterProvider } from 'react-router-6.4'; | |||
There was a problem hiding this comment.
can we move this into a new test file?
|
Let's also make sure we get a docs PR ready for this @onurtemizkan! |
|
thank you for this patch up! |
|
do you have integration guide documentation with createBrowserRouter ? -> you've updated already! |
Resolves: #5872
Adds tracing support for React Router 6.4's
createBrowserRouter(and seamlesslycreateMemoryRouter).React Router 6.4 allows us to reach the router state itself, without having to use React hooks. So, the implementation is much simpler.
Example Spans:
pageload- Linkpageload- Linknavigation- LinkImplementation Details Discussed
This implementation works as is, but I have a few questions to the team, before moving forward:
We don't need to access
useEffectandreact-routerfunctions anymore. It's still required byreactRouterV6Instrumentation, so we may have another instrumentation setup function likereactRouterV64Instrumentation, to avoid this. WDYT?This doesn't fix the issue [react-router-6] Nested Routers produce wrong parametrized URL #5997. When I tried to replicate the issue combining
createBrowserRouter, wildcard paths, and descendant routes defined in separate<Routes>, thematchesobject we're getting from the router is stillpath/*. It's not about our name parameterisation utility, as we are not getting info from the router. Besides that, I'm not sure if this pattern is recommended with the new API, but it still works in React Router 6.4.This implementation doesn't support
createHashRouteryet. It's not much different thancreateBrowserRouter, but we'll need another wrapper after all. Alternatively, we can try wrapping<RouterProvider>, which can end up with an easier single wrapper for all router types. Thinking about the previous issues abouthydration, I'm not very keen to introduce another React component wrapper, but it's also possible. WDYT?