Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SDK: better support for SPA routing #467

Merged
merged 6 commits into from
Aug 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 21 additions & 4 deletions packages/rum/src/viewCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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, () => {
Expand Down Expand Up @@ -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()
Expand All @@ -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']) {
Expand All @@ -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
Expand Down
12 changes: 12 additions & 0 deletions packages/rum/test/specHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
101 changes: 98 additions & 3 deletions packages/rum/test/viewCollection.spec.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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()

Expand All @@ -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()
Expand Down Expand Up @@ -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()
})
})
Expand Down
29 changes: 28 additions & 1 deletion test/e2e/scenario/agents.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
waitServerRumEvents,
withBrowserLogs,
} from './helpers'
import { isRumResourceEvent, isRumUserActionEvent, isRumViewEvent } from './serverTypes'
import { isRumResourceEvent, isRumUserActionEvent, isRumViewEvent, ServerRumViewLoadingType } from './serverTypes'

beforeEach(startSpec)

Expand Down Expand Up @@ -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)
})
})
6 changes: 6 additions & 0 deletions test/e2e/scenario/serverTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/static/async-e2e-page.html
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
3 changes: 3 additions & 0 deletions test/static/bundle-e2e-page.html
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,8 @@
</head>
<body>
<button>click me</button>
<div id="test-anchor">
<a href="#test-anchor">anchor link</a>
</div>
</body>
</html>
3 changes: 3 additions & 0 deletions test/static/npm-e2e-page.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,8 @@
</head>
<body>
<button>click me</button>
<div id="test-anchor">
<a href="#test-anchor">anchor link</a>
</div>
</body>
</html>