Skip to content

Commit

Permalink
🐛[RUMF-320] Remove url-polyfill dependency (#294)
Browse files Browse the repository at this point in the history
* ♻️ extract functions that manage URLs

* ✨ add simple url polyfill

* 🔥 url-polyfill dependency

* ♿ tweaks for IE

* 👌 use URL when available
  • Loading branch information
bcaudan authored Mar 12, 2020
1 parent a4eb2e1 commit 3c525e5
Show file tree
Hide file tree
Showing 13 changed files with 189 additions and 76 deletions.
1 change: 0 additions & 1 deletion LICENSE-3rdparty.csv
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ Component,Origin,License,Copyright
require,lodash.assign,MIT,Copyright jQuery Foundation and other contributors
require,lodash.merge,MIT,Copyright OpenJS Foundation and other contributors
require,tslib,Apache-2.0,Copyright Microsoft Corporation
require,url-polyfill,MIT,Copyright 2017 Valentin Richard
file,tracekit,MIT,Copyright 2013 Onur Can Cakmak and all TraceKit contributors
dev,@types/jasmine,MIT,Copyright Microsoft Corporation
dev,@types/lodash.assign,MIT,Copyright Microsoft Corporation
Expand Down
3 changes: 1 addition & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
"dependencies": {
"lodash.assign": "4.2.0",
"lodash.merge": "4.6.2",
"tslib": "1.10.0",
"url-polyfill": "1.1.7"
"tslib": "1.10.0"
},
"devDependencies": {
"@types/lodash.assign": "4.2.6",
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export {
stopSessionManagement,
} from './sessionManagement'
export { HttpRequest, Batch } from './transport'
export * from './urlPolyfill'
export * from './utils'
export { areCookiesAuthorized, getCookie, setCookie, COOKIE_ACCESS_DELAY } from './cookie'

Expand Down
7 changes: 1 addition & 6 deletions packages/core/src/requestCollection.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import 'url-polyfill'

import { toStackTraceString } from './errorCollection'
import { monitor } from './internalMonitoring'
import { Observable } from './observable'
import { computeStackTrace } from './tracekit'
import { normalizeUrl } from './urlPolyfill'
import { ResourceKind } from './utils'

export enum RequestType {
Expand Down Expand Up @@ -145,10 +144,6 @@ export function trackFetch(observable: RequestObservable) {
})
}

export function normalizeUrl(url: string) {
return new URL(url, window.location.origin).href
}

export function isRejected(request: RequestDetails) {
return request.status === 0 && request.responseType !== 'opaque'
}
Expand Down
68 changes: 68 additions & 0 deletions packages/core/src/urlPolyfill.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { getLinkElementOrigin, getLocationOrigin } from './utils'

export function normalizeUrl(url: string) {
return buildUrl(url, getLocationOrigin()).href
}

export function isValidUrl(url: string) {
try {
return !!buildUrl(url)
} catch {
return false
}
}

export function haveSameOrigin(url1: string, url2: string) {
return getOrigin(url1) === getOrigin(url2)
}

export function getOrigin(url: string) {
return getLinkElementOrigin(buildUrl(url))
}

export function getPathName(url: string) {
const pathname = buildUrl(url).pathname
return pathname[0] === '/' ? pathname : `/${pathname}`
}

export function getSearch(url: string) {
return buildUrl(url).search
}

export function getHash(url: string) {
return buildUrl(url).hash
}

function buildUrl(url: string, base?: string) {
if (checkURLSupported()) {
return new URL(url, base)
}
if (base === undefined && !/:/.test(url)) {
throw new Error(`Invalid URL: '${url}'`)
}
let doc = document
const anchorElement = doc.createElement('a')
if (base !== undefined) {
doc = document.implementation.createHTMLDocument('')
const baseElement = doc.createElement('base')
baseElement.href = base
doc.head.appendChild(baseElement)
doc.body.appendChild(anchorElement)
}
anchorElement.href = url
return anchorElement
}

let isURLSupported: boolean | undefined
function checkURLSupported() {
if (isURLSupported !== undefined) {
return isURLSupported
}
try {
const url = new URL('http://test')
return url.href === 'http://test'
} catch {
isURLSupported = false
}
return isURLSupported
}
16 changes: 16 additions & 0 deletions packages/core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,19 @@ export function getGlobalObject<T>(): T {
// tslint:disable-next-line: function-constructor no-function-constructor-with-string-args
return (typeof globalThis === 'object' ? globalThis : Function('return this')()) as T
}

export function getLocationOrigin() {
return getLinkElementOrigin(window.location)
}

/**
* IE fallback
* https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils/origin
*/
export function getLinkElementOrigin(element: Location | HTMLAnchorElement | URL) {
if (element.origin) {
return element.origin
}
const sanitizedHost = element.host.replace(/(:80|:443)$/, '')
return `${element.protocol}//${sanitizedHost}`
}
31 changes: 1 addition & 30 deletions packages/core/test/requestCollection.spec.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,5 @@
import { Observable } from '../src/observable'
import {
isRejected,
isServerError,
normalizeUrl,
RequestDetails,
RequestType,
trackFetch,
trackXhr,
} from '../src/requestCollection'
import { isRejected, isServerError, RequestDetails, RequestType, trackFetch, trackXhr } from '../src/requestCollection'
import { FetchStub, FetchStubBuilder, FetchStubPromise, isFirefox, isIE } from '../src/specHelper'

describe('fetch tracker', () => {
Expand Down Expand Up @@ -174,27 +166,6 @@ describe('fetch tracker', () => {
})
})

describe('normalize url', () => {
it('should add origin to relative path', () => {
expect(normalizeUrl('/my/path')).toEqual(`${window.location.origin}/my/path`)
})

it('should add protocol to relative url', () => {
expect(normalizeUrl('//foo.com:9876/my/path')).toEqual('http://foo.com:9876/my/path')
})

it('should keep full url unchanged', () => {
expect(normalizeUrl('https://foo.com/my/path')).toEqual('https://foo.com/my/path')
})

it('should keep non http url unchanged', () => {
if (isFirefox()) {
pending('https://bugzilla.mozilla.org/show_bug.cgi?id=1578787')
}
expect(normalizeUrl('file://foo.com/my/path')).toEqual('file://foo.com/my/path')
})
})

describe('xhr tracker', () => {
let originalOpen: typeof XMLHttpRequest.prototype.open
let originalSend: typeof XMLHttpRequest.prototype.send
Expand Down
70 changes: 70 additions & 0 deletions packages/core/test/urlPolyfill.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import {
getHash,
getLocationOrigin,
getOrigin,
getPathName,
getSearch,
isFirefox,
isValidUrl,
normalizeUrl,
} from '../src'

describe('normalize url', () => {
it('should add origin to relative path', () => {
expect(normalizeUrl('/my/path')).toEqual(`${getLocationOrigin()}/my/path`)
})

it('should add protocol to relative url', () => {
expect(normalizeUrl('//foo.com:9876/my/path')).toEqual('http://foo.com:9876/my/path')
})

it('should keep full url unchanged', () => {
expect(normalizeUrl('https://foo.com/my/path')).toEqual('https://foo.com/my/path')
})

it('should keep non http url unchanged', () => {
if (isFirefox()) {
pending('https://bugzilla.mozilla.org/show_bug.cgi?id=1578787')
}
expect(normalizeUrl('file://foo.com/my/path')).toEqual('file://foo.com/my/path')
})
})

describe('isValidUrl', () => {
it('should ensure url is valid', () => {
expect(isValidUrl('http://www.datadoghq.com')).toBe(true)
expect(isValidUrl('http://www.datadoghq.com/foo/bar?a=b#hello')).toBe(true)
expect(isValidUrl('file://www.datadoghq.com')).toBe(true)
expect(isValidUrl('/plop')).toBe(false)
expect(isValidUrl('')).toBe(false)
})
})

describe('getOrigin', () => {
it('should retrieve url origin', () => {
expect(getOrigin('http://www.datadoghq.com')).toBe('http://www.datadoghq.com')
expect(getOrigin('http://www.datadoghq.com/foo/bar?a=b#hello')).toBe('http://www.datadoghq.com')
expect(getOrigin('http://localhost:8080')).toBe('http://localhost:8080')
})
})

describe('getPathName', () => {
it('should retrieve url path name', () => {
expect(getPathName('http://www.datadoghq.com')).toBe('/')
expect(getPathName('http://www.datadoghq.com/foo/bar?a=b#hello')).toBe('/foo/bar')
})
})

describe('getSearch', () => {
it('should retrieve url search', () => {
expect(getSearch('http://www.datadoghq.com')).toBe('')
expect(getSearch('http://www.datadoghq.com/foo/bar?a=b#hello')).toBe('?a=b')
})
})

describe('getHash', () => {
it('should retrieve url hash', () => {
expect(getHash('http://www.datadoghq.com')).toBe('')
expect(getHash('http://www.datadoghq.com/foo/bar?a=b#hello')).toBe('#hello')
})
})
32 changes: 17 additions & 15 deletions packages/rum/src/resourceUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import { addMonitoringMessage, Configuration, includes, msToNs, ResourceKind } from '@datadog/browser-core'
import {
addMonitoringMessage,
Configuration,
getPathName,
haveSameOrigin,
includes,
isValidUrl,
msToNs,
ResourceKind,
} from '@datadog/browser-core'

import { PerformanceResourceDetails } from './rum'

Expand All @@ -25,18 +34,15 @@ const RESOURCE_TYPES: Array<[ResourceKind, (initiatorType: string, path: string)
]

export function computeResourceKind(timing: PerformanceResourceTiming) {
let url: URL | undefined
try {
url = new URL(timing.name)
} catch (e) {
const url = timing.name
if (!isValidUrl(url)) {
addMonitoringMessage(`Failed to construct URL for "${timing.name}"`)
return ResourceKind.OTHER
}
if (url !== undefined) {
const path = url.pathname
for (const [type, isType] of RESOURCE_TYPES) {
if (isType(timing.initiatorType, path)) {
return type
}
const path = getPathName(url)
for (const [type, isType] of RESOURCE_TYPES) {
if (isType(timing.initiatorType, path)) {
return type
}
}
return ResourceKind.OTHER
Expand Down Expand Up @@ -118,7 +124,3 @@ function isBrowserAgentRequest(url: string, configuration: Configuration) {
(configuration.internalMonitoringEndpoint && haveSameOrigin(url, configuration.internalMonitoringEndpoint))
)
}

function haveSameOrigin(url1: string, url2: string) {
return new URL(url1).origin === new URL(url2).origin
}
3 changes: 2 additions & 1 deletion packages/rum/src/rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface PerformanceResourceDetails {
}

export interface RumResourceEvent {
date: number
duration: number
evt: {
category: RumEventCategory.RESOURCE
Expand Down Expand Up @@ -309,7 +310,7 @@ function trackPerformanceTiming(
export function handleResourceEntry(
configuration: Configuration,
entry: PerformanceResourceTiming,
addRumEvent: (event: RumEvent) => void,
addRumEvent: (event: RumResourceEvent) => void,
lifeCycle: LifeCycle
) {
if (!isValidResource(entry.name, configuration)) {
Expand Down
10 changes: 6 additions & 4 deletions packages/rum/test/viewTracker.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 { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
import { PerformanceLongTaskTiming, PerformancePaintTiming, RumEvent, RumViewEvent, UserAction } from '../src/rum'
import { RumSession } from '../src/rumSession'
Expand All @@ -11,10 +13,10 @@ function setup({
lifeCycle?: LifeCycle
} = {}) {
spyOn(history, 'pushState').and.callFake((_: any, __: string, pathname: string) => {
const url = new URL(pathname, 'http://localhost')
fakeLocation.pathname = url.pathname
fakeLocation.search = url.search
fakeLocation.hash = url.hash
const url = `http://localhost${pathname}`
fakeLocation.pathname = getPathName(url)
fakeLocation.search = getSearch(url)
fakeLocation.hash = getHash(url)
})
const fakeLocation: Partial<Location> = { pathname: '/foo' }
const fakeSession = {
Expand Down
18 changes: 6 additions & 12 deletions test/app/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,25 @@
# yarn lockfile v1


"@datadog/browser-core@1.4.1", "@datadog/browser-core@file:../../packages/core":
version "1.4.1"
"@datadog/browser-core@1.7.0", "@datadog/browser-core@file:../../packages/core":
version "1.7.0"
dependencies:
lodash.assign "4.2.0"
lodash.merge "4.6.2"
tslib "1.10.0"
url-polyfill "1.1.7"

"@datadog/browser-logs@file:../../packages/logs":
version "1.4.1"
version "1.7.0"
dependencies:
"@datadog/browser-core" "1.4.1"
"@datadog/browser-core" "1.7.0"
lodash.assign "4.2.0"
lodash.merge "4.6.2"
tslib "1.10.0"

"@datadog/browser-rum@file:../../packages/rum":
version "1.4.1"
version "1.7.0"
dependencies:
"@datadog/browser-core" "1.4.1"
"@datadog/browser-core" "1.7.0"
lodash.assign "4.2.0"
lodash.merge "4.6.2"
tslib "1.10.0"
Expand Down Expand Up @@ -2236,11 +2235,6 @@ urix@^0.1.0:
resolved "https://registry.yarnpkg.com/urix/-/urix-0.1.0.tgz#da937f7a62e21fec1fd18d49b35c2935067a6c72"
integrity sha1-2pN/emLiH+wf0Y1Js1wpNQZ6bHI=

[email protected]:
version "1.1.7"
resolved "https://registry.yarnpkg.com/url-polyfill/-/url-polyfill-1.1.7.tgz#402ee84360eb549bbeb585f4c7971e79a31de9e3"
integrity sha512-ZrAxYWCREjmMtL8gSbSiKKLZZticgihCvVBtrFbUVpyoETt8GQJeG2okMWA8XryDAaHMjJfhnc+rnhXRbI4DXA==

url@^0.11.0:
version "0.11.0"
resolved "https://registry.yarnpkg.com/url/-/url-0.11.0.tgz#3838e97cfc60521eb73c525a8e55bfdd9e2e28f1"
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9713,11 +9713,6 @@ urix@^0.1.0:
resolved "https://registry.yarnpkg.com/urix/-/urix-0.1.0.tgz#da937f7a62e21fec1fd18d49b35c2935067a6c72"
integrity sha1-2pN/emLiH+wf0Y1Js1wpNQZ6bHI=

[email protected]:
version "1.1.7"
resolved "https://registry.yarnpkg.com/url-polyfill/-/url-polyfill-1.1.7.tgz#402ee84360eb549bbeb585f4c7971e79a31de9e3"
integrity sha512-ZrAxYWCREjmMtL8gSbSiKKLZZticgihCvVBtrFbUVpyoETt8GQJeG2okMWA8XryDAaHMjJfhnc+rnhXRbI4DXA==

url@^0.11.0:
version "0.11.0"
resolved "https://registry.yarnpkg.com/url/-/url-0.11.0.tgz#3838e97cfc60521eb73c525a8e55bfdd9e2e28f1"
Expand Down

0 comments on commit 3c525e5

Please sign in to comment.