Skip to content

Commit

Permalink
ref(utils): Remove getGlobalObject() usage from all framework speci…
Browse files Browse the repository at this point in the history
…fic SDKs (#5948)
  • Loading branch information
timfish authored Oct 13, 2022
1 parent 47aabd8 commit 74e787f
Show file tree
Hide file tree
Showing 15 changed files with 63 additions and 85 deletions.
8 changes: 3 additions & 5 deletions packages/angular/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni
import { ActivatedRouteSnapshot, Event, NavigationEnd, NavigationStart, ResolveEnd, Router } from '@angular/router';
import { getCurrentHub } from '@sentry/browser';
import { Span, Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
import { logger, stripUrlQueryAndFragment, timestampWithMs, WINDOW } from '@sentry/utils';
import { Observable, Subscription } from 'rxjs';
import { filter, tap } from 'rxjs/operators';

Expand All @@ -15,8 +15,6 @@ let instrumentationInitialized: boolean;
let stashedStartTransaction: (context: TransactionContext) => Transaction | undefined;
let stashedStartTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

/**
* Creates routing instrumentation for Angular Router.
*/
Expand All @@ -29,9 +27,9 @@ export function routingInstrumentation(
stashedStartTransaction = customStartTransaction;
stashedStartTransactionOnLocationChange = startTransactionOnLocationChange;

if (startTransactionOnPageLoad && global && global.location) {
if (startTransactionOnPageLoad && WINDOW && WINDOW.location) {
customStartTransaction({
name: global.location.pathname,
name: WINDOW.location.pathname,
op: 'pageload',
metadata: { source: 'url' },
});
Expand Down
5 changes: 2 additions & 3 deletions packages/ember/addon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@ import { macroCondition, isDevelopingApp, getOwnConfig } from '@embroider/macros
import { next } from '@ember/runloop';
import { assert, warn } from '@ember/debug';
import Ember from 'ember';
import { timestampWithMs } from '@sentry/utils';
import { timestampWithMs, GLOBAL_OBJ } from '@sentry/utils';
import { GlobalConfig, OwnConfig } from './types';
import { getGlobalObject } from '@sentry/utils';

function _getSentryInitConfig() {
const _global = getGlobalObject<GlobalConfig>();
const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalConfig;
_global.__sentryEmberConfig = _global.__sentryEmberConfig ?? {};
return _global.__sentryEmberConfig;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ import { ExtendedBackburner } from '@sentry/ember/runloop';
import { Span, Transaction, Integration } from '@sentry/types';
import { EmberRunQueues } from '@ember/runloop/-private/types';
import { getActiveTransaction } from '..';
import { browserPerformanceTimeOrigin, getGlobalObject, timestampWithMs } from '@sentry/utils';
import { browserPerformanceTimeOrigin, GLOBAL_OBJ, timestampWithMs } from '@sentry/utils';
import { macroCondition, isTesting, getOwnConfig } from '@embroider/macros';
import { EmberSentryConfig, GlobalConfig, OwnConfig } from '../types';

function getSentryConfig() {
const _global = getGlobalObject<GlobalConfig>();
const _global = GLOBAL_OBJ as typeof GLOBAL_OBJ & GlobalConfig;
_global.__sentryEmberConfig = _global.__sentryEmberConfig ?? {};
const environmentConfig = getOwnConfig<OwnConfig>().sentryConfig;
if (!environmentConfig.sentry) {
Expand Down
14 changes: 6 additions & 8 deletions packages/nextjs/src/performance/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,21 +3,19 @@ import { Primitive, TraceparentData, Transaction, TransactionContext, Transactio
import {
baggageHeaderToDynamicSamplingContext,
extractTraceparentData,
getGlobalObject,
logger,
stripUrlQueryAndFragment,
WINDOW,
} from '@sentry/utils';
import type { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';
import type { ParsedUrlQuery } from 'querystring';

const global = getGlobalObject<
Window & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
}
>();
const global = WINDOW as typeof WINDOW & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
};

type StartTransactionCb = (context: TransactionContext) => Transaction | undefined;

Expand Down
14 changes: 6 additions & 8 deletions packages/nextjs/test/index.client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,14 @@ import { BaseClient, getCurrentHub } from '@sentry/core';
import * as SentryReact from '@sentry/react';
import { Integrations as TracingIntegrations } from '@sentry/tracing';
import { Integration } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { logger, WINDOW } from '@sentry/utils';
import { JSDOM } from 'jsdom';

import { init, Integrations, nextRouterInstrumentation } from '../src/index.client';
import { UserIntegrationsFunction } from '../src/utils/userIntegrations';

const { BrowserTracing } = TracingIntegrations;

const global = getGlobalObject();

const reactInit = jest.spyOn(SentryReact, 'init');
const captureEvent = jest.spyOn(BaseClient.prototype, 'captureEvent');
const loggerLogSpy = jest.spyOn(logger, 'log');
Expand All @@ -23,12 +21,12 @@ const dom = new JSDOM(undefined, { url: 'https://example.com/' });
Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });

const originalGlobalDocument = getGlobalObject<Window>().document;
const originalGlobalLocation = getGlobalObject<Window>().location;
const originalGlobalDocument = WINDOW.document;
const originalGlobalLocation = WINDOW.location;
afterAll(() => {
// Clean up JSDom
Object.defineProperty(global, 'document', { value: originalGlobalDocument });
Object.defineProperty(global, 'location', { value: originalGlobalLocation });
Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });
});

function findIntegrationByName(integrations: Integration[] = [], name: string): Integration | undefined {
Expand All @@ -38,7 +36,7 @@ function findIntegrationByName(integrations: Integration[] = [], name: string):
describe('Client init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
WINDOW.__SENTRY__.hub = undefined;
});

it('inits the React SDK', () => {
Expand Down
8 changes: 3 additions & 5 deletions packages/nextjs/test/index.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import * as SentryNode from '@sentry/node';
import { getCurrentHub, NodeClient } from '@sentry/node';
import { Integration } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { GLOBAL_OBJ, logger } from '@sentry/utils';
import * as domain from 'domain';

import { init } from '../src/index.server';

const { Integrations } = SentryNode;

const global = getGlobalObject();

// normally this is set as part of the build process, so mock it here
(global as typeof global & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';
(GLOBAL_OBJ as typeof GLOBAL_OBJ & { __rewriteFramesDistDir__: string }).__rewriteFramesDistDir__ = '.next';

const nodeInit = jest.spyOn(SentryNode, 'init');
const loggerLogSpy = jest.spyOn(logger, 'log');
Expand All @@ -23,7 +21,7 @@ function findIntegrationByName(integrations: Integration[] = [], name: string):
describe('Server init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
delete process.env.VERCEL;
});

Expand Down
38 changes: 18 additions & 20 deletions packages/nextjs/test/performance/client.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { Transaction } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';
import { JSDOM } from 'jsdom';
import { NEXT_DATA as NextData } from 'next/dist/next-server/lib/utils';
import { default as Router } from 'next/router';

import { nextRouterInstrumentation } from '../../src/performance/client';

const globalObject = getGlobalObject<
Window & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
}
>();
const globalObject = WINDOW as typeof WINDOW & {
__BUILD_MANIFEST?: {
sortedPages?: string[];
};
};

const originalBuildManifest = globalObject.__BUILD_MANIFEST;
const originalBuildManifestRoutes = globalObject.__BUILD_MANIFEST?.sortedPages;
Expand Down Expand Up @@ -60,8 +58,8 @@ function createMockStartTransaction() {
}

describe('nextRouterInstrumentation', () => {
const originalGlobalDocument = getGlobalObject<Window>().document;
const originalGlobalLocation = getGlobalObject<Window>().location;
const originalGlobalDocument = WINDOW.document;
const originalGlobalLocation = WINDOW.location;

function setUpNextPage(pageProperties: {
url: string;
Expand Down Expand Up @@ -93,25 +91,25 @@ describe('nextRouterInstrumentation', () => {
// The Next.js routing instrumentations requires a few things to be present on pageload:
// 1. Access to window.document API for `window.document.getElementById`
// 2. Access to window.location API for `window.location.pathname`
Object.defineProperty(global, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(global, 'location', { value: dom.window.document.location, writable: true });
Object.defineProperty(WINDOW, 'document', { value: dom.window.document, writable: true });
Object.defineProperty(WINDOW, 'location', { value: dom.window.document.location, writable: true });

// Define Next.js clientside build manifest with navigatable routes
(global as any).__BUILD_MANIFEST = {
...(global as any).__BUILD_MANIFEST,
sortedPages: pageProperties.navigatableRoutes,
globalObject.__BUILD_MANIFEST = {
...globalObject.__BUILD_MANIFEST,
sortedPages: pageProperties.navigatableRoutes as string[],
};
}

afterEach(() => {
// Clean up JSDom
Object.defineProperty(global, 'document', { value: originalGlobalDocument });
Object.defineProperty(global, 'location', { value: originalGlobalLocation });
Object.defineProperty(WINDOW, 'document', { value: originalGlobalDocument });
Object.defineProperty(WINDOW, 'location', { value: originalGlobalLocation });

// Reset Next.js' __BUILD_MANIFEST
(global as any).__BUILD_MANIFEST = originalBuildManifest;
if ((global as any).__BUILD_MANIFEST) {
(global as any).__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes;
globalObject.__BUILD_MANIFEST = originalBuildManifest;
if (globalObject.__BUILD_MANIFEST) {
globalObject.__BUILD_MANIFEST.sortedPages = originalBuildManifestRoutes as string[];
}

// Clear all event handlers
Expand Down
8 changes: 3 additions & 5 deletions packages/react/src/reactrouter.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Transaction, TransactionSource } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import * as React from 'react';

Expand All @@ -26,8 +26,6 @@ export type RouteConfig = {
type MatchPath = (pathname: string, props: string | string[] | any, parent?: Match | null) => Match | null;
/* eslint-enable @typescript-eslint/no-explicit-any */

const global = getGlobalObject<Window>();

let activeTransaction: Transaction | undefined;

export function reactRouterV4Instrumentation(
Expand Down Expand Up @@ -57,8 +55,8 @@ function createReactRouterInstrumentation(
return history.location.pathname;
}

if (global && global.location) {
return global.location.pathname;
if (WINDOW && WINDOW.location) {
return WINDOW.location.pathname;
}

return undefined;
Expand Down
10 changes: 4 additions & 6 deletions packages/react/src/reactrouterv3.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Primitive, Transaction, TransactionContext, TransactionSource } from '@sentry/types';
import { getGlobalObject } from '@sentry/utils';
import { WINDOW } from '@sentry/utils';

import { Location, ReactRouterInstrumentation } from './types';

Expand All @@ -21,8 +21,6 @@ export type Match = (

type ReactRouterV3TransactionSource = Extract<TransactionSource, 'url' | 'route'>;

const global = getGlobalObject<Window>();

/**
* Creates routing instrumentation for React Router v3
* Works for React Router >= 3.2.0 and < 4.0.0
Expand All @@ -44,11 +42,11 @@ export function reactRouterV3Instrumentation(
let activeTransaction: Transaction | undefined;
let prevName: string | undefined;

// Have to use global.location because history.location might not be defined.
if (startTransactionOnPageLoad && global && global.location) {
// Have to use window.location because history.location might not be defined.
if (startTransactionOnPageLoad && WINDOW && WINDOW.location) {
normalizeTransactionName(
routes,
global.location as unknown as Location,
WINDOW.location as unknown as Location,
match,
(localName: string, source: ReactRouterV3TransactionSource = 'url') => {
prevName = localName;
Expand Down
6 changes: 2 additions & 4 deletions packages/react/src/reactrouterv6.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// https://gist.github.com/wontondon/e8c4bdf2888875e4c755712e99279536

import { Transaction, TransactionContext, TransactionSource } from '@sentry/types';
import { getGlobalObject, getNumberOfUrlSegments, logger } from '@sentry/utils';
import { getNumberOfUrlSegments, logger, WINDOW } from '@sentry/utils';
import hoistNonReactStatics from 'hoist-non-react-statics';
import React from 'react';

Expand Down Expand Up @@ -58,8 +58,6 @@ let _matchRoutes: MatchRoutes;
let _customStartTransaction: (context: TransactionContext) => Transaction | undefined;
let _startTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

const SENTRY_TAGS = {
'routing.instrumentation': 'react-router-v6',
};
Expand All @@ -76,7 +74,7 @@ export function reactRouterV6Instrumentation(
startTransactionOnPageLoad = true,
startTransactionOnLocationChange = true,
): void => {
const initPathName = global && global.location && global.location.pathname;
const initPathName = WINDOW && WINDOW.location && WINDOW.location.pathname;
if (startTransactionOnPageLoad && initPathName) {
activeTransaction = customStartTransaction({
name: initPathName,
Expand Down
8 changes: 3 additions & 5 deletions packages/remix/src/performance/client.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { ErrorBoundaryProps } from '@sentry/react';
import { withErrorBoundary } from '@sentry/react';
import { Transaction, TransactionContext } from '@sentry/types';
import { getGlobalObject, logger } from '@sentry/utils';
import { logger, WINDOW } from '@sentry/utils';
import * as React from 'react';

const DEFAULT_TAGS = {
Expand Down Expand Up @@ -38,11 +38,9 @@ let _useMatches: UseMatches;
let _customStartTransaction: (context: TransactionContext) => Transaction | undefined;
let _startTransactionOnLocationChange: boolean;

const global = getGlobalObject<Window>();

function getInitPathName(): string | undefined {
if (global && global.location) {
return global.location.pathname;
if (WINDOW && WINDOW.location) {
return WINDOW.location.pathname;
}

return undefined;
Expand Down
6 changes: 2 additions & 4 deletions packages/remix/test/index.client.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import { getCurrentHub } from '@sentry/core';
import * as SentryReact from '@sentry/react';
import { getGlobalObject } from '@sentry/utils';
import { GLOBAL_OBJ } from '@sentry/utils';

import { init } from '../src/index.client';

const global = getGlobalObject();

const reactInit = jest.spyOn(SentryReact, 'init');

describe('Client init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
});

it('inits the React SDK', () => {
Expand Down
6 changes: 2 additions & 4 deletions packages/remix/test/index.server.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,15 @@
import * as SentryNode from '@sentry/node';
import { getCurrentHub } from '@sentry/node';
import { getGlobalObject } from '@sentry/utils';
import { GLOBAL_OBJ } from '@sentry/utils';

import { init } from '../src/index.server';

const global = getGlobalObject();

const nodeInit = jest.spyOn(SentryNode, 'init');

describe('Server init()', () => {
afterEach(() => {
jest.clearAllMocks();
global.__SENTRY__.hub = undefined;
GLOBAL_OBJ.__SENTRY__.hub = undefined;
});

it('inits the Node SDK', () => {
Expand Down
Loading

0 comments on commit 74e787f

Please sign in to comment.