SDK: better support for SPA routing (#467)

* SDK: better support for SPA routing - STEP 1/2 : add hash tracking (#463)

* add hash tracking

* ✅ add hash change tests

* introduce renewViewOnChange

* add better management and comments of the areViewDifferent hash part

* set async tests for hashchanges

* reset window.location.hash of previous tests

* skip tesst using Promise for IE

* replace jasmine promise with callback process

* remove anchor check on this PR

* ✅ and implem reviews

* SDK: better support for SPA routing - STEP 2/2 : filter out html anchor tags changes (#466)

* add hash tracking

* ✅ add hash change tests

* introduce renewViewOnChange

* add better management and comments of the areViewDifferent hash part

* set async tests for hashchanges

* reset window.location.hash of previous tests

* skip tesst using Promise for IE

* replace jasmine promise with callback process

* remove anchor check on this PR

* add anchor nav filter

* patch tslint error

* ✅ and implem reviews

* implement reviews regarding func naming and hash cleaning

* simplify mockGetElementById

* add e2e tests

* simply anchor filter out proccess

* implement reviews

* unit test the hash change acknowledgement after an anchor nav

* fix typo

* fix typo v2

* fix mock hash

Co-authored-by: Bastien Caudan <bastien.caudan@gmail.com>

packages/rum/src/viewCollection.ts

@@ -40,8 +40,7 @@ export function startViewCollection(location: Location, lifeCycle: LifeCycle) {
   const startOrigin = 0
   let currentView = newView(lifeCycle, location, ViewLoadingType.INITIAL_LOAD, startOrigin)
 
-  // Renew view on history changes
-  trackHistory(() => {
+  function renewViewOnChange() {
     if (currentView.isDifferentView(location)) {
       currentView.triggerUpdate()
       currentView.end()
@@ -50,7 +49,13 @@ export function startViewCollection(location: Location, lifeCycle: LifeCycle) {
       currentView.updateLocation(location)
       currentView.triggerUpdate()
     }
-  })
+  }
+
+  // Renew view on history changes
+  trackHistory(renewViewOnChange)
+
+  // Renew view on hash changes
+  trackHash(renewViewOnChange)
 
   // Renew view on session renewal
   lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => {
@@ -148,7 +153,10 @@ function newView(
       stopScheduleViewUpdate()
     },
     isDifferentView(otherLocation: Location) {
-      return location.pathname !== otherLocation.pathname
+      return (
+        location.pathname !== otherLocation.pathname ||
+        (!isHashAnAnchor(otherLocation.hash) && otherLocation.hash !== location.hash)
+      )
     },
     triggerUpdate() {
       updateView()
@@ -159,6 +167,11 @@ function newView(
   }
 }
 
+function isHashAnAnchor(hash: string) {
+  const correspondingId = hash.substr(1)
+  return !!document.getElementById(correspondingId)
+}
+
 function trackHistory(onHistoryChange: () => void) {
   const originalPushState = history.pushState
   history.pushState = monitor(function(this: History['pushState']) {
@@ -173,6 +186,10 @@ function trackHistory(onHistoryChange: () => void) {
   window.addEventListener(DOM_EVENT.POP_STATE, monitor(onHistoryChange))
 }
 
+function trackHash(onHashChange: () => void) {
+  window.addEventListener('hashchange', monitor(onHashChange))
+}
+
 interface Timings {
   domComplete?: number
   domContentLoaded?: number

packages/rum/test/specHelper.ts

@@ -85,6 +85,18 @@ export function setup(): TestSetupBuilder {
       spyOn(history, 'pushState').and.callFake((_: any, __: string, pathname: string) => {
         assign(fakeLocation, buildLocation(pathname, fakeLocation.href!))
       })
+
+      function hashchangeCallBack() {
+        fakeLocation.hash = window.location.hash
+      }
+
+      window.addEventListener('hashchange', hashchangeCallBack)
+
+      cleanupTasks.push(() => {
+        window.removeEventListener('hashchange', hashchangeCallBack)
+        window.location.hash = ''
+      })
+
       return setupBuilder
     },
     withSession(sessionStub: RumSession) {

packages/rum/test/viewCollection.spec.ts

@@ -1,3 +1,5 @@
+import { getHash, getPathName, getSearch } from '@datadog/browser-core'
+
 import { LifeCycleEventType } from '../src/lifeCycle'
 import { ViewContext } from '../src/parentContexts'
 import { PerformanceLongTaskTiming, PerformancePaintTiming } from '../src/rum'
@@ -59,6 +61,12 @@ const FAKE_NAVIGATION_ENTRY_WITH_LOADEVENT_AFTER_ACTIVITY_TIMING = {
   loadEventEnd: BEFORE_PAGE_ACTIVITY_VALIDATION_DELAY * 1.2,
 }
 
+function mockGetElementById() {
+  return spyOn(document, 'getElementById').and.callFake((elementId: string) => {
+    return (elementId === ('testHashValue' as unknown)) as any
+  })
+}
+
 function spyOnViews() {
   const handler = jasmine.createSpy()
 
@@ -79,6 +87,7 @@ describe('rum track url change', () => {
   let createSpy: jasmine.Spy
 
   beforeEach(() => {
+    const fakeLocation: Partial<Location> = { pathname: '/foo', hash: '' }
     setupBuilder = setup()
       .withFakeLocation('/foo')
       .withViewCollection()
@@ -106,21 +115,107 @@ describe('rum track url change', () => {
     expect(viewContext.id).not.toEqual(initialViewId)
   })
 
-  it('should not create new view on search change', () => {
+  it('should create a new view on hash change from history', () => {
     const { lifeCycle } = setupBuilder.build()
     lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
 
-    history.pushState({}, '', '/foo?bar=qux')
+    history.pushState({}, '', '/foo#bar')
+
+    expect(createSpy).toHaveBeenCalled()
+    const viewContext = createSpy.calls.argsFor(0)[0] as ViewContext
+    expect(viewContext.id).not.toEqual(initialViewId)
+  })
+
+  it('should not create a new view on hash change from history when the hash has kept the same value', () => {
+    history.pushState({}, '', '/foo#bar')
+
+    const { lifeCycle } = setupBuilder.build()
+    lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
+
+    history.pushState({}, '', '/foo#bar')
 
     expect(createSpy).not.toHaveBeenCalled()
   })
 
-  it('should not create a new view on hash change', () => {
+  it('should create a new view on hash change', (done) => {
     const { lifeCycle } = setupBuilder.build()
     lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
 
+    function hashchangeCallBack() {
+      expect(createSpy).toHaveBeenCalled()
+      const viewContext = createSpy.calls.argsFor(0)[0] as ViewContext
+      expect(viewContext.id).not.toEqual(initialViewId)
+      window.removeEventListener('hashchange', hashchangeCallBack)
+      done()
+    }
+
+    window.addEventListener('hashchange', hashchangeCallBack)
+
+    window.location.hash = '#bar'
+  })
+
+  it('should not create a new view when the hash has kept the same value', (done) => {
     history.pushState({}, '', '/foo#bar')
 
+    const { lifeCycle } = setupBuilder.build()
+    lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
+
+    function hashchangeCallBack() {
+      expect(createSpy).not.toHaveBeenCalled()
+      window.removeEventListener('hashchange', hashchangeCallBack)
+      done()
+    }
+
+    window.addEventListener('hashchange', hashchangeCallBack)
+
+    window.location.hash = '#bar'
+  })
+
+  it('should not create a new view when it is an Anchor navigation', (done) => {
+    const { lifeCycle } = setupBuilder.build()
+    mockGetElementById()
+    lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
+
+    function hashchangeCallBack() {
+      expect(createSpy).not.toHaveBeenCalled()
+      window.removeEventListener('hashchange', hashchangeCallBack)
+      done()
+    }
+
+    window.addEventListener('hashchange', hashchangeCallBack)
+
+    window.location.hash = '#testHashValue'
+  })
+
+  it('should acknowledge the view location hash change after an Anchor navigation', (done) => {
+    const { lifeCycle } = setupBuilder.build()
+    const spyObj = mockGetElementById()
+    lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
+
+    function hashchangeCallBack() {
+      expect(createSpy).not.toHaveBeenCalled()
+      window.removeEventListener('hashchange', hashchangeCallBack)
+
+      // clear mockGetElementById that fake Anchor nav
+      spyObj.and.callThrough()
+
+      // This is not an Anchor nav anymore but the hash and pathname have not been updated
+      history.pushState({}, '', '/foo#testHashValue')
+      expect(createSpy).not.toHaveBeenCalled()
+      done()
+    }
+
+    window.addEventListener('hashchange', hashchangeCallBack)
+
+    window.location.hash = '#testHashValue'
+  })
+
+  it('should not create new view on search change', () => {
+    const { lifeCycle } = setupBuilder.build()
+    lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, createSpy)
+
+    history.pushState({}, '', '/foo?bar=qux')
+
     expect(createSpy).not.toHaveBeenCalled()
   })
 })

test/e2e/scenario/agents.scenario.ts

@@ -16,7 +16,7 @@ import {
   waitServerRumEvents,
   withBrowserLogs,
 } from './helpers'
-import { isRumResourceEvent, isRumUserActionEvent, isRumViewEvent } from './serverTypes'
+import { isRumResourceEvent, isRumUserActionEvent, isRumViewEvent, ServerRumViewLoadingType } from './serverTypes'
 
 beforeEach(startSpec)
 
@@ -270,3 +270,30 @@ describe('user action collection', () => {
     expect(resourceEvents[0].user_action!.id).toBe(userActionEvents[0].user_action.id!)
   })
 })
+
+describe('anchor navigation', () => {
+  it('should not create a new view when it is an Anchor navigation', async () => {
+    await $('#test-anchor').click()
+
+    await flushEvents()
+    const rumEvents = await waitServerRumEvents()
+    const viewEvents = rumEvents.filter(isRumViewEvent)
+
+    expect(viewEvents.length).toBe(1)
+    expect(viewEvents[0].view.loading_type).toBe(ServerRumViewLoadingType.INITIAL_LOAD)
+  })
+
+  it('should create a new view on hash change', async () => {
+    await browserExecute(() => {
+      window.location.hash = '#bar'
+    })
+
+    await flushEvents()
+    const rumEvents = await waitServerRumEvents()
+    const viewEvents = rumEvents.filter(isRumViewEvent)
+
+    expect(viewEvents.length).toBe(2)
+    expect(viewEvents[0].view.loading_type).toBe(ServerRumViewLoadingType.INITIAL_LOAD)
+    expect(viewEvents[1].view.loading_type).toBe(ServerRumViewLoadingType.ROUTE_CHANGE)
+  })
+})

test/e2e/scenario/serverTypes.ts

@@ -80,6 +80,11 @@ export function isRumUserActionEvent(event: ServerRumEvent): event is ServerRumU
   return event.evt.category === 'user_action'
 }
 
+export enum ServerRumViewLoadingType {
+  INITIAL_LOAD = 'initial_load',
+  ROUTE_CHANGE = 'route_change',
+}
+
 export interface ServerRumViewEvent extends ServerRumEvent {
   evt: {
     category: 'view'
@@ -90,6 +95,7 @@ export interface ServerRumViewEvent extends ServerRumEvent {
   session_id: string
   view: {
     id: string
+    loading_type: ServerRumViewLoadingType
     measures: {
       dom_complete: number
       dom_content_loaded: number

test/static/async-e2e-page.html

@@ -9,6 +9,9 @@
   </head>
   <body>
     <button>click me</button>
+    <div id="test-anchor">
+      <a href="#test-anchor">anchor link</a>
+    </div>
     <script type="text/javascript">
       window.addEventListener('load', () => {
         const intakeOrigin = `http://${window.location.hostname}:4000`

test/static/bundle-e2e-page.html

@@ -36,5 +36,8 @@
   </head>
   <body>
     <button>click me</button>
+    <div id="test-anchor">
+      <a href="#test-anchor">anchor link</a>
+    </div>
   </body>
 </html>

test/static/npm-e2e-page.html

@@ -10,5 +10,8 @@
   </head>
   <body>
     <button>click me</button>
+    <div id="test-anchor">
+      <a href="#test-anchor">anchor link</a>
+    </div>
   </body>
 </html>