Skip to content

Commit

Permalink
ref(tracing): Remove getGlobalObject() usage from @sentry/tracing (
Browse files Browse the repository at this point in the history
  • Loading branch information
timfish authored Oct 13, 2022
1 parent 49cdb28 commit 971ed96
Show file tree
Hide file tree
Showing 10 changed files with 34 additions and 44 deletions.
10 changes: 4 additions & 6 deletions packages/tracing/src/browser/backgroundtab.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
import { getGlobalObject, logger } from '@sentry/utils';
import { logger, WINDOW } from '@sentry/utils';

import { IdleTransaction } from '../idletransaction';
import { SpanStatusType } from '../span';
import { getActiveTransaction } from '../utils';

const global = getGlobalObject<Window>();

/**
* Add a listener that cancels and finishes a transaction when the global
* document is hidden.
*/
export function registerBackgroundTabDetection(): void {
if (global && global.document) {
global.document.addEventListener('visibilitychange', () => {
if (WINDOW && WINDOW.document) {
WINDOW.document.addEventListener('visibilitychange', () => {
const activeTransaction = getActiveTransaction() as IdleTransaction;
if (global.document.hidden && activeTransaction) {
if (WINDOW.document.hidden && activeTransaction) {
const statusType: SpanStatusType = 'cancelled';

__DEBUG_BUILD__ &&
Expand Down
4 changes: 2 additions & 2 deletions packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/* eslint-disable max-lines */
import { Hub } from '@sentry/core';
import { EventProcessor, Integration, Transaction, TransactionContext } from '@sentry/types';
import { baggageHeaderToDynamicSamplingContext, getDomElement, getGlobalObject, logger } from '@sentry/utils';
import { baggageHeaderToDynamicSamplingContext, getDomElement, logger, WINDOW } from '@sentry/utils';

import { startIdleTransaction } from '../hubextensions';
import { DEFAULT_FINAL_TIMEOUT, DEFAULT_HEARTBEAT_INTERVAL, DEFAULT_IDLE_TIMEOUT } from '../idletransaction';
Expand Down Expand Up @@ -265,7 +265,7 @@ export class BrowserTracing implements Integration {
__DEBUG_BUILD__ && logger.log(`[Tracing] Starting ${finalContext.op} transaction on scope`);

const hub = this._getCurrentHub();
const { location } = getGlobalObject() as WindowOrWorkerGlobalScope & { location: Location };
const { location } = WINDOW;

const idleTransaction = startIdleTransaction(
hub,
Expand Down
14 changes: 6 additions & 8 deletions packages/tracing/src/browser/metrics/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable max-lines */
import { Measurements } from '@sentry/types';
import { browserPerformanceTimeOrigin, getGlobalObject, htmlTreeAsString, logger } from '@sentry/utils';
import { browserPerformanceTimeOrigin, htmlTreeAsString, logger, WINDOW } from '@sentry/utils';

import { IdleTransaction } from '../../idletransaction';
import { Transaction } from '../../transaction';
Expand All @@ -13,10 +13,8 @@ import { observe, PerformanceEntryHandler } from '../web-vitals/lib/observe';
import { NavigatorDeviceMemory, NavigatorNetworkInformation } from '../web-vitals/types';
import { _startChild, isMeasurementValue } from './utils';

const global = getGlobalObject<Window>();

function getBrowserPerformanceAPI(): Performance | undefined {
return global && global.addEventListener && global.performance;
return WINDOW && WINDOW.addEventListener && WINDOW.performance;
}

let _performanceCursor: number = 0;
Expand All @@ -32,7 +30,7 @@ export function startTrackingWebVitals(reportAllChanges: boolean = false): void
const performance = getBrowserPerformanceAPI();
if (performance && browserPerformanceTimeOrigin) {
if (performance.mark) {
global.performance.mark('sentry-tracing-init');
WINDOW.performance.mark('sentry-tracing-init');
}
_trackCLS();
_trackLCP(reportAllChanges);
Expand Down Expand Up @@ -112,7 +110,7 @@ function _trackFID(): void {
/** Add performance related spans to a transaction */
export function addPerformanceEntries(transaction: Transaction): void {
const performance = getBrowserPerformanceAPI();
if (!performance || !global.performance.getEntries || !browserPerformanceTimeOrigin) {
if (!performance || !WINDOW.performance.getEntries || !browserPerformanceTimeOrigin) {
// Gatekeeper if performance API not available
return;
}
Expand Down Expand Up @@ -162,7 +160,7 @@ export function addPerformanceEntries(transaction: Transaction): void {
break;
}
case 'resource': {
const resourceName = (entry.name as string).replace(global.location.origin, '');
const resourceName = (entry.name as string).replace(WINDOW.location.origin, '');
_addResourceSpans(transaction, entry, resourceName, startTime, duration, timeOrigin);
break;
}
Expand Down Expand Up @@ -376,7 +374,7 @@ export function _addResourceSpans(
* Capture the information of the user agent.
*/
function _trackNavigator(transaction: Transaction): void {
const navigator = global.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory);
const navigator = WINDOW.navigator as null | (Navigator & NavigatorNetworkInformation & NavigatorDeviceMemory);
if (!navigator) {
return;
}
Expand Down
12 changes: 5 additions & 7 deletions packages/tracing/src/browser/router.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import { Transaction, TransactionContext } from '@sentry/types';
import { addInstrumentationHandler, getGlobalObject, logger } from '@sentry/utils';

const global = getGlobalObject<Window>();
import { addInstrumentationHandler, logger, WINDOW } from '@sentry/utils';

/**
* Default function implementing pageload and navigation transactions
Expand All @@ -11,17 +9,17 @@ export function instrumentRoutingWithDefaults<T extends Transaction>(
startTransactionOnPageLoad: boolean = true,
startTransactionOnLocationChange: boolean = true,
): void {
if (!global || !global.location) {
if (!WINDOW || !WINDOW.location) {
__DEBUG_BUILD__ && logger.warn('Could not initialize routing instrumentation due to invalid location');
return;
}

let startingUrl: string | undefined = global.location.href;
let startingUrl: string | undefined = WINDOW.location.href;

let activeTransaction: T | undefined;
if (startTransactionOnPageLoad) {
activeTransaction = customStartTransaction({
name: global.location.pathname,
name: WINDOW.location.pathname,
op: 'pageload',
metadata: { source: 'url' },
});
Expand Down Expand Up @@ -51,7 +49,7 @@ export function instrumentRoutingWithDefaults<T extends Transaction>(
activeTransaction.finish();
}
activeTransaction = customStartTransaction({
name: global.location.pathname,
name: WINDOW.location.pathname,
op: 'navigation',
metadata: { source: 'url' },
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,14 @@
* limitations under the License.
*/

import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';

import { onHidden } from './onHidden';

let firstHiddenTime = -1;

const initHiddenTime = (): number => {
return getGlobalObject<Window>().document.visibilityState === 'hidden' ? 0 : Infinity;
return WINDOW.document.visibilityState === 'hidden' ? 0 : Infinity;
};

const trackChanges = (): void => {
Expand Down
4 changes: 2 additions & 2 deletions packages/tracing/src/browser/web-vitals/lib/onHidden.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
* limitations under the License.
*/

import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';

export interface OnHiddenCallback {
(event: Event): void;
}

export const onHidden = (cb: OnHiddenCallback, once?: boolean): void => {
const onHiddenOrPageHide = (event: Event): void => {
if (event.type === 'pagehide' || getGlobalObject<Window>().document.visibilityState === 'hidden') {
if (event.type === 'pagehide' || WINDOW.document.visibilityState === 'hidden') {
cb(event);
if (once) {
removeEventListener('visibilitychange', onHiddenOrPageHide, true);
Expand Down
7 changes: 3 additions & 4 deletions packages/tracing/src/index.bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export {
export { SDK_VERSION } from '@sentry/browser';

import { Integrations as BrowserIntegrations } from '@sentry/browser';
import { getGlobalObject } from '@sentry/utils';
import { GLOBAL_OBJ } from '@sentry/utils';

import { BrowserTracing } from './browser';
import { addExtensionMethods } from './hubextensions';
Expand All @@ -63,9 +63,8 @@ export { Span } from './span';
let windowIntegrations = {};

// This block is needed to add compatibility with the integrations packages when used with a CDN
const _window = getGlobalObject<Window>();
if (_window.Sentry && _window.Sentry.Integrations) {
windowIntegrations = _window.Sentry.Integrations;
if (GLOBAL_OBJ.Sentry && GLOBAL_OBJ.Sentry.Integrations) {
windowIntegrations = GLOBAL_OBJ.Sentry.Integrations;
}

const INTEGRATIONS = {
Expand Down
12 changes: 6 additions & 6 deletions packages/tracing/test/browser/browsertracing.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { BrowserClient } from '@sentry/browser';
import { Hub, makeMain } from '@sentry/core';
import type { BaseTransportOptions, ClientOptions, DsnComponents } from '@sentry/types';
import { getGlobalObject, InstrumentHandlerCallback, InstrumentHandlerType } from '@sentry/utils';
import { InstrumentHandlerCallback, InstrumentHandlerType, WINDOW } from '@sentry/utils';
import { JSDOM } from 'jsdom';

import { BrowserTracing, BrowserTracingOptions, getMetaContent } from '../../src/browser/browsertracing';
Expand Down Expand Up @@ -40,11 +40,11 @@ const warnSpy = jest.spyOn(logger, 'warn');
beforeAll(() => {
const dom = new JSDOM();
// @ts-ignore need to override global document
global.document = dom.window.document;
WINDOW.document = dom.window.document;
// @ts-ignore need to override global document
global.window = dom.window;
WINDOW.window = dom.window;
// @ts-ignore need to override global document
global.location = dom.window.location;
WINDOW.location = dom.window.location;
});

describe('BrowserTracing', () => {
Expand Down Expand Up @@ -508,7 +508,7 @@ describe('BrowserTracing', () => {
};

it('extracts window.location/self.location for sampling context in pageload transactions', () => {
getGlobalObject<Window>().location = dogParkLocation as any;
WINDOW.location = dogParkLocation as any;

const tracesSampler = jest.fn();
const options = getDefaultBrowserClientOptions({ tracesSampler });
Expand All @@ -525,7 +525,7 @@ describe('BrowserTracing', () => {
});

it('extracts window.location/self.location for sampling context in navigation transactions', () => {
getGlobalObject<Window>().location = dogParkLocation as any;
WINDOW.location = dogParkLocation as any;

const tracesSampler = jest.fn();
const options = getDefaultBrowserClientOptions({ tracesSampler });
Expand Down
2 changes: 0 additions & 2 deletions packages/tracing/test/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ jest.spyOn(logger, 'warn');
jest.spyOn(logger, 'log');
jest.spyOn(utilsModule, 'isNodeEnv');

// we have to add things into the real global object (rather than mocking the return value of getGlobalObject) because
// there are modules which call getGlobalObject as they load, which is seemingly too early for jest to intervene
addDOMPropertiesToGlobal(['XMLHttpRequest', 'Event', 'location', 'document']);

describe('Hub', () => {
Expand Down
9 changes: 4 additions & 5 deletions packages/tracing/test/testutils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createTransport } from '@sentry/browser';
import { ClientOptions } from '@sentry/types';
import { getGlobalObject, resolvedSyncPromise } from '@sentry/utils';
import { GLOBAL_OBJ, resolvedSyncPromise } from '@sentry/utils';
import { JSDOM } from 'jsdom';

/**
Expand All @@ -13,14 +13,13 @@ import { JSDOM } from 'jsdom';
* @param properties The names of the properties to add
*/
export function addDOMPropertiesToGlobal(properties: string[]): void {
// we have to add things into the real global object (rather than mocking the return value of getGlobalObject)
// because there are modules which call getGlobalObject as they load, which is too early for jest to intervene
// we have to add things into the real global object
// because there are modules which call GLOBAL_OBJ as they load, which is too early for jest to intervene
const { window } = new JSDOM('', { url: 'http://dogs.are.great/' });
const global = getGlobalObject<NodeJS.Global & Window>();

properties.forEach(prop => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
(global as any)[prop] = window[prop];
(GLOBAL_OBJ as any)[prop] = window[prop];
});
}

Expand Down

0 comments on commit 971ed96

Please sign in to comment.