🐛 Prevent duplicate Next.js RUM views from discarded renders

packages/browser-rum-nextjs/README.md

@@ -24,6 +24,17 @@ After configuration, the Datadog App provides instructions for integrating the [
 
 Both routers require **Next.js v15.3+**, which supports the [`instrumentation-client`][3] file convention.
 
+For Next.js v16.3+, enable transition events so the plugin can deduplicate repeated callbacks for one navigation:
+
+```js
+// next.config.js
+module.exports = {
+  experimental: {
+    instrumentationClientRouterTransitionEvents: true,
+  },
+}
+```
+
 ## App router usage
 
 ### 1. Create an `instrumentation-client.js` file in the root of your Next.js project

packages/browser-rum-nextjs/src/domain/nextJSRouter/datadogAppRouter.tsx

@@ -1,20 +1,19 @@
 'use client'
 
-import { useRef } from 'react'
+import { useEffect } from 'react'
 import { usePathname, useParams } from 'next/navigation'
 import { mockable } from '@datadog/browser-core'
-import { startNextjsView } from '../nextjsPlugin'
+import { setNextjsViewName } from '../nextjsPlugin'
 import { computeViewNameFromParams } from './computeViewNameFromParams'
 
 export function DatadogAppRouter() {
   const pathname = mockable(usePathname)()
   const params = mockable(useParams)()
-  const previousPathname = mockable(useRef)<string | null>(null)
+  const viewName = computeViewNameFromParams(pathname, params)
 
-  if (previousPathname.current !== pathname) {
-    previousPathname.current = pathname
-    startNextjsView(computeViewNameFromParams(pathname, params))
-  }
+  useEffect(() => {
+    setNextjsViewName(viewName, pathname)
+  }, [viewName, pathname])
 
   return null
 }

packages/browser-rum-nextjs/src/domain/nextjsPlugin.spec.ts

@@ -5,6 +5,7 @@ import { appendElement } from '../../../browser-rum-core/test'
 import {
   nextjsPlugin,
   startNextjsView,
+  setNextjsViewName,
   onRumInit,
   onRumStart,
   onRouterTransitionStart,
@@ -19,14 +20,19 @@ interface NextjsGlobalObject {
 
 function createPublicApi() {
   const startViewSpy = jasmine.createSpy('startView')
-  return { publicApi: { startView: startViewSpy } as unknown as RumPublicApi, startViewSpy }
+  const setViewNameSpy = jasmine.createSpy('setViewName')
+  return {
+    publicApi: { startView: startViewSpy, setViewName: setViewNameSpy } as unknown as RumPublicApi,
+    startViewSpy,
+    setViewNameSpy,
+  }
 }
 
 function initPlugin() {
-  const { publicApi, startViewSpy } = createPublicApi()
+  const { publicApi, startViewSpy, setViewNameSpy } = createPublicApi()
   const plugin = nextjsPlugin()
   plugin.onInit({ publicApi, initConfiguration: { ...INIT_CONFIGURATION } })
-  return { plugin, publicApi, startViewSpy }
+  return { plugin, publicApi, startViewSpy, setViewNameSpy }
 }
 
 describe('nextjsPlugin', () => {
@@ -63,40 +69,121 @@ describe('nextjsPlugin', () => {
     expect(initConfiguration.trackViewsManually).toBe(true)
   })
 
-  it('does not start a view on init', () => {
+  it('starts the initial app-router view on init', () => {
     const { startViewSpy } = initPlugin()
 
-    expect(startViewSpy).not.toHaveBeenCalled()
+    expect(startViewSpy).toHaveBeenCalledOnceWith({ name: window.location.pathname, url: window.location.href })
   })
 
   it('delegates startNextjsView to publicApi.startView with name', () => {
     const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
 
     startNextjsView('/about')
 
     expect(startViewSpy).toHaveBeenCalledOnceWith({ name: '/about', url: undefined })
   })
 
-  it('uses onRouterTransitionStart URL when available', () => {
+  it('starts a view from onRouterTransitionStart before React renders', () => {
     const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
 
     onRouterTransitionStart('/about?foo=bar')
-    startNextjsView('/about')
 
     expect(startViewSpy).toHaveBeenCalledOnceWith({
       name: '/about',
       url: `${window.location.origin}/about?foo=bar`,
     })
   })
 
-  it('clears onRouterTransitionStart URL after startNextjsView consumes it', () => {
+  it('does not start a duplicate view when Next.js repeats a transition event', () => {
     const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+    const event = { id: 'transition-1' }
 
-    onRouterTransitionStart('/about')
-    startNextjsView('/about')
-    startNextjsView('/other')
+    onRouterTransitionStart('/about', undefined, event)
+    onRouterTransitionStart('/about', undefined, event)
+
+    expect(startViewSpy).toHaveBeenCalledOnceWith({ name: '/about', url: `${window.location.origin}/about` })
+  })
+
+  it('starts views for separate transition events to the same pending pathname', () => {
+    const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart('/about', undefined, { id: 'transition-1' })
+    onRouterTransitionStart('/about', undefined, { id: 'transition-2' })
+
+    expect(startViewSpy).toHaveBeenCalledTimes(2)
+  })
+
+  it('starts a view when a navigation returns to the committed pathname', () => {
+    const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart('/redirect', undefined, { id: 'transition-1' })
+    onRouterTransitionStart(window.location.pathname, undefined, { id: 'transition-2' })
+
+    expect(startViewSpy).toHaveBeenCalledTimes(2)
+    expect(startViewSpy.calls.argsFor(1)[0]).toEqual({
+      name: window.location.pathname,
+      url: window.location.href,
+    })
+  })
+
+  it('does not rename a newer pending view from an older commit', () => {
+    const { startViewSpy, setViewNameSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart('/protected', undefined, { id: 'transition-1' })
+    onRouterTransitionStart('/login', undefined, { id: 'transition-2' })
+    setNextjsViewName('/protected', '/protected')
+
+    expect(startViewSpy).toHaveBeenCalledTimes(2)
+    expect(setViewNameSpy).not.toHaveBeenCalled()
+  })
+
+  it('starts views for successive concrete App Router pathnames', () => {
+    const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart('/user/42?admin=true')
+    setNextjsViewName('/user/[id]', '/user/42')
+    onRouterTransitionStart('/user/999?admin=true')
+
+    expect(startViewSpy).toHaveBeenCalledTimes(2)
+    expect(startViewSpy.calls.argsFor(1)[0]).toEqual({
+      name: '/user/999',
+      url: `${window.location.origin}/user/999?admin=true`,
+    })
+  })
+
+  it('does not start a view for query-string or hash-only navigations', () => {
+    const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart(`${window.location.pathname}?foo=bar`)
+    onRouterTransitionStart(`${window.location.pathname}#section`)
+
+    expect(startViewSpy).not.toHaveBeenCalled()
+  })
+
+  it('does not start a view for external navigations', () => {
+    const { startViewSpy } = initPlugin()
+    startViewSpy.calls.reset()
+
+    onRouterTransitionStart('https://example.com/about')
+
+    expect(startViewSpy).not.toHaveBeenCalled()
+  })
+
+  it('sets the normalized name after the view has started', () => {
+    const { setViewNameSpy } = initPlugin()
+
+    setNextjsViewName('/users/[id]', '/users/42')
+    setNextjsViewName('/users/[id]', '/users/42')
 
-    expect(startViewSpy.calls.mostRecent().args[0]).toEqual({ name: '/other', url: undefined })
+    expect(setViewNameSpy).toHaveBeenCalledOnceWith('/users/[id]')
   })
 
   it('reports app-router when no __NEXT_DATA__ script is present', () => {

packages/browser-rum-nextjs/src/domain/nextjsPlugin.ts

@@ -5,6 +5,7 @@ import type { RumPlugin, RumPublicApi, StartRumResult } from '@datadog/browser-r
 export type NextjsPlugin = Pick<Required<RumPlugin>, 'name' | 'onInit' | 'onRumStart' | 'getConfigurationTelemetry'>
 
 type NextjsRouterType = 'app-router' | 'pages-router'
+type RouterTransitionEvent = { id: string } | null
 interface NextjsGlobalObject {
   next?: { version?: string }
 }
@@ -13,7 +14,12 @@ type StartSubscriber = (addError: StartRumResult['addError']) => void
 
 let globalPublicApi: RumPublicApi | undefined
 let globalAddError: StartRumResult['addError'] | undefined
-let lastNavigationUrl: string | undefined
+let currentViewName: string | undefined
+// Updated by DatadogAppRouter after React commits the route.
+let currentAppRouterPathname: string | undefined
+// Updated immediately when a RUM view starts, so it can point to an uncommitted route.
+let activeAppRouterPathname: string | undefined
+let lastRouterTransitionId: string | undefined
 let routerType: NextjsRouterType | undefined
 
 const onRumInitSubscribers: InitSubscriber[] = []
@@ -27,6 +33,12 @@ export function nextjsPlugin(): NextjsPlugin {
       initConfiguration.trackViewsManually = true
       routerType = mockable(detectNextjsRouterType)()
 
+      if (routerType === 'app-router') {
+        currentAppRouterPathname = window.location.pathname
+        activeAppRouterPathname = window.location.pathname
+        startNextjsView(window.location.pathname, window.location.href)
+      }
+
       for (const subscriber of onRumInitSubscribers) {
         subscriber(publicApi)
       }
@@ -55,18 +67,52 @@ function detectNextjsRouterType(): NextjsRouterType {
   return document.getElementById('__NEXT_DATA__') ? 'pages-router' : 'app-router'
 }
 
-export function startNextjsView(viewName: string) {
+export function startNextjsView(viewName: string, url?: string) {
   if (globalPublicApi) {
-    // Use the URL captured by onRouterTransitionStart if available, since React renders before pushState updates window.location
-    const url = lastNavigationUrl ? buildUrl(lastNavigationUrl, window.location.origin).href : undefined
-    lastNavigationUrl = undefined
+    currentViewName = viewName
     globalPublicApi.startView({ name: viewName, url })
   }
 }
 
-// Must be re-exported from the user's instrumentation-client.ts so we can capture the URL before React renders
-export function onRouterTransitionStart(url: string) {
-  lastNavigationUrl = url
+export function setNextjsViewName(viewName: string, pathname?: string) {
+  // The App Router component calls this after the route has committed.
+  const hasPendingNavigation = activeAppRouterPathname !== currentAppRouterPathname
+  currentAppRouterPathname = pathname ?? currentAppRouterPathname
+
+  // A layout effect may have started a newer navigation before this passive effect runs.
+  if (pathname && pathname !== activeAppRouterPathname && hasPendingNavigation) {
+    return
+  }
+
+  activeAppRouterPathname = pathname ?? activeAppRouterPathname
+
+  if (globalPublicApi && currentViewName !== viewName) {
+    currentViewName = viewName
+    globalPublicApi.setViewName(viewName)
+  }
+}
+
+// Must be re-exported from the user's instrumentation-client.ts so we can start the view before React renders
+export function onRouterTransitionStart(url: string, _navigationType?: string, event?: RouterTransitionEvent) {
+  const navigationUrl = buildUrl(url, window.location.origin)
+
+  if (event && event.id === lastRouterTransitionId) {
+    return
+  }
+
+  // A different transition ID can target the same active pathname while that pathname has not committed yet.
+  // Keep it distinct from a query/hash-only navigation on the committed route.
+  const isNewPendingTransition =
+    event && event.id !== lastRouterTransitionId && navigationUrl.pathname !== currentAppRouterPathname
+
+  if (
+    navigationUrl.origin === window.location.origin &&
+    (navigationUrl.pathname !== activeAppRouterPathname || isNewPendingTransition)
+  ) {
+    lastRouterTransitionId = event?.id
+    activeAppRouterPathname = navigationUrl.pathname
+    startNextjsView(navigationUrl.pathname, navigationUrl.href)
+  }
 }
 
 export function onRumInit(callback: InitSubscriber) {
@@ -90,6 +136,9 @@ export function resetNextjsPlugin() {
   globalAddError = undefined
   onRumInitSubscribers.length = 0
   onRumStartSubscribers.length = 0
-  lastNavigationUrl = undefined
+  currentViewName = undefined
+  currentAppRouterPathname = undefined
+  activeAppRouterPathname = undefined
+  lastRouterTransitionId = undefined
   routerType = undefined
 }

packages/browser-rum-nextjs/test/initializeNextjsPlugin.ts

@@ -5,7 +5,7 @@ import { registerCleanupTask } from '../../browser-core/test'
 
 export function initializeNextjsPlugin({
   initConfiguration = {},
-  publicApi = {},
+  publicApi = { startView: noop, setViewName: noop },
   addError = noop,
 }: {
   initConfiguration?: Partial<RumInitConfiguration>

test/apps/nextjs/app/discardedRenderProbe.tsx

@@ -0,0 +1,29 @@
+'use client'
+
+import { useEffect, useState } from 'react'
+
+let renderAttempt = 0
+
+export function DiscardedRenderProbe() {
+  const enabled =
+    typeof window !== 'undefined' && new URLSearchParams(window.location.search).has('discard-nextjs-render')
+  const [started, setStarted] = useState(false)
+
+  useEffect(() => {
+    if (enabled) {
+      setStarted(true)
+    }
+  }, [enabled])
+
+  if (!started) {
+    return null
+  }
+
+  renderAttempt += 1
+
+  if (renderAttempt === 1) {
+    throw new Promise<void>((resolve) => setTimeout(resolve))
+  }
+
+  return <span data-testid="discarded-render-probe-ready" hidden />
+}

test/apps/nextjs/app/layout.tsx

@@ -1,10 +1,15 @@
+import { Suspense } from 'react'
 import { DatadogAppRouter } from '@datadog/browser-rum-nextjs'
+import { DiscardedRenderProbe } from './discardedRenderProbe'
 
 export default function RootLayout({ children, sidebar }: { children: React.ReactNode; sidebar: React.ReactNode }) {
   return (
     <html lang="en">
       <body style={{ fontFamily: 'system-ui, sans-serif', margin: 0 }}>
-        <DatadogAppRouter />
+        <Suspense fallback={null}>
+          <DatadogAppRouter />
+          <DiscardedRenderProbe />
+        </Suspense>
         <nav style={{ background: '#632ca6', padding: '1rem', marginBottom: '1rem' }}>
           <a href="/" style={{ color: 'white', textDecoration: 'none' }}>
             Home

test/apps/nextjs/app/login/page.tsx

@@ -0,0 +1,3 @@
+export default function LoginPage() {
+  return <h1>Login</h1>
+}

test/apps/nextjs/app/page.tsx

@@ -20,6 +20,23 @@ export default function HomePage() {
         <li>
           <Link href="/global-error-test?throw=true">Go to Global Error</Link>
         </li>
+        <li>
+          <Link href="/slow" prefetch={false}>
+            Go to Slow Page
+          </Link>
+        </li>
+        <li>
+          <Link href="/?discard-nextjs-render">Discard Next.js Render</Link>
+        </li>
+        <li>
+          <Link href="/redirect">Go to Redirect</Link>
+        </li>
+        <li>
+          <Link href="/redirect-home">Redirect Home</Link>
+        </li>
+        <li>
+          <Link href="/protected">Go to Protected</Link>
+        </li>
       </ul>
     </div>
   )