🐛 Allow both history and History prototype to be patched by 3rd party

packages/core/test/emulate/buildLocation.ts

@@ -0,0 +1,11 @@
+import { buildUrl } from '../../src'
+
+export function buildLocation(url: string, base = location.href) {
+  const urlObject = buildUrl(url, base)
+  return {
+    hash: urlObject.hash,
+    href: urlObject.href,
+    pathname: urlObject.pathname,
+    search: urlObject.search,
+  } as Location
+}

packages/core/test/index.ts

@@ -5,7 +5,7 @@ export * from './cookie'
 export * from './registerCleanupTask'
 export * from './interceptRequests'
 export * from './emulate/createNewEvent'
-export * from './emulate/mockLocation'
+export * from './emulate/buildLocation'
 export * from './emulate/mockClock'
 export * from './emulate/mockReportingObserver'
 export * from './emulate/mockZoneJs'

packages/rum-core/src/browser/locationChangeObservable.spec.ts

@@ -1,31 +1,11 @@
-import type { Observable, Subscription } from '@datadog/browser-core'
-import { mockLocation } from '@datadog/browser-core/test'
+import { registerCleanupTask } from '@datadog/browser-core/test'
 import type { RumConfiguration } from '@datadog/browser-rum-core'
-import type { LocationChange } from './locationChangeObservable'
 import { createLocationChangeObservable } from './locationChangeObservable'
 
 describe('locationChangeObservable', () => {
-  let observable: Observable<LocationChange>
-  let observer: jasmine.Spy<(locationChange: LocationChange) => void>
-  let subscription: Subscription
-  let fakeLocation: Partial<Location>
-  let cleanupLocation: () => void
-  let configuration: RumConfiguration
-
-  beforeEach(() => {
-    ;({ location: fakeLocation, cleanup: cleanupLocation } = mockLocation('/foo'))
-    configuration = {} as RumConfiguration
-    observable = createLocationChangeObservable(configuration, fakeLocation as Location)
-    observer = jasmine.createSpy('obs')
-    subscription = observable.subscribe(observer)
-  })
-
-  afterEach(() => {
-    subscription.unsubscribe()
-    cleanupLocation()
-  })
-
   it('should notify observers on history change', () => {
+    const observer = setup()
+
     history.pushState({}, '', '/foo?bar=qux')
 
     const locationChanges = observer.calls.argsFor(0)[0]
@@ -34,6 +14,8 @@ describe('locationChangeObservable', () => {
   })
 
   it('should notify observers on hashchange', (done) => {
+    const observer = setup()
+
     function hashChangeCallback() {
       const locationChanges = observer.calls.argsFor(0)[0]
       expect(locationChanges.oldLocation.href).toMatch(/\/foo$/)
@@ -48,8 +30,84 @@ describe('locationChangeObservable', () => {
   })
 
   it('should not notify if the url has not changed', () => {
+    const observer = setup()
+
     history.pushState({}, '', '/foo')
 
     expect(observer).not.toHaveBeenCalled()
   })
+
+  it('allow frameworks to patch history.pushState', () => {
+    const wrapperSpy = setupHistoryInstancePushStateWrapper()
+    const observer = setup()
+
+    history.pushState({}, '', '/foo?bar=qux')
+
+    const locationChanges = observer.calls.argsFor(0)[0]
+    expect(locationChanges.oldLocation.href).toMatch(/\/foo$/)
+    expect(locationChanges.newLocation.href).toMatch(/\/foo\?bar=qux$/)
+    expect(wrapperSpy).toHaveBeenCalled()
+  })
+
+  it('allow frameworks to patch History.prototype.pushState', () => {
+    const wrapperSpy = setupHistoryPrototypePushStateWrapper()
+    const observer = setup()
+
+    history.pushState({}, '', '/foo?bar=qux')
+
+    const locationChanges = observer.calls.argsFor(0)[0]
+    expect(locationChanges.oldLocation.href).toMatch(/\/foo$/)
+    expect(locationChanges.newLocation.href).toMatch(/\/foo\?bar=qux$/)
+    expect(wrapperSpy).toHaveBeenCalled()
+  })
 })
+
+function setup() {
+  const originalPathname = location.pathname
+
+  history.pushState({}, '', '/foo')
+
+  const observable = createLocationChangeObservable({} as RumConfiguration, location)
+  const observer = jasmine.createSpy('obs')
+  const subscription = observable.subscribe(observer)
+
+  registerCleanupTask(() => {
+    subscription.unsubscribe()
+    history.pushState({}, '', originalPathname)
+  })
+
+  return observer
+}
+
+function setupHistoryInstancePushStateWrapper() {
+  const wrapperSpy = jasmine.createSpy('wrapperSpy')
+  const originalPushState = history.pushState.bind(history)
+
+  history.pushState = (...args) => {
+    wrapperSpy(...args)
+    originalPushState(...args)
+  }
+
+  registerCleanupTask(() => {
+    // @ts-expect-error reseting history instance to its original state
+    delete history.pushState
+  })
+
+  return wrapperSpy
+}
+
+function setupHistoryPrototypePushStateWrapper() {
+  const wrapperSpy = jasmine.createSpy('wrapperSpy')
+  const originalPushState = History.prototype.pushState.bind(history)
+
+  History.prototype.pushState = (...args) => {
+    wrapperSpy(...args)
+    originalPushState(...args)
+  }
+
+  registerCleanupTask(() => {
+    History.prototype.pushState = originalPushState
+  })
+
+  return wrapperSpy
+}

packages/rum-core/src/browser/locationChangeObservable.ts

@@ -33,11 +33,15 @@ export function createLocationChangeObservable(configuration: RumConfiguration,
 }
 
 function trackHistory(configuration: RumConfiguration, onHistoryChange: () => void) {
-  const { stop: stopInstrumentingPushState } = instrumentMethod(History.prototype, 'pushState', ({ onPostCall }) => {
-    onPostCall(onHistoryChange)
-  })
+  const { stop: stopInstrumentingPushState } = instrumentMethod(
+    getHistoryInstrumentationTarget('pushState'),
+    'pushState',
+    ({ onPostCall }) => {
+      onPostCall(onHistoryChange)
+    }
+  )
   const { stop: stopInstrumentingReplaceState } = instrumentMethod(
-    History.prototype,
+    getHistoryInstrumentationTarget('replaceState'),
     'replaceState',
     ({ onPostCall }) => {
       onPostCall(onHistoryChange)
@@ -57,3 +61,9 @@ function trackHistory(configuration: RumConfiguration, onHistoryChange: () => vo
 function trackHash(configuration: RumConfiguration, onHashChange: () => void) {
   return addEventListener(configuration, window, DOM_EVENT.HASH_CHANGE, onHashChange)
 }
+
+function getHistoryInstrumentationTarget(methodName: 'pushState' | 'replaceState') {
+  // Ideally we should always instument the method on the prototype, however some frameworks (e.g [Next.js](https://github.com/vercel/next.js/blob/d3f5532065f3e3bb84fb54bd2dfd1a16d0f03a21/packages/next/src/client/components/app-router.tsx#L429))
+  // are wrapping the instance method. In that case we should also wrap the instance method.
+  return Object.prototype.hasOwnProperty.call(history, methodName) ? history : History.prototype
+}