[NextJS] Nextjs app router integration (AI optimistic)#4234
[NextJS] Nextjs app router integration (AI optimistic)#4234
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
|
✨ Fix all issues with BitsAI or with Cursor
|
|
|
||
| let forceUpdate: () => void | ||
|
|
||
| function App() { |
There was a problem hiding this comment.
I'm not sure I love the idea of doing a component in a unit test. I would prefer to put it in a e2e test and here test behaviour.
| return <>{children}</> | ||
| } | ||
|
|
||
| DatadogRumProvider.displayName = 'DatadogRumProvider' |
There was a problem hiding this comment.
Question: What is displayName this for?
There was a problem hiding this comment.
It's just for the React Browser Extension to be able to display the component name in the React Virtual DOM tree. This is optional but nice to have
| * return ( | ||
| * <html> | ||
| * <body> | ||
| * <DatadogRumProvider>{children}</DatadogRumProvider> |
There was a problem hiding this comment.
I am stealing the following question since it was asked to me too:
Question: Why is this a Provider if it's not wrapping anything? Could it be a function call too?
There was a problem hiding this comment.
Yeah, indeed, it can be implemented as a hook for example. We can't make it a pure function call because we have to call hooks in it, but hooks approach could work, I agree
| "devDependencies": { | ||
| "@types/react": "19.2.14", | ||
| "@types/react-dom": "19.2.3", | ||
| "next": "15.2.2", |
There was a problem hiding this comment.
Question:: So we will support next >= 15.2.2. Shouldn't we try to support sooner versions too?
Suggestion:: For example support version >= 13?
There was a problem hiding this comment.
No, that's just the version we use inside our integration. As it is defined above in the peerDependencies section, this approach supports all the versions >= 13
| import { | ||
| reactPlugin, | ||
| UNSTABLE_ReactComponentTracker as ReactComponentTracker, | ||
| ErrorBoundary, |
There was a problem hiding this comment.
Suggestion: The thing about this is that NextJS has a different signature to report errors.
export default function ErrorPage({ error, reset, }:
So the React Error Boundary would not have the whole context. So we would need to make something for it.
There was a problem hiding this comment.
Yeah, tbh I didn't look too much into sample app implementation made by AI, so it probably has a lots of problems which you raised
| datadogRum.setGlobalContext(window.RUM_CONTEXT) | ||
| } | ||
|
|
||
| function Link({ |
There was a problem hiding this comment.
Question: Isn't this a Next component? Why do we have to re-declare it?
| ) | ||
| } | ||
|
|
||
| function App() { |
There was a problem hiding this comment.
This is not how App Router works for routes. Each file is it's own route. Doc
There was a problem hiding this comment.
This is important so that we use NextJS specific behaviour like client and server components and even prefetching and test against it.
| return useContext(NavigationContext).navigate | ||
| } | ||
|
|
||
| export function NavigationProvider({ |
There was a problem hiding this comment.
Question: What is this component for? Would customer need to create this file themselves? Shouldn't this be handled on the SDK side?
| test.describe(`with ${description}`, () => { | ||
| createTest('should define a view name with DatadogRumProvider') | ||
| .withRum() | ||
| .withReactApp(appName) |
There was a problem hiding this comment.
Question: Why are using ReactApp and not NextApp. IIUC, here you are serving the static html files to the tests, a NextApp needs it's own server to run so as to leverage NextJS features like prefetching, server components, cache, etc.
|
|
||
| // Firefox may delay dispatching error events from React error boundaries, | ||
| // causing flushEvents() to miss them, this timeout ensures the RUM event is captured. | ||
| if (browserName === 'firefox') { |
There was a problem hiding this comment.
I'm not soo sure about this.
✨ Add Next.js App Router integration for
@datadog/browser-rum-reactMotivation
Next.js App Router is a widely adopted routing paradigm (Next.js 13.4+), but RUM view tracking requires manual instrumentation today. This integration provides a
<DatadogRumProvider>component that automatically tracks client-side navigations as RUM views with parameterized route names (e.g.,/users/:id), following the same sub-package pattern as the existing React Router integrations.Changes
Plugin configuration (
packages/rum-react/src/domain/reactPlugin.ts):nextAppRouter?: booleanoption toReactPluginConfigurationtrackViewsManually = truewhennextAppRouteris enabledrouterandnextAppRouterare enabled simultaneouslynextAppRouterin configuration telemetryView name computation (
packages/rum-react/src/domain/nextjs/computeNextViewName.ts):pathname+params(e.g.,/users/123+{ id: '123' }→/users/:id)[...slug]→/:slug), multi-occurrence params, and edge cases (empty/undefined values)Provider component (
packages/rum-react/src/domain/nextjs/datadogRumProvider.tsx):DatadogRumProvidercomponent that readsusePathname()/useParams()fromnext/navigationonRumInitsubscriber pattern (safe for any init order)/users/123→/users/456does not start a new view if both resolve to/users/:idSub-package entry point:
packages/rum-react/nextjs/package.json— shim pointing to compiled artifacts (same pattern asreact-router-v6/,react-router-v7/)packages/rum-react/src/entries/nextjs.ts— entry point exportingDatadogRumProviderSample test app (
test/apps/nextjs-app/):mockNextNavigation.tsx(mockusePathname,useParams,useNavigatebacked by React context)/), dynamic param (/user/:id), tracked component (/tracked), error boundary (/error)DatadogRumProvider,ErrorBoundary,UNSTABLE_ReactComponentTrackerfrom the SDKscripts/build/build-test-apps.tsandtest/e2e/lib/framework/sdkBuilds.tsE2E tests (
test/e2e/scenario/reactPlugin.scenario.ts):/user/42, verify last view name is/user/:idTrackedPagevital with durationframework: 'react'and component stackTest instructions
yarn test:unit --spec packages/rum-react/src/domain/nextjs/computeNextViewName.spec.tsand the other spec files inpackages/rum-react/src/domain/nextjs/yarn test:unit --spec packages/rum-react/src/domain/reactPlugin.spec.tsyarn build:appsyarn test:e2e -g "Next.js App Router"yarn typecheckChecklist